[FFmpeg-devel] [DEVEL][PATCH] ffmpeg: fix channel_layout bug on non-default layout

[pkv.stream]

Hi Michael,

>>   ffmpeg_opt.c |   11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>> 2af07f4366efdfaf1018bb2ea29be672befe0823  0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch
>>  From 4ec55dc88923108132307b41300a1abddf32e6f7 Mon Sep 17 00:00:00 2001
>> From: pkviet <pkv.stream at gmail.com>
>> Date: Mon, 2 Oct 2017 11:14:31 +0200
>> Subject: [PATCH] ffmpeg: fix channel_layout bug on non-default layout
>>
>> Fix for ticket 6706.
>> The -channel_layout option was not working when the channel layout was not
>> a default one (ex: for 4 channels, quad was interpreted as 4.0 which is
>> the default layout for 4 channels; or octagonal interpreted as 7.1).
>> This led to the spurious auto-insertion of an auto-resampler filter
>> remapping the channels even if input and output had identical channel
>> layouts.
>> The fix operates by directly calling the channel layout if defined in
>> options. If the layout is undefined, the default layout is selected as
>> before the fix.
>> ---
>>   ffmpeg_opt.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
>> index 100fa76..cf5a63c 100644
>> --- a/ffmpeg_opt.c
>> +++ b/ffmpeg_opt.c
>> @@ -1804,6 +1804,12 @@ static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in
>>   
>>           MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st);
>>   
>> +        AVDictionaryEntry *output_layout = av_dict_get(o->g->codec_opts,
>> +                                                       "channel_layout",
>> +                                                       NULL, AV_DICT_MATCH_CASE);
> This doesnt look right
>
> not an issue of the patch as such but
> why is the channel_layout option per file and not per stream?


just my ignorance; do you mean this is not the right way to retrieve the 
channel_layout option from an audio stream ?
Could you give me a hint on how to retrieve correctly the option ?

the goal is to get rid of :

l. 2526 in ffmpeg_opt.c :  f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels);

from commit 
https://github.com/FFmpeg/FFmpeg/commit/198e8b8e774659eacaa7058c7f5704029af5bbbf#diff-3662f9bbf5b9e46a224a4e65b17254ba

(merge from libav) which is causing the auto-insert of a filter which 
rematrix a non default channel layout into a default one (ex: octagonal 
to 7.1).




>
> also currently mixed statemts and declarations arent allowed
>
yes sorry; I've learned that from the ongoing discussion  on the 'for  
int loop' and will fix if I am able to find the right way to solve the 
ticket

>> +        if (output_layout)
>> +            ost->enc_ctx->channel_layout = strtoull(output_layout->value, NULL, 10);
> audio_enc
>
ok
>> +
>>           MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
>>           if (sample_fmt &&
>>               (audio_enc->sample_fmt = av_get_sample_fmt(sample_fmt)) == AV_SAMPLE_FMT_NONE) {
>> @@ -2524,7 +2530,10 @@ loop_end:
>>                              (count + 1) * sizeof(*f->sample_rates));
>>                   }
>>                   if (ost->enc_ctx->channels) {
>> -                    f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels);
>> +                    if (ost->enc_ctx->channel_layout)
>> +                        f->channel_layout = ost->enc_ctx->channel_layout;
>> +                    else
>> +                        f->channel_layout = av_get_default_channel_layout(ost->enc_ctx->channels);
> if () {
> } else
>
> is better as tat way future patches do not need to change existing
> lines maing patches more readable if ever a line is added

sure, thanks for the explanation ; will fix

>
> [...]
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel