Finalization of the first version of EchoCanceller 3

webrtc/modules/audio_processing/aec3/echo_remover.cc

Index: webrtc/modules/audio_processing/aec3/echo_remover.cc
diff --git a/webrtc/modules/audio_processing/aec3/echo_remover.cc b/webrtc/modules/audio_processing/aec3/echo_remover.cc
index ab0b68bb168f5b60b14cd02dd89d510c908721f0..af9188025b803aa815ef713c0eedf5e3856af0e7 100644
--- a/webrtc/modules/audio_processing/aec3/echo_remover.cc
+++ b/webrtc/modules/audio_processing/aec3/echo_remover.cc
@@ -10,59 +10,208 @@
 #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;
+  // 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;
 
-  void UpdateEchoLeakageStatus(bool leakage_detected) 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);
 
-// TODO(peah): Add functionality.
-void EchoRemoverImpl::UpdateEchoLeakageStatus(bool leakage_detected) {}
+  // 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);
+}
 
 }  // namespace