[FFmpeg-devel] [PATCH v1] avcodec/cbs: Keep ff_cbs_trace_syntax_element

[Dai, Jianhui J]

> -----Original Message-----
> From: Dai, Jianhui J <jianhui.j.dai at intel.com>
> Sent: Tuesday, October 10, 2023 10:57 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: [PATCH v1] avcodec/cbs: Keep ff_cbs_trace_syntax_element
> 
> Split ff_cbs_trace_syntax_element from ff_cbs_trace_read_log to decouple the
> tracing from GetBitContext. This allows CBS implementations that do not have a
> GetBitContext to directly use ff_cbs_trace_syntax_element to trace syntax
> elements.
> 
> Signed-off-by: Jianhui Dai <jianhui.j.dai at intel.com>
> ---
>  libavcodec/cbs.c          | 41 +++++++++++++++++++++++++--------------
>  libavcodec/cbs_internal.h |  4 ++++
>  2 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index cdd7adebeb..2f2cfcfb31
> 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -498,26 +498,18 @@ void ff_cbs_trace_header(CodedBitstreamContext
> *ctx,
>      av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name);  }
> 
> -void ff_cbs_trace_read_log(void *trace_context,
> -                           GetBitContext *gbc, int length,
> -                           const char *str, const int *subscripts,
> -                           int64_t value)
> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
> +                                 const char *str, const int *subscripts,
> +                                 const char *bits, 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;
> -    int position;
> -
> -    av_assert0(value >= INT_MIN && value <= UINT32_MAX);
> 
> -    position = get_bits_count(gbc);
> +    if (!ctx->trace_enable)
> +        return;
> 
> -    av_assert0(length < 256);
> -    for (i = 0; i < length; i++)
> -        bits[i] = get_bits1(gbc) ? '1' : '0';
> -    bits[length] = 0;
> +    av_assert0(value >= INT_MIN && value <= UINT32_MAX);
> 
>      subs = subscripts ? subscripts[0] : 0;
>      n = 0;
> @@ -545,7 +537,7 @@ void ff_cbs_trace_read_log(void *trace_context,
>      av_assert0(n == subs);
> 
>      name_len = strlen(name);
> -    bits_len = length;
> +    bits_len = strlen(bits);
> 
>      if (name_len + bits_len > 60)
>          pad = bits_len + 2;
> @@ -556,6 +548,25 @@ void ff_cbs_trace_read_log(void *trace_context,
>             position, name, pad, bits, 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 bits[256];
> +    int position;
> +
> +    position = get_bits_count(gbc);
> +
> +    av_assert0(length < 256);
> +    for (int i = 0; i < length; i++)
> +        bits[i] = get_bits1(gbc) ? '1' : '0';
> +    bits[length] = 0;
> +
> +    ff_cbs_trace_syntax_element(ctx, position, str, subscripts, bits,
> +value); }
> +
>  void ff_cbs_trace_write_log(void *trace_context,
>                              PutBitContext *pbc, int length,
>                              const char *str, const int *subscripts, diff --git
> a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h index
> 07220f1f3e..ff90ce467d 100644
> --- a/libavcodec/cbs_internal.h
> +++ b/libavcodec/cbs_internal.h
> @@ -158,6 +158,10 @@ 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

@Mark Thompson <sw at jkqxz.net>
Could you please take a look if it's a valid change based on your last refactor?
It's intended to use for the reviewing cbs_vp8 patch.

> --
> 2.25.1