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

[Hendrik Leppkes]

On Fri, Jul 14, 2017 at 4:08 PM, foo86 <foobaz86 at gmail.com> wrote:
> On Thu, Jul 13, 2017 at 12:27:03PM +0200, Paul B Mahol wrote:
>> +static inline unsigned int get_bits(GetBitContext *s, int n)
>>  {
>> +#ifdef CACHED_BITSTREAM_READER
>> +    register int tmp = 0;
>> +#ifdef BITSTREAM_READER_LE
>> +    uint64_t left = 0;
>> +#endif
>> +
>> +    av_assert2(n>0 && n<=32);
>> +    if (n > s->bits_left) {
>> +        n -= s->bits_left;
>> +#ifdef BITSTREAM_READER_LE
>> +        left = s->bits_left;
>> +#endif
>> +        tmp = get_val(s, s->bits_left);
> This triggers an assert in get_val() if s->bits_left == 0.
>
>> +        refill_32(s);
>> +    }
>> +
>> +#ifdef BITSTREAM_READER_LE
>> +    tmp = get_val(s, n) << left | tmp;
>> +#else
>> +    tmp = get_val(s, n) | tmp << n;
> This causes undefined behavior if n > 30.

get_bits is only valid until n = 25 in the "non-cached" case, so its
not a problem to impose the same limitation on the cached reader.
In fact, if they are to share the exact same API, it should probably
follow that they also share the same constraints, so that we can do
proper performance comparisons between the two, instead of having to
re-write the using code.

- Hendrik