Finalization of the first version of EchoCanceller 3

webrtc/modules/audio_processing/aec3/echo_remover_unittest.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 <memory>
+#include <numeric>
 #include <sstream>
 #include <string>
 
+#include "webrtc/base/random.h"
 #include "webrtc/modules/audio_processing/aec3/aec3_constants.h"
 #include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"
+#include "webrtc/modules/audio_processing/test/echo_canceller_test_tools.h"
 #include "webrtc/test/gtest.h"
 
 namespace webrtc {
 namespace {
 
 std::string ProduceDebugText(int sample_rate_hz) {
   std::ostringstream ss;
   ss << "Sample rate: " << sample_rate_hz;
   return ss.str();
 }
 
+std::string ProduceDebugText(int sample_rate_hz, int delay) {
+  std::ostringstream ss(ProduceDebugText(sample_rate_hz));
+  ss << ", Delay: " << delay;
+  return ss.str();
+}
+
 }  // namespace
 
 // Verifies the basic API call sequence
 TEST(EchoRemover, BasicApiCalls) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     ProduceDebugText(rate);
     std::unique_ptr<EchoRemover> remover(EchoRemover::Create(rate));
 
     std::vector<std::vector<float>> render(NumBandsForRate(rate),
                                            std::vector<float>(kBlockSize, 0.f));
@@ skipping 108 matching lines @@
   EchoPathVariability echo_path_variability(false, false);
   rtc::Optional<size_t> echo_path_delay_samples;
   EXPECT_DEATH(
       remover->ProcessBlock(echo_path_delay_samples, echo_path_variability,
                             false, render, nullptr),
       "");
 }
 
 #endif
 
+// Performs a sanity check that the echo_remover is being able to properly
+// remove echoes.
+TEST(EchoRemover, BasicEchoRemoval) {

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

In this test, the near-end signal is comprised entirely of echo. Is it possible
to write similar test/tests for echo + noise?

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

Absolutely, but I'm not sure if we should have that here/yet as the AEC
functionality is still being developed. I prefer to stay with having  this
sanity check test for now and instead consider adding more refined tests later.
Is that ok?

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

Acknowledged.

+  constexpr int kNumBlocksToProcess = 500;
+  Random random_generator(42U);
+  for (auto rate : {8000, 16000, 32000, 48000}) {
+    std::vector<std::vector<float>> x(NumBandsForRate(rate),
+                                      std::vector<float>(kBlockSize, 0.f));
+    std::vector<std::vector<float>> y(NumBandsForRate(rate),
+                                      std::vector<float>(kBlockSize, 0.f));
+    EchoPathVariability echo_path_variability(false, false);
+    for (size_t delay_samples : {0, 64, 150, 200, 301}) {
+      ProduceDebugText(rate, delay_samples);

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

This doesn't do anything: ProduceDebugText is a function that returns a string
and has no side effects. It's used the same way in decimator_by_4_unittest.
Should it be logged with SCOPED_TRACE?

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

Done.

+      std::unique_ptr<EchoRemover> remover(EchoRemover::Create(rate));
+      std::vector<std::unique_ptr<DelayBuffer<float>>> delay_buffers(x.size());
+      for (size_t j = 0; j < x.size(); ++j) {
+        delay_buffers[j].reset(new DelayBuffer<float>(delay_samples));
+      }
+
+      bool silence = false;
+      float input_energy = 0.f;
+      float output_energy = 0.f;
+      for (int k = 0; k < kNumBlocksToProcess; ++k) {
+        if (silence) {
+          silence = (k % 100) != 0;
+        } else {
+          silence = (k % 10) == 0;
+        }

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

The 'silence' updating construct seems a little strange. Only frames 100-109,
200-209, 300-309, 400-409 will have nonzero samples. Why do we start with 100
blocks of silence?

Also, I think it's rather hard to understand what happens here. Can you change
it to

for k = 0..:
   const bool silence = <t/f condition calculated from 'k'>

?

To keep it working as is, it would be 

 'const bool silence = k < 100 || (k % 100 >= 10)' 

which I think is much better (but still a bit complex).

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

I changed to your proposed one-linear.

The purpose of the silence and speech parts is to ensure that the comfort noise
generator has an estimated noise level that is lower than the non-silence parts,
as that will then allow us to detect the removal of those in the AEC output.

The initial silence of 100 frames is not important, it could as well  have been
the other way around.

+
+        for (size_t j = 0; j < x.size(); ++j) {
+          if (silence) {
+            std::fill(x[j].begin(), x[j].end(), 0.f);
+          } else {
+            RandomizeSampleVector(&random_generator, x[j]);
+          }
+          delay_buffers[j]->Delay(x[j], y[j]);
+        }
+
+        if (k > kNumBlocksToProcess / 2) {
+          for (size_t j = 0; j < x.size(); ++j) {
+            input_energy = std::inner_product(y[j].begin(), y[j].end(),
+                                              y[j].begin(), input_energy);
+          }
+
+          remover->ProcessBlock(rtc::Optional<size_t>(delay_samples),
+                                echo_path_variability, false, x, &y);

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

The remover only sees the last 250 of the samples. That seems wrong. I think the
remover should process all of the samples, but that energy levels should only be
counted starting from kNumBlocksToProcess/2.

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

Good find!

Done.

+
+          for (size_t j = 0; j < x.size(); ++j) {
+            output_energy = std::inner_product(y[j].begin(), y[j].end(),
+                                               y[j].begin(), output_energy);
+          }
+        }
+      }
+      EXPECT_GT(input_energy, 10.f * output_energy);
+    }
+  }
+}
+
 }  // namespace webrtc