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

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

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

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 :
> > Hi,
> > 
> > On Thu, 18 Dec 2025 18:28:24 -0500, Detlev Casanova wrote:
> > > +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
> 
> > 
> > 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

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