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

webrtc/modules/audio_processing/aec/aec_core.cc

 /*
  *  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 967 matching lines @@
   memcpy(e_extended + PART_LEN, e, sizeof(float) * PART_LEN);
   Fft(e_extended, e_fft);
 
   // Scale error signal inversely with far power.
   WebRtcAec_ScaleErrorSignal(filter_step_size, error_threshold, x_pow, e_fft);
   WebRtcAec_FilterAdaptation(num_partitions, *x_fft_buf_block_pos, x_fft_buf,
                              e_fft, h_fft_buf);
   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 int prefBandSize = kPrefBandSize / aec->mult;
   const float prefBandQuant = 0.75f, prefBandQuantLow = 0.5f;
-  const int prefBandSize = kPrefBandSize / aec->mult;
   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;
 
   if (hNlXdAvg < 0.75f && hNlXdAvg < aec->hNlXdAvgMin) {
     aec->hNlXdAvgMin = hNlXdAvg;
   }
 
   if (hNlDeAvg > 0.98f && hNlXdAvg > 0.9f) {
     aec->stNearState = 1;
   } else if (hNlDeAvg < 0.95f || hNlXdAvg < 0.8f) {
     aec->stNearState = 0;
   }
 
   if (aec->hNlXdAvgMin == 1) {
     aec->echoState = 0;
     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;
       hNlFbLow = hNlXdAvg;
     }
   } 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]);
       }
 
       // Select an order statistic from the preferred bands.
       // TODO(peah): Using quicksort now, but a selection algorithm may be
       // preferred.
       memcpy(hNlPref, &hNl[minPrefBand], sizeof(float) * prefBandSize);
       qsort(hNlPref, prefBandSize, sizeof(float), CmpFloat);
       hNlFb = hNlPref[static_cast<int>(floor(prefBandQuant *
                                              (prefBandSize - 1)))];
@@ skipping 27 matching lines @@
 
   // Smooth the overdrive.
   if (aec->overDrive < aec->overdrive_scaling) {
     aec->overdrive_scaling =
         0.99f * aec->overdrive_scaling + 0.01f * aec->overDrive;
   } else {
     aec->overdrive_scaling =
         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.
   ComfortNoise(aec, efw, comfortNoiseHband, aec->noisePow, hNl);
 
   // Inverse error fft.
   ScaledInverseFft(efw, fft, 2.0f, 1);
 
   // Overlap and add to obtain output.
   for (i = 0; i < PART_LEN; i++) {
@@ skipping 782 matching lines @@
 
 int WebRtcAec_system_delay(AecCore* self) {
   return self->system_delay;
 }
 
 void WebRtcAec_SetSystemDelay(AecCore* self, int delay) {
   assert(delay >= 0);
   self->system_delay = delay;
 }
 }  // namespace webrtc