Transparency increasing tuning for AEC3

webrtc/modules/audio_processing/aec3/erle_estimator.cc

 /*
  *  Copyright (c) 2017 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.
  */
 
 #include "webrtc/modules/audio_processing/aec3/erle_estimator.h"
 
 #include <algorithm>
 
 namespace webrtc {
 
 namespace {
 
 constexpr float kMinErle = 1.f;
-constexpr float kMaxErle = 8.f;
+constexpr float kMaxLfErle = 8.f;
+constexpr float kMaxHfErle = 1.5f;
 
 }  // namespace
 
 ErleEstimator::ErleEstimator() {
   erle_.fill(kMinErle);
   hold_counters_.fill(0);
 }
 
 ErleEstimator::~ErleEstimator() = default;
 
 void ErleEstimator::Update(
     const std::array<float, kFftLengthBy2Plus1>& render_spectrum,
     const std::array<float, kFftLengthBy2Plus1>& capture_spectrum,
     const std::array<float, kFftLengthBy2Plus1>& subtractor_spectrum) {
   const auto& X2 = render_spectrum;
   const auto& Y2 = capture_spectrum;
   const auto& E2 = subtractor_spectrum;
 
   // Corresponds of WGN of power -46 dBFS.
   constexpr float kX2Min = 44015068.0f;
 
   // Update the estimates in a clamped minimum statistics manner.
-  for (size_t k = 1; k < kFftLengthBy2; ++k) {
-    if (X2[k] > kX2Min && E2[k] > 0.f) {
-      const float new_erle = Y2[k] / E2[k];
-      if (new_erle > erle_[k]) {
-        hold_counters_[k - 1] = 100;
-        erle_[k] += 0.1f * (new_erle - erle_[k]);
-        erle_[k] = std::max(kMinErle, std::min(erle_[k], kMaxErle));
+  size_t k = 1;
+  size_t band_limit = 32;

[ivoc, 2017/05/22 14:57:59]

I think it would be nice to initialize this to to kFftLengthBy2 / 2.

[peah-webrtc, 2017/05/22 21:52:46]

Good point! I did not think of them being the same.

Done.

+  float max_erle = kMaxLfErle;
+  for (int j = 0; j < 2; ++j) {
+    for (; k < band_limit; ++k) {
+      if (X2[k] > kX2Min && E2[k] > 0.f) {
+        const float new_erle = Y2[k] / E2[k];
+        if (new_erle > erle_[k]) {
+          hold_counters_[k - 1] = 100;
+          erle_[k] += 0.1f * (new_erle - erle_[k]);
+          erle_[k] = std::max(kMinErle, std::min(erle_[k], max_erle));

[aleloi, 2017/05/22 20:41:45]

I think the code gets simpler to follow without the changing band_limit/max_erle
at the end. How about keeping the old structure, but changing 

kMaxErle --> (k < KfftLenghtBy2 / 2 ? kMaxLfErle : kMaxHfErle)

Or perhaps 

for (k = 1 .. kfftby2) {
  const bool is_low_frequency = < KfftLenghtBy2 / 2;
  // ...
  ... = max(..., min(..., is_low_frequency ? kMaxLfErle : kMaxHfErle));
}

[peah-webrtc, 2017/05/22 21:52:46]

I agree that that would produce nicer code, but I don't think it makes sense to
put in an if-statement of known outcome in a loop. But I think I found a way to
rewrite the code so that it looks better using a lambda. I will, however, do
that in a separate CL.

+        }
       }
     }
+    band_limit = kFftLengthBy2;
+    max_erle = kMaxHfErle;
   }
 
   std::for_each(hold_counters_.begin(), hold_counters_.end(),
                 [](int& a) { --a; });
   std::transform(hold_counters_.begin(), hold_counters_.end(),
                  erle_.begin() + 1, erle_.begin() + 1, [](int a, float b) {
                    return a > 0 ? b : std::max(kMinErle, 0.97f * b);
                  });
 
   erle_[0] = erle_[1];
   erle_[kFftLengthBy2] = erle_[kFftLengthBy2 - 1];
 }
 
 }  // namespace webrtc