diff --git a/api/BUILD.gn b/api/BUILD.gn index a2e33ae8e7..1e4d3001cd 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -42,6 +42,7 @@ rtc_source_set("enable_media") { "../pc:media_factory", "../rtc_base/system:rtc_export", "environment", + "//third_party/abseil-cpp/absl/base:nullability", ] } diff --git a/api/audio/BUILD.gn b/api/audio/BUILD.gn index 22c3296ac5..1cede93ae1 100644 --- a/api/audio/BUILD.gn +++ b/api/audio/BUILD.gn @@ -78,6 +78,7 @@ rtc_source_set("audio_processing") { "../../rtc_base/system:arch", "../../rtc_base/system:file_wrapper", "../../rtc_base/system:rtc_export", + "../environment", "../task_queue", "//third_party/abseil-cpp/absl/base:nullability", "//third_party/abseil-cpp/absl/strings:string_view", diff --git a/api/audio/audio_processing.h b/api/audio/audio_processing.h index dca75f2174..ae600dfc9a 100644 --- a/api/audio/audio_processing.h +++ b/api/audio/audio_processing.h @@ -33,6 +33,7 @@ #include "api/array_view.h" #include "api/audio/audio_processing_statistics.h" #include "api/audio/echo_control.h" +#include "api/environment/environment.h" #include "api/ref_count.h" #include "api/scoped_refptr.h" #include "api/task_queue/task_queue_base.h" @@ -733,6 +734,14 @@ class RTC_EXPORT AudioProcessing : public RefCountInterface { static int GetFrameSize(int sample_rate_hz) { return sample_rate_hz / 100; } }; +class AudioProcessingFactory { + public: + virtual ~AudioProcessingFactory() = default; + + virtual absl::Nullable> Create( + const Environment& env) = 0; +}; + // Experimental interface for a custom analysis submodule. class CustomAudioAnalyzer { public: diff --git a/api/enable_media.cc b/api/enable_media.cc index 233de7910f..fc8ae38be2 100644 --- a/api/enable_media.cc +++ b/api/enable_media.cc @@ -13,6 +13,7 @@ #include #include +#include "absl/base/nullability.h" #include "api/environment/environment.h" #include "api/peer_connection_interface.h" #include "api/scoped_refptr.h" @@ -45,11 +46,16 @@ class MediaFactoryImpl : public MediaFactory { std::unique_ptr CreateMediaEngine( const Environment& env, PeerConnectionFactoryDependencies& deps) override { + absl::Nullable> audio_processing = + deps.audio_processing_factory != nullptr + ? deps.audio_processing_factory->Create(env) + : std::move(deps.audio_processing); + auto audio_engine = std::make_unique( &env.task_queue_factory(), deps.adm.get(), std::move(deps.audio_encoder_factory), std::move(deps.audio_decoder_factory), std::move(deps.audio_mixer), - std::move(deps.audio_processing), std::move(deps.audio_frame_processor), + std::move(audio_processing), std::move(deps.audio_frame_processor), env.field_trials()); auto video_engine = std::make_unique( std::move(deps.video_encoder_factory), diff --git a/api/enable_media_with_defaults.cc b/api/enable_media_with_defaults.cc index 7dccfa22f4..af8ab671df 100644 --- a/api/enable_media_with_defaults.cc +++ b/api/enable_media_with_defaults.cc @@ -34,7 +34,11 @@ void EnableMediaWithDefaults(PeerConnectionFactoryDependencies& deps) { if (deps.audio_decoder_factory == nullptr) { deps.audio_decoder_factory = CreateBuiltinAudioDecoderFactory(); } - if (deps.audio_processing == nullptr) { + if (deps.audio_processing == nullptr && + deps.audio_processing_factory == nullptr) { + // TODO: bugs.webrtc.org/369904700 - set `deps.audio_processing_factory` + // instead of `deps.audio_processing` when there is an implementation that + // can replace `AudioProcessingBuilder`. deps.audio_processing = AudioProcessingBuilder().Create(); } diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 2578523d4d..8082a5241d 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1465,7 +1465,10 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final { rtc::scoped_refptr audio_encoder_factory; rtc::scoped_refptr audio_decoder_factory; rtc::scoped_refptr audio_mixer; + // TODO: bugs.webrtc.org/369904700 - Deprecate `audio_processing` in favor + // of `audio_processing_factory`. rtc::scoped_refptr audio_processing; + std::unique_ptr audio_processing_factory; std::unique_ptr audio_frame_processor; std::unique_ptr video_encoder_factory; std::unique_ptr video_decoder_factory; diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index a9c8895cd4..f76d235add 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -289,8 +289,10 @@ if (rtc_include_tests) { ":aec_dump_interface", ":audio_buffer", ":audio_processing", + "../../api:scoped_refptr", "../../api/audio:audio_processing", "../../api/audio:audio_processing_statistics", + "../../api/environment", "../../api/task_queue", "../../test:test_support", "//third_party/abseil-cpp/absl/base:nullability", diff --git a/modules/audio_processing/include/mock_audio_processing.h b/modules/audio_processing/include/mock_audio_processing.h index f8c76bf3d9..eba59c05a9 100644 --- a/modules/audio_processing/include/mock_audio_processing.h +++ b/modules/audio_processing/include/mock_audio_processing.h @@ -17,6 +17,8 @@ #include "absl/strings/string_view.h" #include "api/audio/audio_processing.h" #include "api/audio/audio_processing_statistics.h" +#include "api/environment/environment.h" +#include "api/scoped_refptr.h" #include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/include/aec_dump.h" #include "test/gmock.h" @@ -174,6 +176,14 @@ class MockAudioProcessing : public AudioProcessing { MOCK_METHOD(AudioProcessing::Config, GetConfig, (), (const, override)); }; +class MockAudioProcessingFactory : public AudioProcessingFactory { + public: + MOCK_METHOD(scoped_refptr, + Create, + (const Environment&), + (override)); +}; + } // namespace test } // namespace webrtc diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 48e899b5dd..045ab81721 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2379,6 +2379,7 @@ if (rtc_include_tests && !build_with_chromium) { "../media:rtc_data_sctp_transport_internal", "../media:rtc_media_config", "../media:stream_params", + "../modules/audio_processing:mocks", "../modules/rtp_rtcp:rtp_rtcp_format", "../p2p:basic_port_allocator", "../p2p:connection", diff --git a/pc/peer_connection_factory_unittest.cc b/pc/peer_connection_factory_unittest.cc index a9fb648dc0..518ebeae4d 100644 --- a/pc/peer_connection_factory_unittest.cc +++ b/pc/peer_connection_factory_unittest.cc @@ -23,8 +23,10 @@ #include "api/create_peerconnection_factory.h" #include "api/data_channel_interface.h" #include "api/enable_media.h" +#include "api/enable_media_with_defaults.h" #include "api/environment/environment_factory.h" #include "api/jsep.h" +#include "api/make_ref_counted.h" #include "api/media_stream_interface.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/test/mock_packet_socket_factory.h" @@ -39,6 +41,7 @@ #include "api/video_codecs/video_encoder_factory_template_libvpx_vp9_adapter.h" #include "api/video_codecs/video_encoder_factory_template_open_h264_adapter.h" #include "media/base/fake_frame_source.h" +#include "modules/audio_processing/include/mock_audio_processing.h" #include "p2p/base/fake_port_allocator.h" #include "p2p/base/port.h" #include "p2p/base/port_allocator.h" @@ -65,11 +68,14 @@ namespace webrtc { namespace { using ::testing::_; +using ::testing::A; using ::testing::AtLeast; using ::testing::InvokeWithoutArgs; using ::testing::NiceMock; using ::testing::Return; using ::testing::UnorderedElementsAre; +using ::webrtc::test::MockAudioProcessing; +using ::webrtc::test::MockAudioProcessingFactory; static const char kStunIceServer[] = "stun:stun.l.google.com:19302"; static const char kTurnIceServer[] = "turn:test.com:1234"; @@ -718,5 +724,42 @@ TEST(PeerConnectionFactoryDependenciesTest, UsesPacketSocketFactory) { called.Wait(kWaitTimeout); } +TEST(PeerConnectionFactoryDependenciesTest, + CreatesAudioProcessingWithProvidedFactory) { + auto ap_factory = std::make_unique(); + auto audio_processing = make_ref_counted>(); + // Validate that provided audio_processing is used by expecting that a request + // to start AEC Dump with unnatural size limit is propagated to the + // `audio_processing`. + EXPECT_CALL(*audio_processing, CreateAndAttachAecDump(A(), 24'242, _)); + EXPECT_CALL(*ap_factory, Create).WillOnce(Return(audio_processing)); + + PeerConnectionFactoryDependencies pcf_dependencies; + pcf_dependencies.adm = FakeAudioCaptureModule::Create(); + pcf_dependencies.audio_processing_factory = std::move(ap_factory); + EnableMediaWithDefaults(pcf_dependencies); + + scoped_refptr pcf = + CreateModularPeerConnectionFactory(std::move(pcf_dependencies)); + pcf->StartAecDump(nullptr, 24'242); +} + +TEST(PeerConnectionFactoryDependenciesTest, UsesAudioProcessingWhenProvided) { + // Test legacy way of providing audio_processing. + // TODO: bugs.webrtc.org/369904700 - Delete this test when webrtc users no + // longer set PeerConnectionFactoryDependencies::audio_processing. + auto audio_processing = make_ref_counted>(); + EXPECT_CALL(*audio_processing, CreateAndAttachAecDump(A(), 24'242, _)); + + PeerConnectionFactoryDependencies pcf_dependencies; + pcf_dependencies.adm = FakeAudioCaptureModule::Create(); + pcf_dependencies.audio_processing = std::move(audio_processing); + EnableMediaWithDefaults(pcf_dependencies); + + scoped_refptr pcf = + CreateModularPeerConnectionFactory(std::move(pcf_dependencies)); + pcf->StartAecDump(nullptr, 24'242); +} + } // namespace } // namespace webrtc diff --git a/pc/test/integration_test_helpers.cc b/pc/test/integration_test_helpers.cc index 9a439f41d8..6503b27de3 100644 --- a/pc/test/integration_test_helpers.cc +++ b/pc/test/integration_test_helpers.cc @@ -235,6 +235,10 @@ bool PeerConnectionIntegrationWrapper::Init( pc_factory_dependencies.video_decoder_factory.reset(); } + // TODO: bugs.webrtc.org/369904700 - inject test specific + // audio_processing_factory right away when `EnableMediaWithDefault` would + // always keep audio_processing unchanged and thus can postpone + // AudioProcessing construction. if (!pc_factory_dependencies.audio_processing) { // If the standard Creation method for APM returns a null pointer, instead // use the builder for testing to create an APM object.