[FFmpeg-devel,4/4] lavc/get_bits: add a compat wrapper for the cached bitstream reader

Use that instead of the merged version.
---
 libavcodec/get_bits.h | 297 +++++++-----------------------------------
 1 file changed, 50 insertions(+), 247 deletions(-)

Anton Khirnov:
> Use that instead of the merged version.
> ---
>  libavcodec/get_bits.h | 297 +++++++-----------------------------------
>  1 file changed, 50 insertions(+), 247 deletions(-)
> 
> diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> index 9f2b1784d5..d9fc794c8c 100644
> --- a/libavcodec/get_bits.h
> +++ b/libavcodec/get_bits.h
> @@ -1,6 +1,5 @@
>  /*
>   * Copyright (c) 2004 Michael Niedermayer <michaelni@gmx.at>
> - * Copyright (c) 2016 Alexandra Hájková
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -59,12 +58,43 @@
>  #define CACHED_BITSTREAM_READER 0
>  #endif
>  
> +#if CACHED_BITSTREAM_READER
> +#include "bitstream.h"
> +
> +#define MIN_CACHE_BITS 64

Please don't define this. It makes no sense for the cached bitstream reader.
(And several of the uses of this macro for the other bitstream readers
seem highly dubious for me. The worst is bit_copy() in dvdec.c which
would simply not work (or even assert) when one uses a nondefault
getbit-API. apedec, eatgv, proresdec2, tta and wma use this to check
their input for compliance instead of basing their limits on the
underlying specifications.)

> +
> +typedef BitstreamContext GetBitContext;
> +
> +#define get_bits_count      bitstream_tell
> +#define get_bits_left       bitstream_bits_left
> +#define skip_bits_long      bitstream_skip
> +#define skip_bits           bitstream_skip
> +#define get_bits            bitstream_read

This will add a check for bits == 0 to every get_bits() for which the
compiler can't infer that it is not zero at compile-time. This is
unacceptable.

> +#define get_bitsz           bitstream_read
> +#define get_bits_le         bitstream_read

Patch 3/4 intended to make it possible to use both LE and BE in the same
file; and now you define this here. This is wrong and will break the
parts of utvideodec.c that use get_bits_le() (they are uncovered by FATE).

> +#define get_bits_long       bitstream_read
> +#define get_bits64          bitstream_read_63
> +#define get_xbits           bitstream_read_xbits
> +#define get_sbits           bitstream_read_signed
> +#define get_sbits_long      bitstream_read_signed
> +#define show_bits           bitstream_peek
> +#define show_bits_long      bitstream_peek
> +#define init_get_bits       bitstream_init
> +#define init_get_bits8      bitstream_init8
> +#define init_get_bits8_le   bitstream_init8_le
> +#define align_get_bits      bitstream_align
> +#define get_vlc2            bitstream_read_vlc
> +
> +#define get_bits1(s)        bitstream_read(s, 1)

Why not bitstream_read_bit?

> +#define show_bits1(s)       bitstream_peek(s, 1)
> +#define skip_bits1(s)       bitstream_skip(s, 1)
> +
> +#define skip_1stop_8data_bits bitstream_skip_1stop_8data
> +
> +#else   // CACHED_BITSTREAM_READER
> +
>  typedef struct GetBitContext {
>      const uint8_t *buffer, *buffer_end;
> -#if CACHED_BITSTREAM_READER
> -    uint64_t cache;
> -    unsigned bits_left;
> -#endif
>      int index;
>      int size_in_bits;
>      int size_in_bits_plus8;
> @@ -121,16 +151,12 @@ static inline unsigned int show_bits(GetBitContext *s, int n);
>   * For examples see get_bits, show_bits, skip_bits, get_vlc.
>   */
>  
> -#if CACHED_BITSTREAM_READER
> -#   define MIN_CACHE_BITS 64
> -#elif defined LONG_BITSTREAM_READER
> +#if defined LONG_BITSTREAM_READER
>  #   define MIN_CACHE_BITS 32
>  #else
>  #   define MIN_CACHE_BITS 25
>  #endif
>  
> -#if !CACHED_BITSTREAM_READER
> -
>  #define OPEN_READER_NOSIZE(name, gb)            \
>      unsigned int name ## _index = (gb)->index;  \
>      unsigned int av_unused name ## _cache
> @@ -215,73 +241,12 @@ static inline unsigned int show_bits(GetBitContext *s, int n);
>  
>  #define GET_CACHE(name, gb) ((uint32_t) name ## _cache)
>  
> -#endif
>  
>  static inline int get_bits_count(const GetBitContext *s)
>  {
> -#if CACHED_BITSTREAM_READER
> -    return s->index - s->bits_left;
> -#else
>      return s->index;
> -#endif
>  }
>  
> -#if CACHED_BITSTREAM_READER
> -static inline void refill_32(GetBitContext *s, int is_le)
> -{
> -#if !UNCHECKED_BITSTREAM_READER
> -    if (s->index >> 3 >= s->buffer_end - s->buffer)
> -        return;
> -#endif
> -
> -    if (is_le)
> -        s->cache = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) << s->bits_left | s->cache;
> -    else
> -        s->cache = s->cache | (uint64_t)AV_RB32(s->buffer + (s->index >> 3)) << (32 - s->bits_left);
> -    s->index     += 32;
> -    s->bits_left += 32;
> -}
> -
> -static inline void refill_64(GetBitContext *s, int is_le)
> -{
> -#if !UNCHECKED_BITSTREAM_READER
> -    if (s->index >> 3 >= s->buffer_end - s->buffer)
> -        return;
> -#endif
> -
> -    if (is_le)
> -        s->cache = AV_RL64(s->buffer + (s->index >> 3));
> -    else
> -        s->cache = AV_RB64(s->buffer + (s->index >> 3));
> -    s->index += 64;
> -    s->bits_left = 64;
> -}
> -
> -static inline uint64_t get_val(GetBitContext *s, unsigned n, int is_le)
> -{
> -    uint64_t ret;
> -    av_assert2(n>0 && n<=63);
> -    if (is_le) {
> -        ret = s->cache & ((UINT64_C(1) << n) - 1);
> -        s->cache >>= n;
> -    } else {
> -        ret = s->cache >> (64 - n);
> -        s->cache <<= n;
> -    }
> -    s->bits_left -= n;
> -    return ret;
> -}
> -
> -static inline unsigned show_val(const GetBitContext *s, unsigned n)
> -{
> -#ifdef BITSTREAM_READER_LE
> -    return s->cache & ((UINT64_C(1) << n) - 1);
> -#else
> -    return s->cache >> (64 - n);
> -#endif
> -}
> -#endif
> -
>  /**
>   * Skips the specified number of bits.
>   * @param n the number of bits to skip,
> @@ -291,28 +256,12 @@ static inline unsigned show_val(const GetBitContext *s, unsigned n)
>   */
>  static inline void skip_bits_long(GetBitContext *s, int n)
>  {
> -#if 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);
>  #endif
> -#endif
> -}
> -
> -#if CACHED_BITSTREAM_READER
> -static inline void skip_remaining(GetBitContext *s, unsigned n)
> -{
> -#ifdef BITSTREAM_READER_LE
> -    s->cache >>= n;
> -#else
> -    s->cache <<= n;
> -#endif
> -    s->bits_left -= n;
>  }
> -#endif
>  
>  /**
>   * Read MPEG-1 dc-style VLC (sign bit + mantissa with no MSB).
> @@ -321,13 +270,6 @@ static inline void skip_remaining(GetBitContext *s, unsigned n)
>   */
>  static inline int get_xbits(GetBitContext *s, int n)
>  {
> -#if CACHED_BITSTREAM_READER
> -    int32_t cache = show_bits(s, 32);
> -    int sign = ~cache >> 31;
> -    skip_remaining(s, n);
> -
> -    return ((((uint32_t)(sign ^ cache)) >> (32 - n)) ^ sign) - sign;
> -#else
>      register int sign;
>      register int32_t cache;
>      OPEN_READER(re, s);
> @@ -338,10 +280,8 @@ static inline int get_xbits(GetBitContext *s, int n)
>      LAST_SKIP_BITS(re, s, n);
>      CLOSE_READER(re, s);
>      return (NEG_USR32(sign ^ cache, n) ^ sign) - sign;
> -#endif
>  }
>  
> -#if !CACHED_BITSTREAM_READER
>  static inline int get_xbits_le(GetBitContext *s, int n)
>  {
>      register int sign;
> @@ -355,22 +295,16 @@ static inline int get_xbits_le(GetBitContext *s, int n)
>      CLOSE_READER(re, s);
>      return (zero_extend(sign ^ cache, n) ^ sign) - sign;
>  }
> -#endif
>  
>  static inline int get_sbits(GetBitContext *s, int n)
>  {
>      register int tmp;
> -#if CACHED_BITSTREAM_READER
> -    av_assert2(n>0 && n<=25);
> -    tmp = sign_extend(get_bits(s, n), n);
> -#else
>      OPEN_READER(re, s);
>      av_assert2(n>0 && n<=25);
>      UPDATE_CACHE(re, s);
>      tmp = SHOW_SBITS(re, s, n);
>      LAST_SKIP_BITS(re, s, n);
>      CLOSE_READER(re, s);
> -#endif
>      return tmp;
>  }
>  
> @@ -380,32 +314,12 @@ static inline int get_sbits(GetBitContext *s, int n)
>  static inline unsigned int get_bits(GetBitContext *s, int n)
>  {
>      register unsigned int tmp;
> -#if CACHED_BITSTREAM_READER
> -
> -    av_assert2(n>0 && n<=32);
> -    if (n > s->bits_left) {
> -#ifdef BITSTREAM_READER_LE
> -        refill_32(s, 1);
> -#else
> -        refill_32(s, 0);
> -#endif
> -        if (s->bits_left < 32)
> -            s->bits_left = n;
> -    }
> -
> -#ifdef BITSTREAM_READER_LE
> -    tmp = get_val(s, n, 1);
> -#else
> -    tmp = get_val(s, n, 0);
> -#endif
> -#else
>      OPEN_READER(re, s);
>      av_assert2(n>0 && n<=25);
>      UPDATE_CACHE(re, s);
>      tmp = SHOW_UBITS(re, s, n);
>      LAST_SKIP_BITS(re, s, n);
>      CLOSE_READER(re, s);
> -#endif
>      av_assert2(tmp < UINT64_C(1) << n);
>      return tmp;
>  }
> @@ -420,16 +334,6 @@ static av_always_inline int get_bitsz(GetBitContext *s, int n)
>  
>  static inline unsigned int get_bits_le(GetBitContext *s, int n)
>  {
> -#if CACHED_BITSTREAM_READER
> -    av_assert2(n>0 && n<=32);
> -    if (n > s->bits_left) {
> -        refill_32(s, 1);
> -        if (s->bits_left < 32)
> -            s->bits_left = n;
> -    }
> -
> -    return get_val(s, n, 1);
> -#else
>      register int tmp;
>      OPEN_READER(re, s);
>      av_assert2(n>0 && n<=25);
> @@ -438,7 +342,6 @@ static inline unsigned int get_bits_le(GetBitContext *s, int n)
>      LAST_SKIP_BITS(re, s, n);
>      CLOSE_READER(re, s);
>      return tmp;
> -#endif
>  }
>  
>  /**
> @@ -447,71 +350,22 @@ static inline unsigned int get_bits_le(GetBitContext *s, int n)
>  static inline unsigned int show_bits(GetBitContext *s, int n)
>  {
>      register unsigned int tmp;
> -#if CACHED_BITSTREAM_READER
> -    if (n > s->bits_left)
> -#ifdef BITSTREAM_READER_LE
> -        refill_32(s, 1);
> -#else
> -        refill_32(s, 0);
> -#endif
> -
> -    tmp = show_val(s, n);
> -#else
>      OPEN_READER_NOSIZE(re, s);
>      av_assert2(n>0 && n<=25);
>      UPDATE_CACHE(re, s);
>      tmp = SHOW_UBITS(re, s, n);
> -#endif
>      return tmp;
>  }
>  
>  static inline void skip_bits(GetBitContext *s, int n)
>  {
> -#if CACHED_BITSTREAM_READER
> -    if (n < s->bits_left)
> -        skip_remaining(s, n);
> -    else {
> -        n -= s->bits_left;
> -        s->cache = 0;
> -        s->bits_left = 0;
> -
> -        if (n >= 64) {
> -            unsigned skip = (n / 8) * 8;
> -
> -            n -= skip;
> -            s->index += skip;
> -        }
> -#ifdef BITSTREAM_READER_LE
> -        refill_64(s, 1);
> -#else
> -        refill_64(s, 0);
> -#endif
> -        if (n)
> -            skip_remaining(s, n);
> -    }
> -#else
>      OPEN_READER(re, s);
>      LAST_SKIP_BITS(re, s, n);
>      CLOSE_READER(re, s);
> -#endif
>  }
>  
>  static inline unsigned int get_bits1(GetBitContext *s)
>  {
> -#if CACHED_BITSTREAM_READER
> -    if (!s->bits_left)
> -#ifdef BITSTREAM_READER_LE
> -        refill_64(s, 1);
> -#else
> -        refill_64(s, 0);
> -#endif
> -
> -#ifdef BITSTREAM_READER_LE
> -    return get_val(s, 1, 1);
> -#else
> -    return get_val(s, 1, 0);
> -#endif
> -#else
>      unsigned int index = s->index;
>      uint8_t result     = s->buffer[index >> 3];
>  #ifdef BITSTREAM_READER_LE
> @@ -528,7 +382,6 @@ static inline unsigned int get_bits1(GetBitContext *s)
>      s->index = index;
>  
>      return result;
> -#endif
>  }
>  
>  static inline unsigned int show_bits1(GetBitContext *s)
> @@ -549,10 +402,6 @@ static inline unsigned int get_bits_long(GetBitContext *s, int n)
>      av_assert2(n>=0 && n<=32);
>      if (!n) {
>          return 0;
> -#if CACHED_BITSTREAM_READER
> -    }
> -    return get_bits(s, n);
> -#else
>      } else if (n <= MIN_CACHE_BITS) {
>          return get_bits(s, n);
>      } else {
> @@ -564,7 +413,6 @@ static inline unsigned int get_bits_long(GetBitContext *s, int n)
>          return ret | get_bits(s, n - 16);
>  #endif
>      }
> -#endif
>  }
>  
>  /**
> @@ -610,8 +458,17 @@ static inline unsigned int show_bits_long(GetBitContext *s, int n)
>      }
>  }
>  
> -static inline int init_get_bits_xe(GetBitContext *s, const uint8_t *buffer,
> -                                   int bit_size, int is_le)
> +
> +/**
> + * Initialize GetBitContext.
> + * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes
> + *        larger than the actual read bits because some optimized bitstream
> + *        readers read 32 or 64 bit at once and could read over the end
> + * @param bit_size the size of the buffer in bits
> + * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would overflow.
> + */
> +static inline int init_get_bits(GetBitContext *s, const uint8_t *buffer,
> +                                int bit_size)
>  {
>      int buffer_size;
>      int ret = 0;
> @@ -630,33 +487,9 @@ static inline int init_get_bits_xe(GetBitContext *s, const uint8_t *buffer,
>      s->buffer_end         = buffer + buffer_size;
>      s->index              = 0;
>  
> -#if CACHED_BITSTREAM_READER
> -    s->cache              = 0;
> -    s->bits_left          = 0;
> -    refill_64(s, is_le);
> -#endif
> -
>      return ret;
>  }
>  
> -/**
> - * Initialize GetBitContext.
> - * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes
> - *        larger than the actual read bits because some optimized bitstream
> - *        readers read 32 or 64 bit at once and could read over the end
> - * @param bit_size the size of the buffer in bits
> - * @return 0 on success, AVERROR_INVALIDDATA if the buffer_size would overflow.
> - */
> -static inline int init_get_bits(GetBitContext *s, const uint8_t *buffer,
> -                                int bit_size)
> -{
> -#ifdef BITSTREAM_READER_LE
> -    return init_get_bits_xe(s, buffer, bit_size, 1);
> -#else
> -    return init_get_bits_xe(s, buffer, bit_size, 0);
> -#endif
> -}
> -
>  /**
>   * Initialize GetBitContext.
>   * @param buffer bitstream buffer, must be AV_INPUT_BUFFER_PADDING_SIZE bytes
> @@ -678,7 +511,7 @@ static inline int init_get_bits8_le(GetBitContext *s, const uint8_t *buffer,
>  {
>      if (byte_size > INT_MAX / 8 || byte_size < 0)
>          byte_size = -1;
> -    return init_get_bits_xe(s, buffer, byte_size * 8, 1);
> +    return init_get_bits(s, buffer, byte_size * 8);
>  }
>  
>  static inline const uint8_t *align_get_bits(GetBitContext *s)
> @@ -763,19 +596,6 @@ static inline const uint8_t *align_get_bits(GetBitContext *s)
>          SKIP_BITS(name, gb, n);                                 \
>      } while (0)
>  
> -/* Return the LUT element for the given bitstream configuration. */
> -static inline int set_idx(GetBitContext *s, int code, int *n, int *nb_bits,
> -                          const VLCElem *table)
> -{
> -    unsigned idx;
> -
> -    *nb_bits = -*n;
> -    idx = show_bits(s, *nb_bits) + code;
> -    *n = table[idx].len;
> -
> -    return table[idx].sym;
> -}
> -
>  /**
>   * Parse a vlc code.
>   * @param bits is the number of bits which will be read at once, must be
> @@ -788,24 +608,6 @@ static inline int set_idx(GetBitContext *s, int code, int *n, int *nb_bits,
>  static av_always_inline int get_vlc2(GetBitContext *s, const VLCElem *table,
>                                       int bits, int max_depth)
>  {
> -#if CACHED_BITSTREAM_READER
> -    int nb_bits;
> -    unsigned idx = show_bits(s, bits);
> -    int code = table[idx].sym;
> -    int n    = table[idx].len;
> -
> -    if (max_depth > 1 && n < 0) {
> -        skip_remaining(s, bits);
> -        code = set_idx(s, code, &n, &nb_bits, table);
> -        if (max_depth > 2 && n < 0) {
> -            skip_remaining(s, nb_bits);
> -            code = set_idx(s, code, &n, &nb_bits, table);
> -        }
> -    }
> -    skip_remaining(s, n);
> -
> -    return code;
> -#else
>      int code;
>  
>      OPEN_READER(re, s);
> @@ -816,7 +618,6 @@ static av_always_inline int get_vlc2(GetBitContext *s, const VLCElem *table,
>      CLOSE_READER(re, s);
>  
>      return code;
> -#endif
>  }
>  
>  static inline int decode012(GetBitContext *gb)
> @@ -856,4 +657,6 @@ static inline int skip_1stop_8data_bits(GetBitContext *gb)
>      return 0;
>  }
>  
> +#endif // CACHED_BITSTREAM_READER
> +
>  #endif /* AVCODEC_GET_BITS_H */