Re: [FFmpeg-devel] [PATCH v5] avcodec/cbs_vp8: Add support for VP8 codec bitstream

["Dai, Jianhui J" <jianhui.j.dai-at-intel.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Ronald S. Bultje
> Sent: Monday, November 6, 2023 10:48 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v5] avcodec/cbs_vp8: Add support for
> VP8 codec bitstream
> 
> Hi,
> 
> On Sun, Nov 5, 2023 at 7:56 PM Dai, Jianhui J < jianhui.j.dai-at-
> intel.com@ffmpeg.org> wrote:
> 
> > This commit adds support for VP8 bitstream read methods to the cbs
> > codec. This enables the trace_headers bitstream filter to support VP8,
> > in addition to AV1, H.264, H.265, and VP9. This can be useful for
> > debugging VP8 stream issues.
> >
> > The CBS VP8 implements a simple VP8 boolean decoder using
> > GetBitContext to read the bitstream.
> >
> 
> Is it possible to re-use the existing vp56rac decoder for this? Having two
> arithmetic bool decoders that do the same thing is a bit weird.
> 

Thank for your commentary.

The existing RAC decoder cannot be reused here because:
1. The VPXRangeCoder is optimized for performance, that it always read one more byte. However, it does not apply bounds checking to prevent overreads. 
    It was suggested to implement this simply boolean reader, in the comment of patch v1 and v2.
2. Additionally, all CBS have to use GetBitContext to read bitstream and trace the syntax elements.

> 
> > +static const uint8_t vp8_token_update_probs[4][8][3][11] = {
> >
> 
> It would be nice if these symbols could be re-used from the existing vp8
> native decoder, instead of duplicating them? Both source + binary size are
> relevant here.

Including vp8data.h would introduce many unwanted static tables other than vp8_token_update_probs and increase the binary size. 
As suggested in patch v3, it is better to use local defined vp8_token_update_probs.

> 
> I'm also wondering if - longer-term - it makes sense to try to merge some of
> these concepts back into the native decoders, objects like
> Vp8RawFameHeader, but I'm guessing that's not super-urgent...

Thanks. That can be reused in the future.

> 
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".