Disable Intelligibility Enhancer for high SNRs

webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc

Index: webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc
diff --git a/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc b/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc
index de36b7a8bc75c943fb436326b8f067e4ca948215..23dca2631f2d96cb67a791cb7727c0ad2ed8b76f 100644
--- a/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc
+++ b/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc
@@ -38,6 +38,8 @@ const float kDecayRate = 0.994f;              // Power estimation decay rate.
 const float kMaxRelativeGainChange = 0.006f;
 const float kRho = 0.0004f;  // Default production and interpretation SNR.
 const float kPowerNormalizationFactor = 1.f / (1 << 30);
+const float kMaxActiveSNR = 128.f;  // 21dB
+const float kMinInactiveSNR = 32.f;  // 15dB
 
 // Returns dot product of vectors |a| and |b| with size |length|.
 float DotProduct(const float* a, const float* b, size_t length) {
@@ -84,6 +86,8 @@ IntelligibilityEnhancer::IntelligibilityEnhancer(int sample_rate_hz,
       audio_s16_(chunk_length_),
       chunks_since_voice_(kSpeechOffsetDelay),
       is_speech_(false),
+      snr_(kMaxActiveSNR),
+      is_active_(false),
       noise_estimation_buffer_(num_noise_bins),
       noise_estimation_queue_(kMaxNumNoiseEstimatesToBuffer,
                               std::vector<float>(num_noise_bins),
@@ -135,29 +139,54 @@ void IntelligibilityEnhancer::ProcessAudioBlock(
   if (is_speech_) {
     clear_power_estimator_.Step(in_block[0]);
   }
-  const std::vector<float>& clear_power = clear_power_estimator_.power();
-  const std::vector<float>& noise_power = noise_power_estimator_.power();
-  MapToErbBands(clear_power.data(), render_filter_bank_,
-                filtered_clear_pow_.data());
-  MapToErbBands(noise_power.data(), capture_filter_bank_,
-                filtered_noise_pow_.data());
-  SolveForGainsGivenLambda(kLambdaTop, start_freq_, gains_eq_.data());
-  const float power_target = std::accumulate(
-      filtered_clear_pow_.data(), filtered_clear_pow_.data() + bank_size_, 0.f);
-  const float power_top =
-      DotProduct(gains_eq_.data(), filtered_clear_pow_.data(), bank_size_);
-  SolveForGainsGivenLambda(kLambdaBot, start_freq_, gains_eq_.data());
-  const float power_bot =
-      DotProduct(gains_eq_.data(), filtered_clear_pow_.data(), bank_size_);
-  if (power_target >= power_bot && power_target <= power_top) {
-    SolveForLambda(power_target);
-    UpdateErbGains();
-  }  // Else experiencing power underflow, so do nothing.
+  UpdateActivity();
+  if (is_active_) {
+    MapToErbBands(clear_power_estimator_.power().data(), render_filter_bank_,
+                  filtered_clear_pow_.data());
+    MapToErbBands(noise_power_estimator_.power().data(), capture_filter_bank_,
+                  filtered_noise_pow_.data());
+    SolveForGainsGivenLambda(kLambdaTop, start_freq_, gains_eq_.data());
+    const float power_target = std::accumulate(
+        filtered_clear_pow_.data(),
+        filtered_clear_pow_.data() + bank_size_,
+        0.f);
+    const float power_top =
+        DotProduct(gains_eq_.data(), filtered_clear_pow_.data(), bank_size_);
+    SolveForGainsGivenLambda(kLambdaBot, start_freq_, gains_eq_.data());
+    const float power_bot =
+        DotProduct(gains_eq_.data(), filtered_clear_pow_.data(), bank_size_);
+    if (power_target >= power_bot && power_target <= power_top) {
+      SolveForLambda(power_target);
+      UpdateErbGains();
+    }  // Else experiencing power underflow, so do nothing.
+  }
   for (size_t i = 0; i < in_channels; ++i) {
     gain_applier_.Apply(in_block[i], out_block[i]);
   }
 }
 
+void IntelligibilityEnhancer::UpdateActivity() {

[peah-webrtc, 2016/04/12 13:39:21]

What you are updating here is the is_active flag and gains, right? And the
is_active flag is a flag for whether the effect of the IE should be active or
not, and not for whether there is activity in the render signal, right?

If I did not get that wrong, I think this method should be modified or renamed.
since to me, activity means speech activity.
What about renaming it to ControlEffectApplication, or SnrBasedEffectActivation
which in my mind describe in more detail what is being done.

[aluebs-webrtc, 2016/04/12 18:34:28]

Yes, your understanding is completely right. And I agree that your naming
suggestion is more intuitive. Done.

+  const float* clear_psd = clear_power_estimator_.power().data();
+  const float* noise_psd = noise_power_estimator_.power().data();
+  const float clear_power =
+      std::accumulate(clear_psd, clear_psd + freqs_, 0.f);
+  const float noise_power =
+      std::accumulate(noise_psd, noise_psd + freqs_, 0.f);
+  snr_ = kDecayRate * snr_ + (1.f - kDecayRate) * clear_power / noise_power;

[peah-webrtc, 2016/04/12 13:39:21]

This SNR estimate is an average of the instantaneous SNR. An alternative could
be to use the average of the overall SNR. Have you considered that (I'm not
saying this is wrong).

[peah-webrtc, 2016/04/12 13:39:21]

This SNR estimate is assuming that the ratio of the clear_psd and noise_psd
matches the ratio at the ear of the listener.  What happens if the listener is
using headphones? Then this ratio is very different from what the SNR is at the
ear of the listener. The same is the case if a device is used in speaker mode.

I don't really see any point in adding functionality for tuning the application
of the IE effect based on the digital SNR until a mapping is in place to map
this to the acoustic SNR at the ear. 

But I'm fine with the code change.

[aluebs-webrtc, 2016/04/12 18:34:28]

That is an interesting point. Because the PSDs are already filtered over time
already, this is not exactly an average of the instantaneous SNR, but more of an
average of an averaged SNR, if that makes some sense. This additional filtering
is just to ensure its smoothness.

[aluebs-webrtc, 2016/04/12 18:34:28]

As discussed offline at the beginning of this project, with some broad
assumptions we can estimate acoustic SNRs from the digital one good enough for
the IE to improve the intelligibility of the signal. But also, I am testing
right now on a real device if this holds true and at the same time working on a
mapping I suggested to see if it improves the relation between the SNRs.
On the other hand, what we decided was to enable this feature first only for
headphones and phone mode (no speaker phone), so we can focus on that and delay
the additional tweaking to later on the process.
I think this code is valuable as of today, but I agree that the thresholds will
need to be adjusted if we apply any mapping.

+  if (is_active_) {
+    if (snr_ > kMaxActiveSNR) {
+      is_active_ = false;
+      // Set the target gains to unity.
+      float* gains = gain_applier_.target();
+      for (size_t i = 0; i < freqs_; ++i) {
+        gains[i] = 1.f;
+      }
+    }
+  } else {
+    is_active_ = snr_ < kMinInactiveSNR;
+  }
+}
+
 void IntelligibilityEnhancer::SolveForLambda(float power_target) {
   const float kConvergeThresh = 0.001f;  // TODO(ekmeyerson): Find best values
   const int kMaxIters = 100;             // for these, based on experiments.