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

[Val Packett]

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@xxxxxxxxxxxxxxxx/). 
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