From b37e59d198add002a0776b6a44603f9846f65f56 Mon Sep 17 00:00:00 2001 From: Sam Zackrisson Date: Mon, 27 Apr 2020 08:39:33 +0200 Subject: [PATCH] Add unittests for APM with submodule creation disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Per Åhgren Cr-Commit-Position: refs/heads/master@{#31138} --- modules/audio_processing/BUILD.gn | 14 +- .../audio_processing/audio_processing_impl.cc | 11 +- .../audio_processing/audio_processing_impl.h | 14 ++ .../audio_processing_impl_unittest.cc | 165 ++++++++++++++++++ ...=> optionally_built_submodule_creators.cc} | 8 +- .../optionally_built_submodule_creators.h | 38 ++++ modules/audio_processing/transient/BUILD.gn | 13 +- .../transient/transient_suppressor_creator.h | 26 --- 8 files changed, 246 insertions(+), 43 deletions(-) rename modules/audio_processing/{transient/transient_suppressor_creator.cc => optionally_built_submodule_creators.cc} (72%) create mode 100644 modules/audio_processing/optionally_built_submodule_creators.h delete mode 100644 modules/audio_processing/transient/transient_suppressor_creator.h diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index 46207aa658..a305189bcf 100644 --- a/modules/audio_processing/BUILD.gn +++ b/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", diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index bdef059686..6a87891164 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 @@ 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( diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index 65ab5a60cd..54acee9302 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 @@ 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_; diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc index 3c5458d151..71352bc65a 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(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 apm = + new rtc::RefCountedObject(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 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 buffer; + std::array 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 apm = + new rtc::RefCountedObject(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 apm = + new rtc::RefCountedObject(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 b60c3f69e0..62a1632566 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 @@ -16,10 +16,14 @@ namespace webrtc { -std::unique_ptr CreateTransientSuppressor() { +std::unique_ptr CreateTransientSuppressor( + const ApmSubmoduleCreationOverrides& overrides) { #ifdef WEBRTC_EXCLUDE_TRANSIENT_SUPPRESSOR return nullptr; #else + if (overrides.transient_suppression) { + return nullptr; + } return std::make_unique(); #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 0000000000..c96e66f975 --- /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 + +#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 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 984ffbfc8d..13e319f88e 100644 --- a/modules/audio_processing/transient/BUILD.gn +++ b/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", 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 133b473127..0000000000 --- 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 - -#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 CreateTransientSuppressor(); - -} // namespace webrtc - -#endif // MODULES_AUDIO_PROCESSING_TRANSIENT_TRANSIENT_SUPPRESSOR_CREATOR_H_