diff mbox series

[FFmpeg-devel] fftools/ffmpeg_demux: gracefully ignore mismatching channel layouts for -channel_layout option

Message ID 20240603214847.19205-1-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel] fftools/ffmpeg_demux: gracefully ignore mismatching channel layouts for -channel_layout option | expand

Checks

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

Commit Message

Marton Balint June 3, 2024, 9:48 p.m. UTC
The very old behaviour of -channel_layout was to simply warn the user about
channel layouts which does not have a matching channel count, and ignore them,
instead of reporting an error.

The recent fix re-added support for overriding -channel_layout, but it rejected
mismatching layouts. There is no easy way for the user to specify a channel
layout only for streams with matching number of channels, so this patch
restores the very old behaviour of ignoring mismatching layouts. See the
discussion in ticket #11016.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 fftools/ffmpeg_demux.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Anton Khirnov June 5, 2024, 8:41 a.m. UTC | #1
Quoting Marton Balint (2024-06-03 23:48:47)
> The very old behaviour of -channel_layout was to simply warn the user about
> channel layouts which does not have a matching channel count, and ignore them,
> instead of reporting an error.
> 
> The recent fix re-added support for overriding -channel_layout, but it rejected
> mismatching layouts. There is no easy way for the user to specify a channel
> layout only for streams with matching number of channels, so this patch
> restores the very old behaviour of ignoring mismatching layouts. See the
> discussion in ticket #11016.

I'm ambivalent about this. On one hand it probably doesn't hurt, for now
at least, on the other it seems quite ad-hoc. Previously it worked this
way mostly by accident, whereas if we now restore this behaviour
deliberately we'll be committing to supporting it for the foreseeable
future.
Marton Balint June 10, 2024, 7:23 p.m. UTC | #2
On Wed, 5 Jun 2024, Anton Khirnov wrote:

> Quoting Marton Balint (2024-06-03 23:48:47)
>> The very old behaviour of -channel_layout was to simply warn the user about
>> channel layouts which does not have a matching channel count, and ignore them,
>> instead of reporting an error.
>>
>> The recent fix re-added support for overriding -channel_layout, but it rejected
>> mismatching layouts. There is no easy way for the user to specify a channel
>> layout only for streams with matching number of channels, so this patch
>> restores the very old behaviour of ignoring mismatching layouts. See the
>> discussion in ticket #11016.
>
> I'm ambivalent about this. On one hand it probably doesn't hurt, for now
> at least, on the other it seems quite ad-hoc. Previously it worked this
> way mostly by accident, whereas if we now restore this behaviour
> deliberately we'll be committing to supporting it for the foreseeable
> future.
>

Yeah, I am a bit unsure about it as well. What made me implement this is 
that there is really no easy way I could think of to simulate the old 
behaviour. An alternative idea might be to introduce a new option 
"-try_channel_map" which only gives a warning.

Ideally if we could use a stream specifier something like

-channel_map:a:eval:eq(st.codecpar.ch_layout.nb_channels,1) mono

that should solve this in a generic way, but this requires massive new 
features in the eval API and in the opt API as well.

Regards,
Marton
diff mbox series

Patch

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 1ca8d804ae..2e77210237 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1399,13 +1399,14 @@  static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
                 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",
+                av_log(ist, AV_LOG_WARNING,
+                    "Specified channel layout '%s' has %d channels, but input has %d channels. Ignoring specified layout.\n",
                     ch_layout_str, ch_layout.nb_channels, par->ch_layout.nb_channels);
                 av_channel_layout_uninit(&ch_layout);
-                return AVERROR(EINVAL);
+                ch_layout_str = NULL;
             }
-        } else {
+        }
+        if (!ch_layout_str) {
             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);