Le 18/11/2017 à 9:26 PM, Michael Niedermayer a écrit :
On Sat, Nov 18, 2017 at 11:41:54AM +0100, pkv.stream wrote:
Hi Michael
ffmpeg_opt.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
a7c71fb1ccd7d91b61033be70dfd324b4e3f3c34
0001-ffmpeg-fix-channel_layout-bug-on-non-default-layout.patch
From fb7f7f6e01cc242e13d0e640f583dffe6e7a8ada Mon Sep 17 00:00:00 2001
From: pkviet<pkv.str...@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.
---
fftools/ffmpeg_opt.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index ca6f10d..8941d66 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1785,6 +1785,7 @@ static OutputStream *new_audio_stream(OptionsContext *o,
AVFormatContext *oc, in
AVStream *st;
OutputStream *ost;
AVCodecContext *audio_enc;
+ AVDictionaryEntry *output_layout;
ost = new_output_stream(o, oc, AVMEDIA_TYPE_AUDIO, source_index);
st = ost->st;
@@ -1799,7 +1800,10 @@ static OutputStream *new_audio_stream(OptionsContext *o,
AVFormatContext *oc, in
char *sample_fmt = NULL;
MATCH_PER_STREAM_OPT(audio_channels, i, audio_enc->channels, oc, st);
-
+ output_layout = av_dict_get(ost->encoder_opts,"channel_layout", NULL,
AV_DICT_IGNORE_SUFFIX);
+ if (output_layout) {
+ audio_enc->channel_layout = strtoull(output_layout->value, NULL,
10);
+ }
why is this handled different from audio_channels ?
that is why is this not using MATCH_PER_STREAM_OPT() ?
(yes this would require some changes but i wonder why this would be
handled differently)
Hi
I did try to use the MATCH_PER_STREAM_OPT() but didn't manage to
have it working. Also I was a bit hesitant to modify the
OptionsContext struct, and preferred something minimal.
If you think this can definitely be made to work without too much
coding and prefer such a solution, I'll retry working on a
MATCH_PER_STREAM_OPT() solution.
i dont really know if it "can definitely be made to work without too much
coding", it just seemed odd how this is handled differently
I have another version of the patch working with MATCH_PER_STREAM_OPT() ;
but the changes to code are more important, and it's a bit hacky
(defines an internal OptionDef) ... so not sure it is any better
than the few lines of patch v3.
I've checked that stream specifiers ( :a:0 ) are passed correctly
and that two streams with different layouts are also treated
correctly (the previous patch without MATCH_PER_STREAM_OPT() works
also; those were two points you singled out in your review).
I have no real opinion on the best course, which to pick or even to
let the bug hanging (I'm only trying to fix it due to my work on the
aac PCE patch of atomnuker ; the bug prevents full use of the new
PCE capability).
It's Ok with me if you decide to forgo these attempts to fix the bug
and let the bug stay.
I'm not impacted by the bug in my case use (encode 16 channels in
aac); just trying to be thorough in my (akward) contributions and
trying to give back to the project.
Tell me the best course; or if you see a way to make my
MATCH_PER_STREAM_OPT() code less hacky.
iam sure theres a way to do this less hacky
why do you need a 2nd table ? or rather why does it not work if you
put the entry in the main table ? (so there are 2 entries one for
OPT_SPEC and one for teh callback, will it not send the data to both
matching entries ?
Hi
it does work in the main table.
I didn't want to pollute it. But if you say it is OK then I'll update
accordingly and send a separate patch then.
thanks
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
