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

[Lynne via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 6160 bytes --]

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?

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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