[FFmpeg-devel] [PATCH] libswresample: Use channel count if channel layout is undefined

[Michael Niedermayer]

On Fri, Jul 13, 2018 at 12:43:36PM +0100, Marcin Gorzel wrote:
> Rematrixing supports up to 64 channels. However, there is only a limited number of channel layouts defined. Since the in/out channel count is obtained from the channel layout, for undefined layouts (e.g. for 9, 10, 11 channels etc.) the rematrixing fails.
> 
> In ticket #6790 the problem has been partially addressed by applying a patch to swr_set_matrix() in rematrix.c:72.
> However, a similar change must be also applied to swri_rematrix_init() in rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36.
> 
> This patch adds the following check to the swri_rematrix_init() in rematrix.c:389 and swri_rematrix_init_x86() in rematrix_init.c:36: if channel layout is non-zero, obtain channel count from the channel layout, otherwise, use channel count instead.
> 
> It also modifies the checks in swr_set_matrix() in rematrix.c:72 to match the above checks. (Since av_get_channel_layout_nb_channels(s->user_in_ch_layout) was originally used, there may be preference to rely on the channel layout first (if available) before falling back to the user channel count).
> ---
>  libswresample/rematrix.c          | 18 ++++++++++++------
>  libswresample/x86/rematrix_init.c |  8 ++++++--
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
> index 8227730056..8c9fbf3804 100644
> --- a/libswresample/rematrix.c
> +++ b/libswresample/rematrix.c
> @@ -69,10 +69,12 @@ int swr_set_matrix(struct SwrContext *s, const double *matrix, int stride)
>          return AVERROR(EINVAL);
>      memset(s->matrix, 0, sizeof(s->matrix));
>      memset(s->matrix_flt, 0, sizeof(s->matrix_flt));
> -    nb_in = (s->user_in_ch_count > 0) ? s->user_in_ch_count :
> -        av_get_channel_layout_nb_channels(s->user_in_ch_layout);
> -    nb_out = (s->user_out_ch_count > 0) ? s->user_out_ch_count :
> -        av_get_channel_layout_nb_channels(s->user_out_ch_layout);
> +    nb_in = s->user_in_ch_layout != 0
> +        ? av_get_channel_layout_nb_channels(s->user_in_ch_layout)
> +        : s->user_in_ch_count;
> +    nb_out = s->user_out_ch_layout != 0
> +        ? av_get_channel_layout_nb_channels(s->user_out_ch_layout)
> +        : s->user_out_ch_count;
>      for (out = 0; out < nb_out; out++) {
>          for (in = 0; in < nb_in; in++)
>              s->matrix_flt[out][in] = s->matrix[out][in] = matrix[in];

> @@ -384,8 +386,12 @@ av_cold static int auto_matrix(SwrContext *s)
>  
>  av_cold int swri_rematrix_init(SwrContext *s){
>      int i, j;
> -    int nb_in  = av_get_channel_layout_nb_channels(s->in_ch_layout);
> -    int nb_out = av_get_channel_layout_nb_channels(s->out_ch_layout);
> +    int nb_in  = s->in_ch_layout != 0
> +        ? av_get_channel_layout_nb_channels(s->in_ch_layout)
> +        : s->user_in_ch_count;
> +    int nb_out = s->out_ch_layout != 0
> +        ? av_get_channel_layout_nb_channels(s->out_ch_layout)
> +        : s->user_out_ch_count;

So this is the core of the change (the other hunk is a "duplicate" and one
cosmetic)

The code after this uses C ? A : B;
this implies that A is wrong in some cases and B is wrong in some cases
you explained only one of these, that is that the layout is unable to
represent some cases.

2nd question is, are these the ideal fields.
shouldnt this use s->used_ch_count and s->out.ch_count?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180714/51613881/attachment.sig>