Thread (26 messages) 26 messages, 6 authors, 2025-12-28

Re: [PATCH v7 14/17] media: rkvdec: Add H264 support for the VDPU381 variant

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date: 2025-12-24 19:45:13
Also in: linux-media, linux-rockchip, lkml

Hi Detlev,

Le lundi 22 décembre 2025 à 08:19 -0500, Nicolas Dufresne a écrit :
Hi,

Le dimanche 21 décembre 2025 à 00:46 +0800, Jianfeng Liu a écrit :
quoted
Hi,

On Thu, 18 Dec 2025 18:28:24 -0500, Detlev Casanova wrote:
quoted
+static int rkvdec_h264_start(struct rkvdec_ctx *ctx)
+{
+	struct rkvdec_dev *rkvdec = ctx->dev;
+	struct rkvdec_h264_priv_tbl *priv_tbl;
+	struct rkvdec_h264_ctx *h264_ctx;
+	struct v4l2_ctrl *ctrl;
+	int ret;
+
+	ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl,
+			      V4L2_CID_STATELESS_H264_SPS);
+	if (!ctrl)
+		return -EINVAL;
+
+	h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL);
+	if (!h264_ctx)
+		return -ENOMEM;
I can see the sps validation is removed:

	ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps);
	if (ret)
		return ret;

This should fix decoding issue with chromium when minimum size of h264
decoder is lower than 32. While I find this issue is caused by chromium
not following v4l2 stateless spec, and I think checking sps at start
should be still necessary.
One of the missing part of rkvdec is that once you set the OUTPUT format we only
reset the capture format, but we should also reset the SPS control to a matching
default, matching the resolution and resetting the depth. In fact, I don't even
see code that verify the bit depth against already allocated memory, and the is
not capable of converting.

I tend to agree with Jianfeng, that we should leave that un-modified until we
have time to fully address and test this situation.
I digged more, and by removing this check, userspace would endup doing:

- S_FMT(OUT, 800x600)
- S_CTRL(SPS, 800x600)
- S_FMT(OUT, 640x480)

And still succeed streamon with that SPS. Since the SPS rarely changes, and
there is no requirement to send it again, a frame decode can be requested, and
this miss-match can trigger the hardware to possibly overrun.

For this reason, I cannot take v7.

Nicolas
Nicolas
quoted
I have sent a fix to chromium[1] and it should get merged later.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/7274555

Best regards,
Jianfeng

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help