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

[Paul B Mahol]

On 7/8/17, foo86 <foobaz86 at gmail.com> wrote:
> 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.

I will do same in skip_bits_long(). No need to change anything in codecs.