Re: [PATCH 03/10] staging: media: cedrus: h264: mark long-term references in the frame info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cedrus: h264: lost long-term flag in cedrus_write_frame_list()
--

commit 2185109fe892e8452d1ff6c43246f08d7ba7e368
Author: Jernej Skrabec <jernej.skrabec@gmail.com>

staging: media: cedrus: h264: mark long-term references in the frame info

This commit updates cedrus_fill_ref_pic() and cedrus_write_frame_list() to
populate the top and bottom field reference type bits in the H.264 SRAM
framebuffer list entry so that long-term reference pictures are marked as
long-term instead of short-term.

> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index 892193c12b2f3..a716955655f7d 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c

[ ... ]

> @@ -128,6 +136,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
>  		cedrus_fill_ref_pic(ctx, cedrus_buf,
>  				    dpb->top_field_order_cnt,
>  				    dpb->bottom_field_order_cnt,
> +				    dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM,
>  				    &pic_list[position]);
>  	}
>  
> @@ -174,6 +183,7 @@ static int cedrus_write_frame_list(struct cedrus_ctx *ctx,
>  	cedrus_fill_ref_pic(ctx, output_buf,
>  			    decode->top_field_order_cnt,
>  			    decode->bottom_field_order_cnt,
> +			    false,
>  			    &pic_list[position]);

[Severity: Medium]
When decoding the second field of an interlaced complementary field pair into
the same capture buffer (run->dst), the already-decoded first field is present
in decode->dpb[] with a matching timestamp. In cedrus_write_frame_list(), the
DPB loop skips calling cedrus_fill_ref_pic() for that entry:

	for (i = 0; i < ARRAY_SIZE(decode->dpb); i++) {
		...
		if (run->dst->vb2_buf.timestamp == dpb->reference_ts) {
			output = position;
			continue;
		}

and instead populates pic_list[position] via the cedrus_fill_ref_pic() call on
output_buf above with long_term hardcoded to false.

If the first field was marked as a long-term reference (for example, an IDR
field with long_term_reference_flag set or via MMCO), its DPB entry has
V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM set and the second field can reference it
at pic_list[position].

By passing false here instead of preserving the DPB entry's
V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM flag when output >= 0, won't
pic_list[position] be programmed as VE_H264_FRAME_INFO_REF_TYPE_SHORT_TERM
instead of VE_H264_FRAME_INFO_REF_TYPE_LONG_TERM when decoding the second
field?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102170303.1973892-1-jernej.skrabec@gmail.com?part=3