Cleaning up AEC metrics.

webrtc/modules/audio_processing/aec/aec_core.c

 /*
  *  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 298 matching lines @@
     }
 
     if (wfEn > wfEnMax) {
       wfEnMax = wfEn;
       delay = i;
     }
   }
   return delay;
 }
 
+static void UpdateMetric(Stats* metric, float value) {
+  // Instant.
+  metric->instant = value;
+
+  // Max.
+  if (metric->instant > metric->max)
+    metric->max = metric->instant;
+
+  // Min.
+  if (metric->instant < metric->min)
+    metric->min = metric->instant;
+
+  // Average.
+  metric->counter++;

[peah-webrtc, 2016/01/15 06:52:20]

This increase should handle wraparounds. I know this is very unlikely to happen,
but regardless of that I think we should still have it.
(for 32 bit ints, this will wraparound after 248 days, but for 16 bit ints it
will wraparound after 5 hours).

[tlegrand-webrtc, 2016/04/13 14:33:31]

To Per: Are there any platforms today where int is only 16-bits?

[peah-webrtc, 2016/04/14 13:46:10]

I would guess there are are no important such platforms. As far as I know, int
is in practice only 16 bit on 16 bit platforms. 

[kwiberg-webrtc, 2016/04/14 14:01:47]

"You should assume that an int is at least 32 bits, but don't assume that it has
more than 32 bits."
https://google.github.io/styleguide/cppguide.html#Integer_Types

+  metric->sum += metric->instant;
+  metric->average = metric->sum / metric->counter;
+
+  // Upper mean.
+  if (metric->instant > metric->average) {
+    metric->hicounter++;

[peah-webrtc, 2016/01/15 06:52:20]

As above, please add handling of wraparound.

+    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 289 matching lines @@
     level->frsum += level->framelevel;
     if (level->frcounter > countLen) {
       level->averagelevel = level->frsum / countLen;
       level->frsum = 0;
       level->frcounter = 0;
     }
   }
 }
 
 static void UpdateMetrics(AecCore* aec) {
-  float dtmp, dtmp2;
-
   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->farlevel.frcounter == 0) {
 
     if (aec->farlevel.minlevel < noisyPower) {
       actThreshold = actThresholdClean;
     } else {
       actThreshold = actThresholdNoisy;
     }
 
     if ((aec->stateCounter > (0.5f * countLen * subCountLen)) &&
         (aec->farlevel.sfrcounter == 0)
 
         // Estimate in active far-end segments only
         &&
         (aec->farlevel.averagelevel >
          (actThreshold * aec->farlevel.minlevel))) {
 
-      // Subtract noise power
-      echo = aec->nearlevel.averagelevel - safety * aec->nearlevel.minlevel;
-
       // ERL
-      dtmp = 10 * (float)log10(aec->farlevel.averagelevel /
-                                   aec->nearlevel.averagelevel +
-                               1e-10f);
-      dtmp2 = 10 * (float)log10(aec->farlevel.averagelevel / echo + 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;
-      }
+      UpdateMetric(&aec->erl, 10 * (float)log10(1e-10f +

[peah-webrtc, 2016/01/15 06:52:20]

While we are anyway changing the metrics: is there any reason for logging the
metric in dB? It is anyway stored as a float, and the log function may be costly
on some platforms. 

Storing the metric as the actual ratio would
1) Remove 3 log10 operations from the code.
2) Allow us to remove the 1e-10f constant which bias the values for levels close
to zero.

If the people monitoring these metrics know how to interpret these dB values, we
probably should keep them in dB (provided that there is no way to do that
conversion in the stats monitoring system). 
Otherwise I think we should instead store the actual ratio.

[minyue-webrtc, 2016/01/15 10:24:49]

Good suggestion, but be minded that to an upper level, we need to report integer
(unless we want to change that also). Then I am not sure if dynamic range of
integer is bit enough.

+          aec->farlevel.averagelevel / aec->nearlevel.averagelevel));

[peah-webrtc, 2016/01/15 06:52:20]

Is there any guarantee that averagelevel cannot be 0?

 
       // A_NLP
-      dtmp = 10 * (float)log10(aec->nearlevel.averagelevel /
-                                   (2 * aec->linoutlevel.averagelevel) +
-                               1e-10f);
-
-      // subtract noise power
-      suppressedEcho = 2 * (aec->linoutlevel.averagelevel -
-                            safety * aec->linoutlevel.minlevel);
-
-      dtmp2 = 10 * (float)log10(echo / suppressedEcho + 1e-10f);
-
-      aec->aNlp.instant = dtmp2;
-      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;
-      }
+      UpdateMetric(&aec->aNlp, 10 * (float)log10(1e-10f +
+          aec->nearlevel.averagelevel / (2 * aec->linoutlevel.averagelevel)));

[peah-webrtc, 2016/01/15 06:52:20]

Is there any guarantee that averagelevel cannot be 0?

[minyue-webrtc, 2016/01/15 10:24:49]

I don't see, and I am not sure how it did not run into any problem before. Good
to add epsilon to the denominator any way, IMO.

 
       // ERLE
-
-      // subtract noise power
-      suppressedEcho = 2 * (aec->nlpoutlevel.averagelevel -
-                            safety * aec->nlpoutlevel.minlevel);
-
-      dtmp = 10 * (float)log10(aec->nearlevel.averagelevel /
-                                   (2 * aec->nlpoutlevel.averagelevel) +
-                               1e-10f);
-      dtmp2 = 10 * (float)log10(echo / suppressedEcho + 1e-10f);
-
-      dtmp = dtmp2;
-      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;
-      }
+      UpdateMetric(&aec->erle, 10 * (float)log10(1e-10f +
+          aec->nearlevel.averagelevel / (2 * aec->nlpoutlevel.averagelevel)));

[peah-webrtc, 2016/01/15 06:52:20]

Is there any guarantee that averagelevel cannot be 0?

     }
 
     aec->stateCounter = 0;
   }
 }
 
 static void UpdateDelayMetrics(AecCore* self) {
   int i = 0;
   int delay_values = 0;
   int median = 0;
@@ skipping 1182 matching lines @@
 int WebRtcAec_extended_filter_enabled(AecCore* self) {
   return self->extended_filter_enabled;
 }
 
 int WebRtcAec_system_delay(AecCore* self) { return self->system_delay; }
 
 void WebRtcAec_SetSystemDelay(AecCore* self, int delay) {
   assert(delay >= 0);
   self->system_delay = delay;
 }