[FFmpeg-devel] [PATCH 091/281] matroska: convert to new channel layout API

[James Almer]

On 1/21/2022 4:52 PM, Andreas Rheinhardt wrote:
> James Almer:
>> From: Vittorio Giovara <vittorio.giovara at gmail.com>
>>
>> Signed-off-by: Vittorio Giovara <vittorio.giovara at gmail.com>
>> Signed-off-by: Anton Khirnov <anton at khirnov.net>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>   libavformat/matroskadec.c |  8 ++++++--
>>   libavformat/matroskaenc.c | 10 +++++-----
>>   2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 78e5a4a203..62a36cbb8c 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -2083,7 +2083,7 @@ static int matroska_parse_flac(AVFormatContext *s,
>>                       av_log(s, AV_LOG_WARNING,
>>                              "Invalid value of WAVEFORMATEXTENSIBLE_CHANNEL_MASK\n");
>>                   } else
>> -                    st->codecpar->channel_layout = mask;
>> +                    av_channel_layout_from_mask(&st->codecpar->ch_layout, mask);
>>               }
>>               av_dict_free(&dict);
>>           }
>> @@ -2911,7 +2911,11 @@ static int matroska_parse_tracks(AVFormatContext *s)
>>               st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
>>               st->codecpar->codec_tag   = fourcc;
>>               st->codecpar->sample_rate = track->audio.out_samplerate;
>> -            st->codecpar->channels    = track->audio.channels;
>> +            // channel layout may be already set by codec private checks above
>> +            if (st->codecpar->ch_layout.order == AV_CHANNEL_ORDER_NATIVE &&
>> +                !st->codecpar->ch_layout.u.mask)
>> +                st->codecpar->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
>> +            st->codecpar->ch_layout.nb_channels = track->audio.channels;
>>               if (!st->codecpar->bits_per_coded_sample)
>>                   st->codecpar->bits_per_coded_sample = track->audio.bitdepth;
>>               if (st->codecpar->codec_id == AV_CODEC_ID_MP3 ||
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 152312102a..5278166e8c 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -657,9 +657,9 @@ static int put_wv_codecpriv(AVIOContext *pb, const AVCodecParameters *par)
>>   static int put_flac_codecpriv(AVFormatContext *s, AVIOContext *pb,
>>                                 const AVCodecParameters *par)
>>   {
>> -    int write_comment = (par->channel_layout &&
>> -                         !(par->channel_layout & ~0x3ffffULL) &&
>> -                         !ff_flac_is_native_layout(par->channel_layout));
>> +    int write_comment = (par->ch_layout.order == AV_CHANNEL_ORDER_NATIVE && par->ch_layout.u.mask &&
>> +                         !av_channel_layout_subset(&par->ch_layout, ~0x3ffffULL) &&
> 
> Why are you using this function although you already know that it is a
> native layout, so that the typical & works fine?

I think Vittorio originally wrote it as

int write_comment = (!av_channel_layout_subset(&par->ch_layout, 
~0x3ffffULL) && !ff_flac_is_native_layout(par->ch_layout.u.mask));

Which could potentially access the union's mask field on a layout other 
than native if av_channel_layout_subset() succeeded, so i added that 
check. But you're right it makes calling subset() superfluous, so will 
change it.

> 
>> +                         !ff_flac_is_native_layout(par->ch_layout.u.mask));
>>       int ret = ff_flac_write_header(pb, par->extradata, par->extradata_size,
>>                                      !write_comment);
>>   
>> @@ -673,7 +673,7 @@ static int put_flac_codecpriv(AVFormatContext *s, AVIOContext *pb,
>>           uint8_t buf[32];
>>           int64_t len;
>>   
>> -        snprintf(buf, sizeof(buf), "0x%"PRIx64, par->channel_layout);
>> +        snprintf(buf, sizeof(buf), "0x%"PRIx64, par->ch_layout.u.mask);
>>           av_dict_set(&dict, "WAVEFORMATEXTENSIBLE_CHANNEL_MASK", buf, 0);
>>   
>>           len = ff_vorbiscomment_length(dict, vendor, NULL, 0);
>> @@ -1396,7 +1396,7 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>>               put_ebml_string(pb, MATROSKA_ID_CODECID, "A_MS/ACM");
>>   
>>           subinfo = start_ebml_master(pb, MATROSKA_ID_TRACKAUDIO, 6 + 4 * 9);
>> -        put_ebml_uint  (pb, MATROSKA_ID_AUDIOCHANNELS    , par->channels);
>> +        put_ebml_uint(pb, MATROSKA_ID_AUDIOCHANNELS, par->ch_layout.nb_channels);
>>   
>>           track->sample_rate_offset = avio_tell(pb);
>>           put_ebml_float (pb, MATROSKA_ID_AUDIOSAMPLINGFREQ, sample_rate);
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".