[FFmpeg-devel] [DEVEL][PATCH v3] ffmpeg: fix channel_layout bug on non-default layout
pkv.stream
pkv.stream at gmail.com
Sat Nov 18 12:41:54 EET 2017
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.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.
>>>> ---
>>>> 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.
Regards
> [...]
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
From 00c3c724544b16c19282b39644e2584f9c4a4181 Mon Sep 17 00:00:00 2001
From: pkviet <pkv.stream at gmail.com>
Date: Sat, 18 Nov 2017 00:26:50 +0100
Subject: [PATCH] ffmpeg: fix ticket 6706
Fix regression with channel_layout option which is not passed
correctly from output streams to filters when the channel layout is not
a default one.
---
fftools/cmdutils.h | 1 +
fftools/ffmpeg.h | 3 +++
fftools/ffmpeg_opt.c | 41 +++++++++++++++++++++++++++++++++++++----
3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 2997ee3..fa2b255 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -155,6 +155,7 @@ typedef struct SpecifierOpt {
uint8_t *str;
int i;
int64_t i64;
+ uint64_t ui64;
float f;
double dbl;
} u;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index e0977e1..5c45df4 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -121,6 +121,8 @@ typedef struct OptionsContext {
int nb_frame_sizes;
SpecifierOpt *frame_pix_fmts;
int nb_frame_pix_fmts;
+ SpecifierOpt *channel_layouts;
+ int nb_channel_layouts;
/* input options */
int64_t input_ts_offset;
@@ -624,6 +626,7 @@ extern int vstats_version;
extern const AVIOInterruptCB int_cb;
extern const OptionDef options[];
+extern const OptionDef channel_layout_option[];
extern const HWAccel hwaccels[];
extern AVBufferRef *hw_device_ctx;
#if CONFIG_QSV
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 47d3841..ab614a8 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1803,6 +1803,7 @@ 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);
+ MATCH_PER_STREAM_OPT(channel_layouts, ui64, audio_enc->channel_layout, oc, st);
MATCH_PER_STREAM_OPT(sample_fmts, str, sample_fmt, oc, st);
if (sample_fmt &&
@@ -2527,7 +2528,11 @@ 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])
@@ -3104,8 +3109,9 @@ static int opt_channel_layout(void *optctx, const char *opt, const char *arg)
char layout_str[32];
char *stream_str;
char *ac_str;
- int ret, channels, ac_str_size;
+ int ret, channels, ac_str_size, stream_str_size;
uint64_t layout;
+ const char *spec = "a";
layout = av_get_channel_layout(arg);
if (!layout) {
@@ -3116,12 +3122,33 @@ static int opt_channel_layout(void *optctx, const char *opt, const char *arg)
ret = opt_default_new(o, opt, layout_str);
if (ret < 0)
return ret;
+ /* factored code between 'ac' option and 'channel_layout_spec' option */
+ stream_str = strchr(opt, ':');
+ stream_str_size = (stream_str ? strlen(stream_str) : 0);
+ /* set 'channel_layout_spec' internal option which stores channel_layout
+ * (as uint64) in SpecifierOpt, which enables access to channel_layout through MATCH_PER_STREAM_OPT
+ */
+ GROW_ARRAY(o->channel_layouts, o->nb_channel_layouts);
+ o->channel_layouts[o->nb_channel_layouts - 1].specifier = spec;
+ ac_str_size = 20 + stream_str_size;
+ ac_str = av_mallocz(ac_str_size);
+ if (!ac_str) {
+ return AVERROR(ENOMEM);
+ }
+ av_strlcpy(ac_str, "channel_layout_spec", 20);
+ if (stream_str) {
+ av_strlcat(ac_str, stream_str, ac_str_size);
+ }
+ ret = parse_option(o, ac_str, layout_str, channel_layout_option);
+ if (ret < 0) {
+ return ret;
+ }
+ av_free(ac_str);
/* set 'ac' option based on channel layout */
channels = av_get_channel_layout_nb_channels(layout);
snprintf(layout_str, sizeof(layout_str), "%d", channels);
- stream_str = strchr(opt, ':');
- ac_str_size = 3 + (stream_str ? strlen(stream_str) : 0);
+ ac_str_size = 3 + stream_str_size;
ac_str = av_mallocz(ac_str_size);
if (!ac_str)
return AVERROR(ENOMEM);
@@ -3758,3 +3785,9 @@ const OptionDef options[] = {
{ NULL, },
};
+/* internal OptionDef used to enable storage of channel_layout option in a SpecifierOpt */
+const OptionDef channel_layout_option[] = {
+ { "channel_layout_spec", OPT_AUDIO | HAS_ARG | OPT_INT64 | OPT_SPEC |
+ OPT_INPUT | OPT_OUTPUT,{ .off = OFFSET(channel_layouts) },
+ "stores channel layout in SpecifierOpt ", "layout_hex" },
+};
--
2.10.1.windows.1
More information about the ffmpeg-devel
mailing list