Adding fraction of filter divergence in 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 32 matching lines @@
 namespace webrtc {
 
 // Buffer size (samples)
 static const size_t kBufSizePartitions = 250;  // 1 second of audio in 16 kHz.
 
 // Metrics
 static const size_t kSubCountLen = 4;
 static const size_t kCountLen = 50;
 static const int kDelayMetricsAggregationWindow = 1250;  // 5 seconds at 16 kHz.
 
+// Divergence metric is based on audio level, which gets updated every
+// |kCountLen + 1| * 10 milliseconds. Divergence 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
 static const int freqAvgIc = PART_LEN / 2;
 
 // Matlab code to produce table:
 // win = sqrt(hanning(63)); win = [0 ; win(1:32)];
 // fprintf(1, '\t%.14f, %.14f, %.14f,\n', win);
 ALIGN16_BEG const float ALIGN16_END WebRtcAec_sqrtHanning[65] = {
     0.00000000000000f, 0.02454122852291f, 0.04906767432742f, 0.07356456359967f,
@@ skipping 81 matching lines @@
 
 __inline static float MulRe(float aRe, float aIm, float bRe, float bIm) {
   return aRe * bRe - aIm * bIm;
 }
 
 __inline static float MulIm(float aRe, float aIm, float bRe, float bIm) {
   return aRe * bIm + aIm * bRe;
 }
 
 PowerLevel::PowerLevel()
-// TODO(minyue): Due to a legacy bug, |framelevel| and |averagelevel| use a
-// window, of which the length is 1 unit longer than indicated. Remove "+1"
-// when the code is refactored.
-: framelevel(kSubCountLen + 1),
-  averagelevel(kCountLen + 1) {
+    // TODO(minyue): Due to a legacy bug, |framelevel| and |averagelevel| use a

[peah-webrtc, 2016/04/05 09:15:52]

I'd prefer to have the comments outside of the constructor definition.

[minyue-webrtc, 2016/04/05 09:50:31]

Done.

+    // window, of which the length is 1 unit longer than indicated. Remove "+1"
+    // when the code is refactored.
+    : framelevel(kSubCountLen + 1),
+      averagelevel(kCountLen + 1) {
 }
 
 // TODO(minyue): Moving some initialization from WebRtcAec_CreateAec() to ctor.
-AecCore::AecCore() = default;
+AecCore::AecCore()
+    : fraction_filter_divergent(kDivergeMetricAggregationWindow) {
+}
 
 static int CmpFloat(const void* a, const void* b) {
   const float* da = (const float*)a;
   const float* db = (const float*)b;
 
   return (*da > *db) - (*da < *db);
 }
 
 static void FilterFar(int num_partitions,
                       int x_fft_buf_block_pos,
@@ skipping 382 matching lines @@
   self->stateCounter = 0;
   InitLevel(&self->farlevel);
   InitLevel(&self->nearlevel);
   InitLevel(&self->linoutlevel);
   InitLevel(&self->nlpoutlevel);
 
   InitStats(&self->erl);
   InitStats(&self->erle);
   InitStats(&self->aNlp);
   InitStats(&self->rerl);
+
+  self->fraction_filter_divergent.Reset();
 }
 
 static float CalculatePower(const float* in, size_t num_samples) {
   size_t k;
   float energy = 0.0f;
 
   for (k = 0; k < num_samples; ++k) {
     energy += in[k] * in[k];
   }
   return energy / num_samples;
@@ skipping 23 matching lines @@
 
   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()) {
+    const float near_level = aec->nearlevel.framelevel.GetLatestMean();
+    const float level_increase =
+        aec->linoutlevel.framelevel.GetLatestMean() - near_level;
+    const bool audible = aec->nlpoutlevel.framelevel.GetLatestMean() >

[peah-webrtc, 2016/04/05 09:15:52]

The audible flag basically checks wether the NLP mean is more than 100 times (20
dB) larger than the NLP min, right? To me the name does not really make sense as
I cannot make the connection to what it is that would be audible; the echo or
the nearend?
Please either clarify this in a comment, or rename the variable.

[minyue-webrtc, 2016/04/05 09:50:31]

how about nlpout_active?

[peah-webrtc, 2016/04/05 10:42:03]

That is also vague to me. Do you mean that the NLP is transparent?

audible will be high if 
-the nlp gain varies between low and high, 
but low it either
-the nlp gain is constantly high
-the nlp gain is constantly low.

Therefore I think audible is neither a measure of nlp transparency, nlp
activeness or whether something is audible in the output.

[minyue-webrtc, 2016/04/05 12:29:12]

I used nlpout_active to tell if the final output signal is active or silent. It
is similar to line 644 that measures whether far-end signal is active.

But maybe nlpout_active as a name has too much emphasis on nlp, how about
out_signal_active?

+        100.0 * aec->nlpoutlevel.minlevel;

[minyue-webrtc, 2016/04/04 15:07:44]

I found that 100.0 might be a bit large. I prefer using 40.0, which is
consistent with actThresholdClean (line 606). Experiments showed a bit better
result.

[peah-webrtc, 2016/04/05 09:15:52]

What do you mean here? Do you want to keep it 100 or do you want to change it to
40?

[minyue-webrtc, 2016/04/05 09:50:31]

I want to change to 40, which gave a bit better result in my offline tests. I
did not add a patch set just for this, therefore, I put a comment instead.

[peah-webrtc, 2016/04/05 10:42:03]

Acknowledged.

+    // Level increase should be, in principle, negative, when the filter
+    // does not diverge. Here we allow some margin (0.01 * near end level) and
+    // numerical error (1.0).
+    aec->fraction_filter_divergent.AddValue(audible &&

[peah-webrtc, 2016/04/05 09:15:52]

What about renaming fraction_filter_divergent to filter_divergence?

[peah-webrtc, 2016/04/05 09:15:52]

Here the fraction_filter_divergent object is only used as an integer counter. I
don't think the MeanCalculator should be used for this as that is a class for
computing the blocked mean of floating point variables.

In this case, the all the variables are integers and as the block length is
known and hardcoded the mean does not really need to be computed. 

[minyue-webrtc, 2016/04/05 09:50:31]

I agree that BlockMeanCalculator is for floating point, while fraction is to
count the occurrences. If we want to calculate fraction, we basically need to
introduce three variables:

size_t count_  // counts total samples
size_t occurrences_  // counts occurrences
float latest_fraction_

and do the following

if (event occurred) {
  occurrences_++;
}
count_++
if (count_ == block_length) {
  count_ = 0;
  latest_fraction_ = (float)occurrences_ / block_length;
}

This is essentially what BlockMeanCalculator does. I think using
BlockMeanCalculator eliminates much code duplication.

My suggestion is adding a comment on why we can use BlockMeanCalculator to
calculate fraction. WDYT?

[minyue-webrtc, 2016/04/05 09:50:31]

Tina also wanted to drop "fraction". My idea is that the value is between 0 and
1, and "fraction" directly tells what it means.

Regarding "divergent" or "divergence", I have no preference. "divergent" was
suggested by Karl.

How about "filter_divergence_fraction"?

[peah-webrtc, 2016/04/05 10:42:03]

I think it would be better to use a proper counter or that you create a separate
class for that. The current usage of the AddValue method involves an implicit
cast from bool to int and relies on this to be a summable integer which is added
to the mean.

I think that even though it is fully possible to reuse the BlockMeanCalculator
this complicates the code. (And even though the code is shorter, it is slightly
more complex as the cast is needed and as the AddValue calls is needed even when
the filter is not diverging).

[peah-webrtc, 2016/04/05 10:42:03]

Lets go with that, or with divergent_filter_fraction. It is nice to have a unit
attached to the variable; I think there should be a better unit for that but I
cannot come up with a better suggestion so lets go for that.

[minyue-webrtc, 2016/04/05 12:29:12]

Then I prefer to add a new class: BlockFractionCalculator. It can also be used
for fraction_poor_delays in the future.

+        level_increase > std::max(0.01 * near_level, 1.0) ? 1.0 : 0.0);
+  }
+
   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
@@ skipping 1274 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