[FFmpeg-devel] [PATCH] lavc: export flag for MPEG audio dual channel

[Scott Theisen]

On 9/22/22 08:43, Anton Khirnov wrote:
> Quoting Scott Theisen (2022-09-22 03:04:16)
>> On 9/21/22 15:51, James Almer wrote:
>>> On 9/21/2022 4:44 PM, Rémi Denis-Courmont wrote:
>>>> Le keskiviikkona 21. syyskuuta 2022, 22.26.11 EEST Scott Theisen a
>>>> écrit :
>>>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>>>> index 7db5d1b1c5..bcf3a845a8 100644
>>>>> --- a/libavcodec/avcodec.h
>>>>> +++ b/libavcodec/avcodec.h
>>>>> @@ -2076,6 +2076,17 @@ typedef struct AVCodecContext {
>>>>>         *             The decoder can then override during decoding
>>>>> as needed.
>>>>> */
>>>>>        AVChannelLayout ch_layout;
>>>>> +
>>>>> +    /**
>>>>> +     * Audio only.  This flag is set when MPEG audio mode dual
>>>>> channel has
>>>>> been detected. +     * This signals that the audio is two
>>>>> independent mono
>>>>> channels. +     *
>>>>> +     * 0 normally, 1 if dual channel flag is set.
>>>>> +     *
>>>>> +     * - encoding: currently unused (functionally equivalent to
>>>>> stereo,
>>>>> patch welcome) +     * - decoding: set by lavc
>>>>> +     */
>>>>> +    int mpeg_audio_mode_dual_channel;
>>>>>    } AVCodecContext;
>>>> I agree that the dual mono flag should be exposed to the application
>>>> somehow,
>>>> but isn't this a slient ABI break?
>>> It's not a break, but it's overkill for what's essentially a flag.
>> This is how MythTV customized FFmpeg (in 2006) for this and this way was
>> probably the easiest.  It /is/ a flag, so maybe adding an int as a
>> bitset instead of as a bool so other flags could be added if necessary?
>>
>>> The proper way to do this would be to signal such a layout as an
>>> actual channel layout, using a custom order one where both channels
>>> are set as Front Center. But i don't know if until the old channel
>>> layout API fields are gone we should have decoders setting something
>>> only new API users will understand. Old API field users would look at
>>> channels and see a 2, and channel_layout and see it's empty, but then
>>> old and new API values would technically conflict, so I'd like to hear
>>> some opinions.
>> I'm not very familiar with either channel layout API, but the audio is
>> encoded identically to stereo other than the flag, so could we add
>> another entry, e.g. AV_CHANNEL_ORDER_MONO, to the `enum AVChannelOrder`
>> to signify that each channel is independent, but is otherwise identical
>> to AV_CHANNEL_ORDER_NATIVE?
> The whole point is that it's NOT identical to AV_CHANNEL_ORDER_NATIVE.
> ORDER_CUSTOM with two FC channels is exactly the right way to handle
> this IMO.
>

I don't really disagree that AV_CHANNEL_ORDER_CUSTOM with two Front 
Center channels is one way to do it, I was just hoping for an easier way.

I think that would require modification to the code calling 
ff_mpa_decode_header.  But I'm not sure how to create that custom order 
layout and I would like consensus that the custom order is the way to go 
before doing it.

libavcodec/audiotoolboxdec.c just sets an unspecified order. 
libavcodec/mpegaudio_parser.c uses av_channel_layout_default.

Also, should we concern ourselves with the old API?  It didn't look like 
either of the above files did.

Regards,

Scott Theisen