Message ID | tencent_280E4EA7054B7318E85C93E2537C15DF0905@qq.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avutil/channel_layout: make pre-defined channel layouts C++ friendly | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
NAK, as full codebase have usages of designated initializers.
On 8/16/2023 12:44 PM, Zhao Zhili wrote: > From: Zhao Zhili <zhilizhao@tencent.com> > > C++ doesn't support designated initializers until C++20. We have > a bunch of pre-defined channel layouts, the gains to make them > usable in C++ exceed the losses. > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > --- > libavutil/channel_layout.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > index f345415c55..817a5ad370 100644 > --- a/libavutil/channel_layout.h > +++ b/libavutil/channel_layout.h > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > } AVChannelLayout; > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { .mask = (m) }} > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } Comment out the field names instead of removing them altogether, and add a comment about how this is done for the sake of c++ projects including this header. > > /** > * @name Common pre-defined channel layouts > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) > #define AV_CHANNEL_LAYOUT_22POINT2 AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { .mask = 0 }} > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > /** @} */ > > struct AVBPrint; This needs a minor bump IMO. Something said c++ projects can look for before using these defines.
On Wed, Aug 16, 2023 at 6:21 PM James Almer <jamrial@gmail.com> wrote: > On 8/16/2023 12:44 PM, Zhao Zhili wrote: > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > C++ doesn't support designated initializers until C++20. We have > > a bunch of pre-defined channel layouts, the gains to make them > > usable in C++ exceed the losses. > > > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > > --- > > libavutil/channel_layout.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > > index f345415c55..817a5ad370 100644 > > --- a/libavutil/channel_layout.h > > +++ b/libavutil/channel_layout.h > > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > > } AVChannelLayout; > > > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { > .mask = (m) }} > > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } > > Comment out the field names instead of removing them altogether, and add > a comment about how this is done for the sake of c++ projects including > this header. > > > > > /** > > * @name Common pre-defined channel layouts > > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX AV_CHANNEL_LAYOUT_MASK(2, > AV_CH_LAYOUT_STEREO_DOWNMIX) > > #define AV_CHANNEL_LAYOUT_22POINT2 AV_CHANNEL_LAYOUT_MASK(24, > AV_CH_LAYOUT_22POINT2) > > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { > .mask = 0 }} > > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > > /** @} */ > > > > struct AVBPrint; > > This needs a minor bump IMO. Something said c++ projects can look for > before using these defines. > OK, I had enough, I'm leaving FFmpeg once and forever. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer > Sent: 2023年8月17日 0:22 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avutil/channel_layout: make pre-defined channel layouts C++ friendly > > On 8/16/2023 12:44 PM, Zhao Zhili wrote: > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > C++ doesn't support designated initializers until C++20. We have > > a bunch of pre-defined channel layouts, the gains to make them > > usable in C++ exceed the losses. > > > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > > --- > > libavutil/channel_layout.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > > index f345415c55..817a5ad370 100644 > > --- a/libavutil/channel_layout.h > > +++ b/libavutil/channel_layout.h > > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > > } AVChannelLayout; > > > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { .mask = (m) }} > > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } > > Comment out the field names instead of removing them altogether, and add > a comment about how this is done for the sake of c++ projects including > this header. OK. So it won't be reverted by accident. > > > > > /** > > * @name Common pre-defined channel layouts > > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) > > #define AV_CHANNEL_LAYOUT_22POINT2 AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) > > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { .mask = 0 }} > > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > > /** @} */ > > > > struct AVBPrint; > > This needs a minor bump IMO. Something said c++ projects can look for > before using these defines. Good idea. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 8/16/2023 1:52 PM, Paul B Mahol wrote: > On Wed, Aug 16, 2023 at 6:21 PM James Almer <jamrial@gmail.com> wrote: > >> On 8/16/2023 12:44 PM, Zhao Zhili wrote: >>> From: Zhao Zhili <zhilizhao@tencent.com> >>> >>> C++ doesn't support designated initializers until C++20. We have >>> a bunch of pre-defined channel layouts, the gains to make them >>> usable in C++ exceed the losses. >>> >>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>> --- >>> libavutil/channel_layout.h | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h >>> index f345415c55..817a5ad370 100644 >>> --- a/libavutil/channel_layout.h >>> +++ b/libavutil/channel_layout.h >>> @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { >>> } AVChannelLayout; >>> >>> #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ >>> - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { >> .mask = (m) }} >>> + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } >> >> Comment out the field names instead of removing them altogether, and add >> a comment about how this is done for the sake of c++ projects including >> this header. >> >>> >>> /** >>> * @name Common pre-defined channel layouts >>> @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { >>> #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX AV_CHANNEL_LAYOUT_MASK(2, >> AV_CH_LAYOUT_STEREO_DOWNMIX) >>> #define AV_CHANNEL_LAYOUT_22POINT2 AV_CHANNEL_LAYOUT_MASK(24, >> AV_CH_LAYOUT_22POINT2) >>> #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ >>> - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { >> .mask = 0 }} >>> + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } >>> /** @} */ >>> >>> struct AVBPrint; >> >> This needs a minor bump IMO. Something said c++ projects can look for >> before using these defines. >> > > OK, I had enough, I'm leaving FFmpeg once and forever. We use designated initializers in source files and internal headers, but not in public installed headers, so c++ projects can include them. It's the same reason we avoid bitfields in those too.
ons 2023-08-16 klockan 23:44 +0800 skrev Zhao Zhili: > From: Zhao Zhili <zhilizhao@tencent.com> > > C++ doesn't support designated initializers until C++20. We have > a bunch of pre-defined channel layouts, the gains to make them > usable in C++ exceed the losses. > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > --- > libavutil/channel_layout.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h > index f345415c55..817a5ad370 100644 > --- a/libavutil/channel_layout.h > +++ b/libavutil/channel_layout.h > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > } AVChannelLayout; > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { > .mask = (m) }} > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } > > /** > * @name Common pre-defined channel layouts > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX > AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) > #define AV_CHANNEL_LAYOUT_22POINT2 > AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { > .mask = 0 }} > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } For C++ compat you could use some constructor magic instead, and turn these into proper constants. Hidden behind #ifdef __cplusplus of course. IMO having untyped #defines like this is mega ugly. At the very least it should be (AVChannelLayout){...}. It's likely cargo culted from when channel layouts were bitmasks. /Tomas
> On Aug 17, 2023, at 20:57, Tomas Härdin <git@haerdin.se> wrote: > > ons 2023-08-16 klockan 23:44 +0800 skrev Zhao Zhili: >> From: Zhao Zhili <zhilizhao@tencent.com> >> >> C++ doesn't support designated initializers until C++20. We have >> a bunch of pre-defined channel layouts, the gains to make them >> usable in C++ exceed the losses. >> >> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >> --- >> libavutil/channel_layout.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h >> index f345415c55..817a5ad370 100644 >> --- a/libavutil/channel_layout.h >> +++ b/libavutil/channel_layout.h >> @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { >> } AVChannelLayout; >> >> #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ >> - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { >> .mask = (m) }} >> + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } >> >> /** >> * @name Common pre-defined channel layouts >> @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { >> #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX >> AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) >> #define AV_CHANNEL_LAYOUT_22POINT2 >> AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) >> #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ >> - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { >> .mask = 0 }} >> + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > > For C++ compat you could use some constructor magic instead, and turn > these into proper constants. Hidden behind #ifdef __cplusplus of > course. I’m trying to avoid more debate to not refer to __cplusplus on purpose. > > IMO having untyped #defines like this is mega ugly. At the very least > it should be (AVChannelLayout){...}. It's likely cargo culted from when > channel layouts were bitmasks. (AVChannelLayout){…} can be invalid in C++. AV_TIME_BASE_Q has the problem already. > > /Tomas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
tor 2023-08-17 klockan 22:03 +0800 skrev zhilizhao(赵志立): > > > > On Aug 17, 2023, at 20:57, Tomas Härdin <git@haerdin.se> wrote: > > > > ons 2023-08-16 klockan 23:44 +0800 skrev Zhao Zhili: > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > > > C++ doesn't support designated initializers until C++20. We have > > > a bunch of pre-defined channel layouts, the gains to make them > > > usable in C++ exceed the losses. > > > > > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > > > --- > > > libavutil/channel_layout.h | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavutil/channel_layout.h > > > b/libavutil/channel_layout.h > > > index f345415c55..817a5ad370 100644 > > > --- a/libavutil/channel_layout.h > > > +++ b/libavutil/channel_layout.h > > > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > > > } AVChannelLayout; > > > > > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > > > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u > > > = { > > > .mask = (m) }} > > > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } > > > > > > /** > > > * @name Common pre-defined channel layouts > > > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > > > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX > > > AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) > > > #define AV_CHANNEL_LAYOUT_22POINT2 > > > AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) > > > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > > > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u > > > = { > > > .mask = 0 }} > > > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > > > > For C++ compat you could use some constructor magic instead, and > > turn > > these into proper constants. Hidden behind #ifdef __cplusplus of > > course. > > I’m trying to avoid more debate to not refer to __cplusplus on > purpose. > > > > > IMO having untyped #defines like this is mega ugly. At the very > > least > > it should be (AVChannelLayout){...}. It's likely cargo culted from > > when > > channel layouts were bitmasks. > > (AVChannelLayout){…} can be invalid in C++. AV_TIME_BASE_Q has the > problem > already. We could turn them into proper constants, like so: static const AVChannelLayout AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER = {...}; /Tomas
> On Aug 20, 2023, at 20:53, Tomas Härdin <git@haerdin.se> wrote: > > tor 2023-08-17 klockan 22:03 +0800 skrev zhilizhao(赵志立): >> >> >>> On Aug 17, 2023, at 20:57, Tomas Härdin <git@haerdin.se> wrote: >>> >>> ons 2023-08-16 klockan 23:44 +0800 skrev Zhao Zhili: >>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>> >>>> C++ doesn't support designated initializers until C++20. We have >>>> a bunch of pre-defined channel layouts, the gains to make them >>>> usable in C++ exceed the losses. >>>> >>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>> --- >>>> libavutil/channel_layout.h | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavutil/channel_layout.h >>>> b/libavutil/channel_layout.h >>>> index f345415c55..817a5ad370 100644 >>>> --- a/libavutil/channel_layout.h >>>> +++ b/libavutil/channel_layout.h >>>> @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { >>>> } AVChannelLayout; >>>> >>>> #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ >>>> - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u >>>> = { >>>> .mask = (m) }} >>>> + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } >>>> >>>> /** >>>> * @name Common pre-defined channel layouts >>>> @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { >>>> #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX >>>> AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) >>>> #define AV_CHANNEL_LAYOUT_22POINT2 >>>> AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) >>>> #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ >>>> - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u >>>> = { >>>> .mask = 0 }} >>>> + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } >>> >>> For C++ compat you could use some constructor magic instead, and >>> turn >>> these into proper constants. Hidden behind #ifdef __cplusplus of >>> course. >> >> I’m trying to avoid more debate to not refer to __cplusplus on >> purpose. >> >>> >>> IMO having untyped #defines like this is mega ugly. At the very >>> least >>> it should be (AVChannelLayout){...}. It's likely cargo culted from >>> when >>> channel layouts were bitmasks. >> >> (AVChannelLayout){…} can be invalid in C++. AV_TIME_BASE_Q has the >> problem >> already. > > We could turn them into proper constants, like so: > > static const AVChannelLayout AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER = > {...}; I like this style, but it’s API, so we can’t just replace current implementation by static const variables. And I’m not sure about the coding style regarding to use static const variables in public headers. > > /Tomas > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
mån 2023-08-28 klockan 16:13 +0800 skrev zhilizhao(赵志立): > > > > On Aug 20, 2023, at 20:53, Tomas Härdin <git@haerdin.se> wrote: > > > > tor 2023-08-17 klockan 22:03 +0800 skrev zhilizhao(赵志立): > > > > > > > > > > On Aug 17, 2023, at 20:57, Tomas Härdin <git@haerdin.se> wrote: > > > > > > > > ons 2023-08-16 klockan 23:44 +0800 skrev Zhao Zhili: > > > > > From: Zhao Zhili <zhilizhao@tencent.com> > > > > > > > > > > C++ doesn't support designated initializers until C++20. We > > > > > have > > > > > a bunch of pre-defined channel layouts, the gains to make > > > > > them > > > > > usable in C++ exceed the losses. > > > > > > > > > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > > > > > --- > > > > > libavutil/channel_layout.h | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/libavutil/channel_layout.h > > > > > b/libavutil/channel_layout.h > > > > > index f345415c55..817a5ad370 100644 > > > > > --- a/libavutil/channel_layout.h > > > > > +++ b/libavutil/channel_layout.h > > > > > @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { > > > > > } AVChannelLayout; > > > > > > > > > > #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ > > > > > - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), > > > > > .u > > > > > = { > > > > > .mask = (m) }} > > > > > + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } > > > > > > > > > > /** > > > > > * @name Common pre-defined channel layouts > > > > > @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { > > > > > #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX > > > > > AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) > > > > > #define AV_CHANNEL_LAYOUT_22POINT2 > > > > > AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) > > > > > #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ > > > > > - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, > > > > > .u > > > > > = { > > > > > .mask = 0 }} > > > > > + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } > > > > > > > > For C++ compat you could use some constructor magic instead, > > > > and > > > > turn > > > > these into proper constants. Hidden behind #ifdef __cplusplus > > > > of > > > > course. > > > > > > I’m trying to avoid more debate to not refer to __cplusplus on > > > purpose. > > > > > > > > > > > IMO having untyped #defines like this is mega ugly. At the very > > > > least > > > > it should be (AVChannelLayout){...}. It's likely cargo culted > > > > from > > > > when > > > > channel layouts were bitmasks. > > > > > > (AVChannelLayout){…} can be invalid in C++. AV_TIME_BASE_Q has > > > the > > > problem > > > already. > > > > We could turn them into proper constants, like so: > > > > static const AVChannelLayout > > AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER = > > {...}; > > I like this style, but it’s API, so we can’t just replace current > implementation by static const variables. And I’m not sure about the > coding style regarding to use static const variables in public > headers. The only public header with static const outside of comments is mem.h, and even then they're wrapped in macros (DECLARE_ALIGNED etc) /Tomas
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h index f345415c55..817a5ad370 100644 --- a/libavutil/channel_layout.h +++ b/libavutil/channel_layout.h @@ -359,7 +359,7 @@ typedef struct AVChannelLayout { } AVChannelLayout; #define AV_CHANNEL_LAYOUT_MASK(nb, m) \ - { .order = AV_CHANNEL_ORDER_NATIVE, .nb_channels = (nb), .u = { .mask = (m) }} + { AV_CHANNEL_ORDER_NATIVE, (nb), { m }, NULL } /** * @name Common pre-defined channel layouts @@ -397,7 +397,7 @@ typedef struct AVChannelLayout { #define AV_CHANNEL_LAYOUT_STEREO_DOWNMIX AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO_DOWNMIX) #define AV_CHANNEL_LAYOUT_22POINT2 AV_CHANNEL_LAYOUT_MASK(24, AV_CH_LAYOUT_22POINT2) #define AV_CHANNEL_LAYOUT_AMBISONIC_FIRST_ORDER \ - { .order = AV_CHANNEL_ORDER_AMBISONIC, .nb_channels = 4, .u = { .mask = 0 }} + { AV_CHANNEL_ORDER_AMBISONIC, 4, { 0 }, NULL } /** @} */ struct AVBPrint;