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