[FFmpeg-devel] [PATCH] libavcodec/mpegaudio_parser.c: differentiate MPEG audio dual mono

James Almer jamrial at gmail.com
Wed Dec 11 02:16:51 EET 2024


On 12/10/2024 9:06 PM, Scott Theisen wrote:
> On 12/9/24 02:30, Anton Khirnov wrote:
>> Quoting James Almer (2024-11-30 14:41:20)
>>> On 11/14/2024 1:37 AM, Scott Theisen wrote:
>>>> When attempting to upstream this MythTV change in September 2022, it 
>>>> was recommended to
>>>> use AV_CHANNEL_ORDER_CUSTOM with two AV_CHAN_FRONT_CENTER channels. See
>>>> https://patchwork.ffmpeg.org/project/ffmpeg/ 
>>>> patch/20220921192611.3241-1-scott.the.elm at gmail.com/
>>>> ---
>>>>    libavcodec/audiotoolboxdec.c    |  4 ++--
>>>>    libavcodec/mpegaudio_parser.c   | 12 +++++++++---
>>>>    libavcodec/mpegaudiodecheader.c |  4 +++-
>>>>    libavcodec/mpegaudiodecheader.h |  2 +-
>>>>    tests/ref/fate/pva-demux        |  2 +-
>>>>    5 files changed, 16 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/libavcodec/audiotoolboxdec.c b/libavcodec/ 
>>>> audiotoolboxdec.c
>>>> index 0f7ce8e4eb..d279d7bbc4 100644
>>>> --- a/libavcodec/audiotoolboxdec.c
>>>> +++ b/libavcodec/audiotoolboxdec.c
>>>> @@ -346,10 +346,10 @@ static av_cold int 
>>>> ffat_create_decoder(AVCodecContext *avctx,
>>>>                    avctx->codec_id == AV_CODEC_ID_MP2 ||
>>>>                    avctx->codec_id == AV_CODEC_ID_MP3)) {
>>>>            enum AVCodecID codec_id;
>>>> -        int bit_rate;
>>>> +        int bit_rate, dual_mono;
>>>>            if (ff_mpa_decode_header(AV_RB32(pkt->data), &avctx- 
>>>> >sample_rate,
>>>>                                     &in_format.mChannelsPerFrame, 
>>>> &avctx->frame_size,
>>>> -                                 &bit_rate, &codec_id) < 0)
>>>> +                                 &bit_rate, &codec_id, &dual_mono) 
>>>> < 0)
>>>>                return AVERROR_INVALIDDATA;
>>>>            avctx->bit_rate = bit_rate;
>>>>            in_format.mSampleRate = avctx->sample_rate;
>>>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/ 
>>>> mpegaudio_parser.c
>>>> index d54366f10a..d1a4ee6434 100644
>>>> --- a/libavcodec/mpegaudio_parser.c
>>>> +++ b/libavcodec/mpegaudio_parser.c
>>>> @@ -65,12 +65,12 @@ static int mpegaudio_parse(AVCodecParserContext 
>>>> *s1,
>>>>                }
>>>>            }else{
>>>>                while(i<buf_size){
>>>> -                int ret, sr, channels, bit_rate, frame_size;
>>>> +                int ret, sr, channels, bit_rate, frame_size, 
>>>> dual_mono;
>>>>                    enum AVCodecID codec_id = avctx->codec_id;
>>>>                    state= (state<<8) + buf[i++];
>>>> -                ret = ff_mpa_decode_header(state, &sr, &channels, 
>>>> &frame_size, &bit_rate, &codec_id);
>>>> +                ret = ff_mpa_decode_header(state, &sr, &channels, 
>>>> &frame_size, &bit_rate, &codec_id, &dual_mono);
>>>>                    if (ret < 4) {
>>>>                        if (i > 4)
>>>>                            s->header_count = -2;
>>>> @@ -85,7 +85,13 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
>>>>                        if (s->header_count > header_threshold) {
>>>>                            avctx->sample_rate= sr;
>>>>                            av_channel_layout_uninit(&avctx->ch_layout);
>>>> -                        av_channel_layout_default(&avctx- 
>>>> >ch_layout, channels);
>>>> +                        if (dual_mono) {
>>>> +                            av_channel_layout_custom_init(&avctx- 
>>>> >ch_layout, 2);
>>>> +                            avctx->ch_layout.u.map[0].id = 
>>>> AV_CHAN_FRONT_CENTER;
>>>> +                            avctx->ch_layout.u.map[1].id = 
>>>> AV_CHAN_FRONT_CENTER;
>>> Kind of sucks we have used FC to represent mono for so long that we
>>> can't cleanly introduce a new channel that's strictly mono (with no
>>> speaker location implied) and give it meaningful use.
>> AV_CHAN_UNKNOWN?
>>
> 
> Using AV_CHAN_UNKNOWN would be fine with me.
> 
> However, would using AV_CHANNEL_ORDER_UNSPEC instead of 
> AV_CHANNEL_ORDER_CUSTOM be better?

UNSPEC means the channels have no definition. We could allow the user to 
assume 1 channel is "mono", but definitely not 2 channels for dual mono.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241210/0577a36d/attachment.sig>


More information about the ffmpeg-devel mailing list