From 2dc95ba29953731ca078c62a401873c6f3d53da8 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 15 Oct 2024 12:09:49 +0200 Subject: [PATCH] Add BuiltinAudioProcessingFactory Its implementation is a copy of the AudioProcessingBuilder with intention to replace all usage of AudioProcessingBuilder with the BuiltingAudioProcessingFactory and thus get Environment with propagated field trials available for AudioProcessingImpl at construction. Bug: webrtc:369904700 Change-Id: Iee0eb112dd579402fcd5be56bf1054946179d1fb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365582 Reviewed-by: Harald Alvestrand Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#43242} --- BUILD.gn | 2 +- api/BUILD.gn | 1 + api/audio/BUILD.gn | 36 +++++++ api/audio/audio_processing.h | 2 + api/audio/builtin_audio_processing_factory.cc | 41 ++++++++ api/audio/builtin_audio_processing_factory.h | 98 +++++++++++++++++++ ...iltin_audio_processing_factory_unittest.cc | 48 +++++++++ api/create_peerconnection_factory.cc | 6 +- examples/BUILD.gn | 1 + .../objcnativeapi/objc/objc_call_client.mm | 3 +- modules/audio_processing/BUILD.gn | 2 + .../audio_processing_impl_unittest.cc | 51 +++++----- 12 files changed, 263 insertions(+), 28 deletions(-) create mode 100644 api/audio/builtin_audio_processing_factory.cc create mode 100644 api/audio/builtin_audio_processing_factory.h create mode 100644 api/audio/builtin_audio_processing_factory_unittest.cc diff --git a/BUILD.gn b/BUILD.gn index 98faecf7cd..9c42c12349 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -638,7 +638,7 @@ if (rtc_include_tests && !build_with_chromium) { deps = [ "api:compile_all_headers", "api:rtc_api_unittests", - "api/audio/test:audio_api_unittests", + "api/audio:audio_api_unittests", "api/audio_codecs/test:audio_codecs_api_unittests", "api/numerics:numerics_unittests", "api/task_queue:pending_task_safety_flag_unittests", diff --git a/api/BUILD.gn b/api/BUILD.gn index 1e4d3001cd..6c3b397f65 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -99,6 +99,7 @@ if (!build_with_chromium) { "audio:audio_device", "audio:audio_mixer_api", "audio:audio_processing", + "audio:builtin_audio_processing_factory", "audio_codecs:audio_codecs_api", "video_codecs:video_codecs_api", ] diff --git a/api/audio/BUILD.gn b/api/audio/BUILD.gn index 1cede93ae1..710c490948 100644 --- a/api/audio/BUILD.gn +++ b/api/audio/BUILD.gn @@ -85,6 +85,26 @@ rtc_source_set("audio_processing") { ] } +rtc_library("builtin_audio_processing_factory") { + visibility = [ "*" ] + configs += [ "../../modules/audio_processing:apm_debug_dump" ] + sources = [ + "builtin_audio_processing_factory.cc", + "builtin_audio_processing_factory.h", + ] + deps = [ + ":audio_processing", + ":echo_control", + "..:make_ref_counted", + "..:scoped_refptr", + "../../modules/audio_processing", + "../../rtc_base:logging", + "../../rtc_base/system:rtc_export", + "../environment", + "//third_party/abseil-cpp/absl/base:nullability", + ] +} + rtc_source_set("audio_processing_statistics") { visibility = [ "*" ] sources = [ @@ -143,3 +163,19 @@ rtc_source_set("echo_detector_creator") { "../../modules/audio_processing:residual_echo_detector", ] } + +if (rtc_include_tests) { + rtc_library("audio_api_unittests") { + testonly = true + sources = [ "builtin_audio_processing_factory_unittest.cc" ] + deps = [ + ":audio_processing", + ":builtin_audio_processing_factory", + "..:scoped_refptr", + "../../test:test_support", + "../environment", + "../environment:environment_factory", + "test:audio_api_unittests", + ] + } +} diff --git a/api/audio/audio_processing.h b/api/audio/audio_processing.h index ae600dfc9a..a9a82d91a4 100644 --- a/api/audio/audio_processing.h +++ b/api/audio/audio_processing.h @@ -771,6 +771,8 @@ class CustomProcessing { virtual ~CustomProcessing() {} }; +// TODO: bugs.webrtc.org/369904700 - Deprecate and remove in favor of the +// BuiltinAudioProcessingFactory. class RTC_EXPORT AudioProcessingBuilder { public: AudioProcessingBuilder(); diff --git a/api/audio/builtin_audio_processing_factory.cc b/api/audio/builtin_audio_processing_factory.cc new file mode 100644 index 0000000000..810d3105ea --- /dev/null +++ b/api/audio/builtin_audio_processing_factory.cc @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2024 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 "api/audio/builtin_audio_processing_factory.h" + +#include + +#include "absl/base/nullability.h" +#include "api/audio/audio_processing.h" +#include "api/environment/environment.h" +#include "api/make_ref_counted.h" +#include "api/scoped_refptr.h" +#include "modules/audio_processing/audio_processing_impl.h" +#include "rtc_base/logging.h" + +namespace webrtc { + +absl::Nullable> +BuiltinAudioProcessingFactory::Create(const Environment& /*env*/) { + if (called_create_) { + RTC_DLOG(LS_ERROR) + << "Calling BuiltinAudioProcessingFactory::Create more than once " + "is currently unsupported."; + } + called_create_ = true; + + // TODO: bugs.webrtc.org/369904700 - Pass `env` when AudioProcessingImpl gets + // constructor that accepts it. + return make_ref_counted( + config_, std::move(capture_post_processing_), + std::move(render_pre_processing_), std::move(echo_control_factory_), + std::move(echo_detector_), std::move(capture_analyzer_)); +} + +} // namespace webrtc diff --git a/api/audio/builtin_audio_processing_factory.h b/api/audio/builtin_audio_processing_factory.h new file mode 100644 index 0000000000..041c801260 --- /dev/null +++ b/api/audio/builtin_audio_processing_factory.h @@ -0,0 +1,98 @@ +/* + * Copyright (c) 2024 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 API_AUDIO_BUILTIN_AUDIO_PROCESSING_FACTORY_H_ +#define API_AUDIO_BUILTIN_AUDIO_PROCESSING_FACTORY_H_ + +#include +#include + +#include "absl/base/nullability.h" +#include "api/audio/audio_processing.h" +#include "api/audio/echo_control.h" +#include "api/environment/environment.h" +#include "api/scoped_refptr.h" +#include "rtc_base/system/rtc_export.h" + +namespace webrtc { + +class RTC_EXPORT BuiltinAudioProcessingFactory : public AudioProcessingFactory { + public: + BuiltinAudioProcessingFactory() = default; + explicit BuiltinAudioProcessingFactory(const AudioProcessing::Config& config) + : config_(config) {} + BuiltinAudioProcessingFactory(const BuiltinAudioProcessingFactory&) = delete; + BuiltinAudioProcessingFactory& operator=( + const BuiltinAudioProcessingFactory&) = delete; + ~BuiltinAudioProcessingFactory() override = default; + + // Sets the APM configuration. + BuiltinAudioProcessingFactory& SetConfig( + const AudioProcessing::Config& config) { + config_ = config; + return *this; + } + + // Sets the echo controller factory to inject when APM is created. + BuiltinAudioProcessingFactory& SetEchoControlFactory( + std::unique_ptr echo_control_factory) { + echo_control_factory_ = std::move(echo_control_factory); + return *this; + } + + // Sets the capture post-processing sub-module to inject when APM is created. + BuiltinAudioProcessingFactory& SetCapturePostProcessing( + std::unique_ptr capture_post_processing) { + capture_post_processing_ = std::move(capture_post_processing); + return *this; + } + + // Sets the render pre-processing sub-module to inject when APM is created. + BuiltinAudioProcessingFactory& SetRenderPreProcessing( + std::unique_ptr render_pre_processing) { + render_pre_processing_ = std::move(render_pre_processing); + return *this; + } + + // Sets the echo detector to inject when APM is created. + BuiltinAudioProcessingFactory& SetEchoDetector( + rtc::scoped_refptr echo_detector) { + echo_detector_ = std::move(echo_detector); + return *this; + } + + // Sets the capture analyzer sub-module to inject when APM is created. + BuiltinAudioProcessingFactory& SetCaptureAnalyzer( + std::unique_ptr capture_analyzer) { + capture_analyzer_ = std::move(capture_analyzer); + return *this; + } + + // Creates an APM instance with the specified config or the default one if + // unspecified. Injects the specified components transferring the ownership + // to the newly created APM instance. This implementation of the + // AudioProcessingFactory interface is not designed to be used more than once. + // Calling `Create` second time would return an unspecified object. + absl::Nullable> Create( + const Environment& env) override; + + private: + bool called_create_ = false; + AudioProcessing::Config config_; + std::unique_ptr echo_control_factory_; + std::unique_ptr capture_post_processing_; + std::unique_ptr render_pre_processing_; + scoped_refptr echo_detector_; + std::unique_ptr capture_analyzer_; +}; + +} // namespace webrtc + +#endif // API_AUDIO_BUILTIN_AUDIO_PROCESSING_FACTORY_H_ diff --git a/api/audio/builtin_audio_processing_factory_unittest.cc b/api/audio/builtin_audio_processing_factory_unittest.cc new file mode 100644 index 0000000000..a611213350 --- /dev/null +++ b/api/audio/builtin_audio_processing_factory_unittest.cc @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2024 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 "api/audio/builtin_audio_processing_factory.h" + +#include "api/audio/audio_processing.h" +#include "api/environment/environment.h" +#include "api/environment/environment_factory.h" +#include "api/scoped_refptr.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { + +using ::testing::NotNull; + +TEST(BuiltinAudioProcessingFactoryTest, CreatesWithDefaults) { + EXPECT_THAT(BuiltinAudioProcessingFactory().Create(CreateEnvironment()), + NotNull()); +} + +TEST(BuiltinAudioProcessingFactoryTest, CreatesWithConfig) { + const Environment env = CreateEnvironment(); + AudioProcessing::Config config; + // Change a field to make config different to default one. + config.gain_controller1.enabled = !config.gain_controller1.enabled; + + scoped_refptr ap1 = + BuiltinAudioProcessingFactory(config).Create(env); + ASSERT_THAT(ap1, NotNull()); + EXPECT_EQ(ap1->GetConfig().gain_controller1.enabled, + config.gain_controller1.enabled); + + scoped_refptr ap2 = + BuiltinAudioProcessingFactory().SetConfig(config).Create(env); + ASSERT_THAT(ap2, NotNull()); + EXPECT_EQ(ap2->GetConfig().gain_controller1.enabled, + config.gain_controller1.enabled); +} + +} // namespace webrtc diff --git a/api/create_peerconnection_factory.cc b/api/create_peerconnection_factory.cc index 310b58d4a1..888d1e8a4e 100644 --- a/api/create_peerconnection_factory.cc +++ b/api/create_peerconnection_factory.cc @@ -16,6 +16,7 @@ #include "api/audio/audio_device.h" #include "api/audio/audio_mixer.h" #include "api/audio/audio_processing.h" +#include "api/audio/builtin_audio_processing_factory.h" #include "api/audio_codecs/audio_decoder_factory.h" #include "api/audio_codecs/audio_encoder_factory.h" #include "api/enable_media.h" @@ -60,7 +61,10 @@ rtc::scoped_refptr CreatePeerConnectionFactory( if (audio_processing) { dependencies.audio_processing = std::move(audio_processing); } else { - dependencies.audio_processing = AudioProcessingBuilder().Create(); +#ifndef WEBRTC_EXCLUDE_AUDIO_PROCESSING_MODULE + dependencies.audio_processing_factory = + std::make_unique(); +#endif } dependencies.audio_mixer = std::move(audio_mixer); dependencies.video_encoder_factory = std::move(video_encoder_factory); diff --git a/examples/BUILD.gn b/examples/BUILD.gn index a0dbdcc154..c6bb616df4 100644 --- a/examples/BUILD.gn +++ b/examples/BUILD.gn @@ -490,6 +490,7 @@ if (is_ios || (is_mac && target_cpu != "x86")) { "../api:scoped_refptr", "../api:sequence_checker", "../api/audio:audio_processing", + "../api/audio:builtin_audio_processing_factory", "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", "../api/rtc_event_log:rtc_event_log_factory", diff --git a/examples/objcnativeapi/objc/objc_call_client.mm b/examples/objcnativeapi/objc/objc_call_client.mm index a794f96628..c9c365f01f 100644 --- a/examples/objcnativeapi/objc/objc_call_client.mm +++ b/examples/objcnativeapi/objc/objc_call_client.mm @@ -19,6 +19,7 @@ #import "sdk/objc/helpers/RTCCameraPreviewView.h" #include "api/audio/audio_processing.h" +#include "api/audio/builtin_audio_processing_factory.h" #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/enable_media.h" @@ -124,7 +125,7 @@ void ObjCCallClient::CreatePeerConnectionFactory() { [[RTC_OBJC_TYPE(RTCDefaultVideoEncoderFactory) alloc] init]); dependencies.video_decoder_factory = webrtc::ObjCToNativeVideoDecoderFactory( [[RTC_OBJC_TYPE(RTCDefaultVideoDecoderFactory) alloc] init]); - dependencies.audio_processing = webrtc::AudioProcessingBuilder().Create(); + dependencies.audio_processing_factory = std::make_unique(); webrtc::EnableMedia(dependencies); dependencies.event_log_factory = std::make_unique(); pcf_ = webrtc::CreateModularPeerConnectionFactory(std::move(dependencies)); diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index f76d235add..6973dca152 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -347,7 +347,9 @@ if (rtc_include_tests) { "../../api/audio:aec3_factory", "../../api/audio:audio_frame_api", "../../api/audio:audio_processing", + "../../api/audio:builtin_audio_processing_factory", "../../api/audio:echo_detector_creator", + "../../api/environment:environment_factory", "../../common_audio", "../../common_audio:common_audio_c", "../../rtc_base:checks", diff --git a/modules/audio_processing/audio_processing_impl_unittest.cc b/modules/audio_processing/audio_processing_impl_unittest.cc index 5b00410fbb..f405f38cda 100644 --- a/modules/audio_processing/audio_processing_impl_unittest.cc +++ b/modules/audio_processing/audio_processing_impl_unittest.cc @@ -17,9 +17,10 @@ #include #include "api/audio/audio_processing.h" +#include "api/audio/builtin_audio_processing_factory.h" +#include "api/environment/environment_factory.h" #include "api/make_ref_counted.h" #include "api/scoped_refptr.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" @@ -198,8 +199,8 @@ TEST(AudioProcessingImplTest, AudioParameterChangeTriggersInit) { } TEST(AudioProcessingImplTest, UpdateCapturePreGainRuntimeSetting) { - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting().Create(); + scoped_refptr apm = + BuiltinAudioProcessingFactory().Create(CreateEnvironment()); webrtc::AudioProcessing::Config apm_config; apm_config.pre_amplifier.enabled = true; apm_config.pre_amplifier.fixed_gain_factor = 1.f; @@ -231,8 +232,8 @@ TEST(AudioProcessingImplTest, UpdateCapturePreGainRuntimeSetting) { TEST(AudioProcessingImplTest, LevelAdjustmentUpdateCapturePreGainRuntimeSetting) { - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting().Create(); + scoped_refptr apm = + BuiltinAudioProcessingFactory().Create(CreateEnvironment()); webrtc::AudioProcessing::Config apm_config; apm_config.capture_level_adjustment.enabled = true; apm_config.capture_level_adjustment.pre_gain_factor = 1.f; @@ -264,8 +265,8 @@ TEST(AudioProcessingImplTest, TEST(AudioProcessingImplTest, LevelAdjustmentUpdateCapturePostGainRuntimeSetting) { - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting().Create(); + scoped_refptr apm = + BuiltinAudioProcessingFactory().Create(CreateEnvironment()); webrtc::AudioProcessing::Config apm_config; apm_config.capture_level_adjustment.enabled = true; apm_config.capture_level_adjustment.post_gain_factor = 1.f; @@ -302,10 +303,10 @@ TEST(AudioProcessingImplTest, EchoControllerObservesSetCaptureUsageChange) { const MockEchoControlFactory* echo_control_factory_ptr = echo_control_factory.get(); - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoControlFactory(std::move(echo_control_factory)) - .Create(); + .Create(CreateEnvironment()); constexpr int16_t kAudioLevel = 10000; constexpr int kSampleRateHz = 48000; @@ -384,10 +385,10 @@ TEST(AudioProcessingImplTest, auto echo_control_factory = std::make_unique(); const auto* echo_control_factory_ptr = echo_control_factory.get(); - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoControlFactory(std::move(echo_control_factory)) - .Create(); + .Create(CreateEnvironment()); // Disable AGC. webrtc::AudioProcessing::Config apm_config; apm_config.gain_controller1.enabled = false; @@ -427,10 +428,10 @@ TEST(AudioProcessingImplTest, auto echo_control_factory = std::make_unique(); const auto* echo_control_factory_ptr = echo_control_factory.get(); - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoControlFactory(std::move(echo_control_factory)) - .Create(); + .Create(CreateEnvironment()); // Disable AGC. webrtc::AudioProcessing::Config apm_config; apm_config.gain_controller1.enabled = false; @@ -470,10 +471,10 @@ TEST(AudioProcessingImplTest, auto echo_control_factory = std::make_unique(); const auto* echo_control_factory_ptr = echo_control_factory.get(); - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoControlFactory(std::move(echo_control_factory)) - .Create(); + .Create(CreateEnvironment()); webrtc::AudioProcessing::Config apm_config; // Enable AGC1. apm_config.gain_controller1.enabled = true; @@ -525,10 +526,10 @@ TEST(AudioProcessingImplTest, EchoControllerObservesPlayoutVolumeChange) { auto echo_control_factory = std::make_unique(); const auto* echo_control_factory_ptr = echo_control_factory.get(); - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoControlFactory(std::move(echo_control_factory)) - .Create(); + .Create(CreateEnvironment()); // Disable AGC. webrtc::AudioProcessing::Config apm_config; apm_config.gain_controller1.enabled = false; @@ -582,11 +583,11 @@ TEST(AudioProcessingImplTest, RenderPreProcessorBeforeEchoDetector) { std::unique_ptr test_render_pre_processor( new TestRenderPreProcessor()); // Create APM injecting the test echo detector and render pre-processor. - rtc::scoped_refptr apm = - AudioProcessingBuilderForTesting() + scoped_refptr apm = + BuiltinAudioProcessingFactory() .SetEchoDetector(test_echo_detector) .SetRenderPreProcessing(std::move(test_render_pre_processor)) - .Create(); + .Create(CreateEnvironment()); webrtc::AudioProcessing::Config apm_config; apm_config.pre_amplifier.enabled = true; apm->ApplyConfig(apm_config);