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}
diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn
index 46207aa..a305189 100644
--- a/modules/audio_processing/BUILD.gn
+++ b/modules/audio_processing/BUILD.gn
@@ -156,6 +156,7 @@
":audio_processing_statistics",
":config",
":high_pass_filter",
+ ":optionally_built_submodule_creators",
":rms_level",
":voice_detection",
"../../api:array_view",
@@ -187,7 +188,6 @@
"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("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 @@
":audioproc_protobuf_utils",
":audioproc_test_utils",
":audioproc_unittest_proto",
+ ":optionally_built_submodule_creators",
":rms_level",
":runtime_settings_protobuf_utils",
"../../api/audio:audio_frame_api",
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index bdef059..6a87891 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/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 @@
// 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 @@
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(
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index 65ab5a6..54acee9 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/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 @@
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 @@
// 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_;
diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc
index 3c5458d..71352bc 100644
--- a/modules/audio_processing/audio_processing_impl_unittest.cc
+++ b/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_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
diff --git a/modules/audio_processing/transient/transient_suppressor_creator.cc b/modules/audio_processing/optionally_built_submodule_creators.cc
similarity index 72%
rename from modules/audio_processing/transient/transient_suppressor_creator.cc
rename to modules/audio_processing/optionally_built_submodule_creators.cc
index b60c3f6..62a1632 100644
--- a/modules/audio_processing/transient/transient_suppressor_creator.cc
+++ b/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
}
diff --git a/modules/audio_processing/optionally_built_submodule_creators.h b/modules/audio_processing/optionally_built_submodule_creators.h
new file mode 100644
index 0000000..c96e66f
--- /dev/null
+++ b/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_
diff --git a/modules/audio_processing/transient/BUILD.gn b/modules/audio_processing/transient/BUILD.gn
index 984ffbf..13e319f 100644
--- a/modules/audio_processing/transient/BUILD.gn
+++ b/modules/audio_processing/transient/BUILD.gn
@@ -12,20 +12,9 @@
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",
diff --git a/modules/audio_processing/transient/transient_suppressor_creator.h b/modules/audio_processing/transient/transient_suppressor_creator.h
deleted file mode 100644
index 133b473..0000000
--- a/modules/audio_processing/transient/transient_suppressor_creator.h
+++ /dev/null
@@ -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_
