diff mbox

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

Message ID 55b601f7-541e-e83c-366b-cf9b881770bd@gmail.com
State Superseded
Headers show

Commit Message

pkv.stream Oct. 2, 2017, 7:50 p.m. UTC
Thanks Michael; replaced the offending MSVC function.

Le 02/10/2017 à 9:35 PM, Michael Niedermayer a écrit :
> On Sun, Oct 01, 2017 at 03:17:30AM +0200, pkv.stream wrote:
>> Hello
>>
>> the submitted patch addresses the regression discussed in ticket #6706.
>>
>>   <https://trac.ffmpeg.org/ticket/6706>
>>
>> The -channel_layout option is not working when the channel layout is not
>> a default one (ex: for 4 channels, quad is interpreted as 4.0 which is
>> the default layout for 4 channels; or octagonal interpreted as 7.1).
>> This leads to the spurious auto-insertion of an auto-resampler filter
>> remapping the channels even if input and output have identical channel
>> layouts.
>>
>> Thanks for any comments.
> fails build:
>
> fftools/ffmpeg_opt.c:1810:13: error: implicit declaration of function ‘_strtoui64’ [-Werror=implicit-function-declaration]
>               ost->enc_ctx->channel_layout = _strtoui64(output_layout->value, NULL, 10);
>
>
> [...]
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
From 5a51fc45da691feed05a67ec72a8ee581a2444b3 Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream@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 | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

pkv.stream Oct. 4, 2017, 6:25 p.m. UTC | #1
Le 02/10/2017 à 9:50 PM, pkv.stream a écrit :
> Thanks Michael; replaced the offending MSVC function.
>
> Le 02/10/2017 à 9:35 PM, Michael Niedermayer a écrit :
>> On Sun, Oct 01, 2017 at 03:17:30AM +0200, pkv.stream wrote:
>>> Hello
>>>
>>> the submitted patch addresses the regression discussed in ticket #6706.
>>>
>>>   <https://trac.ffmpeg.org/ticket/6706>
>>>
>>> The -channel_layout option is not working when the channel layout is not
>>> a default one (ex: for 4 channels, quad is interpreted as 4.0 which is
>>> the default layout for 4 channels; or octagonal interpreted as 7.1).
>>> This leads to the spurious auto-insertion of an auto-resampler filter
>>> remapping the channels even if input and output have identical channel
>>> layouts.
>>>
>>> Thanks for any comments.
>> fails build:
>>
>> fftools/ffmpeg_opt.c:1810:13: error: implicit declaration of function ‘_strtoui64’ [-Werror=implicit-function-declaration]
>>               ost->enc_ctx->channel_layout = _strtoui64(output_layout->value, NULL, 10);
>>
>>
>> [...]
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
and ping
Moritz Barsnick Oct. 4, 2017, 9:18 p.m. UTC | #2
On Mon, Oct 02, 2017 at 21:50:50 +0200, pkv.stream wrote:
>      if (!ost->stream_copy) {
> -        char *sample_fmt = NULL;
> +
> +		char *sample_fmt = NULL;
>  

This is very obviously a patch which will not be accepted.


> -        MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
> +		AVDictionaryEntry *output_layout = av_dict_get(o->g->codec_opts, "channel_layout",NULL, AV_DICT_MATCH_CASE);
> +        if (output_layout)
> +            ost->enc_ctx->channel_layout = strtoull(output_layout->value, NULL, 10);
> +
> +		MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);

Your indentation is totally wrong, and makes use of tabs. Please follow
the ffmpeg style.

Moritz
pkv.stream Oct. 4, 2017, 9:26 p.m. UTC | #3
Le 4 oct. 2017 11:24 PM, "Moritz Barsnick" <barsnick@gmx.net> a écrit :

On Mon, Oct 02, 2017 at 21:50:50 +0200, pkv.stream wrote:
>      if (!ost->stream_copy) {
> -        char *sample_fmt = NULL;
> +
> +             char *sample_fmt = NULL;
>

This is very obviously a patch which will not be accepted.


> -        MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
> +             AVDictionaryEntry *output_layout =
av_dict_get(o->g->codec_opts, "channel_layout",NULL, AV_DICT_MATCH_CASE);
> +        if (output_layout)
> +            ost->enc_ctx->channel_layout =
strtoull(output_layout->value, NULL, 10);
> +
> +             MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);

Your indentation is totally wrong, and makes use of tabs. Please follow
the ffmpeg style.


Ah sorry for being careless.
Thanks for pointing out. Will correct.


Moritz
diff mbox

Patch

diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index 100fa76..9dfef50 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -1800,11 +1800,16 @@  static OutputStream *new_audio_stream(OptionsContext *o, AVFormatContext *oc, in
     MATCH_PER_STREAM_OPT(filters,        str, ost->filters,        oc, st);
 
     if (!ost->stream_copy) {
-        char *sample_fmt = NULL;
+
+		char *sample_fmt = NULL;
 
         MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st);
 
-        MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
+		AVDictionaryEntry *output_layout = av_dict_get(o->g->codec_opts, "channel_layout",NULL, AV_DICT_MATCH_CASE);
+        if (output_layout)
+            ost->enc_ctx->channel_layout = strtoull(output_layout->value, NULL, 10);
+
+		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) {
             av_log(NULL, AV_LOG_FATAL, "Invalid sample format '%s'\n", sample_fmt);
@@ -2524,7 +2529,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);
                 } else if (ost->enc->channel_layouts) {
                     count = 0;
                     while (ost->enc->channel_layouts[count])