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

Michael Niedermayer michael at niedermayer.cc
Fri Jul 20 00:37:30 EEST 2018


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.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20180719/ac231121/attachment.sig>


More information about the ffmpeg-devel mailing list