Add unittests for APM with submodule creation disabled

This introduces a function AudioProcessingImpl::SetCreateOptionalSubmodulesForTesting to simulate the exclusion of build-optional submodules, and tests of the currently only excludable submodule.

Bug: webrtc:11292
Change-Id: If492606205c9fdc669a6dce3a8989a434aeeed1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173746
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31138}

modules/audio_processing/BUILD.gn

@@ -156,6 +156,7 @@ rtc_library("audio_processing") {
     ":audio_processing_statistics",
     ":config",
     ":high_pass_filter",
+    ":optionally_built_submodule_creators",
     ":rms_level",
     ":voice_detection",
     "../../api:array_view",
@@ -187,7 +188,6 @@ rtc_library("audio_processing") {
     "agc2:gain_applier",
     "ns",
     "transient:transient_suppressor_api",
-    "transient:transient_suppressor_creator",
     "vad",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
@@ -215,6 +215,17 @@ rtc_library("voice_detection") {
   ]
 }
 
+rtc_library("optionally_built_submodule_creators") {
+  sources = [
+    "optionally_built_submodule_creators.cc",
+    "optionally_built_submodule_creators.h",
+  ]
+  deps = [
+    "transient:transient_suppressor_api",
+    "transient:transient_suppressor_impl",
+  ]
+}
+
 rtc_source_set("rms_level") {
   visibility = [ "*" ]
   sources = [
@@ -421,6 +432,7 @@ if (rtc_include_tests) {
         ":audioproc_protobuf_utils",
         ":audioproc_test_utils",
         ":audioproc_unittest_proto",
+        ":optionally_built_submodule_creators",
         ":rms_level",
         ":runtime_settings_protobuf_utils",
         "../../api/audio:audio_frame_api",

modules/audio_processing/audio_processing_impl.cc

@@ -27,7 +27,7 @@
 #include "modules/audio_processing/common.h"
 #include "modules/audio_processing/include/audio_frame_view.h"
 #include "modules/audio_processing/logging/apm_data_dumper.h"
-#include "modules/audio_processing/transient/transient_suppressor_creator.h"
+#include "modules/audio_processing/optionally_built_submodule_creators.h"
 #include "rtc_base/atomic_ops.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/constructor_magic.h"
@@ -640,6 +640,12 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) {
 // TODO(webrtc:5298): Remove.
 void AudioProcessingImpl::SetExtraOptions(const webrtc::Config& config) {}
 
+void AudioProcessingImpl::OverrideSubmoduleCreationForTesting(
+    const ApmSubmoduleCreationOverrides& overrides) {
+  rtc::CritScope cs(&crit_capture_);
+  submodule_creation_overrides_ = overrides;
+}
+
 int AudioProcessingImpl::proc_sample_rate_hz() const {
   // Used as callback from submodules, hence locking is not allowed.
   return capture_nonlocked_.capture_processing_format.sample_rate_hz();
@@ -1588,7 +1594,8 @@ void AudioProcessingImpl::InitializeTransientSuppressor() {
   if (config_.transient_suppression.enabled) {
     // Attempt to create a transient suppressor, if one is not already created.
     if (!submodules_.transient_suppressor) {
-      submodules_.transient_suppressor = CreateTransientSuppressor();
+      submodules_.transient_suppressor =
+          CreateTransientSuppressor(submodule_creation_overrides_);
     }
     if (submodules_.transient_suppressor) {
       submodules_.transient_suppressor->Initialize(

modules/audio_processing/audio_processing_impl.h

@@ -30,6 +30,7 @@
 #include "modules/audio_processing/include/audio_processing_statistics.h"
 #include "modules/audio_processing/level_estimator.h"
 #include "modules/audio_processing/ns/noise_suppressor.h"
+#include "modules/audio_processing/optionally_built_submodule_creators.h"
 #include "modules/audio_processing/render_queue_item_verifier.h"
 #include "modules/audio_processing/residual_echo_detector.h"
 #include "modules/audio_processing/rms_level.h"
@@ -141,6 +142,15 @@ class AudioProcessingImpl : public AudioProcessing {
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, DefaultBehavior);
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, ValidConfigBehavior);
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, InValidConfigBehavior);
+  FRIEND_TEST_ALL_PREFIXES(ApmWithSubmodulesExcludedTest,
+                           ToggleTransientSuppressor);
+  FRIEND_TEST_ALL_PREFIXES(ApmWithSubmodulesExcludedTest,
+                           ReinitializeTransientSuppressor);
+  FRIEND_TEST_ALL_PREFIXES(ApmWithSubmodulesExcludedTest,
+                           BitexactWithDisabledModules);
+
+  void OverrideSubmoduleCreationForTesting(
+      const ApmSubmoduleCreationOverrides& overrides);
 
   // Class providing thread-safe message pipe functionality for
   // |runtime_settings_|.
@@ -331,6 +341,10 @@ class AudioProcessingImpl : public AudioProcessing {
   // Struct containing the Config specifying the behavior of APM.
   AudioProcessing::Config config_;
 
+  // Overrides for testing the exclusion of some submodules from the build.
+  ApmSubmoduleCreationOverrides submodule_creation_overrides_
+      RTC_GUARDED_BY(crit_capture_);
+
   // Class containing information about what submodules are active.
   SubmoduleStates submodule_states_;
 

modules/audio_processing/audio_processing_impl_unittest.cc

@@ -15,10 +15,13 @@
 
 #include "api/scoped_refptr.h"
 #include "modules/audio_processing/include/audio_processing.h"
+#include "modules/audio_processing/optionally_built_submodule_creators.h"
 #include "modules/audio_processing/test/audio_processing_builder_for_testing.h"
+#include "modules/audio_processing/test/echo_canceller_test_tools.h"
 #include "modules/audio_processing/test/echo_control_mock.h"
 #include "modules/audio_processing/test/test_utils.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/random.h"
 #include "rtc_base/ref_counted_object.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
@@ -406,4 +409,166 @@ TEST(AudioProcessingImplTest, RenderPreProcessorBeforeEchoDetector) {
             test_echo_detector->last_render_audio_first_sample());
 }
 
+// Disabling build-optional submodules and trying to enable them via the APM
+// config should be bit-exact with running APM with said submodules disabled.
+// This mainly tests that SetCreateOptionalSubmodulesForTesting has an effect.
+TEST(ApmWithSubmodulesExcludedTest, BitexactWithDisabledModules) {
+  rtc::scoped_refptr<AudioProcessingImpl> apm =
+      new rtc::RefCountedObject<AudioProcessingImpl>(webrtc::Config());
+  ASSERT_EQ(apm->Initialize(), AudioProcessing::kNoError);
+
+  ApmSubmoduleCreationOverrides overrides;
+  overrides.transient_suppression = true;
+  apm->OverrideSubmoduleCreationForTesting(overrides);
+
+  AudioProcessing::Config apm_config = apm->GetConfig();
+  apm_config.transient_suppression.enabled = true;
+  apm->ApplyConfig(apm_config);
+
+  rtc::scoped_refptr<AudioProcessing> apm_reference =
+      AudioProcessingBuilder().Create();
+  apm_config = apm_reference->GetConfig();
+  apm_config.transient_suppression.enabled = false;
+  apm_reference->ApplyConfig(apm_config);
+
+  constexpr int kSampleRateHz = 16000;
+  constexpr int kNumChannels = 1;
+  std::array<float, kSampleRateHz / 100> buffer;
+  std::array<float, kSampleRateHz / 100> buffer_reference;
+  float* channel_pointers[] = {buffer.data()};
+  float* channel_pointers_reference[] = {buffer_reference.data()};
+  StreamConfig stream_config(/*sample_rate_hz=*/kSampleRateHz,
+                             /*num_channels=*/kNumChannels,
+                             /*has_keyboard=*/false);
+  Random random_generator(2341U);
+  constexpr int kFramesToProcessPerConfiguration = 10;
+
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    std::copy(buffer.begin(), buffer.end(), buffer_reference.begin());
+    ASSERT_EQ(apm->ProcessStream(channel_pointers, stream_config, stream_config,
+                                 channel_pointers),
+              kNoErr);
+    ASSERT_EQ(
+        apm_reference->ProcessStream(channel_pointers_reference, stream_config,
+                                     stream_config, channel_pointers_reference),
+        kNoErr);
+    for (int j = 0; j < kSampleRateHz / 100; ++j) {
+      EXPECT_EQ(buffer[j], buffer_reference[j]);
+    }
+  }
+}
+
+// Disable transient suppressor creation and run APM in ways that should trigger
+// calls to the transient suppressor API.
+TEST(ApmWithSubmodulesExcludedTest, ReinitializeTransientSuppressor) {
+  rtc::scoped_refptr<AudioProcessingImpl> apm =
+      new rtc::RefCountedObject<AudioProcessingImpl>(webrtc::Config());
+  ASSERT_EQ(apm->Initialize(), kNoErr);
+
+  ApmSubmoduleCreationOverrides overrides;
+  overrides.transient_suppression = true;
+  apm->OverrideSubmoduleCreationForTesting(overrides);
+
+  AudioProcessing::Config config = apm->GetConfig();
+  config.transient_suppression.enabled = true;
+  apm->ApplyConfig(config);
+  // 960 samples per frame: 10 ms of <= 48 kHz audio with <= 2 channels.
+  float buffer[960];
+  float* channel_pointers[] = {&buffer[0], &buffer[480]};
+  Random random_generator(2341U);
+  constexpr int kFramesToProcessPerConfiguration = 3;
+
+  StreamConfig initial_stream_config(/*sample_rate_hz=*/16000,
+                                     /*num_channels=*/1,
+                                     /*has_keyboard=*/false);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, initial_stream_config,
+                                 initial_stream_config, channel_pointers),
+              kNoErr);
+  }
+
+  StreamConfig stereo_stream_config(/*sample_rate_hz=*/16000,
+                                    /*num_channels=*/2,
+                                    /*has_keyboard=*/false);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, stereo_stream_config,
+                                 stereo_stream_config, channel_pointers),
+              kNoErr);
+  }
+
+  StreamConfig high_sample_rate_stream_config(/*sample_rate_hz=*/48000,
+                                              /*num_channels=*/1,
+                                              /*has_keyboard=*/false);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(
+        apm->ProcessStream(channel_pointers, high_sample_rate_stream_config,
+                           high_sample_rate_stream_config, channel_pointers),
+        kNoErr);
+  }
+
+  StreamConfig keyboard_stream_config(/*sample_rate_hz=*/16000,
+                                      /*num_channels=*/1,
+                                      /*has_keyboard=*/true);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, keyboard_stream_config,
+                                 keyboard_stream_config, channel_pointers),
+              kNoErr);
+  }
+}
+
+// Disable transient suppressor creation and run APM in ways that should trigger
+// calls to the transient suppressor API.
+TEST(ApmWithSubmodulesExcludedTest, ToggleTransientSuppressor) {
+  rtc::scoped_refptr<AudioProcessingImpl> apm =
+      new rtc::RefCountedObject<AudioProcessingImpl>(webrtc::Config());
+  ASSERT_EQ(apm->Initialize(), AudioProcessing::kNoError);
+
+  ApmSubmoduleCreationOverrides overrides;
+  overrides.transient_suppression = true;
+  apm->OverrideSubmoduleCreationForTesting(overrides);
+
+  //  960 samples per frame: 10 ms of <= 48 kHz audio with <= 2 channels.
+  float buffer[960];
+  float* channel_pointers[] = {&buffer[0], &buffer[480]};
+  Random random_generator(2341U);
+  constexpr int kFramesToProcessPerConfiguration = 3;
+  StreamConfig stream_config(/*sample_rate_hz=*/16000,
+                             /*num_channels=*/1,
+                             /*has_keyboard=*/false);
+
+  AudioProcessing::Config config = apm->GetConfig();
+  config.transient_suppression.enabled = true;
+  apm->ApplyConfig(config);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, stream_config, stream_config,
+                                 channel_pointers),
+              kNoErr);
+  }
+
+  config = apm->GetConfig();
+  config.transient_suppression.enabled = false;
+  apm->ApplyConfig(config);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, stream_config, stream_config,
+                                 channel_pointers),
+              kNoErr);
+  }
+
+  config = apm->GetConfig();
+  config.transient_suppression.enabled = true;
+  apm->ApplyConfig(config);
+  for (int i = 0; i < kFramesToProcessPerConfiguration; ++i) {
+    RandomizeSampleVector(&random_generator, buffer);
+    EXPECT_EQ(apm->ProcessStream(channel_pointers, stream_config, stream_config,
+                                 channel_pointers),
+              kNoErr);
+  }
+}
 }  // namespace webrtc

modules/audio_processing/optionally_built_submodule_creators.cc

@@ -8,7 +8,7 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
-#include "modules/audio_processing/transient/transient_suppressor_creator.h"
+#include "modules/audio_processing/optionally_built_submodule_creators.h"
 
 #include <memory>
 
@@ -16,10 +16,14 @@
 
 namespace webrtc {
 
-std::unique_ptr<TransientSuppressor> CreateTransientSuppressor() {
+std::unique_ptr<TransientSuppressor> CreateTransientSuppressor(
+    const ApmSubmoduleCreationOverrides& overrides) {
 #ifdef WEBRTC_EXCLUDE_TRANSIENT_SUPPRESSOR
   return nullptr;
 #else
+  if (overrides.transient_suppression) {
+    return nullptr;
+  }
   return std::make_unique<TransientSuppressorImpl>();
 #endif
 }

modules/audio_processing/optionally_built_submodule_creators.h

@@ -0,0 +1,38 @@
+/*
+ *  Copyright (c) 2020 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.
+ */
+
+#ifndef MODULES_AUDIO_PROCESSING_OPTIONALLY_BUILT_SUBMODULE_CREATORS_H_
+#define MODULES_AUDIO_PROCESSING_OPTIONALLY_BUILT_SUBMODULE_CREATORS_H_
+
+#include <memory>
+
+#include "modules/audio_processing/transient/transient_suppressor.h"
+
+namespace webrtc {
+
+// These overrides are only to be used for testing purposes.
+// Each flag emulates a preprocessor macro to exclude a submodule of APM from
+// the build, e.g. WEBRTC_EXCLUDE_TRANSIENT_SUPPRESSOR. If the corresponding
+// flag |transient_suppression| is enabled, then the creators will return
+// nullptr instead of a submodule instance, as if the macro had been defined.
+struct ApmSubmoduleCreationOverrides {
+  bool transient_suppression = false;
+};
+
+// Creates a transient suppressor.
+// Will instead return nullptr if one of the following is true:
+// * WEBRTC_EXCLUDE_TRANSIENT_SUPPRESSOR is defined
+// * The corresponding override in |overrides| is enabled.
+std::unique_ptr<TransientSuppressor> CreateTransientSuppressor(
+    const ApmSubmoduleCreationOverrides& overrides);
+
+}  // namespace webrtc
+
+#endif  // MODULES_AUDIO_PROCESSING_OPTIONALLY_BUILT_SUBMODULE_CREATORS_H_

modules/audio_processing/transient/BUILD.gn

@@ -12,20 +12,9 @@ rtc_source_set("transient_suppressor_api") {
   sources = [ "transient_suppressor.h" ]
 }
 
-rtc_library("transient_suppressor_creator") {
-  sources = [
-    "transient_suppressor_creator.cc",
-    "transient_suppressor_creator.h",
-  ]
-  deps = [
-    ":transient_suppressor_api",
-    ":transient_suppressor_impl",
-  ]
-}
-
 rtc_library("transient_suppressor_impl") {
   visibility = [
-    ":transient_suppressor_creator",
+    "..:optionally_built_submodule_creators",
     ":transient_suppression_test",
     ":transient_suppression_unittests",
     ":click_annotate",

modules/audio_processing/transient/transient_suppressor_creator.h

@@ -1,26 +0,0 @@
-/*
- *  Copyright (c) 2020 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.
- */
-
-#ifndef MODULES_AUDIO_PROCESSING_TRANSIENT_TRANSIENT_SUPPRESSOR_CREATOR_H_
-#define MODULES_AUDIO_PROCESSING_TRANSIENT_TRANSIENT_SUPPRESSOR_CREATOR_H_
-
-#include <memory>
-
-#include "modules/audio_processing/transient/transient_suppressor.h"
-
-namespace webrtc {
-
-// Creates a transient suppressor.
-// Will return nullptr if WEBRTC_EXCLUDE_TRANSIENT_SUPPRESSOR is defined.
-std::unique_ptr<TransientSuppressor> CreateTransientSuppressor();
-
-}  // namespace webrtc
-
-#endif  // MODULES_AUDIO_PROCESSING_TRANSIENT_TRANSIENT_SUPPRESSOR_CREATOR_H_