media: rkvdec: set ctx->image_fmt in rkvdec_s_capture_fmt

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

Posted by Jianfeng Liu 1 day, 10 hours ago

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.

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;
+
 	return 0;
 }
 
-- 
2.43.0

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

Posted by Nicolas Dufresne 1 day, 8 hours ago

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

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

Posted by Jianfeng Liu 20 hours ago

Hi,

On Fri, 12 Dec 2025 12:53:53 -0500, Nicolas Dufresne wrote:
>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.

Chromium does follow the spec when decoding 10bit videos[1], but that is
still limited to hevc and vp9. For h264 and 8bit video chromium thinks
this is unnecessary.

>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).

I will try to fix chromium to follow the spec. Since rkvdec driver is the
only driver related, I didn't check the stateless spec.

[1]https://github.com/chromium/chromium/blob/145.0.7577.2/media/gpu/v4l2/v4l2_video_decoder.cc#L648-L650

Best regards,
Jianfeng

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

Posted by Nicolas Dufresne 11 hours ago

Hi,

Le samedi 13 décembre 2025 à 13:59 +0800, Jianfeng Liu a écrit :
> Chromium does follow the spec when decoding 10bit videos[1], but that is
> still limited to hevc and vp9. For h264 and 8bit video chromium thinks
> this is unnecessary.

thanks for the pointer, that explains a lot my surprise and slight confusion.
Now that you mention, on relevant chromebooks, 10bit is only supported with
HEVC/VP9 (perhaps AV1 too, not sure).

Nicolas

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

Posted by Jonas Karlman 1 day, 9 hours ago

Hi,

On 12/12/2025 4:41 PM, Jianfeng Liu wrote:
> 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.
> 
> 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;

ctx->image_fmt is used to limit what CAPTURE pixel format can be set and
forcing it here violates this.

Chromium should be fixed to follow spec at 4.5.3.2. Initialization [1]:

  1. Set the coded format on the OUTPUT queue via VIDIOC_S_FMT().
  2. Call VIDIOC_S_EXT_CTRLS() to set all the controls (parsed headers,
     etc.) required by the OUTPUT format to enumerate the CAPTURE formats.
  ...
  5. [optional] Choose a different CAPTURE format than suggested via
     VIDIOC_S_FMT() on CAPTURE queue.

Regards,
Jonas

[1] https://docs.kernel.org/userspace-api/media/v4l/dev-stateless-decoder.html

> +
>  	return 0;
>  }
>

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

Posted by Jianfeng Liu 20 hours ago

On Fri, 12 Dec 2025 17:43:35 +0100, Jonas Karlman wrote:
>Chromium should be fixed to follow spec at 4.5.3.2. Initialization [1]:

Hi Jonas,

Thank you for guiding me to the spec, I will look into chromium and try
to fix it.

Best regards,
Jianfeng