[FFmpeg-devel] [PATCH 2/2] avfilter/af_amerge: do not guess an output channel layout

[Nicolas George]

Le quintidi 25 frimaire, an CCXXV, Marton Balint a écrit :
> This is the right thing to do, but I am afraid this will break too many
> existing filter chains. How can we implement this properly? Ideas/options:
> 
> - change it, break it, users will fix it
> - add a guess_output_layout option which will be true for now, false after a
>   major bump, and mention this incompatible change in the next release
> - add amerge2 filter, deprecate amerge filter

I do not like the option or the amerge2 solution, because that leaves
traces we can not get rid of.

My gut feeling wants me to say: there already was a warning, people who
ignore warnings deserve what they get. If you want to be extra careful
and are not in a hurry, you could reword the warning to notify that it
will change soon, wait some time (next release) and do the change.

> 
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
>  libavfilter/af_amerge.c      | 4 +---
>  tests/ref/fate/filter-amerge | 4 ++--
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/libavfilter/af_amerge.c b/libavfilter/af_amerge.c
> index 3bc7d89..43a2d95 100644
> --- a/libavfilter/af_amerge.c
> +++ b/libavfilter/af_amerge.c
> @@ -114,9 +114,7 @@ static int query_formats(AVFilterContext *ctx)

>                 "output layout will be determined by the number of distinct input channels\n");

I think the warning needs to be reworded anyway.

>          for (i = 0; i < nb_ch; i++)
>              s->route[i] = i;
> -        outlayout = av_get_default_channel_layout(nb_ch);
> -        if (!outlayout && nb_ch)
> -            outlayout = 0xFFFFFFFFFFFFFFFFULL >> (64 - nb_ch);
> +        outlayout = FF_COUNT2LAYOUT(nb_ch);
>      } else {
>          int *route[SWR_CH_MAX];
>          int c, out_ch_number = 0;
> diff --git a/tests/ref/fate/filter-amerge b/tests/ref/fate/filter-amerge
> index b3e5eb5..8118b4e 100644

> --- a/tests/ref/fate/filter-amerge
> +++ b/tests/ref/fate/filter-amerge
> @@ -2,8 +2,8 @@
>  #media_type 0: audio
>  #codec_id 0: pcm_s16le
>  #sample_rate 0: 44100
> -#channel_layout 0: 3
> -#channel_layout_name 0: stereo
> +#channel_layout 0: 0
> +#channel_layout_name 0: 2 channels

Michael: this is an example of filter graph that breaks: the output was
"stereo", it becomes "2 channels". Since there are
av_get_default_channel_layout() all over the place anyway, I think
something else would have caught it and hidden the difference for all
practical purposes.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161223/3ed6b4ca/attachment.sig>