| Message ID | 20240519195258.13627-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 4bfecde9b9b25568f10135a507b44d1bb623c66b |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/5] fftools/ffmpeg_demux: honor -ch_layout options for overriding input stream channel layout | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
On Sun, 19 May 2024, 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. > > This used to work via the respective AVCodecContext option, but since > 639c2f00497257cb60ecaeeac1aacfa80df3be06 it no longer gets passed to the > decoders. It is actually better if we set it manually, instead of using the > codec option because that way we can also override it on the stream level, so > it will also work for stream copy or bitstream filtering. > > We don't allow changing the number of channels, because that can cause > unexpected results. We disable layout guessing, if a channel layout is > specified. > > Fixes ticket #11016. Will apply the series. Regards, Marton > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > fftools/ffmpeg_demux.c | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index cba63dab5f..1ca8d804ae 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -1386,9 +1386,30 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > > break; > case AVMEDIA_TYPE_AUDIO: { > - int guess_layout_max = INT_MAX; > - MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); > - guess_input_channel_layout(ist, par, guess_layout_max); > + char *ch_layout_str = NULL; > + MATCH_PER_STREAM_OPT(audio_ch_layouts, str, ch_layout_str, ic, st); > + if (ch_layout_str) { > + AVChannelLayout ch_layout; > + ret = av_channel_layout_from_string(&ch_layout, ch_layout_str); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, "Error parsing channel layout %s.\n", ch_layout_str); > + return ret; > + } > + if (par->ch_layout.nb_channels <= 0 || par->ch_layout.nb_channels == ch_layout.nb_channels) { > + av_channel_layout_uninit(&par->ch_layout); > + par->ch_layout = ch_layout; > + } else { > + av_log(ist, AV_LOG_ERROR, > + "Specified channel layout '%s' has %d channels, but input has %d channels.\n", > + ch_layout_str, ch_layout.nb_channels, par->ch_layout.nb_channels); > + av_channel_layout_uninit(&ch_layout); > + return AVERROR(EINVAL); > + } > + } else { > + int guess_layout_max = INT_MAX; > + MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); > + guess_input_channel_layout(ist, par, guess_layout_max); > + } > break; > } > case AVMEDIA_TYPE_DATA: > -- > 2.35.3 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Quoting Marton Balint (2024-05-25 18:57:32) > > > On Sun, 19 May 2024, 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. > > > > This used to work via the respective AVCodecContext option, but since > > 639c2f00497257cb60ecaeeac1aacfa80df3be06 it no longer gets passed to the > > decoders. It is actually better if we set it manually, instead of using the > > codec option because that way we can also override it on the stream level, so > > it will also work for stream copy or bitstream filtering. > > > > We don't allow changing the number of channels, because that can cause > > unexpected results. We disable layout guessing, if a channel layout is > > specified. > > > > Fixes ticket #11016. > > Will apply the series. Could you please wait a few days? I'm just freshly back from a vacation and didn't have time to look at this in detail yet.
Quoting Marton Balint (2024-05-19 21:52:57) > 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. > > This used to work via the respective AVCodecContext option, but since > 639c2f00497257cb60ecaeeac1aacfa80df3be06 it no longer gets passed to the > decoders. It is actually better if we set it manually, instead of using the > codec option because that way we can also override it on the stream level, so > it will also work for stream copy or bitstream filtering. > > We don't allow changing the number of channels, because that can cause > unexpected results. We disable layout guessing, if a channel layout is > specified. > > Fixes ticket #11016. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > fftools/ffmpeg_demux.c | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) Looks ok
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index cba63dab5f..1ca8d804ae 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1386,9 +1386,30 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) break; case AVMEDIA_TYPE_AUDIO: { - int guess_layout_max = INT_MAX; - MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); - guess_input_channel_layout(ist, par, guess_layout_max); + char *ch_layout_str = NULL; + MATCH_PER_STREAM_OPT(audio_ch_layouts, str, ch_layout_str, ic, st); + if (ch_layout_str) { + AVChannelLayout ch_layout; + ret = av_channel_layout_from_string(&ch_layout, ch_layout_str); + if (ret < 0) { + av_log(ist, AV_LOG_ERROR, "Error parsing channel layout %s.\n", ch_layout_str); + return ret; + } + if (par->ch_layout.nb_channels <= 0 || par->ch_layout.nb_channels == ch_layout.nb_channels) { + av_channel_layout_uninit(&par->ch_layout); + par->ch_layout = ch_layout; + } else { + av_log(ist, AV_LOG_ERROR, + "Specified channel layout '%s' has %d channels, but input has %d channels.\n", + ch_layout_str, ch_layout.nb_channels, par->ch_layout.nb_channels); + av_channel_layout_uninit(&ch_layout); + return AVERROR(EINVAL); + } + } else { + int guess_layout_max = INT_MAX; + MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); + guess_input_channel_layout(ist, par, guess_layout_max); + } break; } case AVMEDIA_TYPE_DATA:
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. This used to work via the respective AVCodecContext option, but since 639c2f00497257cb60ecaeeac1aacfa80df3be06 it no longer gets passed to the decoders. It is actually better if we set it manually, instead of using the codec option because that way we can also override it on the stream level, so it will also work for stream copy or bitstream filtering. We don't allow changing the number of channels, because that can cause unexpected results. We disable layout guessing, if a channel layout is specified. Fixes ticket #11016. Signed-off-by: Marton Balint <cus@passwd.hu> --- fftools/ffmpeg_demux.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)