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

[Marton Balint <cus@passwd.hu>]

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



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