Re: [PATCH 06/10] staging: media: cedrus: mpeg2: fix the references of field pictures

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

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

Hi,

Le lundi 02 novembre 2026 à 18:02 +0100, Jernej Skrabec a écrit :
> The second field of an I or P frame can be predicted from the first
> field of the same frame, which the hardware takes from the backward
> reference. The driver programs the backward reference passed by
> userspace instead, which doesn't point to the frame being decoded, so
> the second field is predicted from the wrong picture.
> 
> Use the current frame as the backward reference of I and P pictures,
> like the vendor library does. Also fall back to the current frame
> instead of address 0 when a reference buffer is missing, so the
> hardware never reads from an invalid address.
> 
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 29 ++++++++++++++-----
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> index 4c8accee5389..0a2f8e77fb29 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -56,6 +56,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  	dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
>  	struct cedrus_dev *dev = ctx->dev;
>  	struct vb2_queue *vq;
> +	struct vb2_buffer *fwd, *bwd;
>  	const u8 *matrix;
>  	unsigned int i;
>  	bool field, tff;
> @@ -147,15 +148,29 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
>  
>  	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
>  
> -	/* Forward and backward prediction reference buffers. */
> +	/*
> +	 * Forward and backward prediction reference buffers. As the vendor
> +	 * library does, I and P pictures use the current frame as backward
> +	 * reference: the second field of a frame predicts from the first one.
> +	 * Missing references also fall back to the current frame.
> +	 */
>  	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  
> -	cedrus_write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
> -				  VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
> -				  VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
> -	cedrus_write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
> -				  VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
> -				  VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
> +	fwd = vb2_find_buffer(vq, pic->forward_ref_ts) ?: &run->dst->vb2_buf;
> +	if (pic->picture_coding_type == V4L2_MPEG2_PIC_CODING_TYPE_B)
> +		bwd = vb2_find_buffer(vq, pic->backward_ref_ts) ?:
> +		      &run->dst->vb2_buf;
> +	else
> +		bwd = &run->dst->vb2_buf;
> +
> +	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, fwd, 0));
> +	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, fwd, 1));
> +	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, bwd, 0));
> +	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR,
> +		     cedrus_dst_buf_addr(ctx, bwd, 1));
>  
>  	/* Destination luma and chroma buffers. */
>  

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