diff mbox series

[FFmpeg-devel,v4,01/10] channel_layout: add new channel positions supported by xHE-AAC

Message ID 20240526213739.67158-1-dev@lynne.ee
State New
Headers show
Series [FFmpeg-devel,v4,01/10] channel_layout: add new channel positions supported by xHE-AAC | expand

Checks

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

Commit Message

Lynne May 26, 2024, 9:37 p.m. UTC 
apichanges will be updated upon merging, as well as a version bump.
---
 libavutil/channel_layout.c | 4 ++++
 libavutil/channel_layout.h | 8 ++++++++
 2 files changed, 12 insertions(+)

Comments

Jan Ekström May 27, 2024, 2:11 p.m. UTC | #1 
On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> apichanges will be updated upon merging, as well as a version bump.
> ---
>  libavutil/channel_layout.c | 4 ++++
>  libavutil/channel_layout.h | 8 ++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 98839b7250..2d6963b6df 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -75,6 +75,10 @@ static const struct channel_name channel_names[] = {
>      [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom front center"   },
>      [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom front left"     },
>      [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom front right"    },
> +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side surround left"    },
> +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side surround right"   },

Just as a note, if you want to follow the ISO 23091-3 naming for these
(BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
Rss ("Left side surround", "Right side surround"). Just a word order
thing. This would then also match the Apple identifier,
`kAudioChannelLabel_LeftSideSurround`.

Also we might want to start adding comments into the enum like "///<
+90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
identifiers that this channel is supposed to match?

> +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top surround left"     },
> +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top surround right"    },
>  };

So here the mapping seems to be that ISO 23091-3 calls these Lvs and
Rvs (Left, Right vertical surround) and they are mapped against
BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
mention these two, it contains as U+110 Ltr and Rtr. At this point it
would be nice if someone actually has access to IEC 62574 :D .

as for the enum comment, maybe "///< +110 degrees, +30 degrees
vertical, Lvs, TpLS" ?

references:

BS.2051 https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
ISO 23091-3: ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
(used to be public on N-documents, but now that site always requires a
login :< )
ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
Some D document from 2021 on setting up audio setups that has a table
that attempts to match their names to various actual specifications
(including IEC 62574 where it mentions TpLS and TpRS):
https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
MediaInfo's attempt at matching the different specs:
https://mediaarea.net/AudioChannelLayout

Jan
Marton Balint May 27, 2024, 7:31 p.m. UTC | #2 
On Mon, 27 May 2024, Jan Ekström wrote:

> On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
> <ffmpeg-devel@ffmpeg.org> wrote:
>>
>> apichanges will be updated upon merging, as well as a version bump.
>> ---
>>  libavutil/channel_layout.c | 4 ++++
>>  libavutil/channel_layout.h | 8 ++++++++
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
>> index 98839b7250..2d6963b6df 100644
>> --- a/libavutil/channel_layout.c
>> +++ b/libavutil/channel_layout.c
>> @@ -75,6 +75,10 @@ static const struct channel_name channel_names[] = {
>>      [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom front center"   },
>>      [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom front left"     },
>>      [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom front right"    },
>> +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side surround left"    },
>> +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side surround right"   },
>
> Just as a note, if you want to follow the ISO 23091-3 naming for these
> (BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
> Rss ("Left side surround", "Right side surround"). Just a word order
> thing. This would then also match the Apple identifier,
> `kAudioChannelLabel_LeftSideSurround`.

I guess the idea was to be consistent with existing FFMPEG channel word 
order and short names. I kind of prefer that approach, because the other 
channel names will not follow ISO 23091 anyway, so there is not much 
benefit in being consistent with ISO 23091 for these 4 newly added 
channels only, at the cost of not being consistent with existing ffmpeg 
way... So I think the patch is fine as is.

Regards,
Marton


>
> Also we might want to start adding comments into the enum like "///<
> +90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
> identifiers that this channel is supposed to match?
>
>> +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top surround left"     },
>> +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top surround right"    },
>>  };
>
> So here the mapping seems to be that ISO 23091-3 calls these Lvs and
> Rvs (Left, Right vertical surround) and they are mapped against
> BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
> mention these two, it contains as U+110 Ltr and Rtr. At this point it
> would be nice if someone actually has access to IEC 62574 :D .
>
> as for the enum comment, maybe "///< +110 degrees, +30 degrees
> vertical, Lvs, TpLS" ?
>
> references:
>
> BS.2051 https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
> ISO 23091-3: ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
> (used to be public on N-documents, but now that site always requires a
> login :< )
> ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
> Some D document from 2021 on setting up audio setups that has a table
> that attempts to match their names to various actual specifications
> (including IEC 62574 where it mentions TpLS and TpRS):
> https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
> MediaInfo's attempt at matching the different specs:
> https://mediaarea.net/AudioChannelLayout
>
> Jan
> _______________________________________________
> 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".
Jan Ekström May 28, 2024, 9:20 p.m. UTC | #3 
On Mon, May 27, 2024 at 10:31 PM Marton Balint <cus@passwd.hu> wrote:
>
>
>
> On Mon, 27 May 2024, Jan Ekström wrote:
>
> > On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
> > <ffmpeg-devel@ffmpeg.org> wrote:
> >>
> >> apichanges will be updated upon merging, as well as a version bump.
> >> ---
> >>  libavutil/channel_layout.c | 4 ++++
> >>  libavutil/channel_layout.h | 8 ++++++++
> >>  2 files changed, 12 insertions(+)
> >>
> >> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> >> index 98839b7250..2d6963b6df 100644
> >> --- a/libavutil/channel_layout.c
> >> +++ b/libavutil/channel_layout.c
> >> @@ -75,6 +75,10 @@ static const struct channel_name channel_names[] = {
> >>      [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom front center"   },
> >>      [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom front left"     },
> >>      [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom front right"    },
> >> +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side surround left"    },
> >> +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side surround right"   },
> >
> > Just as a note, if you want to follow the ISO 23091-3 naming for these
> > (BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
> > Rss ("Left side surround", "Right side surround"). Just a word order
> > thing. This would then also match the Apple identifier,
> > `kAudioChannelLabel_LeftSideSurround`.
>
> I guess the idea was to be consistent with existing FFMPEG channel word
> order and short names. I kind of prefer that approach, because the other
> channel names will not follow ISO 23091 anyway, so there is not much
> benefit in being consistent with ISO 23091 for these 4 newly added
> channels only, at the cost of not being consistent with existing ffmpeg
> way... So I think the patch is fine as is.
>

If we explicitly go for a non-standardized names then it probably is
an even better idea to add "///< " comments marking what we actually
based these new enum entries on with regards to matching ISO 23091 and
IEC 62574 / BS.2051 identifiers (even if we leave out the approximate
azimuth and vertical degrees out of it, as that is a whole separate
mess).

I am planning on attempting to add such for at least some of the enum
values we have for the existing ones, but for these new ones we should
have these from the get-go.

Jan

> Regards,
> Marton
>
>
> >
> > Also we might want to start adding comments into the enum like "///<
> > +90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
> > identifiers that this channel is supposed to match?
> >
> >> +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top surround left"     },
> >> +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top surround right"    },
> >>  };
> >
> > So here the mapping seems to be that ISO 23091-3 calls these Lvs and
> > Rvs (Left, Right vertical surround) and they are mapped against
> > BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
> > mention these two, it contains as U+110 Ltr and Rtr. At this point it
> > would be nice if someone actually has access to IEC 62574 :D .
> >
> > as for the enum comment, maybe "///< +110 degrees, +30 degrees
> > vertical, Lvs, TpLS" ?
> >
> > references:
> >
> > BS.2051 https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
> > ISO 23091-3: ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
> > (used to be public on N-documents, but now that site always requires a
> > login :< )
> > ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
> > Some D document from 2021 on setting up audio setups that has a table
> > that attempts to match their names to various actual specifications
> > (including IEC 62574 where it mentions TpLS and TpRS):
> > https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
> > MediaInfo's attempt at matching the different specs:
> > https://mediaarea.net/AudioChannelLayout
> >
> > Jan
> > _______________________________________________
> > 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".
Marton Balint May 28, 2024, 9:38 p.m. UTC | #4 
On Wed, 29 May 2024, Jan Ekström wrote:

> On Mon, May 27, 2024 at 10:31 PM Marton Balint <cus@passwd.hu> wrote:
>>
>>
>>
>> On Mon, 27 May 2024, Jan Ekström wrote:
>>
>> > On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
>> > <ffmpeg-devel@ffmpeg.org> wrote:
>> >>
>> >> apichanges will be updated upon merging, as well as a version bump.
>> >> ---
>> >>  libavutil/channel_layout.c | 4 ++++
>> >>  libavutil/channel_layout.h | 8 ++++++++
>> >>  2 files changed, 12 insertions(+)
>> >>
>> >> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
>> >> index 98839b7250..2d6963b6df 100644
>> >> --- a/libavutil/channel_layout.c
>> >> +++ b/libavutil/channel_layout.c
>> >> @@ -75,6 +75,10 @@ static const struct channel_name channel_names[] = {
>> >>      [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom front center"   },
>> >>      [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom front left"     },
>> >>      [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom front right"    },
>> >> +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side surround left"    },
>> >> +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side surround right"   },
>> >
>> > Just as a note, if you want to follow the ISO 23091-3 naming for these
>> > (BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
>> > Rss ("Left side surround", "Right side surround"). Just a word order
>> > thing. This would then also match the Apple identifier,
>> > `kAudioChannelLabel_LeftSideSurround`.
>>
>> I guess the idea was to be consistent with existing FFMPEG channel word
>> order and short names. I kind of prefer that approach, because the other
>> channel names will not follow ISO 23091 anyway, so there is not much
>> benefit in being consistent with ISO 23091 for these 4 newly added
>> channels only, at the cost of not being consistent with existing ffmpeg
>> way... So I think the patch is fine as is.
>>
>
> If we explicitly go for a non-standardized names then it probably is
> an even better idea to add "///< " comments marking what we actually
> based these new enum entries on with regards to matching ISO 23091 and
> IEC 62574 / BS.2051 identifiers (even if we leave out the approximate
> azimuth and vertical degrees out of it, as that is a whole separate
> mess).
>
> I am planning on attempting to add such for at least some of the enum
> values we have for the existing ones, but for these new ones we should
> have these from the get-go.

Sure, OK, I have no problem with adding comments about the origins of the 
channel. And we should ask MediaArea to add a new column to their table, 
the FFmpeg channel names :)

Thanks,
Marton

>
> Jan
>
>> Regards,
>> Marton
>>
>>
>> >
>> > Also we might want to start adding comments into the enum like "///<
>> > +90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
>> > identifiers that this channel is supposed to match?
>> >
>> >> +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top surround left"     },
>> >> +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top surround right"    },
>> >>  };
>> >
>> > So here the mapping seems to be that ISO 23091-3 calls these Lvs and
>> > Rvs (Left, Right vertical surround) and they are mapped against
>> > BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
>> > mention these two, it contains as U+110 Ltr and Rtr. At this point it
>> > would be nice if someone actually has access to IEC 62574 :D .
>> >
>> > as for the enum comment, maybe "///< +110 degrees, +30 degrees
>> > vertical, Lvs, TpLS" ?
>> >
>> > references:
>> >
>> > BS.2051 https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
>> > ISO 23091-3: ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
>> > (used to be public on N-documents, but now that site always requires a
>> > login :< )
>> > ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
>> > Some D document from 2021 on setting up audio setups that has a table
>> > that attempts to match their names to various actual specifications
>> > (including IEC 62574 where it mentions TpLS and TpRS):
>> > https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
>> > MediaInfo's attempt at matching the different specs:
>> > https://mediaarea.net/AudioChannelLayout
>> >
>> > Jan
>> > _______________________________________________
>> > 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".
> _______________________________________________
> 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".
Lynne May 28, 2024, 10:12 p.m. UTC | #5 
On 28/05/2024 23:38, Marton Balint wrote:
> 
> 
> On Wed, 29 May 2024, Jan Ekström wrote:
> 
>> On Mon, May 27, 2024 at 10:31 PM Marton Balint <cus@passwd.hu> wrote:
>>>
>>>
>>>
>>> On Mon, 27 May 2024, Jan Ekström wrote:
>>>
>>> > On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
>>> > <ffmpeg-devel@ffmpeg.org> wrote:
>>> >>
>>> >> apichanges will be updated upon merging, as well as a version bump.
>>> >> ---
>>> >>  libavutil/channel_layout.c | 4 ++++
>>> >>  libavutil/channel_layout.h | 8 ++++++++
>>> >>  2 files changed, 12 insertions(+)
>>> >>
>>> >> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
>>> >> index 98839b7250..2d6963b6df 100644
>>> >> --- a/libavutil/channel_layout.c
>>> >> +++ b/libavutil/channel_layout.c
>>> >> @@ -75,6 +75,10 @@ static const struct channel_name 
>>> channel_names[] = {
>>> >>      [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom 
>>> front center"   },
>>> >>      [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom 
>>> front left"     },
>>> >>      [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom 
>>> front right"    },
>>> >> +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side 
>>> surround left"    },
>>> >> +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side 
>>> surround right"   },
>>> >
>>> > Just as a note, if you want to follow the ISO 23091-3 naming for these
>>> > (BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
>>> > Rss ("Left side surround", "Right side surround"). Just a word order
>>> > thing. This would then also match the Apple identifier,
>>> > `kAudioChannelLabel_LeftSideSurround`.
>>>
>>> I guess the idea was to be consistent with existing FFMPEG channel word
>>> order and short names. I kind of prefer that approach, because the other
>>> channel names will not follow ISO 23091 anyway, so there is not much
>>> benefit in being consistent with ISO 23091 for these 4 newly added
>>> channels only, at the cost of not being consistent with existing ffmpeg
>>> way... So I think the patch is fine as is.
>>>
>>
>> If we explicitly go for a non-standardized names then it probably is
>> an even better idea to add "///< " comments marking what we actually
>> based these new enum entries on with regards to matching ISO 23091 and
>> IEC 62574 / BS.2051 identifiers (even if we leave out the approximate
>> azimuth and vertical degrees out of it, as that is a whole separate
>> mess).
>>
>> I am planning on attempting to add such for at least some of the enum
>> values we have for the existing ones, but for these new ones we should
>> have these from the get-go.
> 
> Sure, OK, I have no problem with adding comments about the origins of 
> the channel. And we should ask MediaArea to add a new column to their 
> table, the FFmpeg channel names :)
> 
> Thanks,
> Marton
> 
>>
>> Jan
>>
>>> Regards,
>>> Marton
>>>
>>>
>>> >
>>> > Also we might want to start adding comments into the enum like "///<
>>> > +90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
>>> > identifiers that this channel is supposed to match?
>>> >
>>> >> +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top 
>>> surround left"     },
>>> >> +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top 
>>> surround right"    },
>>> >>  };
>>> >
>>> > So here the mapping seems to be that ISO 23091-3 calls these Lvs and
>>> > Rvs (Left, Right vertical surround) and they are mapped against
>>> > BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
>>> > mention these two, it contains as U+110 Ltr and Rtr. At this point it
>>> > would be nice if someone actually has access to IEC 62574 :D .
>>> >
>>> > as for the enum comment, maybe "///< +110 degrees, +30 degrees
>>> > vertical, Lvs, TpLS" ?
>>> >
>>> > references:
>>> >
>>> > BS.2051 
>>> https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
>>> > ISO 23091-3: 
>>> ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
>>> > (used to be public on N-documents, but now that site always requires a
>>> > login :< )
>>> > ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
>>> > Some D document from 2021 on setting up audio setups that has a table
>>> > that attempts to match their names to various actual specifications
>>> > (including IEC 62574 where it mentions TpLS and TpRS):
>>> > 
>>> https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
>>> > MediaInfo's attempt at matching the different specs:
>>> > https://mediaarea.net/AudioChannelLayout
>>> >
>>> > Jan
>>> > _______________________________________________
>>> > 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".
>> _______________________________________________
>> 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".

So which names should I resubmit with?
Marton Balint May 29, 2024, 7:41 p.m. UTC | #6 
On Wed, 29 May 2024, Lynne via ffmpeg-devel wrote:

> On 28/05/2024 23:38, Marton Balint wrote:
>>
>>
>>  On Wed, 29 May 2024, Jan Ekström wrote:
>>
>>>  On Mon, May 27, 2024 at 10:31 PM Marton Balint <cus@passwd.hu> wrote:
>>>> 
>>>> 
>>>>
>>>>  On Mon, 27 May 2024, Jan Ekström wrote:
>>>> 
>>>> >  On Mon, May 27, 2024 at 12:37 AM Lynne via ffmpeg-devel
>>>> >  <ffmpeg-devel@ffmpeg.org> wrote:
>>>> >> 
>>>> >>  apichanges will be updated upon merging, as well as a version bump.
>>>> >>  ---
>>>> >>   libavutil/channel_layout.c | 4 ++++
>>>> >>   libavutil/channel_layout.h | 8 ++++++++
>>>> >>   2 files changed, 12 insertions(+)
>>>> >> 
>>>> >>  diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
>>>> >>  index 98839b7250..2d6963b6df 100644
>>>> >>  --- a/libavutil/channel_layout.c
>>>> >>  +++ b/libavutil/channel_layout.c
>>>> >>  @@ -75,6 +75,10 @@ static const struct channel_name
>>>>  channel_names[] = {
>>>> >>       [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom
>>>>  front center"   },
>>>> >>       [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom
>>>>  front left"     },
>>>> >>       [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom
>>>>  front right"    },
>>>> >>  +    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side
>>>>  surround left"    },
>>>> >>  +    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side
>>>>  surround right"   },
>>>> > 
>>>> >  Just as a note, if you want to follow the ISO 23091-3 naming for these
>>>> >  (BS.2051/IEC 62574 calls them just SiL, SiR), then it would be Lss,
>>>> >  Rss ("Left side surround", "Right side surround"). Just a word order
>>>> >  thing. This would then also match the Apple identifier,
>>>> >  `kAudioChannelLabel_LeftSideSurround`.
>>>>
>>>>  I guess the idea was to be consistent with existing FFMPEG channel word
>>>>  order and short names. I kind of prefer that approach, because the other
>>>>  channel names will not follow ISO 23091 anyway, so there is not much
>>>>  benefit in being consistent with ISO 23091 for these 4 newly added
>>>>  channels only, at the cost of not being consistent with existing ffmpeg
>>>>  way... So I think the patch is fine as is.
>>>> 
>>>
>>>  If we explicitly go for a non-standardized names then it probably is
>>>  an even better idea to add "///< " comments marking what we actually
>>>  based these new enum entries on with regards to matching ISO 23091 and
>>>  IEC 62574 / BS.2051 identifiers (even if we leave out the approximate
>>>  azimuth and vertical degrees out of it, as that is a whole separate
>>>  mess).
>>>
>>>  I am planning on attempting to add such for at least some of the enum
>>>  values we have for the existing ones, but for these new ones we should
>>>  have these from the get-go.
>>
>>  Sure, OK, I have no problem with adding comments about the origins of the
>>  channel. And we should ask MediaArea to add a new column to their table,
>>  the FFmpeg channel names :)
>>
>>  Thanks,
>>  Marton
>> 
>>>
>>>  Jan
>>>
>>>>  Regards,
>>>>  Marton
>>>> 
>>>> 
>>>> > 
>>>> >  Also we might want to start adding comments into the enum like "///<
>>>> >  +90 degrees, Lss, SiL" to note the matching ISO 23091-3 and BS.2051
>>>> >  identifiers that this channel is supposed to match?
>>>> > 
>>>> >>  +    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top
>>>>  surround left"     },
>>>> >>  +    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top
>>>>  surround right"    },
>>>> >>   };
>>>> > 
>>>> >  So here the mapping seems to be that ISO 23091-3 calls these Lvs and
>>>> >  Rvs (Left, Right vertical surround) and they are mapped against
>>>> >  BS.2051/IEC 62574 TpLS and TpRS - except BS.2051 doesn't seem to
>>>> >  mention these two, it contains as U+110 Ltr and Rtr. At this point it
>>>> >  would be nice if someone actually has access to IEC 62574 :D .
>>>> > 
>>>> >  as for the enum comment, maybe "///< +110 degrees, +30 degrees
>>>> >  vertical, Lvs, TpLS" ?
>>>> > 
>>>> >  references:
>>>> > 
>>>> >  BS.2051
>>>>  https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.2051-3-202205-I!!PDF-E.pdf
>>>> >  ISO 23091-3:
>>>>  ISO-IECJTC1-SC29_N19613_Text_of_ISOIEC_23091-3CDAmd1_Information_technology__Coding-independent_code_points__Part_3_Audio__AMENDMENT_1_Headphone_support_SC_29WG_06_N_00045.zip
>>>> >  (used to be public on N-documents, but now that site always requires a
>>>> >  login :< )
>>>> >  ISO 23003-3: USAC spec - contains the same listing as in 23091-3.
>>>> >  Some D document from 2021 on setting up audio setups that has a table
>>>> >  that attempts to match their names to various actual specifications
>>>> >  (including IEC 62574 where it mentions TpLS and TpRS):
>>>> >
>>>>  https://www.audiosciencereview.com/forum/index.php?attachments/dolby-atmos-home-entertainment-studio-technical-guidelines-2021-05-pdf.246631/
>>>> >  MediaInfo's attempt at matching the different specs:
>>>> >  https://mediaarea.net/AudioChannelLayout
>>>> > 
>>>> >  Jan
>>>> >  _______________________________________________
>>>> >  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".
>>>  _______________________________________________
>>>  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".
>
> So which names should I resubmit with?
>

As far as I understand the current names are fine, you only need to add a 
comments to the channel names referencing the origin channel names. E.g:

     AV_CHAN_SIDE_SURROUND_LEFT,   ///<  +90 degrees, Lss, SiL
     AV_CHAN_SIDE_SURROUND_RIGHT,  ///<  -90 degrees, Rss, SiR
     AV_CHAN_TOP_SURROUND_LEFT,    ///< +110 degrees, Lvs, TpLS
     AV_CHAN_TOP_SURROUND_RIGHT,   ///< -110 degrees, Rvs, TpRS

Regards,
Marton
diff mbox series

Patch

diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
index 98839b7250..2d6963b6df 100644
--- a/libavutil/channel_layout.c
+++ b/libavutil/channel_layout.c
@@ -75,6 +75,10 @@  static const struct channel_name channel_names[] = {
     [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",       "bottom front center"   },
     [AV_CHAN_BOTTOM_FRONT_LEFT    ] = { "BFL",       "bottom front left"     },
     [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",       "bottom front right"    },
+    [AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",       "side surround left"    },
+    [AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",       "side surround right"   },
+    [AV_CHAN_TOP_SURROUND_LEFT    ] = { "TTL",       "top surround left"     },
+    [AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",       "top surround right"    },
 };
 
 void av_channel_name_bprint(AVBPrint *bp, enum AVChannel channel_id)
diff --git a/libavutil/channel_layout.h b/libavutil/channel_layout.h
index b26b601065..6625313cc5 100644
--- a/libavutil/channel_layout.h
+++ b/libavutil/channel_layout.h
@@ -79,6 +79,10 @@  enum AVChannel {
     AV_CHAN_BOTTOM_FRONT_CENTER,
     AV_CHAN_BOTTOM_FRONT_LEFT,
     AV_CHAN_BOTTOM_FRONT_RIGHT,
+    AV_CHAN_SIDE_SURROUND_LEFT,
+    AV_CHAN_SIDE_SURROUND_RIGHT,
+    AV_CHAN_TOP_SURROUND_LEFT,
+    AV_CHAN_TOP_SURROUND_RIGHT,
 
     /** Channel is empty can be safely skipped. */
     AV_CHAN_UNUSED = 0x200,
@@ -195,6 +199,10 @@  enum AVChannelOrder {
 #define AV_CH_BOTTOM_FRONT_CENTER    (1ULL << AV_CHAN_BOTTOM_FRONT_CENTER  )
 #define AV_CH_BOTTOM_FRONT_LEFT      (1ULL << AV_CHAN_BOTTOM_FRONT_LEFT    )
 #define AV_CH_BOTTOM_FRONT_RIGHT     (1ULL << AV_CHAN_BOTTOM_FRONT_RIGHT   )
+#define AV_CH_SIDE_SURROUND_LEFT     (1ULL << AV_CHAN_SIDE_SURROUND_LEFT   )
+#define AV_CH_SIDE_SURROUND_RIGHT    (1ULL << AV_CHAN_SIDE_SURROUND_RIGHT  )
+#define AV_CH_TOP_SURROUND_LEFT      (1ULL << AV_CHAN_TOP_SURROUND_LEFT    )
+#define AV_CH_TOP_SURROUND_RIGHT     (1ULL << AV_CHAN_TOP_SURROUND_RIGHT   )
 
 /**
  * @}