Ducking fix #3: Removed the state as an input to the FilterAdaptation function

webrtc/modules/audio_processing/aec/aec_core.c

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 164 matching lines @@
                         xfBuf[1][xPos + j],
                         wfBuf[0][pos + j],
                         wfBuf[1][pos + j]);
     }
   }
 }
 
 static void ScaleErrorSignal(int extended_filter_enabled,
                              float normal_mu,
                              float normal_error_threshold,
-                             float *xPow,
+                             float xPow[PART_LEN1],
                              float ef[2][PART_LEN1]) {
   const float mu = extended_filter_enabled ? kExtendedMu : normal_mu;
   const float error_threshold = extended_filter_enabled
                                     ? kExtendedErrorThreshold
                                     : normal_error_threshold;
   int i;
   float abs_ef;
   for (i = 0; i < (PART_LEN1); i++) {
     ef[0][i] /= (xPow[i] + 1e-10f);
     ef[1][i] /= (xPow[i] + 1e-10f);
     abs_ef = sqrtf(ef[0][i] * ef[0][i] + ef[1][i] * ef[1][i]);
 
     if (abs_ef > error_threshold) {
       abs_ef = error_threshold / (abs_ef + 1e-10f);
       ef[0][i] *= abs_ef;
       ef[1][i] *= abs_ef;
     }
 
     // Stepsize factor
     ef[0][i] *= mu;
     ef[1][i] *= mu;
   }
 }
 
-// Time-unconstrined filter adaptation.
-// TODO(andrew): consider for a low-complexity mode.
-// static void FilterAdaptationUnconstrained(AecCore* aec, float *fft,
-//                                          float ef[2][PART_LEN1]) {
-//  int i, j;
-//  for (i = 0; i < aec->num_partitions; i++) {
-//    int xPos = (i + aec->xfBufBlockPos)*(PART_LEN1);
-//    int pos;
-//    // Check for wrap
-//    if (i + aec->xfBufBlockPos >= aec->num_partitions) {
-//      xPos -= aec->num_partitions * PART_LEN1;
-//    }
-//
-//    pos = i * PART_LEN1;
-//
-//    for (j = 0; j < PART_LEN1; j++) {
-//      aec->wfBuf[0][pos + j] += MulRe(aec->xfBuf[0][xPos + j],
-//                                      -aec->xfBuf[1][xPos + j],
-//                                      ef[0][j], ef[1][j]);
-//      aec->wfBuf[1][pos + j] += MulIm(aec->xfBuf[0][xPos + j],
-//                                      -aec->xfBuf[1][xPos + j],
-//                                      ef[0][j], ef[1][j]);
-//    }
-//  }
-//}
 
-static void FilterAdaptation(AecCore* aec, float* fft, float ef[2][PART_LEN1]) {
+static void FilterAdaptation(
+    int num_partitions,
+    int xfBufBlockPos,

[hlundin-webrtc, 2015/11/20 11:55:20]

Rename parameters.

[peah-webrtc, 2015/11/24 13:03:01]

Done.

+    float xfBuf[2][kExtendedNumPartitions * PART_LEN1],

[hlundin-webrtc, 2015/11/20 11:55:20]

Any of the array input that can be made const?

[peah-webrtc, 2015/11/24 13:03:01]

I checked with kwiberg@ and he said it should be fine. I'll re-revise the
ScaleErrorSignal in the previous CL function accordingly as well in an upcoming
CL.

Done.

+    float ef[2][PART_LEN1],
+    float wfBuf[2][kExtendedNumPartitions * PART_LEN1]) {
   int i, j;
-  for (i = 0; i < aec->num_partitions; i++) {
-    int xPos = (i + aec->xfBufBlockPos) * (PART_LEN1);
+  float fft[PART_LEN2];
+  for (i = 0; i < num_partitions; i++) {
+    int xPos = (i + xfBufBlockPos) * (PART_LEN1);
     int pos;
     // Check for wrap
-    if (i + aec->xfBufBlockPos >= aec->num_partitions) {
-      xPos -= aec->num_partitions * PART_LEN1;
+    if (i + xfBufBlockPos >= num_partitions) {
+      xPos -= num_partitions * PART_LEN1;
     }
 
     pos = i * PART_LEN1;
 
     for (j = 0; j < PART_LEN; j++) {
 
-      fft[2 * j] = MulRe(aec->xfBuf[0][xPos + j],
-                         -aec->xfBuf[1][xPos + j],
+      fft[2 * j] = MulRe(xfBuf[0][xPos + j],
+                         -xfBuf[1][xPos + j],
                          ef[0][j],
                          ef[1][j]);
-      fft[2 * j + 1] = MulIm(aec->xfBuf[0][xPos + j],
-                             -aec->xfBuf[1][xPos + j],
+      fft[2 * j + 1] = MulIm(xfBuf[0][xPos + j],
+                             -xfBuf[1][xPos + j],
                              ef[0][j],
                              ef[1][j]);
     }
-    fft[1] = MulRe(aec->xfBuf[0][xPos + PART_LEN],
-                   -aec->xfBuf[1][xPos + PART_LEN],
+    fft[1] = MulRe(xfBuf[0][xPos + PART_LEN],
+                   -xfBuf[1][xPos + PART_LEN],
                    ef[0][PART_LEN],
                    ef[1][PART_LEN]);
 
     aec_rdft_inverse_128(fft);
     memset(fft + PART_LEN, 0, sizeof(float) * PART_LEN);
 
     // fft scaling
     {
       float scale = 2.0f / PART_LEN2;
       for (j = 0; j < PART_LEN; j++) {
         fft[j] *= scale;
       }
     }
     aec_rdft_forward_128(fft);
 
-    aec->wfBuf[0][pos] += fft[0];
-    aec->wfBuf[0][pos + PART_LEN] += fft[1];
+    wfBuf[0][pos] += fft[0];
+    wfBuf[0][pos + PART_LEN] += fft[1];
 
     for (j = 1; j < PART_LEN; j++) {
-      aec->wfBuf[0][pos + j] += fft[2 * j];
-      aec->wfBuf[1][pos + j] += fft[2 * j + 1];
+      wfBuf[0][pos + j] += fft[2 * j];
+      wfBuf[1][pos + j] += fft[2 * j + 1];
     }
   }
 }
 
 static void OverdriveAndSuppress(AecCore* aec,
                                  float hNl[PART_LEN1],
                                  const float hNlFb,
                                  float efw[2][PART_LEN1]) {
   int i;
   for (i = 0; i < PART_LEN1; i++) {
@@ skipping 546 matching lines @@
         self->num_delay_values;
   }
 
   // Reset histogram.
   memset(self->delay_histogram, 0, sizeof(self->delay_histogram));
   self->num_delay_values = 0;
 
   return;
 }
 
-static void FrequencyToTime(float freq_data[2][PART_LEN1],
-                            float time_data[PART_LEN2]) {
+static void InverseFft(float freq_data[2][PART_LEN1],

[hlundin-webrtc, 2015/11/20 11:55:20]

Make freq_data const.

[peah-webrtc, 2015/11/24 13:03:01]

Done.

+                       float time_data[PART_LEN2]) {
   time_data[0] = freq_data[0][0];
   time_data[1] = freq_data[0][PART_LEN];
   for (int i = 1; i < PART_LEN; i++) {
     time_data[2 * i] = freq_data[0][i];
     time_data[2 * i + 1] = freq_data[1][i];
   }
   aec_rdft_inverse_128(time_data);
 }
 
 
-static void TimeToFrequency(float time_data[PART_LEN2],
-                            float freq_data[2][PART_LEN1],
-                            int window) {
-  int i = 0;
-
-  // TODO(bjornv): Should we have a different function/wrapper for windowed FFT?
-  if (window) {
-    for (i = 0; i < PART_LEN; i++) {
-      time_data[i] *= WebRtcAec_sqrtHanning[i];
-      time_data[PART_LEN + i] *= WebRtcAec_sqrtHanning[PART_LEN - i];
-    }
-  }
-
+static void Fft(float time_data[PART_LEN2],

[hlundin-webrtc, 2015/11/20 11:55:20]

Make time_data const.

[ivoc, 2015/11/20 12:58:02]

I think the aec_rdft_forward_128 function writes the output over the input (and
destoys the input in the process). To be honest, I would prefer a fft function
that writes the output directly to an output array, so the copy action below can
be avoided as well, but I guess this was already part of the code.

[peah-webrtc, 2015/11/24 13:03:01]

That is fully correct. And you have a great point! Ideally I would have liked
the fft to be stored in an interlieved manner, as that would have allowed for
in-place calls which would reduce the stack-space. But as it is now, I don't
think we have much choice of that as it would require a lot of refactoring.

I think that if we redesign this to use a memcpy internally that would increase
the complexity by one memcpy in total, but would make the code cleaner and
easier to refactor. I'll make that change. Please let me know if you disagree!

[peah-webrtc, 2015/11/24 13:03:01]

Done.

+                float freq_data[2][PART_LEN1]) {
   aec_rdft_forward_128(time_data);
-  // Reorder.
+  // Reorder fft output data.
   freq_data[1][0] = 0;
   freq_data[1][PART_LEN] = 0;
   freq_data[0][0] = time_data[0];
   freq_data[0][PART_LEN] = time_data[1];
-  for (i = 1; i < PART_LEN; i++) {
+  for (int i = 1; i < PART_LEN; i++) {
     freq_data[0][i] = time_data[2 * i];
     freq_data[1][i] = time_data[2 * i + 1];
   }
 }
 
 static int MoveFarReadPtrWithoutSystemDelayUpdate(AecCore* self, int elements) {
   WebRtc_MoveReadPtr(self->far_buf_windowed, elements);
 #ifdef WEBRTC_AEC_DEBUG_DUMP
   WebRtc_MoveReadPtr(self->far_time_buf, elements);
 #endif
@@ skipping 66 matching lines @@
     delay_quality = (delay_quality > kDelayQualityThresholdMax ?
         kDelayQualityThresholdMax : delay_quality);
     self->delay_quality_threshold =
         (delay_quality > self->delay_quality_threshold ? delay_quality :
             self->delay_quality_threshold);
   }
   return delay_correction;
 }
 
 static void EchoSubtraction(AecCore* aec,
-                            float* nearend_ptr) {
-  float yf[2][PART_LEN1];
-  float fft[PART_LEN2];
-  float y[PART_LEN];
+                            int num_partitions,
+                            int xfBufBlockPos,

[hlundin-webrtc, 2015/11/20 11:55:20]

Change naming format of input parameters.

[peah-webrtc, 2015/11/24 13:03:01]

Done.

+                            int metricsMode,
+                            int extended_filter_enabled,
+                            float normal_mu,
+                            float normal_error_threshold,
+                            float xfBuf[2][kExtendedNumPartitions * PART_LEN1],
+                            const float* const y,
+                            float xPow[PART_LEN1],

[hlundin-webrtc, 2015/11/20 11:55:20]

Can you make any array parameters const?

[peah-webrtc, 2015/11/24 13:03:01]

Yes, and I'll as a consequense also add const to the the ScaleErrorSignal
parameters that I refrained to do in the earlier CL out of being overcautious. 

On top of that, I needed to properly revise the const-ness of the FilterFar
function. Please have a look at that as well!

Pls let me know if it is not ok!

+                            float wfBuf[2][kExtendedNumPartitions * PART_LEN1],
+                            PowerLevel* linoutlevel,
+                            float echo_subtractor_output[PART_LEN]) {
+  float s_fft[2][PART_LEN1];
+  float e_extended[PART_LEN2];
+  float s_extended[PART_LEN2];
+  float *s;
   float e[PART_LEN];
-  float ef[2][PART_LEN1];
+  float e_fft[2][PART_LEN1];
   float scale;
   int i;
-  memset(yf, 0, sizeof(yf));
+  memset(s_fft, 0, sizeof(s_fft));
 
-  // Produce echo estimate.
-  WebRtcAec_FilterFar(aec->num_partitions,
-                      aec->xfBufBlockPos,
-                      aec->xfBuf,
-                      aec->wfBuf,
-                      yf);
+  // Produce echo estimate s_fft.
+  WebRtcAec_FilterFar(num_partitions,
+                      xfBufBlockPos,
+                      xfBuf,
+                      wfBuf,
+                      s_fft);
 
-  // Inverse fft to obtain echo estimate and error.
-  FrequencyToTime(yf, fft);
-
-  // Extract the output signal and compute the time-domain error
+  // Compute the time-domain echo estimate s.
+  InverseFft(s_fft, s_extended);
   scale = 2.0f / PART_LEN2;
-  for (i = 0; i < PART_LEN; i++) {
-    y[i] = fft[PART_LEN + i] * scale;  // fft scaling
+  s = &s_extended[PART_LEN];
+  for (i = 0; i < PART_LEN; ++i) {
+    s[i] *= scale;

[ivoc, 2015/11/20 12:58:02]

Should scaling be part of the InverseFft function? In my opinion it makes sense.

[peah-webrtc, 2015/11/24 13:03:01]

I think the division by PART_LEN could go there, but not the scaling of 2.0.
Looking at it from an Ifft/fft perspective I think it makes sense, and even
though it is more complex I'll split the scaling in two. Please let me know if
it does not make sense!

Done.

   }
 
-  for (i = 0; i < PART_LEN; i++) {
-    e[i] = nearend_ptr[i] - y[i];
+  // Compute the time-domain echo prediction error.
+  for (i = 0; i < PART_LEN; ++i) {
+    e[i] = y[i] - s[i];

[ivoc, 2015/11/20 12:58:02]

Loop can be merged with previous loop.

[peah-webrtc, 2015/11/24 13:03:01]

Done in a previous CL.
Done.

   }
 
+  // Compute the frequency domain echo prediction error.
+  memset(e_extended, 0, sizeof(float) * PART_LEN);
+  memcpy(e_extended + PART_LEN, e, sizeof(float) * PART_LEN);

[ivoc, 2015/11/20 12:58:02]

I think it makes more sense to skip the e signal, and fill e_extended directly
in the previous loop. Especially since e does not seem to be used anywhere else.

[peah-webrtc, 2015/11/24 13:03:01]

I agree, but it will be used in an upcoming CL, so if ok I prefer to keep it as
it is for now.

+  Fft(e_extended, e_fft);
 
-  // Error fft
-  memcpy(aec->eBuf + PART_LEN, e, sizeof(float) * PART_LEN);
-  memset(fft, 0, sizeof(float) * PART_LEN);
-  memcpy(fft + PART_LEN, e, sizeof(float) * PART_LEN);
-  TimeToFrequency(fft, ef, 0);
 
   RTC_AEC_DEBUG_RAW_WRITE(aec->e_fft_file,
-                          &ef[0][0],
-                          sizeof(ef[0][0]) * PART_LEN1 * 2);
+                          &e_fft[0][0],
+                          sizeof(e_fft[0][0]) * PART_LEN1 * 2);
 
-  if (aec->metricsMode == 1) {
+  if (metricsMode == 1) {
     // Note that the first PART_LEN samples in fft (before transformation) are
     // zero. Hence, the scaling by two in UpdateLevel() should not be
     // performed. That scaling is taken care of in UpdateMetrics() instead.
-    UpdateLevel(&aec->linoutlevel, ef);
+    UpdateLevel(linoutlevel, e_fft);
   }
 
   // Scale error signal inversely with far power.
-  WebRtcAec_ScaleErrorSignal(aec->extended_filter_enabled,
-                             aec->normal_mu,
-                             aec->normal_error_threshold,
-                             aec->xPow,
-                             ef);
-  WebRtcAec_FilterAdaptation(aec, fft, ef);
-
-
-  RTC_AEC_DEBUG_WAV_WRITE(aec->outLinearFile, e, PART_LEN);
+  WebRtcAec_ScaleErrorSignal(extended_filter_enabled,
+                             normal_mu,
+                             normal_error_threshold,
+                             xPow,
+                             e_fft);
+  WebRtcAec_FilterAdaptation(num_partitions,
+                             xfBufBlockPos,
+                             xfBuf,
+                             e_fft,
+                             wfBuf);
+  memcpy(echo_subtractor_output, e, sizeof(float) * PART_LEN);
 }
 
 
 static void EchoSuppression(AecCore* aec,
                             float* output,
                             float* const* outputH) {
   float efw[2][PART_LEN1], xfw[2][PART_LEN1];
   complex_t comfortNoiseHband[PART_LEN1];
   float fft[PART_LEN2];
   float scale, dtmp;
@@ skipping 236 matching lines @@
 
   const float gPow[2] = {0.9f, 0.1f};
 
   // Noise estimate constants.
   const int noiseInitBlocks = 500 * aec->mult;
   const float step = 0.1f;
   const float ramp = 1.0002f;
   const float gInitNoise[2] = {0.999f, 0.001f};
 
   float nearend[PART_LEN];
+  float echo_subtractor_output[PART_LEN];
   float* nearend_ptr = NULL;
   float output[PART_LEN];
   float outputH[NUM_HIGH_BANDS_MAX][PART_LEN];
   float* outputH_ptr[NUM_HIGH_BANDS_MAX];
   float* xf_ptr = NULL;
 
   for (i = 0; i < NUM_HIGH_BANDS_MAX; ++i) {
     outputH_ptr[i] = outputH[i];
   }
 
@@ skipping 19 matching lines @@
     RTC_AEC_DEBUG_WAV_WRITE(aec->nearFile, nearend_ptr, PART_LEN);
   }
 #endif
 
   // We should always have at least one element stored in |far_buf|.
   assert(WebRtc_available_read(aec->far_buf) > 0);
   WebRtc_ReadBuffer(aec->far_buf, (void**)&xf_ptr, &xf[0][0], 1);
 
   // Near fft
   memcpy(fft, aec->dBuf, sizeof(float) * PART_LEN2);
-  TimeToFrequency(fft, df, 0);
+  Fft(fft, df);
 
   // Power smoothing
   for (i = 0; i < PART_LEN1; i++) {
     far_spectrum = (xf_ptr[i] * xf_ptr[i]) +
                    (xf_ptr[PART_LEN1 + i] * xf_ptr[PART_LEN1 + i]);
     aec->xPow[i] =
         gPow[0] * aec->xPow[i] + gPow[1] * aec->num_partitions * far_spectrum;
     // Calculate absolute spectra
     abs_far_spectrum[i] = sqrtf(far_spectrum);
 
@@ skipping 58 matching lines @@
 
   // Buffer xf
   memcpy(aec->xfBuf[0] + aec->xfBufBlockPos * PART_LEN1,
          xf_ptr,
          sizeof(float) * PART_LEN1);
   memcpy(aec->xfBuf[1] + aec->xfBufBlockPos * PART_LEN1,
          &xf_ptr[PART_LEN1],
          sizeof(float) * PART_LEN1);
 
   // Perform echo subtraction.
-  EchoSubtraction(aec, nearend_ptr);
+  EchoSubtraction(aec,
+                  aec->num_partitions,
+                  aec->xfBufBlockPos,
+                  aec->metricsMode,
+                  aec->extended_filter_enabled,
+                  aec->normal_mu,
+                  aec->normal_error_threshold,
+                  aec->xfBuf,
+                  nearend_ptr,
+                  aec->xPow,
+                  aec->wfBuf,
+                  &aec->linoutlevel,
+                  echo_subtractor_output);
+
+  RTC_AEC_DEBUG_WAV_WRITE(aec->outLinearFile, echo_subtractor_output, PART_LEN);
 
   // Perform echo suppression.
+  memcpy(aec->eBuf + PART_LEN,
+         echo_subtractor_output,
+         sizeof(float) * PART_LEN);
   EchoSuppression(aec, output, outputH_ptr);
 
   if (aec->metricsMode == 1) {
     // Update power levels and echo metrics
     UpdateLevel(&aec->farlevel, (float(*)[PART_LEN1])xf_ptr);
     UpdateLevel(&aec->nearlevel, df);
     UpdateMetrics(aec);
   }
 
   // Store the output block.
@@ skipping 322 matching lines @@
 void WebRtcAec_BufferFarendPartition(AecCore* aec, const float* farend) {
   float fft[PART_LEN2];
   float xf[2][PART_LEN1];
 
   // Check if the buffer is full, and in that case flush the oldest data.
   if (WebRtc_available_write(aec->far_buf) < 1) {
     WebRtcAec_MoveFarReadPtr(aec, 1);
   }
   // Convert far-end partition to the frequency domain without windowing.
   memcpy(fft, farend, sizeof(float) * PART_LEN2);
-  TimeToFrequency(fft, xf, 0);
+  Fft(fft, xf);
   WebRtc_WriteBuffer(aec->far_buf, &xf[0][0], 1);
 
   // Convert far-end partition to the frequency domain with windowing.
-  memcpy(fft, farend, sizeof(float) * PART_LEN2);
-  TimeToFrequency(fft, xf, 1);
+  WindowData(fft, farend);
+  Fft(fft, xf);
   WebRtc_WriteBuffer(aec->far_buf_windowed, &xf[0][0], 1);
 }
 
 int WebRtcAec_MoveFarReadPtr(AecCore* aec, int elements) {
   int elements_moved = MoveFarReadPtrWithoutSystemDelayUpdate(aec, elements);
   aec->system_delay -= elements_moved * PART_LEN;
   return elements_moved;
 }
 
 void WebRtcAec_ProcessFrames(AecCore* aec,
@@ skipping 191 matching lines @@
 int WebRtcAec_extended_filter_enabled(AecCore* self) {
   return self->extended_filter_enabled;
 }
 
 int WebRtcAec_system_delay(AecCore* self) { return self->system_delay; }
 
 void WebRtcAec_SetSystemDelay(AecCore* self, int delay) {
   assert(delay >= 0);
   self->system_delay = delay;
 }