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

[Dikshita Agarwal <quic_dikshita@quicinc.com>]

On 5/5/2025 6:07 PM, Nicolas Dufresne wrote:
> 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.
> 
one such stream is
JCT-VC-HEVC_V1 -tv NoOutPrior_A_Qualcomm_1 (fluster stream)

for this stream, POC 17, 22 and 24 are not supposed to be displayed (output
is false), the corresponding capture buffers are returned with 0 filled
length by firmware to driver and expected to be dropped by the client.

Thanks,
Dikshita

> 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;
>>>>  }
>>>>