Adding fraction of filter divergence in AEC metrics.

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 1b5f03fec4e7a255f25c681218bbb7470ebc0770..436a1b2458a3a5d574898e593662284ba922a5df 100644
--- a/webrtc/modules/audio_processing/aec/aec_core.cc
+++ b/webrtc/modules/audio_processing/aec/aec_core.cc
@@ -50,6 +50,12 @@ static const int subCountLen = 4;
 static const int countLen = 50;
 static const int kDelayMetricsAggregationWindow = 1250;  // 5 seconds at 16 kHz.
 
+// Diverge metric is based on audio level, which gets updated which every
+// |subCountLen + 1| * 10 milliseconds. Diverge metric takes the statistics of
+// |kDivergeMetricAggregationWindow| samples. Current value corresponds to  0.5
+// seconds at 16 kHz.
+static const int kDivergeMetricAggregationWindow = 25;
+
 // Quantities to control H band scaling for SWB input
 static const float cnScaleHband = 0.4f;  // scale for comfort noise in H band.
 // Initial bin for averaging nlp gain in low band
@@ -556,6 +562,8 @@ static void InitMetrics(AecCore* self) {
   InitStats(&self->erle);
   InitStats(&self->aNlp);
   InitStats(&self->rerl);
+
+  self->fraction_filter_divergent->Flush();
 }
 
 static float CalculatePower(const float* in, size_t num_samples) {
@@ -613,6 +621,16 @@ static void UpdateMetrics(AecCore* aec) {
     aec->stateCounter++;
   }
 
+  if (aec->linoutlevel.sfrcounter == 0) {
+    const float level_increase =

[peah-webrtc, 2016/03/14 09:12:53]

What about renaming level_increase to level_difference?

[minyue-webrtc, 2016/03/14 14:51:41]

I thought about it, "increase" gives a direction: we don't want level to
increase, and therefore there should be a "<" logic on it later.

[peah-webrtc, 2016/03/15 09:18:40]

Ok.
(I think it sounds weird with a negative increase, but I think your point is
definitely valid).

+        aec->linoutlevel.framelevel - aec->nearlevel.framelevel;
+    // Level increase should be, in principle, negative, when the filter
+    // does not diverge. Here we allow some margin (0.001 * near end level) and
+    // numerical error (1.0).
+    aec->fraction_filter_divergent->AddSample(level_increase >

[peah-webrtc, 2016/03/14 09:12:53]

Is a sliding window average measure really needed for this? It is quite
expensive compared to the alternatives, both in terms of computations, amount of
code and amount of memory.

Have you considered using a counter that counts occurrences when this happens?
Since you are logging binary 1 or 0 that should be an option.

What about a sliding average? Have you looked into that?

[minyue-webrtc, 2016/03/14 14:51:41]

I wrote a new MeanCalculator, with the hope that it can be used for other mean
calculations, e.g., the near/far/linout/nlpout audio levels. I think, yes, it
increases memory usage, but benefits are that 1) the update frequency (time when
we want to obtain the mean) is determined by the averaging window length, 2) the
mean can become smoothed.

Let me know if these benefits are worthy for introducing MeanCalculator.

Computationally, it is really cheap: sum = sum - sample[t - window] + sample[t]

[peah-webrtc, 2016/03/15 09:18:40]

If you foresee future uses of the class I think you should add the class only
when you introduce that use of it.

As it is now, I think the class is not really fitting what is done as it is much
too general. Regardless of the class being already available, I think the way it
is used in this place motivates a different implementation of it (one option
could be to use a template class that allow computing the means for a certain
type but at this stage, with only one place using it, I think adding such a
class is too early).

[peah-webrtc, 2016/03/15 09:18:40]

What do you mean by that the update frequency is determined by the window
length? I don't really follow that.

What do you mean by the smoothing of the mean? If smoothing is needed for the
mean is needed, I think it makes sense to add that in the scope of this CL as
such a smoothing would alter the MeanCalculator class.

[peah-webrtc, 2016/03/15 09:18:40]

I think that one thing that is missing is that the metric is currently not  used
anywhere, so it is really hard to see that this kind of update frequency/order
of accuracy is needed. Could you please add the reporting of the metric as well
in this CL?

[minyue-webrtc, 2016/04/03 21:42:05]

Better to add in a follow up CL IMO (and it is drafted already)

[minyue-webrtc, 2016/04/03 21:42:05]

Ok. I'd remove MeanCalculator

[minyue-webrtc, 2016/04/03 21:42:05]

as we offline discussed

+        std::max(0.001 * aec->nearlevel.framelevel, 1.0) ? 1.0 : 0.0);
+  }
+
   if (aec->farlevel.frcounter == 0) {
     if (aec->farlevel.minlevel < noisyPower) {
       actThreshold = actThresholdClean;
@@ -1365,7 +1383,7 @@ static void ProcessBlock(AecCore* aec) {
 
 AecCore* WebRtcAec_CreateAec() {
   int i;
-  AecCore* aec = reinterpret_cast<AecCore*>(malloc(sizeof(AecCore)));
+  AecCore* aec = new AecCore;
   if (!aec) {
     return NULL;
   }
@@ -1429,6 +1447,14 @@ AecCore* WebRtcAec_CreateAec() {
     WebRtcAec_FreeAec(aec);
     return NULL;
   }
+
+  aec->fraction_filter_divergent.reset(

[peah-webrtc, 2016/03/14 09:12:53]

Does this need to be dynamically allocated? Since it is initialized using a
constant, it should be possible to not have it dynamically allocated.

[minyue-webrtc, 2016/03/14 14:51:41]

Problem is |aec| does not have a constructor (unless we want to add one in this
CL)

[peah-webrtc, 2016/03/15 09:18:40]

Acknowledged.

+      new webrtc::MeanCalculator(kDivergeMetricAggregationWindow));
+  if (aec->fraction_filter_divergent.get() == nullptr) {
+    WebRtcAec_FreeAec(aec);
+    return NULL;
+  }
+
 #ifdef WEBRTC_ANDROID
   aec->delay_agnostic_enabled = 1;  // DA-AEC enabled by default.
   // DA-AEC assumes the system is causal from the beginning and will self adjust