Re: [PATCH] media: rkvdec: set ctx->image_fmt in rkvdec_s_capture_fmt

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

[-- Attachment #1: Type: text/plain, Size: 2875 bytes --]

Hi,

Le vendredi 12 décembre 2025 à 23:41 +0800, Jianfeng Liu a écrit :
> ctx->image_fmt is initialized as RKVDEC_IMG_FMT_ANY at
> rkvdec_s_output_fmt, and get set at rkvdec_s_ctrl when userspace sends
> SPS info via VIDIOC_S_EXT_CTRLS. This works fine with gstreamer because
> it sends SPS info to kernel driver before requesting capture queue bufs.
> 
> But chromium requests capture queue bufs first and then sends SPS info
> to kernel, then rkvdec_s_ctrl will return -EBUSY, and the video is
> displayed green.
> 
> Chromium calls VIDIOC_S_FMT to capture queue instead before requesting
> capture queue bufs, so setting ctx->image_fmt in rkvdec_s_capture_fmt
> will make rkvdec_s_ctrl return 0 when the first SPS info sent to driver.

I'm reading you here, and reading the spec again [0], and you are saying the
Chromium isn't following the initialization process. That means, it should be
impossible to allocate 10bit capture buffer, since the SPS in place specify the
color depth. Other parameters in other codec can influence the reference buffer
size, so you could endup with the wrong allocation.


[0] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-stateless-decoder.html#initialization


> 
> Signed-off-by: Jianfeng Liu <liujianfeng1994@gmail.com>
> Fixes: d35c64eccf3b1 ("media: rkvdec: Add get_image_fmt ops")
> ---
> 
>  drivers/media/platform/rockchip/rkvdec/rkvdec.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> index 5af9aa5ab353..e7939d604f64 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -537,6 +537,18 @@ static int rkvdec_s_capture_fmt(struct file *file, void
> *priv,
>  		return ret;
>  
>  	ctx->decoded_fmt = *f;
> +
> +	u32 fourcc = f->fmt.pix_mp.pixelformat;
> +
> +	if (fourcc == V4L2_PIX_FMT_NV12)
> +		ctx->image_fmt = RKVDEC_IMG_FMT_420_8BIT;
> +	else if (fourcc == V4L2_PIX_FMT_NV15)
> +		ctx->image_fmt = RKVDEC_IMG_FMT_420_10BIT;
> +	else if (fourcc == V4L2_PIX_FMT_NV16)
> +		ctx->image_fmt = RKVDEC_IMG_FMT_422_8BIT;
> +	else if (fourcc == V4L2_PIX_FMT_NV20)
> +		ctx->image_fmt = RKVDEC_IMG_FMT_422_10BIT;

You cannot accept a format that falls against the color depth in the SPS
control, and rkvdec does not do color conversion implicitly. This can otherwise
lead to letting the HW overrun the buffer (forcing 8bit with 10bit content). Can
you check with Chromium dev if they can perhaps adhere to the spec instead ?
This is all news to me, but I probably never had to test 10bit playback in
Chromium outside of MTK (which might not be less strict, hopefully done the
right way).

regards,
Nicolas

> +
>  	return 0;
>  }
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]