On 19.07.2018 23:37, Michael Niedermayer wrote:
On Thu, Jul 19, 2018 at 01:53:09PM +0200, Tobias Rapp wrote:
On 18.07.2018 19:31, 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.
---
  libswresample/rematrix.c          | 6 ++++--
  libswresample/x86/rematrix_init.c | 6 ++++--
  2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/libswresample/rematrix.c b/libswresample/rematrix.c
index 8227730056..d1a0cfe7f8 100644
--- a/libswresample/rematrix.c
+++ b/libswresample/rematrix.c
@@ -384,8 +384,10 @@ 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_count > 0) ? s->in.ch_count :
+        av_get_channel_layout_nb_channels(s->in_ch_layout);
+    int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+        av_get_channel_layout_nb_channels(s->out_ch_layout);
      s->mix_any_f = NULL;
diff --git a/libswresample/x86/rematrix_init.c 
b/libswresample/x86/rematrix_init.c
index d71b41a73e..4f9c92e4ab 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -33,8 +33,10 @@ 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_count > 0) ? s->in.ch_count :
+        av_get_channel_layout_nb_channels(s->in_ch_layout);
+    int nb_out = (s->out.ch_count > 0) ? s->out.ch_count :
+        av_get_channel_layout_nb_channels(s->out_ch_layout);
      int num    = nb_in * nb_out;
      int i,j;


Patch looks good to me, except that the title should be updated to reflect
the new logic. I suggest something like "swresample: Use channel count for
rematrix initialization if defined".

The patch does not look good to me
There should be a field that represents the count of channels.
No conditional should be needed

please check that every field is wrong in at least some case.
If true a new field must be added or a existing one needs to be set
differently
But there should be a field that represents the channel count.

If I understand correctly you say that the fall-back to av_get_channel_layout_nb_channels() is not needed here as both functions are internal and called only as part of swr_init() so we may safely assume that the in/out.ch_count fields have been initialized before this code is reached.

Fine with me. Marcin, would you update the patch once more? And there are some additional FATE tests for the pan filter that I can post once this patch is through, if you haven't made up your own tests.

Regards,
Tobias

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to