Re: [PATCH v3 13/23] media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

Hi Dikshita,

Le dimanche 04 mai 2025 à 20:53 +0530, Dikshita Agarwal a écrit :
> 
> 
> On 5/3/2025 9:39 PM, Nicolas Dufresne wrote:
> > Hi Dikshita,
> > 
> > Le vendredi 02 mai 2025 à 00:43 +0530, Dikshita Agarwal a écrit :
> > > Firmware sends buffers with 0 filled length which needs to be dropped,
> > > to achieve the same, add V4L2_BUF_FLAG_ERROR to such buffers.
> > > Also make sure:
> > > - These 0 length buffers are not returned as result of flush.
> > > - Its not a buffer with LAST flag enabled which will also have 0 filled
> > >   length.
> > 
> > This message is quite vague, is this about capture or output buffers ?
> > If its output buffers that don't produce capture, I don't see why they
> > have to be flagged as errors, or why the payload size matter. Then, if
> > its about assigned capture buffers that did not get used in the end, you
> > should put them back in the queue instead of returning them to user
> > space.
> > 
> > Returning a capture buffers to userspace should only be used if a frame
> > could not be produced. That imply copying the cookie timestamp from the
> > src buffers into the capture buffer. Please make sure you don't endup
> > returning fake erorrs to userspace, which may lead to some frame
> > metadata being dropped erroneously.
> > 
> The capture buffers which I am trying to handle here are of 0 byteused
> which means they don't have any valid data and they have the timestamp
> copied from src buffers.
> How these buffers will be handled by client? if we don't associate error
> flag to such buffers?

Please share a link to the stream and specify which frames are handled
this way by your firmware. The answer to your question is entirely
dependent on the stream you are decoding.

Nicolas

> 
> Thanks,
> Dikshita
> > Nicolas
> > 
> > > 
> > > Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
> > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
> > > ---
> > >  drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > index 4488540d1d41..3bb326843a7b 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> > > @@ -378,6 +378,12 @@ static int iris_hfi_gen2_handle_output_buffer(struct iris_inst *inst,
> > >  
> > >  	buf->flags = iris_hfi_gen2_get_driver_buffer_flags(inst, hfi_buffer->flags);
> > >  
> > > +	if (!buf->data_size && inst->state == IRIS_INST_STREAMING &&
> > > +	    !(hfi_buffer->flags & HFI_BUF_FW_FLAG_LAST) &&
> > > +	    !(inst->sub_state & IRIS_INST_SUB_DRC)) {
> > > +		buf->flags |= V4L2_BUF_FLAG_ERROR;
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >