[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