Re: [FFmpeg-devel] [PATCH] avutil/channel_layout: make pre-defined channel layouts C++ friendly

From: "\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avutil/channel_layout: make pre-defined channel layouts C++ friendly
Date: Thu, 17 Aug 2023 22:03:43 +0800
Message-ID: <tencent_D915B52C6ADE003569B67CDAAF0A38A5C30A@qq.com> (raw)
In-Reply-To: <efc640f9e1ca9555b683490c6b08aee4dbae82c1.camel@haerdin.se>

> 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".

_______________________________________________
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".