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

webrtc/modules/audio_processing/aec/aec_core.c

Index: webrtc/modules/audio_processing/aec/aec_core.c
diff --git a/webrtc/modules/audio_processing/aec/aec_core.c b/webrtc/modules/audio_processing/aec/aec_core.c
index e6a353921cfd429f1282490d1de576e2139ac62c..4d552154e648e509e6678c739ffde46b4d335981 100644
--- a/webrtc/modules/audio_processing/aec/aec_core.c
+++ b/webrtc/modules/audio_processing/aec/aec_core.c
@@ -182,7 +182,7 @@ static void FilterFar(int num_partitions,
 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
@@ -207,57 +207,38 @@ static void ScaleErrorSignal(int extended_filter_enabled,
   }
 }
 
-// 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]);
 
@@ -273,12 +254,12 @@ static void FilterAdaptation(AecCore* aec, float* fft, float ef[2][PART_LEN1]) {
     }
     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];
     }
   }
 }
@@ -845,8 +826,8 @@ static void UpdateDelayMetrics(AecCore* self) {
   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++) {
@@ -857,26 +838,15 @@ static void FrequencyToTime(float freq_data[2][PART_LEN1],
 }
 
 
-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];
   }
@@ -963,64 +933,77 @@ static int SignalBasedDelayCorrection(AecCore* self) {
 }
 
 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));
-
-  // Produce echo estimate.
-  WebRtcAec_FilterFar(aec->num_partitions,
-                      aec->xfBufBlockPos,
-                      aec->xfBuf,
-                      aec->wfBuf,
-                      yf);
+  memset(s_fft, 0, sizeof(s_fft));
 
-  // Inverse fft to obtain echo estimate and error.
-  FrequencyToTime(yf, fft);
+  // Produce echo estimate s_fft.
+  WebRtcAec_FilterFar(num_partitions,
+                      xfBufBlockPos,
+                      xfBuf,
+                      wfBuf,
+                      s_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);
 }
 
 
@@ -1277,6 +1260,7 @@ static void ProcessBlock(AecCore* aec) {
   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];
@@ -1316,7 +1300,7 @@ static void ProcessBlock(AecCore* aec) {
 
   // 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++) {
@@ -1395,9 +1379,26 @@ static void ProcessBlock(AecCore* aec) {
          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) {
@@ -1740,12 +1741,12 @@ void WebRtcAec_BufferFarendPartition(AecCore* aec, const float* farend) {
   }
   // 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);
 }