Re: [PATCH] media: cedrus: hevc: Add check for invalid timestamp

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

Le lundi 18 juillet 2022 à 18:56 +0200, Jernej Skrabec a écrit :
> Not all DPB entries will be used most of the time. Unused entries will
> thus have invalid timestamps. They will produce negative buffer index
> which is not specifically handled. This works just by chance in current
> code. It will even produce bogus pointer, but since it's not used, it
> won't do any harm.
> 
> Let's fix that brittle design by skipping writing DPB entry altogether
> if timestamp is invalid.
> 
> Fixes: 86caab29da78 ("media: cedrus: Add HEVC/H.265 decoding support")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_h265.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> index 1afc6797d806..687f87598f78 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -147,6 +147,9 @@ static void cedrus_h265_frame_info_write_dpb(struct cedrus_ctx *ctx,
>  			dpb[i].pic_order_cnt_val
>  		};
>  
> +		if (buffer_index < 0)
> +			continue;

When I compare to other codecs, when the buffer_index does not exist, the addr 0
is being programmed into the HW. With this implementation is is left to whatever
it was set for the previous decode operation. I think its is nicer done the
other way.

> +
>  		cedrus_h265_frame_info_write_single(ctx, i, dpb[i].field_pic,
>  						    pic_order_cnt,
>  						    buffer_index);