Adding second layer of the echo canceller 3 functionality.

webrtc/modules/audio_processing/aec3/block_processor_unittest.cc

 /*
  *  Copyright (c) 2016 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/block_processor.h"
 
 #include <memory>
 #include <sstream>
 #include <string>
 #include <vector>
 
+#include "webrtc/base/checks.h"
+#include "webrtc/base/random.h"
 #include "webrtc/modules/audio_processing/aec3/aec3_constants.h"
+#include "webrtc/modules/audio_processing/aec3/mock/mock_echo_remover.h"
+#include "webrtc/modules/audio_processing/aec3/mock/mock_render_delay_buffer.h"
+#include "webrtc/modules/audio_processing/aec3/mock/mock_render_delay_controller.h"
+#include "webrtc/modules/audio_processing/test/echo_canceller_test_tools.h"
+#include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 
 namespace webrtc {
 namespace {
 
+using testing::AtLeast;
+using testing::Return;
+using testing::StrictMock;
+using testing::_;
+
 // Verifies that the basic BlockProcessor functionality works and that the API
 // methods are callable.
 void RunBasicSetupAndApiCallTest(int sample_rate_hz) {
   std::unique_ptr<BlockProcessor> block_processor(
       BlockProcessor::Create(sample_rate_hz));
   std::vector<std::vector<float>> block(NumBandsForRate(sample_rate_hz),
                                         std::vector<float>(kBlockSize, 0.f));
 
   EXPECT_FALSE(block_processor->BufferRender(&block));
   block_processor->ProcessCapture(false, false, &block);
-  block_processor->ReportEchoLeakage(false);
+  block_processor->UpdateEchoLeakageStatus(false);
 }
 
 #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
 void RunRenderBlockSizeVerificationTest(int sample_rate_hz) {
   std::unique_ptr<BlockProcessor> block_processor(
       BlockProcessor::Create(sample_rate_hz));
   std::vector<std::vector<float>> block(
       NumBandsForRate(sample_rate_hz), std::vector<float>(kBlockSize - 1, 0.f));
 
   EXPECT_DEATH(block_processor->BufferRender(&block), "");
@@ skipping 34 matching lines @@
 #endif
 
 std::string ProduceDebugText(int sample_rate_hz) {
   std::ostringstream ss;
   ss << "Sample rate: " << sample_rate_hz;
   return ss.str();
 }
 
 }  // namespace
 
+// Verifies that the delay controller functionality is properly integrated with
+// the render delay buffer inside block processor.
+// TODO(peah): Activate the unittest once the required code has been landed.
+TEST(BlockProcessor, DISABLED_DelayControllerIntegration) {
+  constexpr size_t kNumBlocks = 310;
+  Random random_generator(42U);
+  for (auto rate : {8000, 16000, 32000, 48000}) {

[hlundin-webrtc, 2017/01/18 13:08:48]

Are any of the tested code paths dependent on the sample rate? If not, I would
argue that you can settle for testing one.

[peah-webrtc, 2017/01/19 15:33:04]

I agree that it is probably sufficient to only test one rate. I would, however,
prefer to keep it as it is as the test is very cheap, and that as doing that it
does not include any assumption on the code being tested. Such an assumption
could easily be broken during future code development and would very likely be
missed as the test will (may) not brake for that code change.

WDYT, can we keep it as it is? The test is cheaper, and the test will become
weaker if we only test one rate.

[hlundin-webrtc, 2017/01/20 09:31:44]

Acknowledged.

[peah-webrtc, 2017/01/23 14:16:39]

Acknowledged.

+    SCOPED_TRACE(ProduceDebugText(rate));
+    std::unique_ptr<testing::StrictMock<webrtc::test::MockRenderDelayBuffer>>
+        render_delay_buffer_mock(
+            new StrictMock<webrtc::test::MockRenderDelayBuffer>(rate));
+    EXPECT_CALL(*render_delay_buffer_mock, Insert(_))
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*render_delay_buffer_mock, IsBlockAvailable())
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*render_delay_buffer_mock, SetDelay(9)).Times(AtLeast(1));
+    EXPECT_CALL(*render_delay_buffer_mock, MaxDelay())
+        .Times(1)

[hlundin-webrtc, 2017/01/18 13:08:48]

Use .WillOnce(Return(x)) instead of .Times(1).WillRepeatedly(Return(x)).

[peah-webrtc, 2017/01/19 15:33:04]

Done.

+        .WillRepeatedly(Return(30));
+    EXPECT_CALL(*render_delay_buffer_mock, MaxApiJitter())
+        .Times(1)

[hlundin-webrtc, 2017/01/18 13:08:48]

WillOnce

[peah-webrtc, 2017/01/19 15:33:04]

Done.

+        .WillRepeatedly(Return(30));
+    EXPECT_CALL(*render_delay_buffer_mock, Delay())
+        .Times(kNumBlocks + 1)
+        .WillRepeatedly(Return(0));
+    std::unique_ptr<BlockProcessor> block_processor(
+        BlockProcessor::Create(rate, std::move(render_delay_buffer_mock)));
+
+    std::vector<std::vector<float>> render_block(
+        NumBandsForRate(rate), std::vector<float>(kBlockSize, 0.f));
+    std::vector<std::vector<float>> capture_block(
+        NumBandsForRate(rate), std::vector<float>(kBlockSize, 0.f));
+    DelayBuffer<float> signal_delay_buffer(640);
+    for (size_t k = 0; k < kNumBlocks; ++k) {
+      RandomizeSampleVector(&random_generator, render_block[0]);
+      signal_delay_buffer.Delay(render_block[0], capture_block[0]);
+      EXPECT_FALSE(block_processor->BufferRender(&render_block));
+      block_processor->ProcessCapture(false, false, &capture_block);
+    }
+  }
+}
+
+// Verifies that BlockProcessor submodules are called in a proper manner.
+TEST(BlockProcessor, SubmoduleIntegration) {
+  constexpr size_t kNumBlocks = 310;
+  Random random_generator(42U);
+  for (auto rate : {8000, 16000, 32000, 48000}) {

[hlundin-webrtc, 2017/01/18 13:08:48]

Same here; would testing one rate suffice?

[peah-webrtc, 2017/01/19 15:33:04]

Probably yes, but for the same arguments as above, I'd like to keep it.

WDYT?

[hlundin-webrtc, 2017/01/20 09:31:44]

Acknowledged.

[peah-webrtc, 2017/01/23 14:16:39]

Acknowledged.

+    SCOPED_TRACE(ProduceDebugText(rate));
+    std::unique_ptr<testing::StrictMock<webrtc::test::MockRenderDelayBuffer>>
+        render_delay_buffer_mock(
+            new StrictMock<webrtc::test::MockRenderDelayBuffer>(rate));
+    std::unique_ptr<
+        testing::StrictMock<webrtc::test::MockRenderDelayController>>
+        render_delay_controller_mock(
+            new StrictMock<webrtc::test::MockRenderDelayController>());
+    std::unique_ptr<testing::StrictMock<webrtc::test::MockEchoRemover>>
+        echo_remover_mock(new StrictMock<webrtc::test::MockEchoRemover>());
+
+    EXPECT_CALL(*render_delay_buffer_mock, Insert(_))
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*render_delay_buffer_mock, IsBlockAvailable())
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*render_delay_buffer_mock, SetDelay(9)).Times(AtLeast(1));
+    EXPECT_CALL(*render_delay_buffer_mock, MaxDelay()).Times(0);

[hlundin-webrtc, 2017/01/18 13:08:48]

You don't need this when you have a StrictMock.

[peah-webrtc, 2017/01/19 15:33:04]

Done.

+    EXPECT_CALL(*render_delay_buffer_mock, MaxApiJitter()).Times(0);

[hlundin-webrtc, 2017/01/18 13:08:48]

You don't need this when you have a StrictMock.

[peah-webrtc, 2017/01/19 15:33:04]

Done.

+    EXPECT_CALL(*render_delay_buffer_mock, Delay())
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(0));
+    EXPECT_CALL(*render_delay_controller_mock, SelectDelay(_))
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(9));
+    EXPECT_CALL(*render_delay_controller_mock, AnalyzeRender(_))
+        .Times(kNumBlocks)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*render_delay_controller_mock, AlignmentHeadroom())
+        .Times(kNumBlocks);
+    EXPECT_CALL(*echo_remover_mock, ProcessBlock(_, _, _, _, _))
+        .Times(kNumBlocks);
+    EXPECT_CALL(*echo_remover_mock, UpdateEchoLeakageStatus(_))
+        .Times(kNumBlocks);
+
+    std::unique_ptr<BlockProcessor> block_processor(BlockProcessor::Create(
+        rate, std::move(render_delay_buffer_mock),
+        std::move(render_delay_controller_mock), std::move(echo_remover_mock)));
+
+    std::vector<std::vector<float>> render_block(
+        NumBandsForRate(rate), std::vector<float>(kBlockSize, 0.f));
+    std::vector<std::vector<float>> capture_block(
+        NumBandsForRate(rate), std::vector<float>(kBlockSize, 0.f));
+    DelayBuffer<float> signal_delay_buffer(640);
+    for (size_t k = 0; k < kNumBlocks; ++k) {
+      RandomizeSampleVector(&random_generator, render_block[0]);
+      signal_delay_buffer.Delay(render_block[0], capture_block[0]);
+      EXPECT_FALSE(block_processor->BufferRender(&render_block));
+      block_processor->ProcessCapture(false, false, &capture_block);
+      block_processor->UpdateEchoLeakageStatus(false);
+    }
+  }
+}
+
 TEST(BlockProcessor, BasicSetupAndApiCalls) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     SCOPED_TRACE(ProduceDebugText(rate));
     RunBasicSetupAndApiCallTest(rate);
   }
 }
 
 #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
-// TODO(peah): Enable all DEATH tests below, or add suppressions, once clarity
-// has been reached on why they fail on the trybots.
-TEST(BlockProcessor, DISABLED_VerifyRenderBlockSizeCheck) {
+TEST(BlockProcessor, VerifyRenderBlockSizeCheck) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     SCOPED_TRACE(ProduceDebugText(rate));
     RunRenderBlockSizeVerificationTest(rate);
   }
 }
 
-TEST(BlockProcessor, DISABLED_VerifyCaptureBlockSizeCheck) {
+TEST(BlockProcessor, VerifyCaptureBlockSizeCheck) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     SCOPED_TRACE(ProduceDebugText(rate));
     RunCaptureBlockSizeVerificationTest(rate);
   }
 }
 
-TEST(BlockProcessor, DISABLED_VerifyRenderNumBandsCheck) {
+TEST(BlockProcessor, VerifyRenderNumBandsCheck) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     SCOPED_TRACE(ProduceDebugText(rate));
     RunRenderNumBandsVerificationTest(rate);
   }
 }
 
-TEST(BlockProcessor, DISABLED_VerifyCaptureNumBandsCheck) {
+TEST(BlockProcessor, VerifyCaptureNumBandsCheck) {
   for (auto rate : {8000, 16000, 32000, 48000}) {
     SCOPED_TRACE(ProduceDebugText(rate));
     RunCaptureNumBandsVerificationTest(rate);
   }
 }
 
 // Verifiers that the verification for null ProcessCapture input works.
-TEST(BlockProcessor, DISABLED_NullProcessCaptureParameter) {
+TEST(BlockProcessor, NullProcessCaptureParameter) {
   EXPECT_DEATH(std::unique_ptr<BlockProcessor>(BlockProcessor::Create(8000))
                    ->ProcessCapture(false, false, nullptr),
                "");
 }
 
 // Verifiers that the verification for null BufferRender input works.
-TEST(BlockProcessor, DISABLED_NullBufferRenderParameter) {
+TEST(BlockProcessor, NullBufferRenderParameter) {
   EXPECT_DEATH(std::unique_ptr<BlockProcessor>(BlockProcessor::Create(8000))
                    ->BufferRender(nullptr),
                "");
 }
 
 #endif
 
 }  // namespace webrtc