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

[Vikash Garodia]

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@xxxxxxxxxx>
> > > ---
> > > Â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