cbs: Make tracing more general

Turn tracing into callbacks for each syntax element, with default
callbacks to match current trace_headers behaviour for debug.  Move
the construction of bit strings into the trace callback, which
simplifies all of the read and write functions.

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>

8 files changed, 373 insertions, 376 deletions

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 3ec8285e21..daf7f66427 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -117,8 +117,9 @@ av_cold int ff_cbs_init(CodedBitstreamContext **ctx_ptr,
 
     ctx->decompose_unit_types = NULL;
 
-    ctx->trace_enable = 0;
-    ctx->trace_level  = AV_LOG_TRACE;
+    ctx->trace_enable  = 0;
+    ctx->trace_level   = AV_LOG_TRACE;
+    ctx->trace_context = ctx;
 
     *ctx_ptr = ctx;
     return 0;
@@ -496,19 +497,27 @@ void ff_cbs_trace_header(CodedBitstreamContext *ctx,
     av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name);
 }
 
-void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
-                                 const char *str, const int *subscripts,
-                                 const char *bits, int64_t value)
+void ff_cbs_trace_read_log(void *trace_context,
+                           GetBitContext *gbc, int length,
+                           const char *str, const int *subscripts,
+                           int64_t value)
 {
+    CodedBitstreamContext *ctx = trace_context;
     char name[256];
+    char bits[256];
     size_t name_len, bits_len;
     int pad, subs, i, j, k, n;
-
-    if (!ctx->trace_enable)
-        return;
+    int position;
 
     av_assert0(value >= INT_MIN && value <= UINT32_MAX);
 
+    position = get_bits_count(gbc);
+
+    av_assert0(length < 256);
+    for (i = 0; i < length; i++)
+        bits[i] = get_bits1(gbc) ? '1' : '0';
+    bits[length] = 0;
+
     subs = subscripts ? subscripts[0] : 0;
     n = 0;
     for (i = j = 0; str[i];) {
@@ -535,7 +544,7 @@ void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
     av_assert0(n == subs);
 
     name_len = strlen(name);
-    bits_len = strlen(bits);
+    bits_len = length;
 
     if (name_len + bits_len > 60)
         pad = bits_len + 2;
@@ -546,6 +555,36 @@ void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
            position, name, pad, bits, value);
 }
 
+void ff_cbs_trace_write_log(void *trace_context,
+                            PutBitContext *pbc, int length,
+                            const char *str, const int *subscripts,
+                            int64_t value)
+{
+    CodedBitstreamContext *ctx = trace_context;
+
+    // Ensure that the syntax element is written to the output buffer,
+    // make a GetBitContext pointed at the start position, then call the
+    // read log function which can read the bits back to log them.
+
+    GetBitContext gbc;
+    int position;
+
+    if (length > 0) {
+        PutBitContext flush;
+        flush = *pbc;
+        flush_put_bits(&flush);
+    }
+
+    position = put_bits_count(pbc);
+    av_assert0(position >= length);
+
+    init_get_bits(&gbc, pbc->buf, position);
+
+    skip_bits_long(&gbc, position - length);
+
+    ff_cbs_trace_read_log(ctx, &gbc, length, str, subscripts, value);
+}
+
 static av_always_inline int cbs_read_unsigned(CodedBitstreamContext *ctx,
                                               GetBitContext *gbc,
                                               int width, const char *name,
@@ -555,7 +594,8 @@ static av_always_inline int cbs_read_unsigned(CodedBitstreamContext *ctx,
                                               uint32_t range_max)
 {
     uint32_t value;
-    int position;
+
+    CBS_TRACE_READ_START();
 
     av_assert0(width > 0 && width <= 32);
 
@@ -565,21 +605,9 @@ static av_always_inline int cbs_read_unsigned(CodedBitstreamContext *ctx,
         return AVERROR_INVALIDDATA;
     }
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
-
     value = get_bits_long(gbc, width);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
-    }
+    CBS_TRACE_READ_END();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -613,6 +641,8 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
                           const int *subscripts, uint32_t value,
                           uint32_t range_min, uint32_t range_max)
 {
+    CBS_TRACE_WRITE_START();
+
     av_assert0(width > 0 && width <= 32);
 
     if (value < range_min || value > range_max) {
@@ -625,22 +655,13 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
     if (put_bits_left(pbc) < width)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     if (width < 32)
         put_bits(pbc, width, value);
     else
         put_bits32(pbc, value);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
@@ -657,7 +678,8 @@ int ff_cbs_read_signed(CodedBitstreamContext *ctx, GetBitContext *gbc,
                        int32_t range_min, int32_t range_max)
 {
     int32_t value;
-    int position;
+
+    CBS_TRACE_READ_START();
 
     av_assert0(width > 0 && width <= 32);
 
@@ -667,21 +689,9 @@ int ff_cbs_read_signed(CodedBitstreamContext *ctx, GetBitContext *gbc,
         return AVERROR_INVALIDDATA;
     }
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
-
     value = get_sbits_long(gbc, width);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = value & (1U << (width - i - 1)) ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
-    }
+    CBS_TRACE_READ_END();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -699,6 +709,8 @@ int ff_cbs_write_signed(CodedBitstreamContext *ctx, PutBitContext *pbc,
                         const int *subscripts, int32_t value,
                         int32_t range_min, int32_t range_max)
 {
+    CBS_TRACE_WRITE_START();
+
     av_assert0(width > 0 && width <= 32);
 
     if (value < range_min || value > range_max) {
@@ -711,22 +723,13 @@ int ff_cbs_write_signed(CodedBitstreamContext *ctx, PutBitContext *pbc,
     if (put_bits_left(pbc) < width)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = value & (1U << (width - i - 1)) ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     if (width < 32)
         put_sbits(pbc, width, value);
     else
         put_bits32(pbc, value);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index b4131db5fe..ffb2797761 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -168,6 +168,51 @@ typedef struct CodedBitstreamFragment {
     CodedBitstreamUnit *units;
 } CodedBitstreamFragment;
 
+
+struct CodedBitstreamContext;
+struct GetBitContext;
+struct PutBitContext;
+
+/**
+ * Callback type for read tracing.
+ *
+ * @param ctx         User-set trace context.
+ * @param gbc         A GetBitContext set at the start of the syntax
+ *                    element.  This is a copy, the callee does not
+ *                    need to preserve it.
+ * @param length      Length in bits of the syntax element.
+ * @param name        String name of the syntax elements.
+ * @param subscripts  If the syntax element is an array, a pointer to
+ *                    an array of subscripts into the array.
+ * @param value       Parsed value of the syntax element.
+ */
+typedef void (*CBSTraceReadCallback)(void *trace_context,
+                                     struct GetBitContext *gbc,
+                                     int start_position,
+                                     const char *name,
+                                     const int *subscripts,
+                                     int64_t value);
+
+/**
+ * Callback type for write tracing.
+ *
+ * @param ctx         User-set trace context.
+ * @param pbc         A PutBitContext set at the end of the syntax
+ *                    element.  The user must not modify this, but may
+ *                    inspect it to determine state.
+ * @param length      Length in bits of the syntax element.
+ * @param name        String name of the syntax elements.
+ * @param subscripts  If the syntax element is an array, a pointer to
+ *                    an array of subscripts into the array.
+ * @param value       Written value of the syntax element.
+ */
+typedef void (*CBSTraceWriteCallback)(void *trace_context,
+                                      struct PutBitContext *pbc,
+                                      int start_position,
+                                      const char *name,
+                                      const int *subscripts,
+                                      int64_t value);
+
 /**
  * Context structure for coded bitstream operations.
  */
@@ -211,11 +256,29 @@ typedef struct CodedBitstreamContext {
      */
     int trace_enable;
     /**
-     * Log level to use for trace output.
+     * Log level to use for default trace output.
      *
      * From AV_LOG_*; defaults to AV_LOG_TRACE.
      */
     int trace_level;
+    /**
+     * User context pointer to pass to trace callbacks.
+     */
+    void *trace_context;
+    /**
+     * Callback for read tracing.
+     *
+     * If tracing is enabled then this is called once for each syntax
+     * element parsed.
+     */
+    CBSTraceReadCallback  trace_read_callback;
+    /**
+     * Callback for write tracing.
+     *
+     * If tracing is enabled then this is called once for each syntax
+     * element written.
+     */
+    CBSTraceWriteCallback trace_write_callback;
 
     /**
      * Write buffer. Used as intermediate buffer when writing units.
@@ -450,4 +513,27 @@ void ff_cbs_discard_units(CodedBitstreamContext *ctx,
                           enum AVDiscard skip,
                           int flags);
 
+
+/**
+ * Helper function for read tracing which formats the syntax element
+ * and logs the result.
+ *
+ * Trace context should be set to the CodedBitstreamContext.
+ */
+void ff_cbs_trace_read_log(void *trace_context,
+                           struct GetBitContext *gbc, int length,
+                           const char *str, const int *subscripts,
+                           int64_t value);
+
+/**
+ * Helper function for write tracing which formats the syntax element
+ * and logs the result.
+ *
+ * Trace context should be set to the CodedBitstreamContext.
+ */
+void ff_cbs_trace_write_log(void *trace_context,
+                            struct PutBitContext *pbc, int length,
+                            const char *str, const int *subscripts,
+                            int64_t value);
+
 #endif /* AVCODEC_CBS_H */
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 7fb5bd8b42..6c478603f1 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -31,10 +31,8 @@ static int cbs_av1_read_uvlc(CodedBitstreamContext *ctx, GetBitContext *gbc,
                              uint32_t range_min, uint32_t range_max)
 {
     uint32_t zeroes, bits_value, value;
-    int position;
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
     zeroes = 0;
     while (1) {
@@ -50,6 +48,9 @@ static int cbs_av1_read_uvlc(CodedBitstreamContext *ctx, GetBitContext *gbc,
     }
 
     if (zeroes >= 32) {
+        // Note that the spec allows an arbitrarily large number of
+        // zero bits followed by a one bit in this case, but the
+        // libaom implementation does not support it.
         value = MAX_UINT_BITS(32);
     } else {
         if (get_bits_left(gbc) < zeroes) {
@@ -62,36 +63,7 @@ static int cbs_av1_read_uvlc(CodedBitstreamContext *ctx, GetBitContext *gbc,
         value = bits_value + (UINT32_C(1) << zeroes) - 1;
     }
 
-    if (ctx->trace_enable) {
-        char bits[65];
-        int i, j, k;
-
-        if (zeroes >= 32) {
-            while (zeroes > 32) {
-                k = FFMIN(zeroes - 32, 32);
-                for (i = 0; i < k; i++)
-                    bits[i] = '0';
-                bits[i] = 0;
-                ff_cbs_trace_syntax_element(ctx, position, name,
-                                            NULL, bits, 0);
-                zeroes -= k;
-                position += k;
-            }
-        }
-
-        for (i = 0; i < zeroes; i++)
-            bits[i] = '0';
-        bits[i++] = '1';
-
-        if (zeroes < 32) {
-            for (j = 0; j < zeroes; j++)
-                bits[i++] = (bits_value >> (zeroes - j - 1) & 1) ? '1' : '0';
-        }
-
-        bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, position, name,
-                                    NULL, bits, value);
-    }
+    CBS_TRACE_READ_END_NO_SUBSCRIPTS();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -109,7 +81,9 @@ static int cbs_av1_write_uvlc(CodedBitstreamContext *ctx, PutBitContext *pbc,
                               uint32_t range_min, uint32_t range_max)
 {
     uint32_t v;
-    int position, zeroes;
+    int zeroes;
+
+    CBS_TRACE_WRITE_START();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -118,28 +92,17 @@ static int cbs_av1_write_uvlc(CodedBitstreamContext *ctx, PutBitContext *pbc,
         return AVERROR_INVALIDDATA;
     }
 
-    if (ctx->trace_enable)
-        position = put_bits_count(pbc);
-
     zeroes = av_log2(value + 1);
     v = value - (1U << zeroes) + 1;
+
+    if (put_bits_left(pbc) < 2 * zeroes + 1)
+        return AVERROR(ENOSPC);
+
     put_bits(pbc, zeroes, 0);
     put_bits(pbc, 1, 1);
     put_bits(pbc, zeroes, v);
 
-    if (ctx->trace_enable) {
-        char bits[65];
-        int i, j;
-        i = 0;
-        for (j = 0; j < zeroes; j++)
-            bits[i++] = '0';
-        bits[i++] = '1';
-        for (j = 0; j < zeroes; j++)
-            bits[i++] = (v >> (zeroes - j - 1) & 1) ? '1' : '0';
-        bits[i++] = 0;
-        ff_cbs_trace_syntax_element(ctx, position, name, NULL,
-                                    bits, value);
-    }
+    CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
 
     return 0;
 }
@@ -148,20 +111,19 @@ static int cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
                                const char *name, uint64_t *write_to)
 {
     uint64_t value;
-    int position, err, i;
+    uint32_t byte;
+    int i;
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
     value = 0;
     for (i = 0; i < 8; i++) {
-        int subscript[2] = { 1, i };
-        uint32_t byte;
-        err = ff_cbs_read_unsigned(ctx, gbc, 8, "leb128_byte[i]", subscript,
-                                   &byte, 0x00, 0xff);
-        if (err < 0)
-            return err;
-
+        if (get_bits_left(gbc) < 8) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid leb128 at "
+                   "%s: bitstream ended.\n", name);
+            return AVERROR_INVALIDDATA;
+        }
+        byte = get_bits(gbc, 8);
         value |= (uint64_t)(byte & 0x7f) << (i * 7);
         if (!(byte & 0x80))
             break;
@@ -170,8 +132,7 @@ static int cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
     if (value > UINT32_MAX)
         return AVERROR_INVALIDDATA;
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position, name, NULL, "", value);
+    CBS_TRACE_READ_END_NO_SUBSCRIPTS();
 
     *write_to = value;
     return 0;
@@ -180,29 +141,25 @@ static int cbs_av1_read_leb128(CodedBitstreamContext *ctx, GetBitContext *gbc,
 static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc,
                                 const char *name, uint64_t value)
 {
-    int position, err, len, i;
+    int len, i;
     uint8_t byte;
 
-    len = (av_log2(value) + 7) / 7;
+    CBS_TRACE_WRITE_START();
 
-    if (ctx->trace_enable)
-        position = put_bits_count(pbc);
+    len = (av_log2(value) + 7) / 7;
 
     for (i = 0; i < len; i++) {
-        int subscript[2] = { 1, i };
+        if (put_bits_left(pbc) < 8)
+            return AVERROR(ENOSPC);
 
         byte = value >> (7 * i) & 0x7f;
         if (i < len - 1)
             byte |= 0x80;
 
-        err = ff_cbs_write_unsigned(ctx, pbc, 8, "leb128_byte[i]", subscript,
-                                    byte, 0x00, 0xff);
-        if (err < 0)
-            return err;
+        put_bits(pbc, 8, byte);
     }
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position, name, NULL, "", value);
+    CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
 
     return 0;
 }
@@ -212,12 +169,11 @@ static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc,
                            const int *subscripts, uint32_t *write_to)
 {
     uint32_t m, v, extra_bit, value;
-    int position, w;
+    int w;
 
-    av_assert0(n > 0);
+    CBS_TRACE_READ_START();
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    av_assert0(n > 0);
 
     w = av_log2(n) + 1;
     m = (1 << w) - n;
@@ -240,18 +196,7 @@ static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc,
         value = (v << 1) - m + extra_bit;
     }
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < w - 1; i++)
-            bits[i] = (v >> i & 1) ? '1' : '0';
-        if (v >= m)
-            bits[i++] = extra_bit ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position,
-                                    name, subscripts, bits, value);
-    }
+    CBS_TRACE_READ_END();
 
     *write_to = value;
     return 0;
@@ -262,7 +207,8 @@ static int cbs_av1_write_ns(CodedBitstreamContext *ctx, PutBitContext *pbc,
                             const int *subscripts, uint32_t value)
 {
     uint32_t w, m, v, extra_bit;
-    int position;
+
+    CBS_TRACE_WRITE_START();
 
     if (value > n) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -271,9 +217,6 @@ static int cbs_av1_write_ns(CodedBitstreamContext *ctx, PutBitContext *pbc,
         return AVERROR_INVALIDDATA;
     }
 
-    if (ctx->trace_enable)
-        position = put_bits_count(pbc);
-
     w = av_log2(n) + 1;
     m = (1 << w) - n;
 
@@ -290,18 +233,7 @@ static int cbs_av1_write_ns(CodedBitstreamContext *ctx, PutBitContext *pbc,
         put_bits(pbc, 1, extra_bit);
     }
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < w - 1; i++)
-            bits[i] = (v >> i & 1) ? '1' : '0';
-        if (value >= m)
-            bits[i++] = extra_bit ? '1' : '0';
-        bits[i] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position,
-                                    name, subscripts, bits, value);
-    }
+    CBS_TRACE_WRITE_END();
 
     return 0;
 }
@@ -311,33 +243,24 @@ static int cbs_av1_read_increment(CodedBitstreamContext *ctx, GetBitContext *gbc
                                   const char *name, uint32_t *write_to)
 {
     uint32_t value;
-    int position, i;
-    char bits[33];
 
-    av_assert0(range_min <= range_max && range_max - range_min < sizeof(bits) - 1);
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
-    for (i = 0, value = range_min; value < range_max;) {
+    av_assert0(range_min <= range_max && range_max - range_min < 32);
+
+    for (value = range_min; value < range_max;) {
         if (get_bits_left(gbc) < 1) {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid increment value at "
                    "%s: bitstream ended.\n", name);
             return AVERROR_INVALIDDATA;
         }
-        if (get_bits1(gbc)) {
-            bits[i++] = '1';
+        if (get_bits1(gbc))
             ++value;
-        } else {
-            bits[i++] = '0';
+        else
             break;
-        }
     }
 
-    if (ctx->trace_enable) {
-        bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, position,
-                                    name, NULL, bits, value);
-    }
+    CBS_TRACE_READ_END_NO_SUBSCRIPTS();
 
     *write_to = value;
     return 0;
@@ -349,6 +272,8 @@ static int cbs_av1_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
 {
     int len;
 
+    CBS_TRACE_WRITE_START();
+
     av_assert0(range_min <= range_max && range_max - range_min < 32);
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -364,23 +289,11 @@ static int cbs_av1_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
     if (put_bits_left(pbc) < len)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < len; i++) {
-            if (range_min + i == value)
-                bits[i] = '0';
-            else
-                bits[i] = '1';
-        }
-        bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, NULL, bits, value);
-    }
-
     if (len > 0)
         put_bits(pbc, len, (1 << len) - 1 - (value != range_max));
 
+    CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
+
     return 0;
 }
 
@@ -388,12 +301,10 @@ static int cbs_av1_read_subexp(CodedBitstreamContext *ctx, GetBitContext *gbc,
                                uint32_t range_max, const char *name,
                                const int *subscripts, uint32_t *write_to)
 {
-    uint32_t value;
-    int position, err;
-    uint32_t max_len, len, range_offset, range_bits;
+    uint32_t value, max_len, len, range_offset, range_bits;
+    int err;
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
     av_assert0(range_max > 0);
     max_len = av_log2(range_max - 1) - 3;
@@ -425,9 +336,7 @@ static int cbs_av1_read_subexp(CodedBitstreamContext *ctx, GetBitContext *gbc,
     }
     value += range_offset;
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position,
-                                    name, subscripts, "", value);
+    CBS_TRACE_READ_END_VALUE_ONLY();
 
     *write_to = value;
     return err;
@@ -437,9 +346,11 @@ static int cbs_av1_write_subexp(CodedBitstreamContext *ctx, PutBitContext *pbc,
                                 uint32_t range_max, const char *name,
                                 const int *subscripts, uint32_t value)
 {
-    int position, err;
+    int err;
     uint32_t max_len, len, range_offset, range_bits;
 
+    CBS_TRACE_WRITE_START();
+
     if (value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
                "%"PRIu32", but must be in [0,%"PRIu32"].\n",
@@ -447,9 +358,6 @@ static int cbs_av1_write_subexp(CodedBitstreamContext *ctx, PutBitContext *pbc,
         return AVERROR_INVALIDDATA;
     }
 
-    if (ctx->trace_enable)
-        position = put_bits_count(pbc);
-
     av_assert0(range_max > 0);
     max_len = av_log2(range_max - 1) - 3;
 
@@ -489,9 +397,7 @@ static int cbs_av1_write_subexp(CodedBitstreamContext *ctx, PutBitContext *pbc,
             return err;
     }
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position,
-                                    name, subscripts, "", value);
+    CBS_TRACE_WRITE_END_VALUE_ONLY();
 
     return err;
 }
diff --git a/libavcodec/cbs_bsf.c b/libavcodec/cbs_bsf.c
index 069f6e9918..b25285483b 100644
--- a/libavcodec/cbs_bsf.c
+++ b/libavcodec/cbs_bsf.c
@@ -123,6 +123,11 @@ int ff_cbs_bsf_generic_init(AVBSFContext *bsf, const CBSBSFType *type)
     if (err < 0)
         return err;
 
+    ctx->output->trace_enable = 1;
+    ctx->output->trace_level  = AV_LOG_TRACE;
+    ctx->output->trace_context = ctx->output;
+    ctx->output->trace_write_callback = ff_cbs_trace_write_log;
+
     if (bsf->par_in->extradata) {
         err = ff_cbs_read_extradata(ctx->input, frag, bsf->par_in);
         if (err < 0) {
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 318c997d94..0a1c8ea426 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -36,41 +36,38 @@ static int cbs_read_ue_golomb(CodedBitstreamContext *ctx, GetBitContext *gbc,
                               uint32_t *write_to,
                               uint32_t range_min, uint32_t range_max)
 {
-    uint32_t value;
-    int position, i, j;
-    unsigned int k;
-    char bits[65];
+    uint32_t leading_bits, value;
+    int max_length, leading_zeroes;
 
-    position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
-    for (i = 0; i < 32; i++) {
-        if (get_bits_left(gbc) < i + 1) {
+    max_length = FFMIN(get_bits_left(gbc), 32);
+
+    leading_bits = show_bits_long(gbc, max_length);
+    if (leading_bits == 0) {
+        if (max_length >= 32) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid ue-golomb code at "
+                   "%s: more than 31 zeroes.\n", name);
+            return AVERROR_INVALIDDATA;
+        } else {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid ue-golomb code at "
                    "%s: bitstream ended.\n", name);
             return AVERROR_INVALIDDATA;
         }
-        k = get_bits1(gbc);
-        bits[i] = k ? '1' : '0';
-        if (k)
-            break;
     }
-    if (i >= 32) {
+
+    leading_zeroes = max_length - 1 - av_log2(leading_bits);
+    skip_bits_long(gbc, leading_zeroes);
+
+    if (get_bits_left(gbc) < leading_zeroes + 1) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid ue-golomb code at "
-               "%s: more than 31 zeroes.\n", name);
+               "%s: bitstream ended.\n", name);
         return AVERROR_INVALIDDATA;
     }
-    value = 1;
-    for (j = 0; j < i; j++) {
-        k = get_bits1(gbc);
-        bits[i + j + 1] = k ? '1' : '0';
-        value = value << 1 | k;
-    }
-    bits[i + j + 1] = 0;
-    --value;
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
+    value = get_bits_long(gbc, leading_zeroes + 1) - 1;
+
+    CBS_TRACE_READ_END();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -88,45 +85,44 @@ static int cbs_read_se_golomb(CodedBitstreamContext *ctx, GetBitContext *gbc,
                               int32_t *write_to,
                               int32_t range_min, int32_t range_max)
 {
+    uint32_t leading_bits, unsigned_value;
+    int max_length, leading_zeroes;
     int32_t value;
-    int position, i, j;
-    unsigned int k;
-    uint32_t v;
-    char bits[65];
 
-    position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
-    for (i = 0; i < 32; i++) {
-        if (get_bits_left(gbc) < i + 1) {
+    max_length = FFMIN(get_bits_left(gbc), 32);
+
+    leading_bits = show_bits_long(gbc, max_length);
+    if (leading_bits == 0) {
+        if (max_length >= 32) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid se-golomb code at "
+                   "%s: more than 31 zeroes.\n", name);
+            return AVERROR_INVALIDDATA;
+        } else {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid se-golomb code at "
                    "%s: bitstream ended.\n", name);
             return AVERROR_INVALIDDATA;
         }
-        k = get_bits1(gbc);
-        bits[i] = k ? '1' : '0';
-        if (k)
-            break;
     }
-    if (i >= 32) {
+
+    leading_zeroes = max_length - 1 - av_log2(leading_bits);
+    skip_bits_long(gbc, leading_zeroes);
+
+    if (get_bits_left(gbc) < leading_zeroes + 1) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid se-golomb code at "
-               "%s: more than 31 zeroes.\n", name);
+               "%s: bitstream ended.\n", name);
         return AVERROR_INVALIDDATA;
     }
-    v = 1;
-    for (j = 0; j < i; j++) {
-        k = get_bits1(gbc);
-        bits[i + j + 1] = k ? '1' : '0';
-        v = v << 1 | k;
-    }
-    bits[i + j + 1] = 0;
-    if (v & 1)
-        value = -(int32_t)(v / 2);
+
+    unsigned_value = get_bits_long(gbc, leading_zeroes + 1);
+
+    if (unsigned_value & 1)
+        value = -(int32_t)(unsigned_value / 2);
     else
-        value = v / 2;
+        value = unsigned_value / 2;
 
-    if (ctx->trace_enable)
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
+    CBS_TRACE_READ_END();
 
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -146,6 +142,8 @@ static int cbs_write_ue_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
 {
     int len;
 
+    CBS_TRACE_WRITE_START();
+
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
                "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
@@ -158,27 +156,14 @@ static int cbs_write_ue_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
     if (put_bits_left(pbc) < 2 * len + 1)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[65];
-        int i;
-
-        for (i = 0; i < len; i++)
-            bits[i] = '0';
-        bits[len] = '1';
-        for (i = 0; i < len; i++)
-            bits[len + i + 1] = (value + 1) >> (len - i - 1) & 1 ? '1' : '0';
-        bits[len + len + 1] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     put_bits(pbc, len, 0);
     if (len + 1 < 32)
         put_bits(pbc, len + 1, value + 1);
     else
         put_bits32(pbc, value + 1);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
@@ -190,6 +175,8 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
     int len;
     uint32_t uvalue;
 
+    CBS_TRACE_WRITE_START();
+
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
                "%"PRId32", but must be in [%"PRId32",%"PRId32"].\n",
@@ -209,27 +196,14 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
     if (put_bits_left(pbc) < 2 * len + 1)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[65];
-        int i;
-
-        for (i = 0; i < len; i++)
-            bits[i] = '0';
-        bits[len] = '1';
-        for (i = 0; i < len; i++)
-            bits[len + i + 1] = (uvalue + 1) >> (len - i - 1) & 1 ? '1' : '0';
-        bits[len + len + 1] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     put_bits(pbc, len, 0);
     if (len + 1 < 32)
         put_bits(pbc, len + 1, uvalue + 1);
     else
         put_bits32(pbc, uvalue + 1);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
index da84697a29..285deb40d5 100644
--- a/libavcodec/cbs_internal.h
+++ b/libavcodec/cbs_internal.h
@@ -157,10 +157,6 @@ typedef struct CodedBitstreamType {
 void ff_cbs_trace_header(CodedBitstreamContext *ctx,
                          const char *name);
 
-void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
-                                 const char *name, const int *subscripts,
-                                 const char *bitstring, int64_t value);
-
 
 // Helper functions for read/write of common bitstream elements, including
 // generation of trace output. The simple functions are equivalent to
@@ -206,6 +202,87 @@ int ff_cbs_write_signed(CodedBitstreamContext *ctx, PutBitContext *pbc,
 // range_min in the above functions.
 #define MIN_INT_BITS(length) (-(INT64_C(1) << ((length) - 1)))
 
+
+// Start of a syntax element during read tracing.
+#define CBS_TRACE_READ_START() \
+    GetBitContext trace_start; \
+    do { \
+        if (ctx->trace_enable) \
+            trace_start = *gbc; \
+    } while (0)
+
+// End of a syntax element for tracing, make callback.
+#define CBS_TRACE_READ_END() \
+    do { \
+        if (ctx->trace_enable) { \
+            int start_position = get_bits_count(&trace_start); \
+            int end_position   = get_bits_count(gbc); \
+            av_assert0(start_position <= end_position); \
+            ctx->trace_read_callback(ctx->trace_context, &trace_start, \
+                                     end_position - start_position, \
+                                     name, subscripts, value); \
+        } \
+    } while (0)
+
+// End of a syntax element with no subscript entries.
+#define CBS_TRACE_READ_END_NO_SUBSCRIPTS() \
+    do { \
+        const int *subscripts = NULL; \
+        CBS_TRACE_READ_END(); \
+    } while (0)
+
+// End of a syntax element which is made up of subelements which
+// are aleady traced, so we are only showing the value.
+#define CBS_TRACE_READ_END_VALUE_ONLY() \
+    do { \
+        if (ctx->trace_enable) { \
+            ctx->trace_read_callback(ctx->trace_context, &trace_start, 0, \
+                                     name, subscripts, value); \
+        } \
+    } while (0)
+
+// Start of a syntax element during write tracing.
+#define CBS_TRACE_WRITE_START() \
+    int start_position; \
+    do { \
+        if (ctx->trace_enable) \
+            start_position = put_bits_count(pbc);; \
+    } while (0)
+
+// End of a syntax element for tracing, make callback.
+#define CBS_TRACE_WRITE_END() \
+    do { \
+        if (ctx->trace_enable) { \
+            int end_position   = put_bits_count(pbc); \
+            av_assert0(start_position <= end_position); \
+            ctx->trace_write_callback(ctx->trace_context, pbc, \
+                                      end_position - start_position, \
+                                      name, subscripts, value); \
+        } \
+    } while (0)
+
+// End of a syntax element with no subscript entries.
+#define CBS_TRACE_WRITE_END_NO_SUBSCRIPTS() \
+    do { \
+        const int *subscripts = NULL; \
+        CBS_TRACE_WRITE_END(); \
+    } while (0)
+
+// End of a syntax element which is made up of subelements which are
+// aleady traced, so we are only showing the value.  This forges a
+// PutBitContext to point to the position of the start of the syntax
+// element, but the other state doesn't matter because length is zero.
+#define CBS_TRACE_WRITE_END_VALUE_ONLY() \
+    do { \
+        if (ctx->trace_enable) { \
+            PutBitContext tmp; \
+            init_put_bits(&tmp, pbc->buf, start_position); \
+            skip_put_bits(&tmp, start_position); \
+            ctx->trace_write_callback(ctx->trace_context, &tmp, 0, \
+                                      name, subscripts, value); \
+        } \
+    } while (0)
+
 #define TYPE_LIST(...) { __VA_ARGS__ }
 #define CBS_UNIT_TYPE_POD(type_, structure) { \
         .nb_unit_types  = 1, \
diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
index b0d5bd8763..816d06da04 100644
--- a/libavcodec/cbs_vp9.c
+++ b/libavcodec/cbs_vp9.c
@@ -28,11 +28,10 @@ static int cbs_vp9_read_s(CodedBitstreamContext *ctx, GetBitContext *gbc,
                           const int *subscripts, int32_t *write_to)
 {
     uint32_t magnitude;
-    int position, sign;
+    int sign;
     int32_t value;
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
     if (get_bits_left(gbc) < width + 1) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at "
@@ -44,17 +43,7 @@ static int cbs_vp9_read_s(CodedBitstreamContext *ctx, GetBitContext *gbc,
     sign      = get_bits1(gbc);
     value     = sign ? -(int32_t)magnitude : magnitude;
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = magnitude >> (width - i - 1) & 1 ? '1' : '0';
-        bits[i] = sign ? '1' : '0';
-        bits[i + 1] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
-    }
+    CBS_TRACE_READ_END();
 
     *write_to = value;
     return 0;
@@ -67,27 +56,19 @@ static int cbs_vp9_write_s(CodedBitstreamContext *ctx, PutBitContext *pbc,
     uint32_t magnitude;
     int sign;
 
+    CBS_TRACE_WRITE_START();
+
     if (put_bits_left(pbc) < width + 1)
         return AVERROR(ENOSPC);
 
     sign      = value < 0;
     magnitude = sign ? -value : value;
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (i = 0; i < width; i++)
-            bits[i] = magnitude >> (width - i - 1) & 1 ? '1' : '0';
-        bits[i] = sign ? '1' : '0';
-        bits[i + 1] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     put_bits(pbc, width, magnitude);
     put_bits(pbc, 1, sign);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
@@ -96,32 +77,24 @@ static int cbs_vp9_read_increment(CodedBitstreamContext *ctx, GetBitContext *gbc
                                   const char *name, uint32_t *write_to)
 {
     uint32_t value;
-    int position, i;
-    char bits[8];
 
-    av_assert0(range_min <= range_max && range_max - range_min < sizeof(bits) - 1);
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    CBS_TRACE_READ_START();
 
-    for (i = 0, value = range_min; value < range_max;) {
+    av_assert0(range_min <= range_max && range_max - range_min < 32);
+
+    for (value = range_min; value < range_max;) {
         if (get_bits_left(gbc) < 1) {
             av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid increment value at "
                    "%s: bitstream ended.\n", name);
             return AVERROR_INVALIDDATA;
         }
-        if (get_bits1(gbc)) {
-            bits[i++] = '1';
+        if (get_bits1(gbc))
             ++value;
-        } else {
-            bits[i++] = '0';
+        else
             break;
-        }
     }
 
-    if (ctx->trace_enable) {
-        bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, position, name, NULL, bits, value);
-    }
+    CBS_TRACE_READ_END_NO_SUBSCRIPTS();
 
     *write_to = value;
     return 0;
@@ -133,6 +106,8 @@ static int cbs_vp9_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
 {
     int len;
 
+    CBS_TRACE_WRITE_START();
+
     av_assert0(range_min <= range_max && range_max - range_min < 8);
     if (value < range_min || value > range_max) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
@@ -148,23 +123,11 @@ static int cbs_vp9_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
     if (put_bits_left(pbc) < len)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[8];
-        int i;
-        for (i = 0; i < len; i++) {
-            if (range_min + i == value)
-                bits[i] = '0';
-            else
-                bits[i] = '1';
-        }
-        bits[i] = 0;
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, NULL, bits, value);
-    }
-
     if (len > 0)
         put_bits(pbc, len, (1 << len) - 1 - (value != range_max));
 
+    CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
+
     return 0;
 }
 
@@ -173,12 +136,11 @@ static int cbs_vp9_read_le(CodedBitstreamContext *ctx, GetBitContext *gbc,
                            const int *subscripts, uint32_t *write_to)
 {
     uint32_t value;
-    int position, b;
+    int b;
 
-    av_assert0(width % 8 == 0);
+    CBS_TRACE_READ_START();
 
-    if (ctx->trace_enable)
-        position = get_bits_count(gbc);
+    av_assert0(width % 8 == 0);
 
     if (get_bits_left(gbc) < width) {
         av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid le value at "
@@ -190,17 +152,7 @@ static int cbs_vp9_read_le(CodedBitstreamContext *ctx, GetBitContext *gbc,
     for (b = 0; b < width; b += 8)
         value |= get_bits(gbc, 8) << b;
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (b = 0; b < width; b += 8)
-            for (i = 0; i < 8; i++)
-                bits[b + i] = value >> (b + i) & 1 ? '1' : '0';
-        bits[b] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
-                                    bits, value);
-    }
+    CBS_TRACE_READ_END();
 
     *write_to = value;
     return 0;
@@ -212,26 +164,18 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
 {
     int b;
 
+    CBS_TRACE_WRITE_START();
+
     av_assert0(width % 8 == 0);
 
     if (put_bits_left(pbc) < width)
         return AVERROR(ENOSPC);
 
-    if (ctx->trace_enable) {
-        char bits[33];
-        int i;
-        for (b = 0; b < width; b += 8)
-            for (i = 0; i < 8; i++)
-                bits[b + i] = value >> (b + i) & 1 ? '1' : '0';
-        bits[b] = 0;
-
-        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
-                                    name, subscripts, bits, value);
-    }
-
     for (b = 0; b < width; b += 8)
         put_bits(pbc, 8, value >> b & 0xff);
 
+    CBS_TRACE_WRITE_END();
+
     return 0;
 }
 
diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c
index 028b0a1e59..8781f5f100 100644
--- a/libavcodec/trace_headers_bsf.c
+++ b/libavcodec/trace_headers_bsf.c
@@ -44,6 +44,8 @@ static int trace_headers_init(AVBSFContext *bsf)
 
     ctx->cbc->trace_enable = 1;
     ctx->cbc->trace_level  = AV_LOG_INFO;
+    ctx->cbc->trace_context = ctx->cbc;
+    ctx->cbc->trace_read_callback = ff_cbs_trace_read_log;
 
     if (bsf->par_in->extradata) {
         CodedBitstreamFragment *frag = &ctx->fragment;