[FFmpeg-devel] [PATCH 1/2] avcodec/cbs: Add specialization for ff_cbs_(read|write)_unsigned()

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Sun Jul 30 19:52:11 EEST 2023


Andreas Rheinhardt:
> These functions allow not only to read and write unsigned values,
> but also to check ranges and to emit trace output which can be
> beautified when processing arrays (indices like "[i]" are replaced
> by their actual numbers).
> 
> Yet lots of callers actually only need something simpler:
> Their range is only implicitly restricted by the amount
> of bits used and they are not part of arrays, hence don't
> need this beautification.
> 
> This commit adds specializations for these callers;
> this is very beneficial size-wise (it reduced the size
> of .text by 14928 bytes here), as a call is now cheaper.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>  libavcodec/cbs.c          | 34 ++++++++++++++++++++++++++++++----
>  libavcodec/cbs_av1.c      | 28 +++++++++++++++++++---------
>  libavcodec/cbs_h2645.c    | 15 +++++++++++++--
>  libavcodec/cbs_internal.h | 11 ++++++++++-
>  libavcodec/cbs_mpeg2.c    | 15 +++++++++++++--
>  libavcodec/cbs_vp9.c      | 14 ++++++++++++--
>  6 files changed, 97 insertions(+), 20 deletions(-)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index e829caa0a0..8bd63aa831 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -536,10 +536,13 @@ void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
>             position, name, pad, bits, value);
>  }
>  
> -int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> -                         int width, const char *name,
> -                         const int *subscripts, uint32_t *write_to,
> -                         uint32_t range_min, uint32_t range_max)
> +static av_always_inline int cbs_read_unsigned(CodedBitstreamContext *ctx,
> +                                              GetBitContext *gbc,
> +                                              int width, const char *name,
> +                                              const int *subscripts,
> +                                              uint32_t *write_to,
> +                                              uint32_t range_min,
> +                                              uint32_t range_max)
>  {
>      uint32_t value;
>      int position;
> @@ -579,6 +582,22 @@ int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
>      return 0;
>  }
>  
> +int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> +                         int width, const char *name,
> +                         const int *subscripts, uint32_t *write_to,
> +                         uint32_t range_min, uint32_t range_max)
> +{
> +    return cbs_read_unsigned(ctx, gbc, width, name, subscripts,
> +                             write_to, range_min, range_max);
> +}
> +
> +int ff_cbs_read_simple_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> +                                int width, const char *name, uint32_t *write_to)
> +{
> +    return cbs_read_unsigned(ctx, gbc, width, name, NULL,
> +                             write_to, 0, UINT32_MAX);
> +}
> +
>  int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>                            int width, const char *name,
>                            const int *subscripts, uint32_t value,
> @@ -615,6 +634,13 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>      return 0;
>  }
>  
> +int ff_cbs_write_simple_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
> +                                 int width, const char *name, uint32_t value)
> +{
> +    return ff_cbs_write_unsigned(ctx, pbc, width, name, NULL,
> +                                 value, 0, MAX_UINT_BITS(width));
> +}
> +
>  int ff_cbs_read_signed(CodedBitstreamContext *ctx, GetBitContext *gbc,
>                         int width, const char *name,
>                         const int *subscripts, int32_t *write_to,
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 1229480567..114d8580b3 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -412,9 +412,8 @@ static int cbs_av1_read_subexp(CodedBitstreamContext *ctx, GetBitContext *gbc,
>      }
>  
>      if (len < max_len) {
> -        err = ff_cbs_read_unsigned(ctx, gbc, range_bits,
> -                                   "subexp_bits", NULL, &value,
> -                                   0, MAX_UINT_BITS(range_bits));
> +        err = ff_cbs_read_simple_unsigned(ctx, gbc, range_bits,
> +                                          "subexp_bits", &value);
>          if (err < 0)
>              return err;
>  
> @@ -476,10 +475,9 @@ static int cbs_av1_write_subexp(CodedBitstreamContext *ctx, PutBitContext *pbc,
>          return err;
>  
>      if (len < max_len) {
> -        err = ff_cbs_write_unsigned(ctx, pbc, range_bits,
> -                                    "subexp_bits", NULL,
> -                                    value - range_offset,
> -                                    0, MAX_UINT_BITS(range_bits));
> +        err = ff_cbs_write_simple_unsigned(ctx, pbc, range_bits,
> +                                           "subexp_bits",
> +                                           value - range_offset);
>          if (err < 0)
>              return err;
>  
> @@ -546,8 +544,6 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  
>  #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL)
>  
> -#define fb(width, name) \
> -        xf(width, name, current->name, 0, MAX_UINT_BITS(width), 0, )
>  #define fc(width, name, range_min, range_max) \
>          xf(width, name, current->name, range_min, range_max, 0, )
>  #define flag(name) fb(1, name)
> @@ -573,6 +569,13 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  #define READWRITE read
>  #define RWContext GetBitContext
>  
> +#define fb(width, name) do { \
> +        uint32_t value; \
> +        CHECK(ff_cbs_read_simple_unsigned(ctx, rw, width, \
> +                                          #name, &value)); \
> +        current->name = value; \
> +    } while (0)
> +
>  #define xf(width, name, var, range_min, range_max, subs, ...) do { \
>          uint32_t value; \
>          CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> @@ -645,6 +648,7 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  #undef READ
>  #undef READWRITE
>  #undef RWContext
> +#undef fb
>  #undef xf
>  #undef xsu
>  #undef uvlc
> @@ -661,6 +665,11 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  #define READWRITE write
>  #define RWContext PutBitContext
>  
> +#define fb(width, name) do { \
> +        CHECK(ff_cbs_write_simple_unsigned(ctx, rw, width, #name, \
> +                                           current->name)); \
> +    } while (0)
> +
>  #define xf(width, name, var, range_min, range_max, subs, ...) do { \
>          CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
>                                      SUBSCRIPTS(subs, __VA_ARGS__), \
> @@ -723,6 +732,7 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  #undef WRITE
>  #undef READWRITE
>  #undef RWContext
> +#undef fb
>  #undef xf
>  #undef xsu
>  #undef uvlc
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 10b3bcc70b..54f6da186b 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -263,8 +263,6 @@ static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t paylo
>  
>  #define u(width, name, range_min, range_max) \
>          xu(width, name, current->name, range_min, range_max, 0, )
> -#define ub(width, name) \
> -        xu(width, name, current->name, 0, MAX_UINT_BITS(width), 0, )
>  #define flag(name) ub(1, name)
>  #define ue(name, range_min, range_max) \
>          xue(name, current->name, range_min, range_max, 0, )
> @@ -300,6 +298,12 @@ static int cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t paylo
>  #define READWRITE read
>  #define RWContext GetBitContext
>  
> +#define ub(width, name) do { \
> +        uint32_t value; \
> +        CHECK(ff_cbs_read_simple_unsigned(ctx, rw, width, #name, \
> +                                          &value)); \
> +        current->name = value; \
> +    } while (0)
>  #define xu(width, name, var, range_min, range_max, subs, ...) do { \
>          uint32_t value; \
>          CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> @@ -374,6 +378,7 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext *gbc)
>  #undef READ
>  #undef READWRITE
>  #undef RWContext
> +#undef ub
>  #undef xu
>  #undef xi
>  #undef xue
> @@ -389,6 +394,11 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext *gbc)
>  #define READWRITE write
>  #define RWContext PutBitContext
>  
> +#define ub(width, name) do { \
> +        uint32_t value = current->name; \
> +        CHECK(ff_cbs_write_simple_unsigned(ctx, rw, width, #name, \
> +                                           value)); \
> +    } while (0)
>  #define xu(width, name, var, range_min, range_max, subs, ...) do { \
>          uint32_t value = var; \
>          CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
> @@ -452,6 +462,7 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext *gbc)
>  #undef WRITE
>  #undef READWRITE
>  #undef RWContext
> +#undef ub
>  #undef xu
>  #undef xi
>  #undef xue
> diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
> index f853086fa3..2838be6b9e 100644
> --- a/libavcodec/cbs_internal.h
> +++ b/libavcodec/cbs_internal.h
> @@ -147,18 +147,27 @@ void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
>  
>  
>  // Helper functions for read/write of common bitstream elements, including
> -// generation of trace output.
> +// generation of trace output. The simple functions are equivalent to
> +// their non-simple counterparts except that their range is unrestricted
> +// (i.e. only limited by the amount of bits used) and they lack
> +// the ability to use subscripts.
>  
>  int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
>                           int width, const char *name,
>                           const int *subscripts, uint32_t *write_to,
>                           uint32_t range_min, uint32_t range_max);
>  
> +int ff_cbs_read_simple_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> +                                int width, const char *name, uint32_t *write_to);
> +
>  int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>                            int width, const char *name,
>                            const int *subscripts, uint32_t value,
>                            uint32_t range_min, uint32_t range_max);
>  
> +int ff_cbs_write_simple_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
> +                                 int width, const char *name, uint32_t value);
> +
>  int ff_cbs_read_signed(CodedBitstreamContext *ctx, GetBitContext *gbc,
>                         int width, const char *name,
>                         const int *subscripts, int32_t *write_to,
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 33bd3e0998..df58b50bab 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -40,8 +40,6 @@
>  
>  #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL)
>  
> -#define ui(width, name) \
> -        xui(width, name, current->name, 0, MAX_UINT_BITS(width), 0, )
>  #define uir(width, name) \
>          xui(width, name, current->name, 1, MAX_UINT_BITS(width), 0, )
>  #define uis(width, name, subs, ...) \
> @@ -65,6 +63,12 @@
>  #define READWRITE read
>  #define RWContext GetBitContext
>  
> +#define ui(width, name) do { \
> +        uint32_t value; \
> +        CHECK(ff_cbs_read_simple_unsigned(ctx, rw, width, #name, \
> +                                          &value)); \
> +        current->name = value; \
> +    } while (0)
>  #define xuia(width, string, var, range_min, range_max, subs, ...) do { \
>          uint32_t value; \
>          CHECK(ff_cbs_read_unsigned(ctx, rw, width, string, \
> @@ -95,6 +99,7 @@
>  #undef READ
>  #undef READWRITE
>  #undef RWContext
> +#undef ui
>  #undef xuia
>  #undef xsi
>  #undef nextbits
> @@ -105,6 +110,11 @@
>  #define READWRITE write
>  #define RWContext PutBitContext
>  
> +#define ui(width, name) do { \
> +        CHECK(ff_cbs_write_simple_unsigned(ctx, rw, width, #name, \
> +                                           current->name)); \
> +    } while (0)
> +
>  #define xuia(width, string, var, range_min, range_max, subs, ...) do { \
>          CHECK(ff_cbs_write_unsigned(ctx, rw, width, string, \
>                                      SUBSCRIPTS(subs, __VA_ARGS__), \
> @@ -134,6 +144,7 @@
>  #undef WRITE
>  #undef READWRITE
>  #undef RWContext
> +#undef ui
>  #undef xuia
>  #undef xsi
>  #undef nextbits
> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
> index ae7f88a8a3..def2ae0d21 100644
> --- a/libavcodec/cbs_vp9.c
> +++ b/libavcodec/cbs_vp9.c
> @@ -251,8 +251,6 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  
>  #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL)
>  
> -#define f(width, name) \
> -        xf(width, name, current->name, 0, )
>  #define s(width, name) \
>          xs(width, name, current->name, 0, )
>  #define fs(width, name, subs, ...) \
> @@ -264,6 +262,12 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #define READWRITE read
>  #define RWContext GetBitContext
>  
> +#define f(width, name) do { \
> +        uint32_t value; \
> +        CHECK(ff_cbs_read_simple_unsigned(ctx, rw, width, #name, \
> +                                          &value)); \
> +        current->name = value; \
> +    } while (0)
>  #define xf(width, name, var, subs, ...) do { \
>          uint32_t value; \
>          CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> @@ -329,6 +333,7 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #undef READ
>  #undef READWRITE
>  #undef RWContext
> +#undef f
>  #undef xf
>  #undef xs
>  #undef increment
> @@ -344,6 +349,10 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #define READWRITE write
>  #define RWContext PutBitContext
>  
> +#define f(width, name) do { \
> +        CHECK(ff_cbs_write_simple_unsigned(ctx, rw, width, #name, \
> +                                           current->name)); \
> +    } while (0)
>  #define xf(width, name, var, subs, ...) do { \
>          CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
>                                      SUBSCRIPTS(subs, __VA_ARGS__), \
> @@ -396,6 +405,7 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #undef WRITE
>  #undef READWRITE
>  #undef RWContext
> +#undef f
>  #undef xf
>  #undef xs
>  #undef increment

Seems I also forgot to apply this. Will apply it tonight unless there
are objections.

- Andreas



More information about the ffmpeg-devel mailing list