[FFmpeg-devel] [PATCH] avcodec/cbs: Avoid leaving the ... out in calls to variadic macros

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Tue Mar 31 16:16:45 EEST 2020


Andreas Rheinhardt:
> According to C99, there has to be at least one argument for every ...
> in a variadic function-like macro. In practice most (all?) compilers also
> allow to leave it completely out, but it is nevertheless required: In a
> variadic macro "there shall be more arguments in the invocation than there
> are parameters in the macro definition (excluding the ...)." (C99,
> 6.10.3.4).
> 
> CBS (not the framework itself, but the macros used in the
> cbs_*_syntax_template.c files) relies on the compiler allowing to leave
> a variadic macro argument out. This leads to warnings when compiling in
> -pedantic mode, e.g. "warning: must specify at least one argument for
> '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]"
> from Clang.
> 
> Most of these warnings can be easily avoided: The syntax_templates
> mostly contain helper macros that expand to more complex variadic macros
> and these helper macros often omit an argument for the .... Modifying
> them to always expand to complex macros with an empty argument for the
> ... at the end fixes most of these warnings: The number of warnings went
> down from 400 to 0 for cbs_av1, from 1114 to 32 for cbs_h2645, from 38 to
> 0 for cbs_jpeg, from 166 to 0 for cbs_mpeg2 and from 110 to 8 for cbs_vp9.
> 
> These eight remaining warnings for cbs_vp9 have been fixed by switching
> to another macro in cbs_vp9_syntax_template: The fixed values for the
> sync bytes as well as the trailing bits for byte-alignment are now read
> via the fixed() macro (this also adds a check to ensure that trailing
> bits are indeed zero as they have to be).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> There are two ways to fix the remaining 32 warnings from cbs_h2645:
> 
> Simply add ", " to all macro calls that make use of the complex macros;
> this has the drawback of adding uglyness to cbs_h26x_syntax_template.c.
> 
> Or add new macros for these macro calls: The places that produce
> warnings use the complex macros directly, because they use names
> different from the default names that the helper macros use, but they do
> not use subscripts and therefore leave the variadic argument (designed
> for subscripts) out. I would have implemented the second solution if it
> were not for the problem of the naming of the new macros.
> 
> (There is of course also the possibility not to care about the remaining
> ones.)
> 
>  libavcodec/cbs_av1.c                 | 16 ++++++++--------
>  libavcodec/cbs_h2645.c               | 14 +++++++-------
>  libavcodec/cbs_jpeg.c                |  2 +-
>  libavcodec/cbs_mpeg2.c               |  6 +++---
>  libavcodec/cbs_vp9.c                 | 13 ++++++-------
>  libavcodec/cbs_vp9_syntax_template.c | 21 ++++-----------------
>  6 files changed, 29 insertions(+), 43 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 16eb7143b6..fc228086c2 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -550,12 +550,12 @@ 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)
> +        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)
> +        xf(width, name, current->name, range_min, range_max, 0, )
>  #define flag(name) fb(1, name)
>  #define su(width, name) \
> -        xsu(width, name, current->name, 0)
> +        xsu(width, name, current->name, 0, )
>  
>  #define fbs(width, name, subs, ...) \
>          xf(width, name, current->name, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__)
> @@ -568,7 +568,7 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  
>  #define fixed(width, name, value) do { \
>          av_unused uint32_t fixed_value = value; \
> -        xf(width, name, fixed_value, value, value, 0); \
> +        xf(width, name, fixed_value, value, value, 0, ); \
>      } while (0)
>  
>  
> @@ -623,9 +623,9 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>  #define delta_q(name) do { \
>          uint8_t delta_coded; \
>          int8_t delta_q; \
> -        xf(1, name.delta_coded, delta_coded, 0, 1, 0); \
> +        xf(1, name.delta_coded, delta_coded, 0, 1, 0, ); \
>          if (delta_coded) \
> -            xsu(1 + 6, name.delta_q, delta_q, 0); \
> +            xsu(1 + 6, name.delta_q, delta_q, 0, ); \
>          else \
>              delta_q = 0; \
>          current->name = delta_q; \
> @@ -700,9 +700,9 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc)
>      } while (0)
>  
>  #define delta_q(name) do { \
> -        xf(1, name.delta_coded, current->name != 0, 0, 1, 0); \
> +        xf(1, name.delta_coded, current->name != 0, 0, 1, 0, ); \
>          if (current->name) \
> -            xsu(1 + 6, name.delta_q, current->name, 0); \
> +            xsu(1 + 6, name.delta_q, current->name, 0, ); \
>      } while (0)
>  
>  #define leb128(name) do { \
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 7a4eecf439..d42073cc5a 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -250,18 +250,18 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL)
>  
>  #define u(width, name, range_min, range_max) \
> -        xu(width, name, current->name, range_min, range_max, 0)
> +        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)
> +        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)
> +        xue(name, current->name, range_min, range_max, 0, )
>  #define i(width, name, range_min, range_max) \
> -        xi(width, name, current->name, range_min, range_max, 0)
> +        xi(width, name, current->name, range_min, range_max, 0, )
>  #define ib(width, name) \
> -        xi(width, name, current->name, MIN_INT_BITS(width), MAX_INT_BITS(width), 0)
> +        xi(width, name, current->name, MIN_INT_BITS(width), MAX_INT_BITS(width), 0, )
>  #define se(name, range_min, range_max) \
> -        xse(name, current->name, range_min, range_max, 0)
> +        xse(name, current->name, range_min, range_max, 0, )
>  
>  #define us(width, name, range_min, range_max, subs, ...) \
>          xu(width, name, current->name, range_min, range_max, subs, __VA_ARGS__)
> @@ -280,7 +280,7 @@ static int cbs_write_se_golomb(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  
>  #define fixed(width, name, value) do { \
>          av_unused uint32_t fixed_value = value; \
> -        xu(width, name, fixed_value, value, value, 0); \
> +        xu(width, name, fixed_value, value, value, 0, ); \
>      } while (0)
>  
>  
> diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> index 89512a26bb..4ff04ae52d 100644
> --- a/libavcodec/cbs_jpeg.c
> +++ b/libavcodec/cbs_jpeg.c
> @@ -34,7 +34,7 @@
>  #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL)
>  
>  #define u(width, name, range_min, range_max) \
> -    xu(width, name, range_min, range_max, 0)
> +    xu(width, name, range_min, range_max, 0, )
>  #define us(width, name, sub, range_min, range_max) \
>      xu(width, name, range_min, range_max, 1, sub)
>  
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 0e5d08ecbf..97f7889cbb 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -41,9 +41,9 @@
>  #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)
> +        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)
> +        xui(width, name, current->name, 1, MAX_UINT_BITS(width), 0, )
>  #define uis(width, name, subs, ...) \
>          xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__)
>  #define uirs(width, name, subs, ...) \
> @@ -57,7 +57,7 @@
>          bit("marker_bit", 1)
>  #define bit(string, value) do { \
>          av_unused uint32_t bit = value; \
> -        xuia(1, string, bit, value, value, 0); \
> +        xuia(1, string, bit, value, value, 0, ); \
>      } while (0)
>  
>  
> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
> index ec82f11c76..eef603bfb2 100644
> --- a/libavcodec/cbs_vp9.c
> +++ b/libavcodec/cbs_vp9.c
> @@ -253,15 +253,14 @@ 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)
> +        xf(width, name, current->name, 0, )
>  #define s(width, name) \
> -        xs(width, name, current->name, 0)
> +        xs(width, name, current->name, 0, )
>  #define fs(width, name, subs, ...) \
>          xf(width, name, current->name, subs, __VA_ARGS__)
>  #define ss(width, name, subs, ...) \
>          xs(width, name, current->name, subs, __VA_ARGS__)
>  
> -
>  #define READ
>  #define READWRITE read
>  #define RWContext GetBitContext
> @@ -295,9 +294,9 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>  #define delta_q(name) do { \
>          uint8_t delta_coded; \
>          int8_t delta_q; \
> -        xf(1, name.delta_coded, delta_coded, 0); \
> +        xf(1, name.delta_coded, delta_coded, 0, ); \
>          if (delta_coded) \
> -            xs(4, name.delta_q, delta_q, 0); \
> +            xs(4, name.delta_q, delta_q, 0, ); \
>          else \
>              delta_q = 0; \
>          current->name = delta_q; \
> @@ -366,9 +365,9 @@ static int cbs_vp9_write_le(CodedBitstreamContext *ctx, PutBitContext *pbc,
>      } while (0)
>  
>  #define delta_q(name) do { \
> -        xf(1, name.delta_coded, !!current->name, 0); \
> +        xf(1, name.delta_coded, !!current->name, 0, ); \
>          if (current->name) \
> -            xs(4, name.delta_q, current->name, 0); \
> +            xs(4, name.delta_q, current->name, 0, ); \
>      } while (0)
>  
>  #define prob(name, subs, ...) do { \
> diff --git a/libavcodec/cbs_vp9_syntax_template.c b/libavcodec/cbs_vp9_syntax_template.c
> index 125eb02589..2f08eccf18 100644
> --- a/libavcodec/cbs_vp9_syntax_template.c
> +++ b/libavcodec/cbs_vp9_syntax_template.c
> @@ -19,23 +19,11 @@
>  static int FUNC(frame_sync_code)(CodedBitstreamContext *ctx, RWContext *rw,
>                                   VP9RawFrameHeader *current)
>  {
> -    uint8_t frame_sync_byte_0 = VP9_FRAME_SYNC_0;
> -    uint8_t frame_sync_byte_1 = VP9_FRAME_SYNC_1;
> -    uint8_t frame_sync_byte_2 = VP9_FRAME_SYNC_2;
>      int err;
>  
> -    xf(8, frame_sync_byte_0, frame_sync_byte_0, 0);
> -    xf(8, frame_sync_byte_1, frame_sync_byte_1, 0);
> -    xf(8, frame_sync_byte_2, frame_sync_byte_2, 0);
> -
> -    if (frame_sync_byte_0 != VP9_FRAME_SYNC_0 ||
> -        frame_sync_byte_1 != VP9_FRAME_SYNC_1 ||
> -        frame_sync_byte_2 != VP9_FRAME_SYNC_2) {
> -        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid frame sync code: "
> -               "%02x %02x %02x.\n", frame_sync_byte_0,
> -               frame_sync_byte_1, frame_sync_byte_2);
> -        return AVERROR_INVALIDDATA;
> -    }
> +    fixed(8, frame_sync_byte_0, VP9_FRAME_SYNC_0);
> +    fixed(8, frame_sync_byte_1, VP9_FRAME_SYNC_1);
> +    fixed(8, frame_sync_byte_2, VP9_FRAME_SYNC_2);
>  
>      return 0;
>  }
> @@ -396,9 +384,8 @@ static int FUNC(uncompressed_header)(CodedBitstreamContext *ctx, RWContext *rw,
>  static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, RWContext *rw)
>  {
>      int err;
> -    av_unused int zero = 0;
>      while (byte_alignment(rw) != 0)
> -        xf(1, zero_bit, zero, 0);
> +        fixed(1, zero_bit, 0);
>  
>      return 0;
>  }
> 
Ping.

- Andreas


More information about the ffmpeg-devel mailing list