On 13.07.2018 13:43, 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];
Sorry for jumping into the discussion late but I don't think this change
is necessary. If only one of the user_*_ch_count / user_*_ch_layout
field pair is set, the outcome will be the same. If both field values
are set they must be consistent or undefined behavior will occur. So if
we assume the field values are consistent, why use the value that has to
be calculated first from the layout mask instead of the explicit count
value?
@@ -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;
s->mix_any_f = NULL;
diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c
index d71b41a73e..a6ae074926 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,12 @@ D(int16, sse2)
av_cold int swri_rematrix_init_x86(struct SwrContext *s){
#if HAVE_X86ASM
int mm_flags = av_get_cpu_flags();
- 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;
int num = nb_in * nb_out;
int i,j;
Like said above I think the same effect can be achieved with less CPU
cycles by using a "(count > 0) ? count :
av_get_channel_layout_nb_channels(layout)" code pattern.
Also not sure what is the difference between the "in_ch_layout" field
used here and the "user_in_ch_layout" field used in function swr_set_matrix.
Minor nit: In my personal opinion putting parentheses around the
condition part of the ternary operator would improve readability.
Regards,
Tobias
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
