Re: [FFmpeg-devel] [PATCH 1/3] avutil/channel_layout: rename 7.1(top) channel layout to 5.1.2

[James Almer <jamrial@gmail.com>]

On 10/23/2023 8:49 PM, Hendrik Leppkes wrote:
> On Tue, Oct 24, 2023 at 1:46 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>
>> On Tue, Oct 24, 2023 at 1:24 AM James Almer <jamrial@gmail.com> wrote:
>>>
>>> This layout maps to ITU-R BS.2051-3 "Sound System C" and ITU-R BS.1196-8 "Channel
>>> Configuration 14", and it being the first layout with top layer channels, it's
>>> best to use a different scheme to properly convey the presence and amount of said
>>> channels.
>>> The new name will also be a better fit for the additions in the following commits.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
>>> index ac2ddfa022..8c40c81b44 100644
>>> --- a/libavutil/channel_layout.h
>>> +++ b/libavutil/channel_layout.h
>>> @@ -232,13 +232,15 @@ enum AVChannelOrder {
>>>   #define AV_CH_LAYOUT_7POINT1           (AV_CH_LAYOUT_5POINT1|AV_CH_BACK_LEFT|AV_CH_BACK_RIGHT)
>>>   #define AV_CH_LAYOUT_7POINT1_WIDE      (AV_CH_LAYOUT_5POINT1|AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER)
>>>   #define AV_CH_LAYOUT_7POINT1_WIDE_BACK (AV_CH_LAYOUT_5POINT1_BACK|AV_CH_FRONT_LEFT_OF_CENTER|AV_CH_FRONT_RIGHT_OF_CENTER)
>>> -#define AV_CH_LAYOUT_7POINT1_TOP_BACK  (AV_CH_LAYOUT_5POINT1_BACK|AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT)
>>> +#define AV_CH_LAYOUT_5POINT1POINT2     (AV_CH_LAYOUT_5POINT1_BACK|AV_CH_TOP_FRONT_LEFT|AV_CH_TOP_FRONT_RIGHT)
>>
>> Looking at the specs and the naming of the thing, I don't think it
>> should be using AV_CH_LAYOUT_5POINT1_BACK as the base layer, but
>> rather AV_CH_LAYOUT_5POINT1. "5.1" is that, and 5.1.2 should extend
>> that, and not swap the base layer speakers.
>> Looking at the specifications, they don't map properly to our speaker
>> definitions, as they assign it a generic "surround" title (at 100-120
>> degrees azimuth), while a rear/back speaker should be at 135 degrees,
>> and a side speaker at 90 (as is the standard for 7.1 setups).
>>
>> Yes, this doesn't match the one you are renaming, but I think
>> consistency within our own defined formats is important here. 5.1 ->
>> 5.1.2 should just add two.
>>
>> The renamed one could be 5.1.2(back) or whatever we want to name those.
>>
>> Or we do it properly and actually include a proper "surround" speaker
>> definition and get rid of the confusion between side and back. But
>> that breaks compat with WAVEFORMATEXTENSIBLE that our channel
>> definitions are mostly sourced from, so might not be worth it.
> 
> Actually I got confused by our terrible naming. The define
> AV_CHANNEL_LAYOUT_5POINT1 maps to "5.1(side)", but the define
> AV_CHANNEL_LAYOUT_5POINT1_BACK maps to "5.1"? What is up with that?
> Regardless, rather see consistency within those as well, so while the
> name of the format is fine, rename the define to include the _BACK
> maybe?

Yes, it's a mess.

Do i use "5.1.2" alongside AV_CH_LAYOUT_5POINT1POINT2_BACK, then?

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