Finalization of the first version of EchoCanceller 3

webrtc/modules/audio_processing/aec3/echo_remover.cc

 /*
  *  Copyright (c) 2017 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.
  */
 #include "webrtc/modules/audio_processing/aec3/echo_remover.h"
 
 #include <algorithm>
-#include <vector>
-
+#include <numeric>
+#include <string>
+
+#include "webrtc/base/atomicops.h"
 #include "webrtc/base/constructormagic.h"
-#include "webrtc/base/checks.h"
-#include "webrtc/base/optional.h"
+#include "webrtc/base/array_view.h"

[aleloi, 2017/02/13 16:44:50]

Include order. Also, array_view is not used.

[peah-webrtc, 2017/02/20 07:37:16]

Reordered the includes. I did, however, keep the include as it is now again
used.

Done.

 #include "webrtc/modules/audio_processing/aec3/aec3_constants.h"
+#include "webrtc/modules/audio_processing/aec3/aec_state.h"
+#include "webrtc/modules/audio_processing/aec3/fft_data.h"
+#include "webrtc/modules/audio_processing/aec3/echo_path_variability.h"
+#include "webrtc/modules/audio_processing/aec3/comfort_noise_generator.h"
+#include "webrtc/modules/audio_processing/aec3/delay_handler.h"
+#include "webrtc/modules/audio_processing/aec3/power_echo_model.h"
+#include "webrtc/modules/audio_processing/aec3/output_selector.h"
+#include "webrtc/modules/audio_processing/aec3/fft_buffer.h"
+#include "webrtc/modules/audio_processing/aec3/render_delay_buffer.h"
+#include "webrtc/modules/audio_processing/aec3/residual_echo_estimator.h"
+#include "webrtc/modules/audio_processing/aec3/subtractor.h"
+#include "webrtc/modules/audio_processing/aec3/suppression_filter.h"
+#include "webrtc/modules/audio_processing/aec3/suppression_gain.h"
+#include "webrtc/modules/audio_processing/audio_buffer.h"
+#include "webrtc/modules/audio_processing/include/audio_processing.h"

[aleloi, 2017/02/13 16:44:50]

Is audio_buffer and audio_processing used?

[peah-webrtc, 2017/02/20 07:37:16]

Done.

+#include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"

[aleloi, 2017/02/13 16:44:50]

Include order

[peah-webrtc, 2017/02/20 07:37:16]

But this one is correctly ordered, right?

[aleloi, 2017/02/23 10:56:44]

Acknowledged.

 
 namespace webrtc {
 
 namespace {
+
+void LinearEchoPower(const FftData& E,
+                     const FftData& Y,
+                     std::array<float, kFftLengthBy2Plus1>* S2) {
+  for (size_t k = 0; k < E.re.size(); ++k) {
+    (*S2)[k] = (Y.re[k] - E.re[k]) * (Y.re[k] - E.re[k]) +
+               (Y.im[k] - E.im[k]) * (Y.im[k] - E.im[k]);
+  }
+}
+
+// Class for removing the echo from the capture signal.
 class EchoRemoverImpl final : public EchoRemover {
  public:
   explicit EchoRemoverImpl(int sample_rate_hz);
   ~EchoRemoverImpl() override;
 
-  void ProcessBlock(const rtc::Optional<size_t>& echo_path_delay_samples,
-                    const EchoPathVariability& echo_path_variability,
-                    bool capture_signal_saturation,
-                    const std::vector<std::vector<float>>& render,
-                    std::vector<std::vector<float>>* capture) override;
-
-  void UpdateEchoLeakageStatus(bool leakage_detected) override;
+  // Removes the echo from a block of samples from the capture signal. The
+  // supplied render signal is assumed to be pre-aligned with the capture
+  // signal.
+  void ProcessBlock(
+      const rtc::Optional<size_t>& external_echo_path_delay_estimate,
+      const EchoPathVariability& echo_path_variability,
+      bool capture_signal_saturation,
+      const std::vector<std::vector<float>>& render,
+      std::vector<std::vector<float>>* capture) override;
+
+  // Updates the status on whether echo leakage is detected in the output of the
+  // echo remover.
+  void UpdateEchoLeakageStatus(bool leakage_detected) override {
+    echo_leakage_detected_ = leakage_detected;
+  }
 
  private:
+  static int instance_count_;
+  const Aec3Fft fft_;
+  ApmDataDumper* data_dumper_;
   const int sample_rate_hz_;
+  Subtractor subtractor_;
+  SuppressionGain suppression_gain_;
+  ComfortNoiseGenerator cng_;
+  SuppressionFilter suppression_filter_;
+  PowerEchoModel power_echo_model_;
+  FftBuffer X_buffer_;
+  RenderSignalAnalyzer render_signal_analyzer_;
+  OutputSelector output_selector_;
+  DelayHandler delay_handler_;
+  ResidualEchoEstimator residual_echo_estimator_;
+  bool echo_leakage_detected_ = false;
+  std::array<float, kBlockSize> x_old_;
+  AecState aec_state_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(EchoRemoverImpl);
 };
 
-// TODO(peah): Add functionality.
+int EchoRemoverImpl::instance_count_ = 0;
+
 EchoRemoverImpl::EchoRemoverImpl(int sample_rate_hz)
-    : sample_rate_hz_(sample_rate_hz) {
-  RTC_DCHECK(ValidFullBandRate(sample_rate_hz_));
+    : data_dumper_(
+          new ApmDataDumper(rtc::AtomicOps::Increment(&instance_count_))),
+      sample_rate_hz_(sample_rate_hz),
+      subtractor_(data_dumper_),
+      suppression_filter_(sample_rate_hz_),
+      X_buffer_(std::max(subtractor_.MinFarendBufferLength(),
+                         power_echo_model_.MinFarendBufferLength()),
+                subtractor_.NumBlocksInRenderSums()) {
+  RTC_DCHECK(ValidFullBandRate(sample_rate_hz));
+  x_old_.fill(0.f);
 }
 
 EchoRemoverImpl::~EchoRemoverImpl() = default;
 
-// TODO(peah): Add functionality.
 void EchoRemoverImpl::ProcessBlock(
     const rtc::Optional<size_t>& echo_path_delay_samples,
     const EchoPathVariability& echo_path_variability,
     bool capture_signal_saturation,
     const std::vector<std::vector<float>>& render,
     std::vector<std::vector<float>>* capture) {
-  RTC_DCHECK(capture);
-  RTC_DCHECK_EQ(render.size(), NumBandsForRate(sample_rate_hz_));
-  RTC_DCHECK_EQ(capture->size(), NumBandsForRate(sample_rate_hz_));
-  RTC_DCHECK_EQ(render[0].size(), kBlockSize);
-  RTC_DCHECK_EQ((*capture)[0].size(), kBlockSize);
+  const std::vector<std::vector<float>>& x = render;
+  std::vector<std::vector<float>>* y = capture;

[aleloi, 2017/02/13 16:44:50]

Is it infeasible to change the inner vectors to array<float, kBlockSize>? That
would affect everything starting from FrameBlocker, right?

[peah-webrtc, 2017/02/20 07:37:16]

We should definitely do that. But as you point out, that needs to start from the
top. Ok if we take that in another CL?

[aleloi, 2017/02/23 10:56:44]

SGTM

+

[aleloi, 2017/02/13 16:44:50]

A few general comments:

I think it would improve readability somewhat to declare some of the arrays
closer to point of use.

Also, perhaps consider inner scopes. Both to limit what parts of the program an
array is visible in, and to structure the control flow a bit. E.g. 'G' can be
declared, computed and used in its own scope.

[peah-webrtc, 2017/02/20 07:37:16]

I definitely think you have a point. However, doing that will increase the size
of the core of the functionality. As it is now, having the variables declared on
top. The actual functionality of the method is visible on one single page on
screen, which I really like as it increases readability.

WDYT? Is it ok to have it as it is?

[aleloi, 2017/02/23 10:56:44]

SGTM

+  RTC_DCHECK(y);
+  RTC_DCHECK_EQ(x.size(), NumBandsForRate(sample_rate_hz_));
+  RTC_DCHECK_EQ(y->size(), NumBandsForRate(sample_rate_hz_));
+  RTC_DCHECK_EQ(x[0].size(), kBlockSize);
+  RTC_DCHECK_EQ((*y)[0].size(), kBlockSize);
+  const std::vector<float>& x0 = x[0];
+  std::vector<float>& y0 = (*y)[0];
+
+  data_dumper_->DumpWav("aec3_processblock_capture_input", y0,
+                        LowestBandRate(sample_rate_hz_), 1);
+  data_dumper_->DumpWav("aec3_processblock_render_input", x0,
+                        LowestBandRate(sample_rate_hz_), 1);
+
+  if (echo_path_variability.AudioPathChanged()) {
+    subtractor_.HandleEchoPathChange(echo_path_variability);
+    power_echo_model_.HandleEchoPathChange(echo_path_variability);
+  }
+
+  std::array<float, kFftLengthBy2Plus1> Y2;
+  std::array<float, kFftLengthBy2Plus1> S2_power;
+  std::array<float, kFftLengthBy2Plus1> R2;
+  std::array<float, kFftLengthBy2Plus1> S2_linear;
+  std::array<float, kFftLengthBy2Plus1> G;
+  FftData X;
+  FftData Y;
+  FftData comfort_noise;
+  FftData high_band_comfort_noise;
+  SubtractorOutput subtractor_output;
+  FftData& E_main = subtractor_output.E_main;
+  std::array<float, kFftLengthBy2Plus1>& E2_main = subtractor_output.E2_main;
+  std::array<float, kFftLengthBy2Plus1>& E2_shadow =
+      subtractor_output.E2_shadow;
+  std::array<float, kBlockSize>& e_main = subtractor_output.e_main;
+  std::array<float, kBlockSize>& e_shadow = subtractor_output.e_shadow;
+
+  // Update the render signal buffer.
+  fft_.PaddedFft(x0, x_old_, &X);
+  X_buffer_.Insert(X);
+
+  // Analyze the render signal.
+  render_signal_analyzer_.Update(X_buffer_, delay_handler_.FilterDelay());
+
+  // Perform linear echo cancellation.
+  subtractor_.Process(X_buffer_, y0, render_signal_analyzer_,
+                      capture_signal_saturation, &subtractor_output);
+
+  // Detect basic doubletalk.
+  const float e_main_energy =
+      std::accumulate(e_main.begin(), e_main.end(), 0.f,
+                      [](float a, float b) -> float { return a + b * b; });
+  const float e_shadow_energy =
+      std::accumulate(e_shadow.begin(), e_shadow.end(), 0.f,
+                      [](float a, float b) -> float { return a + b * b; });

[aleloi, 2017/02/09 17:02:59]

Maybe use std::inner_product? Less lambdas is simpler.

[peah-webrtc, 2017/02/20 07:37:16]

Inner product looks perfect for this. I have some concerns, however, that the
inner product might be more expensive as it uses two iterators.

I broke this out into a separate method for now that we definitely should have
an optimized version for later on. I kept the accumulate for now though.

WDYT, can inner_product be more expensive than accumulate when it operates on
one single vector?

[aleloi, 2017/02/23 10:56:45]

I think you are right: 

https://godbolt.org/g/ZVubS0

Both the accumulate and the inner product version use 128 bit registers and loop
out-rolling, but the inner product one also uses a 32-bit register for
something. Also, the accumulate outrolls more: it handles 32 words in the loop
body. The inner product does 16.

+  const bool doubletalk = e_shadow_energy < e_main_energy;
+
+  // Compute spectra.
+  fft_.ZeroPaddedFft(y0, &Y);
+  LinearEchoPower(E_main, Y, &S2_linear);
+
+  Y.Spectrum(&Y2);
+
+  // Update the internal delay estimates.
+  delay_handler_.UpdateDelays(subtractor_.FilterFrequencyResponse(),
+                              echo_path_delay_samples);
+
+  // Use the power model to estimate the echo.
+  power_echo_model_.EstimateEcho(X_buffer_, Y2, delay_handler_,
+                                 capture_signal_saturation, &S2_power);
+
+  // Update the AEC state information.
+  aec_state_.Update(X_buffer_, E2_main, E2_shadow, Y2, x0, delay_handler_,
+                    echo_path_variability, echo_leakage_detected_);
+
+  // Estimate the residual echo power.
+  residual_echo_estimator_.Estimate(
+      aec_state_, X_buffer_, subtractor_.FilterFrequencyResponse(), E2_main,
+      E2_shadow, S2_linear, S2_power, Y2, delay_handler_, &R2);
+
+  // Estimate the comfort noise.
+  cng_.Compute(Y2, &comfort_noise, &high_band_comfort_noise);
+
+  // Choose the linear output.
+  output_selector_.FormLinearOutput(e_main, y0);
+  data_dumper_->DumpWav("aec3_output_linear", y0,
+                        LowestBandRate(sample_rate_hz_), 1);
+  const std::array<float, kFftLengthBy2Plus1>& E2 =
+      output_selector_.UseSubtractorOutput() ? E2_main : Y2;
+
+  // A choose and apply echo suppression gain.
+  suppression_gain_.GetGain(E2, R2, cng_.NoiseSpectrum(),
+                            doubletalk ? 0.001f : 0.0001f, &G);
+  suppression_filter_.ApplyGain(comfort_noise, high_band_comfort_noise, G, y);
+
+  data_dumper_->DumpWav("aec3_output", y0, LowestBandRate(sample_rate_hz_), 1);
 }
 
-// TODO(peah): Add functionality.
-void EchoRemoverImpl::UpdateEchoLeakageStatus(bool leakage_detected) {}
-
 }  // namespace
 
 EchoRemover* EchoRemover::Create(int sample_rate_hz) {
   return new EchoRemoverImpl(sample_rate_hz);
 }
 
 }  // namespace webrtc