diff mbox series

[FFmpeg-devel] avcodec/get_bits: Avoid reading multiple times in get_bits_long

Message ID AS8P250MB0744DBC515F3FDD40747950E8FEEA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit b0fb8e82dde375efb8d5602f6cd479f210c1e93c
Headers show
Series [FFmpeg-devel] avcodec/get_bits: Avoid reading multiple times in get_bits_long | expand

Commit Message

Andreas Rheinhardt Sept. 7, 2023, 5:16 p.m. UTC 
Due to non-byte-alignment a read of 32 bits guarantees only
25 usable bits; therefore get_bits_long() (which is used to
potentially read more than 25 bits) performs two reads in case
it needs to read more than 25 bits and combines the result.

Yet this is not necessary: One can just read 64 bits at a time
to get 32 usable bits (57 would be possible). This commit does so.

This reduced the size of .text by 30144B for GCC 11.4 and 5648B
for Clang 14 (both with -O3).

(get_bits_long() is a building block of show_bits_long()
and get_ue_golomb_long(), so this patch affects these, too.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/get_bits.h | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

Comments

Andreas Rheinhardt Sept. 10, 2023, 10:20 a.m. UTC | #1 
Andreas Rheinhardt:
> Due to non-byte-alignment a read of 32 bits guarantees only
> 25 usable bits; therefore get_bits_long() (which is used to
> potentially read more than 25 bits) performs two reads in case
> it needs to read more than 25 bits and combines the result.
> 
> Yet this is not necessary: One can just read 64 bits at a time
> to get 32 usable bits (57 would be possible). This commit does so.
> 
> This reduced the size of .text by 30144B for GCC 11.4 and 5648B
> for Clang 14 (both with -O3).
> 
> (get_bits_long() is a building block of show_bits_long()
> and get_ue_golomb_long(), so this patch affects these, too.)
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/get_bits.h | 38 +++++++++++++++++++++++++++++---------
>  1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> index 0594e104bb..f43bcae91e 100644
> --- a/libavcodec/get_bits.h
> +++ b/libavcodec/get_bits.h
> @@ -187,21 +187,28 @@ static inline unsigned int show_bits(GetBitContext *s, int n);
>  
>  #define CLOSE_READER(name, gb) (gb)->index = name ## _index
>  
> +#define UPDATE_CACHE_BE_EXT(name, gb, bits, dst_bits) name ## _cache = \
> +    AV_RB ## bits((gb)->buffer + (name ## _index >> 3)) << (name ## _index & 7) >> (bits - dst_bits)
> +
> +#define UPDATE_CACHE_LE_EXT(name, gb, bits, dst_bits) name ## _cache = \
> +    (uint ## dst_bits ## _t)(AV_RL ## bits((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7))
> +
> +/* Using these two macros ensures that 32 bits are available. */
> +# define UPDATE_CACHE_LE_32(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 64, 32)
> +
> +# define UPDATE_CACHE_BE_32(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 64, 32)
> +
>  # ifdef LONG_BITSTREAM_READER
>  
> -# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
> -      AV_RL64((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
> +# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_32(name, (gb))
>  
> -# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
> -      AV_RB64((gb)->buffer + (name ## _index >> 3)) >> (32 - (name ## _index & 7))
> +# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_32(name, (gb))
>  
>  #else
>  
> -# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
> -      AV_RL32((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
> +# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 32, 32)
>  
> -# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
> -      AV_RB32((gb)->buffer + (name ## _index >> 3)) << (name ## _index & 7)
> +# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 32, 32)
>  
>  #endif
>  
> @@ -209,12 +216,14 @@ static inline unsigned int show_bits(GetBitContext *s, int n);
>  #ifdef BITSTREAM_READER_LE
>  
>  # define UPDATE_CACHE(name, gb) UPDATE_CACHE_LE(name, gb)
> +# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_LE_32(name, (gb))
>  
>  # define SKIP_CACHE(name, gb, num) name ## _cache >>= (num)
>  
>  #else
>  
>  # define UPDATE_CACHE(name, gb) UPDATE_CACHE_BE(name, gb)
> +# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_BE_32(name, (gb))
>  
>  # define SKIP_CACHE(name, gb, num) name ## _cache <<= (num)
>  
> @@ -414,15 +423,26 @@ static inline unsigned int get_bits_long(GetBitContext *s, int n)
>      av_assert2(n>=0 && n<=32);
>      if (!n) {
>          return 0;
> -    } else if (n <= MIN_CACHE_BITS) {
> +    } else if ((!HAVE_FAST_64BIT || av_builtin_constant_p(n <= MIN_CACHE_BITS))
> +               && n <= MIN_CACHE_BITS) {
>          return get_bits(s, n);
>      } else {
> +#if HAVE_FAST_64BIT
> +        unsigned tmp;
> +        OPEN_READER(re, s);
> +        UPDATE_CACHE_32(re, s);
> +        tmp = SHOW_UBITS(re, s, n);
> +        LAST_SKIP_BITS(re, s, n);
> +        CLOSE_READER(re, s);
> +        return tmp;
> +#else
>  #ifdef BITSTREAM_READER_LE
>          unsigned ret = get_bits(s, 16);
>          return ret | (get_bits(s, n - 16) << 16);
>  #else
>          unsigned ret = get_bits(s, 16) << (n - 16);
>          return ret | get_bits(s, n - 16);
> +#endif
>  #endif
>      }
>  }

Will apply this tonight unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index 0594e104bb..f43bcae91e 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -187,21 +187,28 @@  static inline unsigned int show_bits(GetBitContext *s, int n);
 
 #define CLOSE_READER(name, gb) (gb)->index = name ## _index
 
+#define UPDATE_CACHE_BE_EXT(name, gb, bits, dst_bits) name ## _cache = \
+    AV_RB ## bits((gb)->buffer + (name ## _index >> 3)) << (name ## _index & 7) >> (bits - dst_bits)
+
+#define UPDATE_CACHE_LE_EXT(name, gb, bits, dst_bits) name ## _cache = \
+    (uint ## dst_bits ## _t)(AV_RL ## bits((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7))
+
+/* Using these two macros ensures that 32 bits are available. */
+# define UPDATE_CACHE_LE_32(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 64, 32)
+
+# define UPDATE_CACHE_BE_32(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 64, 32)
+
 # ifdef LONG_BITSTREAM_READER
 
-# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
-      AV_RL64((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
+# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_32(name, (gb))
 
-# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
-      AV_RB64((gb)->buffer + (name ## _index >> 3)) >> (32 - (name ## _index & 7))
+# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_32(name, (gb))
 
 #else
 
-# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
-      AV_RL32((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
+# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 32, 32)
 
-# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
-      AV_RB32((gb)->buffer + (name ## _index >> 3)) << (name ## _index & 7)
+# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 32, 32)
 
 #endif
 
@@ -209,12 +216,14 @@  static inline unsigned int show_bits(GetBitContext *s, int n);
 #ifdef BITSTREAM_READER_LE
 
 # define UPDATE_CACHE(name, gb) UPDATE_CACHE_LE(name, gb)
+# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_LE_32(name, (gb))
 
 # define SKIP_CACHE(name, gb, num) name ## _cache >>= (num)
 
 #else
 
 # define UPDATE_CACHE(name, gb) UPDATE_CACHE_BE(name, gb)
+# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_BE_32(name, (gb))
 
 # define SKIP_CACHE(name, gb, num) name ## _cache <<= (num)
 
@@ -414,15 +423,26 @@  static inline unsigned int get_bits_long(GetBitContext *s, int n)
     av_assert2(n>=0 && n<=32);
     if (!n) {
         return 0;
-    } else if (n <= MIN_CACHE_BITS) {
+    } else if ((!HAVE_FAST_64BIT || av_builtin_constant_p(n <= MIN_CACHE_BITS))
+               && n <= MIN_CACHE_BITS) {
         return get_bits(s, n);
     } else {
+#if HAVE_FAST_64BIT
+        unsigned tmp;
+        OPEN_READER(re, s);
+        UPDATE_CACHE_32(re, s);
+        tmp = SHOW_UBITS(re, s, n);
+        LAST_SKIP_BITS(re, s, n);
+        CLOSE_READER(re, s);
+        return tmp;
+#else
 #ifdef BITSTREAM_READER_LE
         unsigned ret = get_bits(s, 16);
         return ret | (get_bits(s, n - 16) << 16);
 #else
         unsigned ret = get_bits(s, 16) << (n - 16);
         return ret | get_bits(s, n - 16);
+#endif
 #endif
     }
 }