Compensate for the IntelligibilityEnhancer processing delay in high bands

webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc

 /*
  *  Copyright (c) 2014 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 98 matching lines @@
                                        kbd_window.data());
   render_mangler_.reset(new LappedTransform(
       num_render_channels_, num_render_channels_, chunk_length_,
       kbd_window.data(), window_size, window_size / 2, this));
 }
 
 IntelligibilityEnhancer::~IntelligibilityEnhancer() {
   // Don't rely on this log, since the destructor isn't called when the app/tab
   // is killed.
   LOG(LS_INFO) << "Intelligibility Enhancer was active for "
-               << static_cast<float>(num_active_chunks_) / num_chunks_
+               << 100.f * static_cast<float>(num_active_chunks_) / num_chunks_

[peah-webrtc, 2016/09/09 09:12:16]

This will cause an exception if IE is destroyed right after having been created,
right (due to the division by 0)?
Please add proper handling of that, and/or add a checked division.

[aluebs-webrtc, 2016/09/10 00:47:55]

Done.

                << "% of the call.";
 }
 
 void IntelligibilityEnhancer::SetCaptureNoiseEstimate(
     std::vector<float> noise, float gain) {
   RTC_DCHECK_EQ(noise.size(), num_noise_bins_);
   for (auto& bin : noise) {
     bin *= gain;
   }
   // Disregarding return value since buffer overflow is acceptable, because it
   // is not critical to get each noise estimate.
   if (noise_estimation_queue_.Insert(&noise)) {
   };
 }
 
-void IntelligibilityEnhancer::ProcessRenderAudio(float* const* audio,
-                                                 int sample_rate_hz,
-                                                 size_t num_channels) {
+void IntelligibilityEnhancer::ProcessRenderAudio(AudioBuffer* audio,
+                                                 int sample_rate_hz) {

[peah-webrtc, 2016/09/09 09:12:16]

Is it really necessary to pass the sample rate to this call? It is only used for
checking that is invariant.
No other submodule inside APM does this and since you anyway change the
parameters of the method it would make sense to change that at the same time.

Also, should it not now be possible to use the sample_rate() method from
AudioBuffer?

[aluebs-webrtc, 2016/09/10 00:47:55]

AudioBuffer doesn't have a sample_rate() method. But dropped the checking and
input parameter.

   RTC_CHECK_EQ(sample_rate_hz_, sample_rate_hz);
-  RTC_CHECK_EQ(num_render_channels_, num_channels);
+  RTC_CHECK_EQ(num_render_channels_, audio->num_channels());

[peah-webrtc, 2016/09/09 09:12:16]

This looks like something that there should be a DCHECK for instead. Same thing
goes for the sample rate CHECK.

[aluebs-webrtc, 2016/09/10 00:47:55]

Done.

   while (noise_estimation_queue_.Remove(&noise_estimation_buffer_)) {
     noise_power_estimator_.Step(noise_estimation_buffer_.data());
   }
-  is_speech_ = IsSpeech(audio[0]);
-  render_mangler_->ProcessChunk(audio, audio);
+  float* const* in_low_band = audio->split_channels_f(kBand0To8kHz);
+  float* const* out_low_band = is_active_ ? in_low_band : nullptr;
+  is_speech_ = IsSpeech(in_low_band[0]);
+  render_mangler_->ProcessChunk(in_low_band, out_low_band);

[peah-webrtc, 2016/09/09 09:12:16]

With this approach, you only let the audio pass through the IE processing when
is_active is true, right? And is_active is true when the SNR is small, right? 
So am I correct in that IE can toggle between applying and not applying the IE
effect during a call?

The drawback with that approach is that each time this toggling happens, there
is a click. The correct way is instead to apply the processing all the time, but
with a fully transparent effect if is_active is false. That will not cause
clicks.

If you want to go back and forth between having the effect on and off you need
to do that in a smooth manner so that there is no click. But since the delay is
changed, I'd definitely prefer the above approach.

[aluebs-webrtc, 2016/09/10 00:47:55]

Good catch. I am not sure why I wrongly assumed this would be smoothed out by
the windowing. Now it always goes a transparent processing that introduces the
blocking delay.

+  if (is_active_) {

[peah-webrtc, 2016/09/09 09:12:16]

You cannot turn this on/off during the call as that will cause a glitch. Either
you have it on or off, but do not toggle between those states.

[aluebs-webrtc, 2016/09/10 00:47:55]

Same as above.

+    DelayHighBands(audio);
+  }
 }
 
 void IntelligibilityEnhancer::ProcessAudioBlock(
     const std::complex<float>* const* in_block,
     size_t in_channels,
     size_t frames,
     size_t /* out_channels */,
     std::complex<float>* const* out_block) {
   RTC_DCHECK_EQ(freqs_, frames);
   if (is_speech_) {
     clear_power_estimator_.Step(in_block[0]);
   }
-  SnrBasedEffectActivation();
-  ++num_chunks_;
   if (is_active_) {
     ++num_active_chunks_;
     if (num_chunks_ % kGainUpdatePeriod == 0) {
       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.
     }
   }
+  SnrBasedEffectActivation();
+  ++num_chunks_;
   for (size_t i = 0; i < in_channels; ++i) {
     gain_applier_.Apply(in_block[i], out_block[i]);
   }
 }
 
 void IntelligibilityEnhancer::SnrBasedEffectActivation() {
   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);
@@ skipping 170 matching lines @@
   FloatToS16(audio, chunk_length_, audio_s16_.data());
   vad_.ProcessChunk(audio_s16_.data(), chunk_length_, sample_rate_hz_);
   if (vad_.last_voice_probability() > kVoiceProbabilityThreshold) {
     chunks_since_voice_ = 0;
   } else if (chunks_since_voice_ < kSpeechOffsetDelay) {
     ++chunks_since_voice_;
   }
   return chunks_since_voice_ < kSpeechOffsetDelay;
 }
 
+void IntelligibilityEnhancer::DelayHighBands(AudioBuffer* audio) {
+  size_t num_bands = audio->num_bands();
+  if (num_bands != high_bands_buffers_.size() + 1u) {

[peah-webrtc, 2016/09/09 09:12:16]

Please put the initialization of this in the constructor instead. As it is now,
it constitutes such a big part of the method that it hides what it really does.

[aluebs-webrtc, 2016/09/10 00:47:55]

Good point. I thought it needed to be dynamic, but actually it doesn't, because
the APM will rebuild the whole IE if the sample_rate changes. My bad. Moved to
constructor.

+    high_bands_buffers_.clear();
+    const size_t initial_delay = render_mangler_->initial_delay();
+    for (size_t i = 0u; i < num_bands - 1; ++i) {
+      high_bands_buffers_.push_back(
+          std::unique_ptr<AudioRingBuffer>(new AudioRingBuffer(
+              num_render_channels_, chunk_length_ + initial_delay)));
+      high_bands_buffers_[i]->MoveReadPositionBackward(initial_delay);
+    }
+  }
+  for (size_t i = 0u; i < num_bands - 1; ++i) {
+    Band band = static_cast<Band>(i + 1);
+    high_bands_buffers_[i]->Write(audio->split_channels_const_f(band),
+                                  num_render_channels_, chunk_length_);
+    high_bands_buffers_[i]->Read(audio->split_channels_f(band),
+                                 num_render_channels_, chunk_length_);
+  }
+}
+
 }  // namespace webrtc