[FFmpeg-devel] [PATCH v5 2/8] cbs: Make tracing more general
Neal Gompa
ngompa13 at gmail.com
Wed Sep 20 03:41:11 EEST 2023
On Mon, Sep 11, 2023 at 3:53 AM <fei.w.wang-at-intel.com at ffmpeg.org> wrote:
>
> From: Mark Thompson <sw at jkqxz.net>
>
> 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 at intel.com>
> ---
> libavcodec/cbs.c | 121 +++++++++----------
> libavcodec/cbs.h | 88 +++++++++++++-
> libavcodec/cbs_av1.c | 206 +++++++++------------------------
> libavcodec/cbs_bsf.c | 5 +
> libavcodec/cbs_h2645.c | 134 +++++++++------------
> libavcodec/cbs_internal.h | 85 +++++++++++++-
> libavcodec/cbs_vp9.c | 108 +++++------------
> libavcodec/trace_headers_bsf.c | 2 +
> 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;
> --
> 2.25.1
>
Code looks good to me, the parts I can exercise works with my hardware.
Reviewed-by: Neal Gompa <ngompa13 at gmail.com>
--
真実はいつも一つ！/ Always, there's only one truth!
More information about the ffmpeg-devel
mailing list