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

[Hendrik Leppkes <h.leppkes@gmail.com>]

On Tue, Oct 24, 2023 at 1:57 AM James Almer <jamrial@gmail.com> wrote:
>
> 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?

That works for me.

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