Dne ponedeljek, 18. julij 2022 ob 19:41:48 CEST je Nicolas Dufresne
napisal(a):
> 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@xxxxxxxxx>
> > ---
> >
> > 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.
It's done the same way as it's done in vendor lib. As I stated in commit
message, actual values don't matter for unused entries. If it is used by
accident, HW reaction on all zero pointers can only be worse than using old,
but valid entry.
Due to no real documentation and Allwinner unwillingness to share details,
we'll probably never know what's best response for each error. Some things can
be deduced from vendor code, but not all.
I would rather not complicate this fix, especially since it's candidate for
backporting.
Best regards,
Jernej
>
> > +
> >
> > cedrus_h265_frame_info_write_single(ctx, i,
dpb[i].field_pic,
> >
> >
pic_order_cnt,
> >
buffer_index);
