Cleaning up AEC metrics.

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 10 matching lines @@
 #include <algorithm>
 #include <assert.h>
 #include <math.h>
 #include <stddef.h>  // size_t
 #include <stdlib.h>
 #include <string.h>
 
 extern "C" {
 #include "webrtc/common_audio/ring_buffer.h"
 }
+#include "webrtc/base/checks.h"
 #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
 #include "webrtc/modules/audio_processing/aec/aec_common.h"
 #include "webrtc/modules/audio_processing/aec/aec_core_internal.h"
 #include "webrtc/modules/audio_processing/aec/aec_rdft.h"
 #include "webrtc/modules/audio_processing/logging/aec_logging.h"
 #include "webrtc/modules/audio_processing/utility/delay_estimator_wrapper.h"
 #include "webrtc/system_wrappers/include/cpu_features_wrapper.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
@@ skipping 317 matching lines @@
     }
 
     if (wfEn > wfEnMax) {
       wfEnMax = wfEn;
       delay = i;
     }
   }
   return delay;
 }
 
+// Update metric with 10 * log10(numerator / denominator).
+static void UpdateLogRatioMetric(Stats* metric, float numerator,
+                                 float denominator) {
+  RTC_DCHECK(metric);
+  RTC_CHECK(numerator >= 0);

[tlegrand-webrtc, 2016/04/25 13:09:56]

Why isn't numerator =  0 allowed?

[kwiberg-webrtc, 2016/04/25 13:23:58]

It is. But what I'm wondering is why we allow denominator == 0...

(Also, for the future, please use RTC_DCHECK_GE, _GT, etc.---they print better
error messages.)

[minyue-webrtc, 2016/04/25 13:26:56]

It is allowed, isn't it?

[tlegrand-webrtc, 2016/04/28 08:17:30]

Right, my mistake. I was thinking 0 wouldn't be allowed for the denominator and
confused myself. But I realize now that 0 should be (and is) allowed for both.

+  RTC_CHECK(denominator >= 0);
+
+  const float log_numerator = log10(numerator + 1e-10f);
+  const float log_denominator = log10(denominator + 1e-10f);

[tlegrand-webrtc, 2016/04/25 13:09:56]

Which is more computational complex, log10 or division? I'm thinking log10 is
more expensive, but I'm not sure. If it is, we are wasting cycles here, just to
get aroung the division with 0, right?

[kwiberg-webrtc, 2016/04/25 13:23:58]

Yes, I'd be surprised if division wasn't significantly faster than log10. See
e.g.
http://stackoverflow.com/questions/2858483/how-can-i-compare-the-performance-...

[minyue-webrtc, 2016/04/25 13:26:56]

Per also asked about this. My reasoning was that it is not only to avoid
dividing by zero, but also to avoid overflow. Say if a/b results in infinity (b
does not need to be zero), it does not mean that log(a/b) is infinity. So log a
- log b has a better representability.

[tlegrand-webrtc, 2016/04/28 08:17:30]

Acknowledged.

+  metric->instant = 10.0f * (log_numerator - log_denominator);
+
+  // Max.

[tlegrand-webrtc, 2016/04/25 13:09:56]

This comment doesn't add any value. Same with "Min." below.

[minyue-webrtc, 2016/04/25 13:26:56]

I agree. I'd like to make a separate CL, which may refactor hicounter etc. as
well.

[tlegrand-webrtc, 2016/04/28 08:17:30]

Acknowledged.

+  if (metric->instant > metric->max)
+    metric->max = metric->instant;
+
+  // Min.
+  if (metric->instant < metric->min)
+    metric->min = metric->instant;
+
+  // Average.
+  metric->counter++;
+  // This is to protect overflow, which should almost never happen.
+  RTC_CHECK_NE(0u, metric->counter);
+  metric->sum += metric->instant;
+  metric->average = metric->sum / metric->counter;
+
+  // Upper mean.

[tlegrand-webrtc, 2016/04/25 13:09:56]

hicounter, hisum and himean doesn't follow the naming convention. Should be on
the format hi_counter, but I'd like to see something more descriptive than "hi".

[minyue-webrtc, 2016/04/25 13:26:56]

True, this will need to refactor other part of AEC. We may better do it in a
separate CL.

[tlegrand-webrtc, 2016/04/28 08:17:30]

Acknowledged.

+  if (metric->instant > metric->average) {
+    metric->hicounter++;
+    // This is to protect overflow, which should almost never happen.
+    RTC_CHECK_NE(0u, metric->hicounter);
+    metric->hisum += metric->instant;
+    metric->himean = metric->hisum / metric->hicounter;
+  }
+}
+
 // Threshold to protect against the ill-effects of a zero far-end.
 const float WebRtcAec_kMinFarendPSD = 15;
 
 // Updates the following smoothed  Power Spectral Densities (PSD):
 //  - sd  : near-end
 //  - se  : residual echo
 //  - sx  : far-end
 //  - sde : cross-PSD of near-end and residual echo
 //  - sxd : cross-PSD of near-end and far-end
 //
@@ skipping 253 matching lines @@
         level->minlevel = new_frame_level;  // New minimum.
       } else {
         level->minlevel *= (1 + 0.001f);  // Small increase.
       }
     }
     level->averagelevel.AddValue(new_frame_level);
   }
 }
 
 static void UpdateMetrics(AecCore* aec) {
-  float dtmp;
-
   const float actThresholdNoisy = 8.0f;
   const float actThresholdClean = 40.0f;
-  const float safety = 0.99995f;
 
   const float noisyPower = 300000.0f;
 
   float actThreshold;
-  float echo, suppressedEcho;
 
   if (aec->echoState) {  // Check if echo is likely present
     aec->stateCounter++;
   }
 
   if (aec->linoutlevel.framelevel.EndOfBlock()) {
     aec->divergent_filter_fraction.AddObservation(aec->nearlevel,
                                                   aec->linoutlevel,
                                                   aec->nlpoutlevel);
   }
 
   if (aec->farlevel.averagelevel.EndOfBlock()) {
     if (aec->farlevel.minlevel < noisyPower) {
       actThreshold = actThresholdClean;
     } else {
       actThreshold = actThresholdNoisy;
     }
 
     const float far_average_level = aec->farlevel.averagelevel.GetLatestMean();
 
     // The last condition is to let estimation be made in active far-end
     // segments only.
     if ((aec->stateCounter > (0.5f * kCountLen * kSubCountLen)) &&
         (aec->farlevel.framelevel.EndOfBlock()) &&
         (far_average_level > (actThreshold * aec->farlevel.minlevel))) {
 
+      // ERL: error return loss.
       const float near_average_level =
           aec->nearlevel.averagelevel.GetLatestMean();
+      UpdateLogRatioMetric(&aec->erl, far_average_level, near_average_level);
 
-      // Subtract noise power
-      echo = near_average_level - safety * aec->nearlevel.minlevel;
-
-      // ERL
-      dtmp = 10 * static_cast<float>(log10(far_average_level /
-                                           near_average_level + 1e-10f));
-
-      aec->erl.instant = dtmp;
-      if (dtmp > aec->erl.max) {
-        aec->erl.max = dtmp;
-      }
-
-      if (dtmp < aec->erl.min) {
-        aec->erl.min = dtmp;
-      }
-
-      aec->erl.counter++;
-      aec->erl.sum += dtmp;
-      aec->erl.average = aec->erl.sum / aec->erl.counter;
-
-      // Upper mean
-      if (dtmp > aec->erl.average) {
-        aec->erl.hicounter++;
-        aec->erl.hisum += dtmp;
-        aec->erl.himean = aec->erl.hisum / aec->erl.hicounter;
-      }
-
-      // A_NLP
+      // A_NLP: error return loss enhanced before the nonlinear suppression.
       const float linout_average_level =
           aec->linoutlevel.averagelevel.GetLatestMean();
-      dtmp = 10 * static_cast<float>(log10(near_average_level /
-                                           linout_average_level + 1e-10f));
+      UpdateLogRatioMetric(&aec->aNlp, near_average_level,
+                           linout_average_level);
 
-      // subtract noise power
-      suppressedEcho =
-          linout_average_level - safety * aec->linoutlevel.minlevel;
-
-      aec->aNlp.instant =
-          10 * static_cast<float>(log10(echo / suppressedEcho + 1e-10f));
-
-      if (dtmp > aec->aNlp.max) {
-        aec->aNlp.max = dtmp;
-      }
-
-      if (dtmp < aec->aNlp.min) {
-        aec->aNlp.min = dtmp;
-      }
-
-      aec->aNlp.counter++;
-      aec->aNlp.sum += dtmp;
-      aec->aNlp.average = aec->aNlp.sum / aec->aNlp.counter;
-
-      // Upper mean
-      if (dtmp > aec->aNlp.average) {
-        aec->aNlp.hicounter++;
-        aec->aNlp.hisum += dtmp;
-        aec->aNlp.himean = aec->aNlp.hisum / aec->aNlp.hicounter;
-      }
-
-      // ERLE
+      // ERLE: error return loss enhanced.
       const float nlpout_average_level =
           aec->nlpoutlevel.averagelevel.GetLatestMean();
-      // subtract noise power
-      suppressedEcho =
-          nlpout_average_level - safety * aec->nlpoutlevel.minlevel;
-      dtmp = 10 * static_cast<float>(log10(echo / suppressedEcho + 1e-10f));
-
-      aec->erle.instant = dtmp;
-      if (dtmp > aec->erle.max) {
-        aec->erle.max = dtmp;
-      }
-
-      if (dtmp < aec->erle.min) {
-        aec->erle.min = dtmp;
-      }
-
-      aec->erle.counter++;
-      aec->erle.sum += dtmp;
-      aec->erle.average = aec->erle.sum / aec->erle.counter;
-
-      // Upper mean
-      if (dtmp > aec->erle.average) {
-        aec->erle.hicounter++;
-        aec->erle.hisum += dtmp;
-        aec->erle.himean = aec->erle.hisum / aec->erle.hicounter;
-      }
+      UpdateLogRatioMetric(&aec->erle, near_average_level,
+                           nlpout_average_level);
     }
 
     aec->stateCounter = 0;
   }
 }
 
 static void UpdateDelayMetrics(AecCore* self) {
   int i = 0;
   int delay_values = 0;
   int median = 0;
@@ skipping 1168 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