linux-kernel - Re: [PATCH 26/28] venus: implementing multi-stream support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <26dd5ee8a6659dc8f86637516d60d01c@codeaurora.org>
Date:   Thu, 03 May 2018 17:16:05 +0530
From:   Vikash Garodia <vgarodia@...eaurora.org>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil@...all.nl>, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-media-owner@...r.kernel.org
Subject: Re: [PATCH 26/28] venus: implementing multi-stream support

Hi Stanimir,

On 2018-05-03 12:42, Stanimir Varbanov wrote:
> Hi Vikash,
> 
> Please write the comments for the chunk of code for which they are 
> refer to.
I see that the patch was about handling multistream mode, but the code 
to handle
the dpb buffer response is missing. My comment is basically to add the 
required
code.

> On  2.05.2018 10:40, Vikash Garodia wrote:
>> Hello Stanimir,
>> 
>> On 2018-04-24 18:14, Stanimir Varbanov wrote:
>>> This is implementing a multi-stream decoder support. The multi
>>> stream gives an option to use the secondary decoder output
>>> with different raw format (or the same in case of crop).
>>> 
>>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@...aro.org>
>>> ---
>>>  drivers/media/platform/qcom/venus/core.h    |   1 +
>>>  drivers/media/platform/qcom/venus/helpers.c | 204 
>>> +++++++++++++++++++++++++++-
>>>  drivers/media/platform/qcom/venus/helpers.h |   6 +
>>>  drivers/media/platform/qcom/venus/vdec.c    |  91 ++++++++++++-
>>>  drivers/media/platform/qcom/venus/venc.c    |   1 +
>>>  5 files changed, 299 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/drivers/media/platform/qcom/venus/core.h
>>> b/drivers/media/platform/qcom/venus/core.h
>>> index 4d6c05f156c4..85e66e2dd672 100644
>>> --- a/drivers/media/platform/qcom/venus/core.h
>>> +++ b/drivers/media/platform/qcom/venus/core.h
>>> @@ -259,6 +259,7 @@ struct venus_inst {
>>>      struct list_head list;
>>>      struct mutex lock;
>>>      struct venus_core *core;
>>> +    struct list_head dpbbufs;
>>>      struct list_head internalbufs;
>>>      struct list_head registeredbufs;
>>>      struct list_head delayed_process;
> 
> <snip>
> 
>> 
>> The dpb buffers queued to hardware will be returned back to host 
>> either during flush
>> or when the session is stopped. Host should not send these buffers to 
>> client.
> 
> That's correct.
> 
>> vdec_buf_done should be handling in a way to drop dpb buffers from 
>> sending to client.
> 
> That is also correct, vdec_buf_done is trying to find the buffer by
> index from a list of queued buffers from v4l2 clients. See
> venus_helper_vb2_buf_queue where it is calling v4l2_m2m_buf_queue.
> 
> So for the dpb buffers venus_helper_find_buf() will return NULL.

My bad, i could see that the DPB buffers are not sent to client in the 
existing patch.

Instead of bailing out on NULL, i was thinking it is better to keep 
explicit check for dpb
buffers and exit with a debug log.

> 
> regards,
> Stan