Re: [PATCH] media: iris: set default size when S_FMT is called with zero size

[Val Packett <val@packett.cool>]

On 10/28/25 10:19 AM, Bryan O'Donoghue wrote:
> On 13/10/2025 00:50, Val Packett wrote:
>> drivers/media/platform/qcom/iris/iris_vdec.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c 
>> b/drivers/media/platform/qcom/iris/iris_vdec.c
>> index ae13c3e1b426..6be09d82e24d 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
>> @@ -196,6 +196,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, 
>> struct v4l2_format *f)
>>       if (vb2_is_busy(q))
>>           return -EBUSY;
>>
>> +    if (f->fmt.pix_mp.width == 0 && f->fmt.pix_mp.height == 0) {
>> +        f->fmt.pix_mp.width = DEFAULT_WIDTH;
>> +        f->fmt.pix_mp.height = DEFAULT_HEIGHT;
>> +    }
>> +
>>       iris_vdec_try_fmt(inst, f);
>>
>>       switch (f->type) {
>> -- 
>
> Doesn't venus do
>
> orig_pixmap = *pixmap;
>
> try_fmt();
>
> format.fmt.pix_mp.width = orig_pixmp.width;
> format.fmt.pix_mp.height = orig_pixmp.height;
>
> should you fall back to DEFAULT_WIDTH/HEIGHT or to 
> orig_pixmp.width/height ? 

Hi– sorry for the late reply and happy holidays :)

The patch does the fallback *before* try_fmt().

I have just retested everything, even with yesterday's new "misc fixes" 
patchset 
(https://lore.kernel.org/all/20251224-iris-fixes-v1-0-5f79861700ec@oss.qualcomm.com/). 
The issue is still present, the 0x0 size is what iris_vdec_s_fmt gets 
before try_fmt().

What the rpi-ffmpeg developer said 
(https://github.com/jc-kynesim/rpi-ffmpeg/issues/103) is:

- V4L2 very clearly says that you should be able to pass anything into 
S_FMT and it should set the returned values to something valid
- S_FMT should not only not complain, it should return valid values in 
the structure passed to it which will then be used in the REQBUFS
- the buffers being allocated are for the coded bitstream (remember in 
V4L2 speak OUTPUT=source, CAPTURE=destination); width/height doesn't 
really mean a lot here but buffer size does
- 
https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-decoder.html 
4.5.1.5 states "width, height: coded resolution of the stream; required 
only if it cannot be parsed from the stream for the given coded format; 
otherwise the decoder will use this resolution as a placeholder 
resolution that will likely change as soon as it can parse the actual 
coded resolution from the stream"

So since this is a deliberate zero request, and V4L2 requires returning 
a valid placeholder resolution, it should be DEFAULT indeed.

I'll resend with a comment as requested by Dikshita.

~val