diff mbox series

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

Message ID 20220617133206.23643-5-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,1/5] get_bits: move check_marker() to mpegvideodec.h | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/configure_armv7_RPi4 warning Failed to apply patch

Commit Message

Anton Khirnov June 17, 2022, 1:32 p.m. UTC
Use that instead of the merged version.
---
 libavcodec/get_bits.h | 296 +++++++-----------------------------------
 1 file changed, 50 insertions(+), 246 deletions(-)

Comments

James Almer June 17, 2022, 3 p.m. UTC | #1
On 6/17/2022 12:00 PM, Paul B Mahol wrote:
> NAK

You'll have to give a reason for your NAK.
Paul B Mahol June 17, 2022, 3 p.m. UTC | #2
NAK
Paul B Mahol June 17, 2022, 3:26 p.m. UTC | #3
On Fri, Jun 17, 2022 at 5:01 PM James Almer <jamrial@gmail.com> wrote:

> On 6/17/2022 12:00 PM, Paul B Mahol wrote:
> > NAK
>
> You'll have to give a reason for your NAK.
>

Remove Alexandra from copyright from your refactoring of get_bits.h

And make sure changes are not making decoders slower than currently are.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Jean-Baptiste Kempf June 18, 2022, 2:31 p.m. UTC | #4
On Fri, 17 Jun 2022, at 17:26, Paul B Mahol wrote:
> On Fri, Jun 17, 2022 at 5:01 PM James Almer <jamrial@gmail.com> wrote:
>
>> On 6/17/2022 12:00 PM, Paul B Mahol wrote:
>> > NAK
>>
>> You'll have to give a reason for your NAK.>

> Remove Alexandra from copyright from your refactoring of get_bits.h

Copyright does not work like that.
Paul B Mahol June 18, 2022, 3:40 p.m. UTC | #5
On Sat, Jun 18, 2022 at 4:32 PM Jean-Baptiste Kempf <jb@videolan.org> wrote:

> On Fri, 17 Jun 2022, at 17:26, Paul B Mahol wrote:
> > On Fri, Jun 17, 2022 at 5:01 PM James Almer <jamrial@gmail.com> wrote:
> >
> >> On 6/17/2022 12:00 PM, Paul B Mahol wrote:
> >> > NAK
> >>
> >> You'll have to give a reason for your NAK.>
>
> > Remove Alexandra from copyright from your refactoring of get_bits.h
>
> Copyright does not work like that.
>


You are not the one who put that there.

I added her to that file, and due removal of her code from that file it is
no longer correct to keep it.

You are extremely biased and malice person, everybody can check this fact.



>
> --
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Anton Khirnov June 23, 2022, 11:57 a.m. UTC | #6
Quoting Paul B Mahol (2022-06-17 17:26:23)
> On Fri, Jun 17, 2022 at 5:01 PM James Almer <jamrial@gmail.com> wrote:
> 
> And make sure changes are not making decoders slower than currently are.

fraps
* before: 2.07428 +- 0.00101 seconds time elapsed
* after:  2.073444 +- 0.000678 seconds time elapsed
* -> same within 0.05%

magicyuv
* before: 1.65126 +- 0.00306 seconds time elapsed
* after:  1.64686 +- 0.00326 seconds time elapsed
* -> same within 0.3%

utvideo
* before: 1.364421 +- 0.000214 seconds time elapsed
* after: 1.364200 +- 0.000220 seconds time elapsed
* -> same within 0.02%

mvha, photocd, sheervideo: cannot test, no samples in FATE
Anton Khirnov June 23, 2022, 12:01 p.m. UTC | #7
Quoting Jean-Baptiste Kempf (2022-06-18 16:31:55)
> On Fri, 17 Jun 2022, at 17:26, Paul B Mahol wrote:
> > On Fri, Jun 17, 2022 at 5:01 PM James Almer <jamrial@gmail.com> wrote:
> >
> >> On 6/17/2022 12:00 PM, Paul B Mahol wrote:
> >> > NAK
> >>
> >> You'll have to give a reason for your NAK.>
> 
> > Remove Alexandra from copyright from your refactoring of get_bits.h
> 
> Copyright does not work like that.

I see no problem with removing her - after this patch she really has no
code in that header.
diff mbox series

Patch

diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
index 498ce4ed35..06192a25a7 100644
--- a/libavcodec/get_bits.h
+++ b/libavcodec/get_bits.h
@@ -59,12 +59,43 @@ 
 #define CACHED_BITSTREAM_READER 0
 #endif
 
+#if CACHED_BITSTREAM_READER
+#include "bitstream.h"
+
+#define MIN_CACHE_BITS 64
+
+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
+#define get_bitsz           bitstream_read
+#define get_bits_le         bitstream_read
+#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)
+#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 +152,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 +242,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 +257,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 +271,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 +281,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 +296,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 +315,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 +335,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 +343,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 +351,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 +383,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 +403,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 +414,6 @@  static inline unsigned int get_bits_long(GetBitContext *s, int n)
         return ret | get_bits(s, n - 16);
 #endif
     }
-#endif
 }
 
 /**
@@ -610,8 +459,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 +488,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 +512,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 +597,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,
-                          VLC_TYPE (*table)[2])
-{
-    unsigned idx;
-
-    *nb_bits = -*n;
-    idx = show_bits(s, *nb_bits) + code;
-    *n = table[idx][1];
-
-    return table[idx][0];
-}
-
 /**
  * Parse a vlc code.
  * @param bits is the number of bits which will be read at once, must be
@@ -788,24 +609,6 @@  static inline int set_idx(GetBitContext *s, int code, int *n, int *nb_bits,
 static av_always_inline int get_vlc2(GetBitContext *s, VLC_TYPE (*table)[2],
                                      int bits, int max_depth)
 {
-#if CACHED_BITSTREAM_READER
-    int nb_bits;
-    unsigned idx = show_bits(s, bits);
-    int code = table[idx][0];
-    int n    = table[idx][1];
-
-    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 +619,6 @@  static av_always_inline int get_vlc2(GetBitContext *s, VLC_TYPE (*table)[2],
     CLOSE_READER(re, s);
 
     return code;
-#endif
 }
 
 static inline int decode012(GetBitContext *gb)
@@ -856,4 +658,6 @@  static inline int skip_1stop_8data_bits(GetBitContext *gb)
     return 0;
 }
 
+#endif // CACHED_BITSTREAM_READER
+
 #endif /* AVCODEC_GET_BITS_H */