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

[Hendrik Leppkes]

On Tue, Oct 24, 2023 at 1:46 AM Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>
> On Tue, Oct 24, 2023 at 1:24 AM James Almer <jamrial at 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 at 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?

- Hendrik