diff mbox series

[FFmpeg-devel,v3,1/2] avcodec/put_bits: Parametrize bit buffer type

Message ID 20200718145303.17059-1-steinar+ffmpeg@gunderson.no
State Accepted
Commit c63c303a1f2b58677d480505ec93a90f77dd25b5
Headers show
Series [FFmpeg-devel,v3,1/2] avcodec/put_bits: Parametrize bit buffer type
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Steinar H. Gunderson July 18, 2020, 2:53 p.m. UTC
Preparatory patch for making the bit buffer different size on different
platforms; make a typedef and make all the hardcoded sizes into expressions
deriving from this size.

No functional change; generated assembler is near-identical.
---
 libavcodec/mpegvideo_enc.c |  2 +-
 libavcodec/put_bits.h      | 95 +++++++++++++++++++++-----------------
 2 files changed, 53 insertions(+), 44 deletions(-)

Comments

Michael Niedermayer July 19, 2020, 6:49 a.m. UTC | #1
On Sat, Jul 18, 2020 at 04:53:02PM +0200, Steinar H. Gunderson wrote:
> Preparatory patch for making the bit buffer different size on different
> platforms; make a typedef and make all the hardcoded sizes into expressions
> deriving from this size.
> 
> No functional change; generated assembler is near-identical.
> ---
>  libavcodec/mpegvideo_enc.c |  2 +-
>  libavcodec/put_bits.h      | 95 +++++++++++++++++++++-----------------
>  2 files changed, 53 insertions(+), 44 deletions(-)

will apply patchset after some more tests

thx

[...]
Paul B Mahol July 19, 2020, 11:47 a.m. UTC | #2
Please revert immediately:

libavcodec/gif.c:347:40: warning: incompatible pointer types passing
'void (PutBitContext *, int, BitBuf)' (aka 'void (struct PutBitContext
*, int, unsigned long)') to parameter of type 'void (*)(struct
PutBitContext *, int, unsigned int)' [-Wincompatible-pointer-types]
                       12, FF_LZW_GIF, put_bits);
                                       ^~~~~~~~
libavcodec/lzw.h:58:32: note: passing argument to parameter 'lzw_put_bits' here
                        void (*lzw_put_bits)(struct


On 7/19/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Sat, Jul 18, 2020 at 04:53:02PM +0200, Steinar H. Gunderson wrote:
>> Preparatory patch for making the bit buffer different size on different
>> platforms; make a typedef and make all the hardcoded sizes into
>> expressions
>> deriving from this size.
>>
>> No functional change; generated assembler is near-identical.
>> ---
>>  libavcodec/mpegvideo_enc.c |  2 +-
>>  libavcodec/put_bits.h      | 95 +++++++++++++++++++++-----------------
>>  2 files changed, 53 insertions(+), 44 deletions(-)
>
> will apply patchset after some more tests
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If the United States is serious about tackling the national security threats
>
> related to an insecure 5G network, it needs to rethink the extent to which
> it
> values corporate profits and government espionage over security.-Bruce
> Schneier
>
diff mbox series

Patch

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c3ef40556a..21c30a9f8a 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -3914,7 +3914,7 @@  static int encode_picture(MpegEncContext *s, int picture_number)
     s->avctx->execute(s->avctx, encode_thread, &s->thread_context[0], NULL, context_count, sizeof(void*));
     for(i=1; i<context_count; i++){
         if (s->pb.buf_end == s->thread_context[i]->pb.buf)
-            set_put_bits_buffer_size(&s->pb, FFMIN(s->thread_context[i]->pb.buf_end - s->pb.buf, INT_MAX/8-32));
+            set_put_bits_buffer_size(&s->pb, FFMIN(s->thread_context[i]->pb.buf_end - s->pb.buf, INT_MAX/8-BUF_BITS));
         merge_context_after_encode(s, s->thread_context[i]);
     }
     emms_c();
diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
index 7d11a3576a..c6a8f3ac14 100644
--- a/libavcodec/put_bits.h
+++ b/libavcodec/put_bits.h
@@ -32,8 +32,14 @@ 
 #include "libavutil/intreadwrite.h"
 #include "libavutil/avassert.h"
 
+typedef uint32_t BitBuf;
+#define AV_WBBUF AV_WB32
+#define AV_WLBUF AV_WL32
+
+static const int BUF_BITS = 8 * sizeof(BitBuf);
+
 typedef struct PutBitContext {
-    uint32_t bit_buf;
+    BitBuf bit_buf;
     int bit_left;
     uint8_t *buf, *buf_ptr, *buf_end;
     int size_in_bits;
@@ -57,7 +63,7 @@  static inline void init_put_bits(PutBitContext *s, uint8_t *buffer,
     s->buf          = buffer;
     s->buf_end      = s->buf + buffer_size;
     s->buf_ptr      = s->buf;
-    s->bit_left     = 32;
+    s->bit_left     = BUF_BITS;
     s->bit_buf      = 0;
 }
 
@@ -66,7 +72,7 @@  static inline void init_put_bits(PutBitContext *s, uint8_t *buffer,
  */
 static inline int put_bits_count(PutBitContext *s)
 {
-    return (s->buf_ptr - s->buf) * 8 + 32 - s->bit_left;
+    return (s->buf_ptr - s->buf) * 8 + BUF_BITS - s->bit_left;
 }
 
 /**
@@ -92,7 +98,7 @@  static inline void rebase_put_bits(PutBitContext *s, uint8_t *buffer,
  */
 static inline int put_bits_left(PutBitContext* s)
 {
-    return (s->buf_end - s->buf_ptr) * 8 - 32 + s->bit_left;
+    return (s->buf_end - s->buf_ptr) * 8 - BUF_BITS + s->bit_left;
 }
 
 /**
@@ -101,33 +107,33 @@  static inline int put_bits_left(PutBitContext* s)
 static inline void flush_put_bits(PutBitContext *s)
 {
 #ifndef BITSTREAM_WRITER_LE
-    if (s->bit_left < 32)
+    if (s->bit_left < BUF_BITS)
         s->bit_buf <<= s->bit_left;
 #endif
-    while (s->bit_left < 32) {
+    while (s->bit_left < BUF_BITS) {
         av_assert0(s->buf_ptr < s->buf_end);
 #ifdef BITSTREAM_WRITER_LE
         *s->buf_ptr++ = s->bit_buf;
         s->bit_buf  >>= 8;
 #else
-        *s->buf_ptr++ = s->bit_buf >> 24;
+        *s->buf_ptr++ = s->bit_buf >> (BUF_BITS - 8);
         s->bit_buf  <<= 8;
 #endif
         s->bit_left  += 8;
     }
-    s->bit_left = 32;
+    s->bit_left = BUF_BITS;
     s->bit_buf  = 0;
 }
 
 static inline void flush_put_bits_le(PutBitContext *s)
 {
-    while (s->bit_left < 32) {
+    while (s->bit_left < BUF_BITS) {
         av_assert0(s->buf_ptr < s->buf_end);
         *s->buf_ptr++ = s->bit_buf;
         s->bit_buf  >>= 8;
         s->bit_left  += 8;
     }
-    s->bit_left = 32;
+    s->bit_left = BUF_BITS;
     s->bit_buf  = 0;
 }
 
@@ -161,29 +167,29 @@  void avpriv_copy_bits(PutBitContext *pb, const uint8_t *src, int length);
  * Write up to 31 bits into a bitstream.
  * Use put_bits32 to write 32 bits.
  */
-static inline void put_bits(PutBitContext *s, int n, unsigned int value)
+static inline void put_bits(PutBitContext *s, int n, BitBuf value)
 {
-    unsigned int bit_buf;
+    BitBuf bit_buf;
     int bit_left;
 
-    av_assert2(n <= 31 && value < (1U << n));
+    av_assert2(n <= 31 && value < (1UL << n));
 
     bit_buf  = s->bit_buf;
     bit_left = s->bit_left;
 
     /* XXX: optimize */
 #ifdef BITSTREAM_WRITER_LE
-    bit_buf |= value << (32 - bit_left);
+    bit_buf |= value << (BUF_BITS - bit_left);
     if (n >= bit_left) {
-        if (3 < s->buf_end - s->buf_ptr) {
-            AV_WL32(s->buf_ptr, bit_buf);
-            s->buf_ptr += 4;
+        if (s->buf_end - s->buf_ptr >= sizeof(BitBuf)) {
+            AV_WLBUF(s->buf_ptr, bit_buf);
+            s->buf_ptr += sizeof(BitBuf);
         } else {
             av_log(NULL, AV_LOG_ERROR, "Internal error, put_bits buffer too small\n");
             av_assert2(0);
         }
         bit_buf     = value >> bit_left;
-        bit_left   += 32;
+        bit_left   += BUF_BITS;
     }
     bit_left -= n;
 #else
@@ -193,14 +199,14 @@  static inline void put_bits(PutBitContext *s, int n, unsigned int value)
     } else {
         bit_buf   <<= bit_left;
         bit_buf    |= value >> (n - bit_left);
-        if (3 < s->buf_end - s->buf_ptr) {
-            AV_WB32(s->buf_ptr, bit_buf);
-            s->buf_ptr += 4;
+        if (s->buf_end - s->buf_ptr >= sizeof(BitBuf)) {
+            AV_WBBUF(s->buf_ptr, bit_buf);
+            s->buf_ptr += sizeof(BitBuf);
         } else {
             av_log(NULL, AV_LOG_ERROR, "Internal error, put_bits buffer too small\n");
             av_assert2(0);
         }
-        bit_left   += 32 - n;
+        bit_left   += BUF_BITS - n;
         bit_buf     = value;
     }
 #endif
@@ -209,27 +215,27 @@  static inline void put_bits(PutBitContext *s, int n, unsigned int value)
     s->bit_left = bit_left;
 }
 
-static inline void put_bits_le(PutBitContext *s, int n, unsigned int value)
+static inline void put_bits_le(PutBitContext *s, int n, BitBuf value)
 {
-    unsigned int bit_buf;
+    BitBuf bit_buf;
     int bit_left;
 
-    av_assert2(n <= 31 && value < (1U << n));
+    av_assert2(n <= 31 && value < (1UL << n));
 
     bit_buf  = s->bit_buf;
     bit_left = s->bit_left;
 
-    bit_buf |= value << (32 - bit_left);
+    bit_buf |= value << (BUF_BITS - bit_left);
     if (n >= bit_left) {
-        if (3 < s->buf_end - s->buf_ptr) {
-            AV_WL32(s->buf_ptr, bit_buf);
-            s->buf_ptr += 4;
+        if (s->buf_end - s->buf_ptr >= sizeof(BitBuf)) {
+            AV_WLBUF(s->buf_ptr, bit_buf);
+            s->buf_ptr += sizeof(BitBuf);
         } else {
             av_log(NULL, AV_LOG_ERROR, "Internal error, put_bits buffer too small\n");
             av_assert2(0);
         }
         bit_buf     = value >> bit_left;
-        bit_left   += 32;
+        bit_left   += BUF_BITS;
     }
     bit_left -= n;
 
@@ -249,17 +255,17 @@  static inline void put_sbits(PutBitContext *pb, int n, int32_t value)
  */
 static void av_unused put_bits32(PutBitContext *s, uint32_t value)
 {
-    unsigned int bit_buf;
+    BitBuf bit_buf;
     int bit_left;
 
     bit_buf  = s->bit_buf;
     bit_left = s->bit_left;
 
 #ifdef BITSTREAM_WRITER_LE
-    bit_buf |= value << (32 - bit_left);
-    if (3 < s->buf_end - s->buf_ptr) {
-        AV_WL32(s->buf_ptr, bit_buf);
-        s->buf_ptr += 4;
+    bit_buf |= (BitBuf)value << (BUF_BITS - bit_left);
+    if (s->buf_end - s->buf_ptr >= sizeof(BitBuf)) {
+        AV_WLBUF(s->buf_ptr, bit_buf);
+        s->buf_ptr += sizeof(BitBuf);
     } else {
         av_log(NULL, AV_LOG_ERROR, "Internal error, put_bits buffer too small\n");
         av_assert2(0);
@@ -267,10 +273,10 @@  static void av_unused put_bits32(PutBitContext *s, uint32_t value)
     bit_buf     = (uint64_t)value >> bit_left;
 #else
     bit_buf     = (uint64_t)bit_buf << bit_left;
-    bit_buf    |= value >> (32 - bit_left);
-    if (3 < s->buf_end - s->buf_ptr) {
-        AV_WB32(s->buf_ptr, bit_buf);
-        s->buf_ptr += 4;
+    bit_buf    |= (BitBuf)value >> (BUF_BITS - bit_left);
+    if (s->buf_end - s->buf_ptr >= sizeof(BitBuf)) {
+        AV_WBBUF(s->buf_ptr, bit_buf);
+        s->buf_ptr += sizeof(BitBuf);
     } else {
         av_log(NULL, AV_LOG_ERROR, "Internal error, put_bits buffer too small\n");
         av_assert2(0);
@@ -333,7 +339,7 @@  static inline uint8_t *put_bits_ptr(PutBitContext *s)
 static inline void skip_put_bytes(PutBitContext *s, int n)
 {
     av_assert2((put_bits_count(s) & 7) == 0);
-    av_assert2(s->bit_left == 32);
+    av_assert2(s->bit_left == BUF_BITS);
     av_assert0(n <= s->buf_end - s->buf_ptr);
     s->buf_ptr += n;
 }
@@ -346,8 +352,8 @@  static inline void skip_put_bytes(PutBitContext *s, int n)
 static inline void skip_put_bits(PutBitContext *s, int n)
 {
     s->bit_left -= n;
-    s->buf_ptr  -= 4 * (s->bit_left >> 5);
-    s->bit_left &= 31;
+    s->buf_ptr  -= sizeof(BitBuf) * ((unsigned)s->bit_left / BUF_BITS);
+    s->bit_left &= (BUF_BITS - 1);
 }
 
 /**
@@ -357,9 +363,12 @@  static inline void skip_put_bits(PutBitContext *s, int n)
  */
 static inline void set_put_bits_buffer_size(PutBitContext *s, int size)
 {
-    av_assert0(size <= INT_MAX/8 - 32);
+    av_assert0(size <= INT_MAX/8 - BUF_BITS);
     s->buf_end = s->buf + size;
     s->size_in_bits = 8*size;
 }
 
+#undef AV_WBBUF
+#undef AV_WLBUF
+
 #endif /* AVCODEC_PUT_BITS_H */