According to 4.5.1.5 of the M2M stateful decoder UAPI documentation,
providing the width and the height is "required only if it cannot be
parsed from the stream", otherwise zero can be passed.

The iris driver would only set the state fields to DEFAULT_WIDTH/HEIGHT
once upon init, but they would get overwritten with zeroes from one S_FMT
call and the next S_FMT call would already see zeroes in place of the
defaults. For clients that used that sequence and did not pass a size,
such as rpi-ffmpeg, this would then result in REQBUFS failing due to
the zero size remembered in the state.

Fix by explicitly falling back to the defaults in S_FMT.

Fixes: b530b95de22c ("media: iris: implement s_fmt, g_fmt and try_fmt ioctls")
Link: https://github.com/jc-kynesim/rpi-ffmpeg/issues/103
Signed-off-by: Val Packett <val@packett.cool>
---
Somehow Venus didn't have this issue and didn't explicitly handle this..

I'm not familiar with this code so if there's a better way to comply
with the UAPI requirements by just not even getting to overwrite the
state with the provided 0 size, I could not figure it out.

Still, let's get this fixed one way or another.

Thanks,
~val
---
 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) {
-- 
2.51.0

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 ?

---
bod

On 10/13/2025 5:20 AM, Val Packett wrote:
> According to 4.5.1.5 of the M2M stateful decoder UAPI documentation,
> providing the width and the height is "required only if it cannot be
> parsed from the stream", otherwise zero can be passed.
> 
> The iris driver would only set the state fields to DEFAULT_WIDTH/HEIGHT
> once upon init, but they would get overwritten with zeroes from one S_FMT
> call and the next S_FMT call would already see zeroes in place of the
> defaults. For clients that used that sequence and did not pass a size,
> such as rpi-ffmpeg, this would then result in REQBUFS failing due to
> the zero size remembered in the state.
> 
> Fix by explicitly falling back to the defaults in S_FMT.
> 
> Fixes: b530b95de22c ("media: iris: implement s_fmt, g_fmt and try_fmt ioctls")
> Link: https://github.com/jc-kynesim/rpi-ffmpeg/issues/103
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> Somehow Venus didn't have this issue and didn't explicitly handle this..

In venus driver, try_fmt is clamping the resolution between min and max
supported which might be handling this case.

> 
> I'm not familiar with this code so if there's a better way to comply
> with the UAPI requirements by just not even getting to overwrite the
> state with the provided 0 size, I could not figure it out.
> 
> Still, let's get this fixed one way or another.
> 
> Thanks,
> ~val
> ---
>  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;
>  

Can you pls add a small comment here.

with that,

Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

> +	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) {