Moved the AEC echo suppression gain computation code to a separate method

webrtc/modules/audio_processing/aec/aec_core.cc

Index: webrtc/modules/audio_processing/aec/aec_core.cc
diff --git a/webrtc/modules/audio_processing/aec/aec_core.cc b/webrtc/modules/audio_processing/aec/aec_core.cc
index 97b5c94ccddf1db05377fe21d907f1fe68f97c2c..9aa9b521b0744601a24ee3d1bb3b31ca00655cef 100644
--- a/webrtc/modules/audio_processing/aec/aec_core.cc
+++ b/webrtc/modules/audio_processing/aec/aec_core.cc
@@ -985,96 +985,30 @@ static void EchoSubtraction(int num_partitions,
   memcpy(echo_subtractor_output, e, sizeof(float) * PART_LEN);
 }
 
-static void EchoSuppression(AecCore* aec,
-                            float farend[PART_LEN2],
-                            float* echo_subtractor_output,
-                            float* output,
-                            float* const* outputH) {
-  float efw[2][PART_LEN1];
-  float xfw[2][PART_LEN1];
-  float dfw[2][PART_LEN1];
-  float comfortNoiseHband[2][PART_LEN1];
-  float fft[PART_LEN2];
-  float nlpGainHband;
-  int i;
-  size_t j;
-
-  // Coherence and non-linear filter
-  float cohde[PART_LEN1], cohxd[PART_LEN1];
+static void FormSuppressionGain(AecCore* aec,
+                                float cohde[PART_LEN1],
+                                float cohxd[PART_LEN1],
+                                float hNl[PART_LEN1]) {
   float hNlDeAvg, hNlXdAvg;
-  float hNl[PART_LEN1];
   float hNlPref[kPrefBandSize];
   float hNlFb = 0, hNlFbLow = 0;
-  const float prefBandQuant = 0.75f, prefBandQuantLow = 0.5f;
   const int prefBandSize = kPrefBandSize / aec->mult;
+  const float prefBandQuant = 0.75f, prefBandQuantLow = 0.5f;
   const int minPrefBand = 4 / aec->mult;
   // Power estimate smoothing coefficients.
   const float* min_overdrive = aec->extended_filter_enabled
                                    ? kExtendedMinOverDrive
                                    : kNormalMinOverDrive;
 
-  // Filter energy
-  const int delayEstInterval = 10 * aec->mult;
-
-  float* xfw_ptr = NULL;
-
-  // Update eBuf with echo subtractor output.
-  memcpy(aec->eBuf + PART_LEN, echo_subtractor_output,
-         sizeof(float) * PART_LEN);
-
-  // Analysis filter banks for the echo suppressor.
-  // Windowed near-end ffts.
-  WindowData(fft, aec->dBuf);
-  aec_rdft_forward_128(fft);
-  StoreAsComplex(fft, dfw);
-
-  // Windowed echo suppressor output ffts.
-  WindowData(fft, aec->eBuf);
-  aec_rdft_forward_128(fft);
-  StoreAsComplex(fft, efw);
-
-  // NLP
-
-  // Convert far-end partition to the frequency domain with windowing.
-  WindowData(fft, farend);
-  Fft(fft, xfw);
-  xfw_ptr = &xfw[0][0];
-
-  // Buffer far.
-  memcpy(aec->xfwBuf, xfw_ptr, sizeof(float) * 2 * PART_LEN1);
-
-  aec->delayEstCtr++;
-  if (aec->delayEstCtr == delayEstInterval) {
-    aec->delayEstCtr = 0;
-    aec->delayIdx = WebRtcAec_PartitionDelay(aec->num_partitions, aec->wfBuf);
-  }
-
-  // Use delayed far.
-  memcpy(xfw, aec->xfwBuf + aec->delayIdx * PART_LEN1,
-         sizeof(xfw[0][0]) * 2 * PART_LEN1);
-
-  WebRtcAec_UpdateCoherenceSpectra(aec->mult, aec->extended_filter_enabled == 1,
-                                   efw, dfw, xfw, &aec->coherence_state,
-                                   &aec->divergeState,
-                                   &aec->extreme_filter_divergence);
-
-  WebRtcAec_ComputeCoherence(&aec->coherence_state, cohde, cohxd);
-
-  // Select the microphone signal as output if the filter is deemed to have
-  // diverged.
-  if (aec->divergeState) {
-    memcpy(efw, dfw, sizeof(efw[0][0]) * 2 * PART_LEN1);
-  }
-
   hNlXdAvg = 0;
-  for (i = minPrefBand; i < prefBandSize + minPrefBand; i++) {
+  for (int i = minPrefBand; i < prefBandSize + minPrefBand; ++i) {
     hNlXdAvg += cohxd[i];
   }
   hNlXdAvg /= prefBandSize;
   hNlXdAvg = 1 - hNlXdAvg;
 
   hNlDeAvg = 0;
-  for (i = minPrefBand; i < prefBandSize + minPrefBand; i++) {
+  for (int i = minPrefBand; i < prefBandSize + minPrefBand; ++i) {
     hNlDeAvg += cohde[i];
   }
   hNlDeAvg /= prefBandSize;
@@ -1094,11 +1028,11 @@ static void EchoSuppression(AecCore* aec,
     aec->overDrive = min_overdrive[aec->nlp_mode];
 
     if (aec->stNearState == 1) {
-      memcpy(hNl, cohde, sizeof(hNl));
+      memcpy(hNl, cohde, sizeof(hNl[0]) * PART_LEN1);

[ivoc, 2016/05/12 10:42:43]

Is this better than sizeof(hNl), or does that not work with the new structure?

[peah-webrtc, 2016/05/12 11:07:50]

No, I would have preferred the previous way to do it, but the compiler
complained so I needed to change the memcpy call to be like this. I think the
difference may be that the compiler only has access to a pointer to the matrix
in this case, while it in the former case had full access to the actual variable
declaration.

[ivoc, 2016/05/12 11:19:39]

I see, that makes sense, thanks.

[kwiberg-webrtc, 2016/05/12 11:29:58]

It'll work if you pass a reference to the array as an argument. Passing just the
array like you do here is equivalent to passing a pointer, for historical
reasons.

The syntax for passing a reference to an array is a wee bit awkward because you
need extra parentheses: T (&array)[N].

Side note: Since this code is C++ now, you can use std::copy instead of memcpy.
Once they are arrays, you ought to be able to write e.g.

  std::copy(std::begin(cohde), std::end(cohde), std::begin(hNl));

[peah-webrtc, 2016/05/13 04:05:47]

Thanks! The ::copy approach is definitely much nicer than the memcpy. Will adopt
that asap :-)

[kwiberg-webrtc, 2016/05/13 06:23:09]

Do note that while it's specialized for trivial types, IIRC it uses memmove
rather than memcpy since it can't assume nonoverlapping source and target
arrays. So in really really performance-critical situations, memcpy is still a
valid choice. In this case the ease-of-use seems like a good trade-off, though.

       hNlFb = hNlDeAvg;
       hNlFbLow = hNlDeAvg;
     } else {
-      for (i = 0; i < PART_LEN1; i++) {
+      for (int i = 0; i < PART_LEN1; ++i) {
         hNl[i] = 1 - cohxd[i];
       }
       hNlFb = hNlXdAvg;
@@ -1107,12 +1041,12 @@ static void EchoSuppression(AecCore* aec,
   } else {
     if (aec->stNearState == 1) {
       aec->echoState = 0;
-      memcpy(hNl, cohde, sizeof(hNl));
+      memcpy(hNl, cohde, sizeof(hNl[0]) * PART_LEN1);
       hNlFb = hNlDeAvg;
       hNlFbLow = hNlDeAvg;
     } else {
       aec->echoState = 1;
-      for (i = 0; i < PART_LEN1; i++) {
+      for (int i = 0; i < PART_LEN1; ++i) {
         hNl[i] = WEBRTC_SPL_MIN(cohde[i], 1 - cohxd[i]);
       }
 
@@ -1160,7 +1094,83 @@ static void EchoSuppression(AecCore* aec,
         0.9f * aec->overdrive_scaling + 0.1f * aec->overDrive;
   }
 
+  // Apply the overdrive.
   WebRtcAec_Overdrive(aec->overdrive_scaling, hNlFb, hNl);
+}
+
+static void EchoSuppression(AecCore* aec,
+                            float farend[PART_LEN2],
+                            float* echo_subtractor_output,
+                            float* output,
+                            float* const* outputH) {
+  float efw[2][PART_LEN1];
+  float xfw[2][PART_LEN1];
+  float dfw[2][PART_LEN1];
+  float comfortNoiseHband[2][PART_LEN1];
+  float fft[PART_LEN2];
+  float nlpGainHband;
+  int i;
+  size_t j;
+
+  // Coherence and non-linear filter
+  float cohde[PART_LEN1], cohxd[PART_LEN1];
+  float hNl[PART_LEN1];
+
+  // Filter energy
+  const int delayEstInterval = 10 * aec->mult;
+
+  float* xfw_ptr = NULL;
+
+  // Update eBuf with echo subtractor output.
+  memcpy(aec->eBuf + PART_LEN, echo_subtractor_output,
+         sizeof(float) * PART_LEN);
+
+  // Analysis filter banks for the echo suppressor.
+  // Windowed near-end ffts.
+  WindowData(fft, aec->dBuf);
+  aec_rdft_forward_128(fft);
+  StoreAsComplex(fft, dfw);
+
+  // Windowed echo suppressor output ffts.
+  WindowData(fft, aec->eBuf);
+  aec_rdft_forward_128(fft);
+  StoreAsComplex(fft, efw);
+
+  // NLP
+
+  // Convert far-end partition to the frequency domain with windowing.
+  WindowData(fft, farend);
+  Fft(fft, xfw);
+  xfw_ptr = &xfw[0][0];
+
+  // Buffer far.
+  memcpy(aec->xfwBuf, xfw_ptr, sizeof(float) * 2 * PART_LEN1);
+
+  aec->delayEstCtr++;
+  if (aec->delayEstCtr == delayEstInterval) {
+    aec->delayEstCtr = 0;
+    aec->delayIdx = WebRtcAec_PartitionDelay(aec->num_partitions, aec->wfBuf);
+  }
+
+  // Use delayed far.
+  memcpy(xfw, aec->xfwBuf + aec->delayIdx * PART_LEN1,
+         sizeof(xfw[0][0]) * 2 * PART_LEN1);
+
+  WebRtcAec_UpdateCoherenceSpectra(aec->mult, aec->extended_filter_enabled == 1,
+                                   efw, dfw, xfw, &aec->coherence_state,
+                                   &aec->divergeState,
+                                   &aec->extreme_filter_divergence);
+
+  WebRtcAec_ComputeCoherence(&aec->coherence_state, cohde, cohxd);
+
+  // Select the microphone signal as output if the filter is deemed to have
+  // diverged.
+  if (aec->divergeState) {
+    memcpy(efw, dfw, sizeof(efw[0][0]) * 2 * PART_LEN1);
+  }
+
+  FormSuppressionGain(aec, cohde, cohxd, hNl);
+
   WebRtcAec_Suppress(hNl, efw);
 
   // Add comfort noise.