| Message ID | 20240518161116.8661-1-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] fftools/ffmpeg_demux: also set -ch_layout avcodec option for -ch_layout CLI param | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Sat, May 18, 2024 at 06:11:12PM +0200, Marton Balint wrote: > The code only set the channel layout of the AVFormatContext, so the user could > not override the channel layout if the demuxer did not have such parameter. > Let's set the specified channel layouts as codec options as well. > > Fixes ticket #11016. > > A regression since 639c2f00497257cb60ecaeeac1aacfa80df3be06. > > Signed-off-by: Marton Balint <cus@passwd.hu> breaks: ./ffmpeg -f u8 -ar 8000 -ac 1 -i /dev/zero -acodec aac -y -t 1 -b:a 48k /tmp/aac.nut [aac @ 0x55d4ed264800] Unsupported channel layout "1 channels" [aac @ 0x55d4ed264800] Qavg: nan [aost#0:0/aac @ 0x55d4ed24d800] Error while opening encoder - maybe incorrect parameters such as bit_rate, rate, width or height. [af#0:0 @ 0x55d4ed264cc0] Error sending frames to consumers: Invalid argument [af#0:0 @ 0x55d4ed264cc0] Task finished with error code: -22 (Invalid argument) [af#0:0 @ 0x55d4ed264cc0] Terminating thread with return code -22 (Invalid argument) [aost#0:0/aac @ 0x55d4ed24d800] Could not open encoder before EOF [aost#0:0/aac @ 0x55d4ed24d800] Task finished with error code: -22 (Invalid argument) [aost#0:0/aac @ 0x55d4ed24d800] Terminating thread with return code -22 (Invalid argument) [out#0/nut @ 0x55d4ed254040] Nothing was written into output file, because at least one of its streams received no packets. size= 0KiB time=N/A bitrate=N/A speed=N/A thx [...]
Quoting Marton Balint (2024-05-18 18:11:12) > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index cba63dab5f..6e23079ceb 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -1524,6 +1524,33 @@ static Demuxer *demux_alloc(void) > return d; > } > > +static int set_input_ch_layout_opts(const OptionsContext *o) > +{ > + /* "ch_layout" is only a valid format option for some formats, but we set > + * it anyway, because it is also a codec option and we don't report > + * unconsumed format options if they are codec options as well. */ > + for (int i = 0; i < o->audio_channels.nb_opt; i++) { > + char val[32]; > + char *spec = o->audio_channels.opt[i].specifier; > + char *key = av_asprintf("ch_layout%s%s", spec[0] ? ":" : "", spec); > + if (!key) > + return AVERROR(ENOMEM); > + snprintf(val, sizeof(val), "%dC", o->audio_channels.opt[i].u.i); > + av_dict_set(&o->g->format_opts, key, val, 0); > + av_dict_set(&o->g->codec_opts, key, val, AV_DICT_DONT_STRDUP_KEY); I don't like modifying OptionsContext in its consumers (it's const for a reason), and it's probably not even necessary - if the semantics of the option is "override demuxer-reported channel layout", then you can just override the demuxer-reported channel layout (similarly to how guess_input_channel_layout() does it) and it will be communicated to the decoder automagically.
Quoting Anton Khirnov (2024-05-27 09:51:49) > I don't like modifying OptionsContext in its consumers (it's const for a > reason), and it's probably not even necessary - if the semantics of the > option is "override demuxer-reported channel layout", then you can just > override the demuxer-reported channel layout (similarly to how > guess_input_channel_layout() does it) and it will be communicated to the > decoder automagically. I see that's pretty much what you did in v2, so disregard the above email.
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index da37e3ad37..83db6584fd 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1689,9 +1689,10 @@ demuxers and is mapped to the corresponding demuxer options. @item -aq @var{q} (@emph{output}) Set the audio quality (codec-specific, VBR). This is an alias for -q:a. @item -ac[:@var{stream_specifier}] @var{channels} (@emph{input/output,per-stream}) -Set the number of audio channels. For output streams it is set by -default to the number of input audio channels. For input streams -this option only makes sense for audio grabbing devices and raw demuxers +Set the number of audio channels. For output streams it is set by default to +the number of input audio channels. For input streams it overrides the number +of channels if the decoder allows it. When used without a stream specifier it +also sets the input channel count for audio grabbing devices and raw demuxers and is mapped to the corresponding demuxer options. @item -an (@emph{input/output}) As an input option, blocks all audio streams of a file from being filtered or diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index cba63dab5f..6e23079ceb 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1524,6 +1524,33 @@ static Demuxer *demux_alloc(void) return d; } +static int set_input_ch_layout_opts(const OptionsContext *o) +{ + /* "ch_layout" is only a valid format option for some formats, but we set + * it anyway, because it is also a codec option and we don't report + * unconsumed format options if they are codec options as well. */ + for (int i = 0; i < o->audio_channels.nb_opt; i++) { + char val[32]; + char *spec = o->audio_channels.opt[i].specifier; + char *key = av_asprintf("ch_layout%s%s", spec[0] ? ":" : "", spec); + if (!key) + return AVERROR(ENOMEM); + snprintf(val, sizeof(val), "%dC", o->audio_channels.opt[i].u.i); + av_dict_set(&o->g->format_opts, key, val, 0); + av_dict_set(&o->g->codec_opts, key, val, AV_DICT_DONT_STRDUP_KEY); + } + for (int i = 0; i < o->audio_ch_layouts.nb_opt; i++) { + char *val = o->audio_ch_layouts.opt[i].u.str; + char *spec = o->audio_ch_layouts.opt[i].specifier; + char *key = av_asprintf("ch_layout%s%s", spec[0] ? ":" : "", spec); + if (!key) + return AVERROR(ENOMEM); + av_dict_set(&o->g->format_opts, key, val, 0); + av_dict_set(&o->g->codec_opts, key, val, AV_DICT_DONT_STRDUP_KEY); + } + return 0; +} + int ifile_open(const OptionsContext *o, const char *filename, Scheduler *sch) { Demuxer *d; @@ -1592,24 +1619,6 @@ int ifile_open(const OptionsContext *o, const char *filename, Scheduler *sch) if (o->audio_sample_rate.nb_opt) { av_dict_set_int(&o->g->format_opts, "sample_rate", o->audio_sample_rate.opt[o->audio_sample_rate.nb_opt - 1].u.i, 0); } - if (o->audio_channels.nb_opt) { - const AVClass *priv_class; - if (file_iformat && (priv_class = file_iformat->priv_class) && - av_opt_find(&priv_class, "ch_layout", NULL, 0, - AV_OPT_SEARCH_FAKE_OBJ)) { - char buf[32]; - snprintf(buf, sizeof(buf), "%dC", o->audio_channels.opt[o->audio_channels.nb_opt - 1].u.i); - av_dict_set(&o->g->format_opts, "ch_layout", buf, 0); - } - } - if (o->audio_ch_layouts.nb_opt) { - const AVClass *priv_class; - if (file_iformat && (priv_class = file_iformat->priv_class) && - av_opt_find(&priv_class, "ch_layout", NULL, 0, - AV_OPT_SEARCH_FAKE_OBJ)) { - av_dict_set(&o->g->format_opts, "ch_layout", o->audio_ch_layouts.opt[o->audio_ch_layouts.nb_opt - 1].u.str, 0); - } - } if (o->frame_rates.nb_opt) { const AVClass *priv_class; /* set the format-level framerate option; @@ -1626,6 +1635,7 @@ int ifile_open(const OptionsContext *o, const char *filename, Scheduler *sch) } if (o->frame_pix_fmts.nb_opt) av_dict_set(&o->g->format_opts, "pixel_format", o->frame_pix_fmts.opt[o->frame_pix_fmts.nb_opt - 1].u.str, 0); + ret = set_input_ch_layout_opts(o); video_codec_name = opt_match_per_type_str(&o->codec_names, 'v'); audio_codec_name = opt_match_per_type_str(&o->codec_names, 'a');
The code only set the channel layout of the AVFormatContext, so the user could not override the channel layout if the demuxer did not have such parameter. Let's set the specified channel layouts as codec options as well. Fixes ticket #11016. A regression since 639c2f00497257cb60ecaeeac1aacfa80df3be06. Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/ffmpeg.texi | 7 ++++--- fftools/ffmpeg_demux.c | 46 +++++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 21 deletions(-)