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.
> +#endif
> +
> +#else
> register int tmp;
> OPEN_READER(re, s);
> av_assert2(n>0 && n<=25);
> UPDATE_CACHE(re, s);
> - tmp = SHOW_SBITS(re, s, n);
> + tmp = SHOW_UBITS(re, s, n);
> LAST_SKIP_BITS(re, s, n);
> CLOSE_READER(re, s);
> +#endif
> return tmp;
> }
The code under #ifdef CACHED_BITSTREAM_READER can probably be simplified
like this (analogous to show_bits()):
if (n > s->bits_left)
refill_32(s);
tmp = get_val(s, n);
This avoids UB and is simpler/faster. Or am I missing something here?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
