[FFmpeg-devel] [PATCH] avcodec/cbs: Avoid leaving the ... out in calls to variadic macros
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Tue Apr 7 15:51:06 EEST 2020
Andreas Rheinhardt:
> 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
>
Ping.
- Andreas
More information about the ffmpeg-devel
mailing list