avfilter/af_headphone: Fix channel assignment

The documentation of the map argument of the headphone filter states:

"Set mapping of input streams for convolution. The argument is a
’|’-separated list of channel names in order as they are given as
additional stream inputs for filter."

Yet this has not been honoured at all. Instead for the kth given HRIR
channel pair it was checked whether there was a kth mapping and if the
channel position so given was present in the channel layout of the main
input; if so, then the given HRIR channel pair was matched to the kth
channel of the main input. It should actually have been matched to the
channel given by the kth mapping. A consequence of the current algorithm
is that if N additional HRIR channel pairs are given, a permutation of
the first N entries of the mapping does not affect the output at all.

The old code might even set arrays belonging to streams that don't exist
(i.e. whose index is >= the number of channels of the main input
stream); these parts were not read lateron at all. The new code doesn't
do this any longer and therefore the number of elements of some of the
allocated arrays has been reduced (in case the number of mappings was
bigger than the number of channels of the first input stream).

Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index fb6af7a966ef7c07211d360b70b209146ffbe7b4..d6647ff80bc6ab443d4697de73c8e1087e933f15 100644 (file)
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -117,8 +117,6 @@ static void parse_map(AVFilterContext *ctx)
             continue;
         }
         used_channels        |= out_channel;
-        if (out_channel == AV_CH_LOW_FREQUENCY)
-            s->lfe_channel = s->nb_irs;
         s->mapping[s->nb_irs] = out_channel;
         s->nb_irs++;
     }
@@ -368,7 +366,6 @@ static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
 {
     struct HeadphoneContext *s = ctx->priv;
     const int ir_len = s->ir_len;
-    int nb_irs = s->nb_irs;
     int nb_input_channels = ctx->inputs[0]->channels;
     float gain_lin = expf((s->gain - 3 * nb_input_channels) / 20 * M_LN10);
     FFTComplex *fft_in_l = NULL;
@@ -430,15 +427,15 @@ static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
         s->temp_src[0] = av_calloc(s->air_len, sizeof(float));
         s->temp_src[1] = av_calloc(s->air_len, sizeof(float));
 
-        s->data_ir[0] = av_calloc(nb_irs * s->air_len, sizeof(*s->data_ir[0]));
-        s->data_ir[1] = av_calloc(nb_irs * s->air_len, sizeof(*s->data_ir[1]));
+        s->data_ir[0] = av_calloc(nb_input_channels * s->air_len, sizeof(*s->data_ir[0]));
+        s->data_ir[1] = av_calloc(nb_input_channels * s->air_len, sizeof(*s->data_ir[1]));
         if (!s->data_ir[0] || !s->data_ir[1] || !s->temp_src[0] || !s->temp_src[1]) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
     } else {
-        s->data_hrtf[0] = av_calloc(n_fft, sizeof(*s->data_hrtf[0]) * nb_irs);
-        s->data_hrtf[1] = av_calloc(n_fft, sizeof(*s->data_hrtf[1]) * nb_irs);
+        s->data_hrtf[0] = av_calloc(n_fft, sizeof(*s->data_hrtf[0]) * nb_input_channels);
+        s->data_hrtf[1] = av_calloc(n_fft, sizeof(*s->data_hrtf[1]) * nb_input_channels);
         if (!s->data_hrtf[0] || !s->data_hrtf[1]) {
             ret = AVERROR(ENOMEM);
             goto fail;
@@ -459,7 +456,9 @@ static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
 
             for (j = 0; j < inlink->channels; j++) {
                 if ((av_channel_layout_extract_channel(inlink->channel_layout, j)) == s->mapping[i]) {
-                    idx = i;
+                    idx = j;
+                    if (s->mapping[i] == AV_CH_LOW_FREQUENCY)
+                        s->lfe_channel = j;
                     break;
                 }
             }
@@ -499,14 +498,16 @@ static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
 
                 for (j = 0; j < inlink->channels; j++) {
                     if ((av_channel_layout_extract_channel(inlink->channel_layout, j)) == s->mapping[k]) {
-                        idx = k;
+                        idx = j;
+                        if (s->mapping[k] == AV_CH_LOW_FREQUENCY)
+                            s->lfe_channel = j;
                         break;
                     }
                 }
                 if (idx == -1)
                     continue;
 
-                I = idx * 2;
+                I = k * 2;
                 if (s->type == TIME_DOMAIN) {
                     float *data_ir_l = s->data_ir[0] + idx * s->air_len;
                     float *data_ir_r = s->data_ir[1] + idx * s->air_len;