[FFmpeg-devel] [PATCH 1/3] avcodec/get_bits: add cached bitstream reader

[foo86]

On Sat, Jul 08, 2017 at 08:07:49PM +0200, Hendrik Leppkes wrote:
> On Sat, Jul 8, 2017 at 7:09 PM, foo86 <foobaz86 at gmail.com> wrote:
> >> +static inline void skip_bits_long(GetBitContext *s, int n)
> >> +{
> >> +#ifdef CACHED_BITSTREAM_READER
> >> +    skip_bits(s, n);
> >> +#else
> >> +#if UNCHECKED_BITSTREAM_READER
> >> +    s->index += n;
> >> +#else
> >> +    s->index += av_clip(n, -s->index, s->size_in_bits_plus8 - s->index);
> > Uncached bitstream reader allows seeking back by passing negative n
> > here. If cached bitstream reader disallows this, there should be a
> > comment saying so (and possibly an assert).
> 
> This seems like an undocumented and possibly insecure/invalid use of
> the API, maybe we should just generally discourage such use.

If skip_bits_long() is not supposed to seek backward, then it's fine by
me. (I thought it was looking at its implementation which allows
negative n).

> Why would you need to skip backwards anyway? Usually code uses
> show_bits, or creates a copy of the reader so it can revert to the
> original if needed.

DCA XLL decoder needs to seek backward to recover from segment overread.
I will probably change it to create a copy of the GetBitContext as you
suggested, seems to be a better solution.