[FFmpeg-devel] [PATCH 4/5] lavc/bitstream: make skip_remaining() public

[Andreas Rheinhardt]

Anton Khirnov:
> Also rename it to bitstream_skip_cache(), which is more descriptive and
> follows the naming conventions of tis API.
> ---
>  libavcodec/bitstream.h          |  1 +
>  libavcodec/bitstream_template.h | 19 ++++++++++++-------
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/bitstream.h b/libavcodec/bitstream.h
> index 3fa63695d3..364245453b 100644
> --- a/libavcodec/bitstream.h
> +++ b/libavcodec/bitstream.h
> @@ -116,6 +116,7 @@ static inline void bitstream_unget(BitstreamContext *bc, uint64_t value,
>  #define bitstream_peek          bitstream_peek_be
>  #define bitstream_peek_signed   bitstream_peek_signed_be
>  #define bitstream_skip          bitstream_skip_be
> +#define bitstream_skip_cache    bitstream_skip_cache_be
>  #define bitstream_seek          bitstream_seek_be
>  #define bitstream_align         bitstream_align_be
>  #define bitstream_read_xbits    bitstream_read_xbits_be
> diff --git a/libavcodec/bitstream_template.h b/libavcodec/bitstream_template.h
> index 14a420139c..717473ca40 100644
> --- a/libavcodec/bitstream_template.h
> +++ b/libavcodec/bitstream_template.h
> @@ -225,7 +225,12 @@ static inline int BS_FUNC(bitstream_peek_signed)(BitstreamContext *bc, unsigned
>      return sign_extend(BS_FUNC(bitstream_peek)(bc, n), n);
>  }
>  
> -static inline void BS_FUNC(skip_remaining)(BitstreamContext *bc, unsigned int n)
> +/**
> + * Skip n bits from the cache. This may only be called if at least n bits are
> + * guaranteed to be in the cache, e.g. right after bitstream_peek(n).
> + * Otherwise use bitstream_skip().
> + */
> +static inline void BS_FUNC(bitstream_skip_cache)(BitstreamContext *bc, unsigned int n)
>  {
>  #ifdef BITSTREAM_LE
>      bc->bits >>= n;
> @@ -241,7 +246,7 @@ static inline void BS_FUNC(skip_remaining)(BitstreamContext *bc, unsigned int n)
>  static inline void BS_FUNC(bitstream_skip)(BitstreamContext *bc, unsigned int n)
>  {
>      if (n < bc->bits_left)
> -        BS_FUNC(skip_remaining)(bc, n);
> +        BS_FUNC(bitstream_skip_cache)(bc, n);
>      else {
>          n -= bc->bits_left;
>          bc->bits      = 0;
> @@ -255,7 +260,7 @@ static inline void BS_FUNC(bitstream_skip)(BitstreamContext *bc, unsigned int n)
>          }
>          BS_FUNC(refill_64)(bc);
>          if (n)
> -            BS_FUNC(skip_remaining)(bc, n);
> +            BS_FUNC(bitstream_skip_cache)(bc, n);
>      }
>  }
>  
> @@ -291,7 +296,7 @@ static inline int BS_FUNC(bitstream_read_xbits)(BitstreamContext *bc, unsigned i
>  {
>      int32_t cache = BS_FUNC(bitstream_peek)(bc, 32);
>      int sign = ~cache >> 31;
> -    BS_FUNC(skip_remaining)(bc, n);
> +    BS_FUNC(bitstream_skip_cache)(bc, n);
>  
>      return ((((uint32_t)(sign ^ cache)) >> (32 - n)) ^ sign) - sign;
>  }
> @@ -374,14 +379,14 @@ static inline int BS_FUNC(bitstream_read_vlc)(BitstreamContext *bc, VLC_TYPE (*t
>      int n        = table[idx][1];
>  
>      if (max_depth > 1 && n < 0) {
> -        BS_FUNC(skip_remaining)(bc, bits);
> +        BS_FUNC(bitstream_skip_cache)(bc, bits);
>          code = BS_FUNC(set_idx)(bc, code, &n, &nb_bits, table);
>          if (max_depth > 2 && n < 0) {
> -            BS_FUNC(skip_remaining)(bc, nb_bits);
> +            BS_FUNC(bitstream_skip_cache)(bc, nb_bits);
>              code = BS_FUNC(set_idx)(bc, code, &n, &nb_bits, table);
>          }
>      }
> -    BS_FUNC(skip_remaining)(bc, n);
> +    BS_FUNC(bitstream_skip_cache)(bc, n);
>  
>      return code;
>  }

How do you intend to avoid the issue described here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297850.html?
Bitstream_peek() does not guarantee that there are bits bits in the
cache even though your documentation says otherwise.

- Andreas