| Message ID | 1473018301-23871-1-git-send-email-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
Le nonidi 19 fructidor, an CCXXIV, Marton Balint a écrit : > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > doc/filters.texi | 11 +++++++---- > libavfilter/af_amerge.c | 12 ++++++++---- > 2 files changed, 15 insertions(+), 8 deletions(-) Can you explain the use case? Regards,
On Sun, 4 Sep 2016, Nicolas George wrote: > Le nonidi 19 fructidor, an CCXXIV, Marton Balint a écrit : >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> doc/filters.texi | 11 +++++++---- >> libavfilter/af_amerge.c | 12 ++++++++---- >> 2 files changed, 15 insertions(+), 8 deletions(-) > > Can you explain the use case? > Some source files might have a mono track, and a stereo track. If I merge these, the order will change, because mono is handled as center. Regards, Marton
Le nonidi 19 fructidor, an CCXXIV, Marton Balint a écrit : > Some source files might have a mono track, and a stereo track. If I merge > these, the order will change, because mono is handled as center. And this is completely correct. What would you want instead? Regards,
On Sun, 4 Sep 2016, Nicolas George wrote: > Le nonidi 19 fructidor, an CCXXIV, Marton Balint a écrit : >> Some source files might have a mono track, and a stereo track. If I merge >> these, the order will change, because mono is handled as center. > > And this is completely correct. What would you want instead? The order of the channels to remain unchanged regardless of the input channel layout (which is often guessed only). Filtering in ffmpeg is unfortunately forcing you to use channel layouts, even if you don't want to, even if you only want to say: this track has 2 channels, that stream track has 1 channels. Don't assume anything else. Another example: You have a file, with mono tracks. Some tracks have correct metadata describing the channel layout of the files, others don't. You wan't the channels in file-order, regardless of the correctness of the input channel layout metadata in the file. Regards, Marton
Hi! 2016-09-05 0:17 GMT+02:00 Marton Balint <cus@passwd.hu>: > Filtering in ffmpeg is unfortunately forcing you to use channel > layouts, even if you don't want to, even if you only want to say: > this track has 2 channels, that stream track has 1 channels. But can't this be fixed? Carl Eugen
Le decadi 20 fructidor, an CCXXIV, Marton Balint a écrit : > The order of the channels to remain unchanged regardless of the input > channel layout (which is often guessed only). Filtering in ffmpeg is > unfortunately forcing you to use channel layouts, even if you don't want to, > even if you only want to say: this track has 2 channels, that stream track > has 1 channels. Don't assume anything else. I feel Carl Eugen has the right of it. libavfilter can work with streams with unknown layouts, but some individual filters may not. To make them work is relatively easy. First, make sure they do not use the channel layout: most filters from the fork will use the channel layout to know the number of channels, we have a separated field for that. Then declare accordingly the list of formats in query formats. The real problem is ffmpeg.c: it will try to guess the channel layout when none is specified. I think I added an option to avoid that, but it may have been lost at some point. And of course, most of the time, there is a channel layout, you just have to make sure it is properly set at input and let the logic do its work. > Another example: You have a file, with mono tracks. Some tracks have correct > metadata describing the channel layout of the files, others don't. ^^^^^^^^^^^^ This is what needs fixing. > You wan't > the channels in file-order, regardless of the correctness of the input > channel layout metadata in the file. Not true. Codecs work with the channel layouts, for inter-channel correlation, for the cutoff frequency on the subwoofer, etc. Regards,
On Mon, 5 Sep 2016, Nicolas George wrote: > Le decadi 20 fructidor, an CCXXIV, Marton Balint a écrit : >> The order of the channels to remain unchanged regardless of the input >> channel layout (which is often guessed only). Filtering in ffmpeg is >> unfortunately forcing you to use channel layouts, even if you don't want to, >> even if you only want to say: this track has 2 channels, that stream track >> has 1 channels. Don't assume anything else. > > I feel Carl Eugen has the right of it. > > libavfilter can work with streams with unknown layouts, but some individual > filters may not. To make them work is relatively easy. First, make sure they > do not use the channel layout: most filters from the fork will use the > channel layout to know the number of channels, we have a separated field for > that. Then declare accordingly the list of formats in query formats. > > The real problem is ffmpeg.c: it will try to guess the channel layout when > none is specified. I think I added an option to avoid that, but it may have > been lost at some point. > > And of course, most of the time, there is a channel layout, you just have to > make sure it is properly set at input and let the logic do its work. The work needed fixing this is definitely not comparable to the work I did creating this patch, or the work anyone will have to do maintaining it. Asking me to go in this direction, seems an unreasonable amount of extra work for me, and most probably I will not do it, and rather abandon the patch, or maintain it privately. >> Another example: You have a file, with mono tracks. Some tracks have correct >> metadata describing the channel layout of the files, others don't. > ^^^^^^^^^^^^ > This is what needs fixing. I don't trust the input channel layouts, I want to specify that all my input channel layouts are unkown. Can/will I do this with -channel_layout 0 or something like that? >> You wan't >> the channels in file-order, regardless of the correctness of the input >> channel layout metadata in the file. > > Not true. Codecs work with the channel layouts, for inter-channel > correlation, for the cutoff frequency on the subwoofer, etc. > Sure, I plan to use uncompressed audio on the output side. Regards, Marton
On Mon, Sep 05, 2016 at 04:43:02PM +0200, Nicolas George wrote: > Le decadi 20 fructidor, an CCXXIV, Marton Balint a écrit : > > The order of the channels to remain unchanged regardless of the input > > channel layout (which is often guessed only). Filtering in ffmpeg is > > unfortunately forcing you to use channel layouts, even if you don't want to, > > even if you only want to say: this track has 2 channels, that stream track > > has 1 channels. Don't assume anything else. > > I feel Carl Eugen has the right of it. > > libavfilter can work with streams with unknown layouts, but some individual > filters may not. To make them work is relatively easy. First, make sure they > do not use the channel layout: most filters from the fork will use the > channel layout to know the number of channels, we have a separated field for > that. Then declare accordingly the list of formats in query formats. is there some easy way to identify test for filters that require the layout unneccessary ? "git grep av_get_channel_layout_nb_channels" and manual checking ? I might be interrested to help fixing these ... thx [...]
On 05.09.2016 16:43, Nicolas George wrote: > Le decadi 20 fructidor, an CCXXIV, Marton Balint a écrit : >> The order of the channels to remain unchanged regardless of the input >> channel layout (which is often guessed only). Filtering in ffmpeg is >> unfortunately forcing you to use channel layouts, even if you don't want to, >> even if you only want to say: this track has 2 channels, that stream track >> has 1 channels. Don't assume anything else. > > I feel Carl Eugen has the right of it. > > libavfilter can work with streams with unknown layouts, but some individual > filters may not. To make them work is relatively easy. First, make sure they > do not use the channel layout: most filters from the fork will use the > channel layout to know the number of channels, we have a separated field for > that. Then declare accordingly the list of formats in query formats. > > The real problem is ffmpeg.c: it will try to guess the channel layout when > none is specified. I think I added an option to avoid that, but it may have > been lost at some point. -guess_layout_max 0 But indeed when using it I stumbled over similar filter issues (amerge or pan?). In the end I stopped using it and let the guessing do its work, instead I added "-write_channel_mask off" on AVI output side. And there seems to be no option to explicitly set an unknown input channel layout (as Marton noted). > And of course, most of the time, there is a channel layout, you just have to > make sure it is properly set at input and let the logic do its work. > >> Another example: You have a file, with mono tracks. Some tracks have correct >> metadata describing the channel layout of the files, others don't. > ^^^^^^^^^^^^ > This is what needs fixing. > >> You wan't >> the channels in file-order, regardless of the correctness of the input >> channel layout metadata in the file. > > Not true. Codecs work with the channel layouts, for inter-channel > correlation, for the cutoff frequency on the subwoofer, etc. Regards, Tobias
2016-09-05 20:51 GMT+02:00 Marton Balint <cus@passwd.hu>: > Can/will I do this with -channel_layout 0 or > something like that? This (-channel_layout 0) was broken in 2012. Carl Eugen
Le decadi 20 fructidor, an CCXXIV, Marton Balint a écrit : > The work needed fixing this is definitely not comparable to the work I did > creating this patch, or the work anyone will have to do maintaining it. > Asking me to go in this direction, seems an unreasonable amount of extra > work for me, and most probably I will not do it, and rather abandon the > patch, or maintain it privately. I understand that, but the patch as is goes in the wrong direction. I am sure we can discuss this a bit further and find a solution that goes in the right direction but is still acceptable with regard to the efforts you can propose. > I don't trust the input channel layouts, I want to specify that all my input > channel layouts are unkown. Can/will I do this with -channel_layout 0 or > something like that? I think the tool should allow to override all values from the input, as long as it makes sense, and this applies to the channel layout without doubt. In other words, I strongly support an option to override the channel layout detected from inputs, including to set it to unknown whenever it makes sense. I do not know if "-channel_layout" is the right option for the job. At a quick glance, it seems to do a lot of things. A simpler option that checks the number of channels and overrides the layout may be a better idea; and naming all the overriding options consistently could be a nice extra feature. But... > Sure, I plan to use uncompressed audio on the output side. ... I do not think that allowing unknown layouts too easily is a good idea. Sometimes, it makes sense to have an unknown layout (at least until we have a more expressive system, if it happens); for example, dual-language-disguised-as-stereo as found in some TV broadcasts can not be expressed as a layout. But most of the time, there is a channel layout, and the correct solution is to make sure the tools and library know it as early as possible. I am pretty sure you are able to make the difference. But even like that you may prepare yourself trouble for the future: maybe at some point you will update your code to change uncompressed PCM to Opus, forget you neglected the layout issue, and thus lose inter-channel correlation. Unfortunately, our average users may not have that clear an understanding. I suspect a lot of them, subjected to an issue, would try setting an unknown channel layout as suggested by an anonymous on some barely-related stackexchange question, see it cuts right through their problem and not realize they are creating slightly invalid files. Regards,
2016-09-10 11:58 GMT+02:00 Nicolas George <george@nsup.org>: > In other words, I strongly support an option to override the > channel layout detected from inputs, including to set it to > unknown whenever it makes sense. > > I do not know if "-channel_layout" is the right option for the job. Why not? Afair, I had tested it extensively before it was broken. Carl Eugen
Le decadi 30 fructidor, an CCXXIV, Carl Eugen Hoyos a écrit : > Why not? > Afair, I had tested it extensively before it was broken. Because "providing a value that is not available" and "overriding a value" are two very different tasks. Sometimes you have to specify the channel layout because the format does not provide the information, for example raw PCM. What we are discussing here is overriding information provided by the format but supposed wrong. I would like to avoid people doing the second when they think they are doing the first one, or just have no clue about what they are doing exactly. Regards,
2016-09-15 15:46 GMT+02:00 Nicolas George <george@nsup.org>: > Le decadi 30 fructidor, an CCXXIV, Carl Eugen Hoyos a écrit : >> Why not? >> Afair, I had tested it extensively before it was broken. > > Because "providing a value that is not available" and > "overriding a value" are two very different tasks. But are we doing this differentiation for any FFmpeg option? Carl Eugen
diff --git a/doc/filters.texi b/doc/filters.texi index c12b093..b1e3890 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -1069,15 +1069,18 @@ The filter accepts the following options: @item inputs Set the number of inputs. Default is 2. +@item reorder_inputs +Try to reorder the inputs based on their channel layout. Default is true. + @end table If the channel layouts of the inputs are disjoint, and therefore compatible, the channel layout of the output will be set accordingly and the channels will be reordered as necessary. If the channel layouts of the inputs are not -disjoint, the output will have all the channels of the first input then all -the channels of the second input, in that order, and the channel layout of -the output will be the default value corresponding to the total number of -channels. +disjoint or @var{reorder_inputs} is set to false, the output will have all the +channels of the first input then all the channels of the second input, in that +order, and the channel layout of the output will be the default value +corresponding to the total number of channels. For example, if the first input is in 2.1 (FL+FR+LF) and the second input is FC+BL+BR, then the output will be in 5.1, with the channels in the diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c index 2b4edb0..419e696 100644 --- a/libavfilter/af_amerge.c +++ b/libavfilter/af_amerge.c @@ -37,6 +37,7 @@ typedef struct { const AVClass *class; int nb_inputs; + int reorder_inputs; int route[SWR_CH_MAX]; /**< channels routing, see copy_samples */ int bps; struct amerge_input { @@ -53,6 +54,8 @@ typedef struct { static const AVOption amerge_options[] = { { "inputs", "specify the number of inputs", OFFSET(nb_inputs), AV_OPT_TYPE_INT, { .i64 = 2 }, 2, SWR_CH_MAX, FLAGS }, + { "reorder_inputs", "try to reorder inputs based on their channel layout", OFFSET(reorder_inputs), + AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, FLAGS }, { NULL } }; @@ -103,10 +106,11 @@ static int query_formats(AVFilterContext *ctx) av_log(ctx, AV_LOG_ERROR, "Too many channels (max %d)\n", SWR_CH_MAX); return AVERROR(EINVAL); } - if (overlap) { - av_log(ctx, AV_LOG_WARNING, - "Input channel layouts overlap: " - "output layout will be determined by the number of distinct input channels\n"); + if (!s->reorder_inputs || overlap) { + if (overlap) + av_log(ctx, AV_LOG_WARNING, + "Input channel layouts overlap: " + "output layout will be determined by the number of distinct input channels\n"); for (i = 0; i < nb_ch; i++) s->route[i] = i; outlayout = av_get_default_channel_layout(nb_ch);
Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/filters.texi | 11 +++++++---- libavfilter/af_amerge.c | 12 ++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-)