On Fri, Jul 14, 2017 at 4:08 PM, foo86 <fooba...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
