Introduce AudioProcessingFactory interface

This interface allows to delegate construction of AudioProcessing to
the PeerConnectionFactory where it can provide propagated field trials

Bug: webrtc:369904700
Change-Id: Ie05cd771e4a869fa5f43173e127256800ae0727f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365320
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43233}

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",
   ]
 }
 

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",

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<scoped_refptr<AudioProcessing>> Create(
+      const Environment& env) = 0;
+};
+
 // Experimental interface for a custom analysis submodule.
 class CustomAudioAnalyzer {
  public:

api/enable_media.cc

@@ -13,6 +13,7 @@
 #include <memory>
 #include <utility>
 
+#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<MediaEngineInterface> CreateMediaEngine(
       const Environment& env,
       PeerConnectionFactoryDependencies& deps) override {
+    absl::Nullable<scoped_refptr<AudioProcessing>> audio_processing =
+        deps.audio_processing_factory != nullptr
+            ? deps.audio_processing_factory->Create(env)
+            : std::move(deps.audio_processing);
+
     auto audio_engine = std::make_unique<WebRtcVoiceEngine>(
         &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<WebRtcVideoEngine>(
         std::move(deps.video_encoder_factory),

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();
   }
 

api/peer_connection_interface.h

@@ -1465,7 +1465,10 @@ struct RTC_EXPORT PeerConnectionFactoryDependencies final {
   rtc::scoped_refptr<AudioEncoderFactory> audio_encoder_factory;
   rtc::scoped_refptr<AudioDecoderFactory> audio_decoder_factory;
   rtc::scoped_refptr<AudioMixer> audio_mixer;
+  // TODO: bugs.webrtc.org/369904700 - Deprecate `audio_processing` in favor
+  // of `audio_processing_factory`.
   rtc::scoped_refptr<AudioProcessing> audio_processing;
+  std::unique_ptr<AudioProcessingFactory> audio_processing_factory;
   std::unique_ptr<AudioFrameProcessor> audio_frame_processor;
   std::unique_ptr<VideoEncoderFactory> video_encoder_factory;
   std::unique_ptr<VideoDecoderFactory> video_decoder_factory;

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",

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<AudioProcessing>,
+              Create,
+              (const Environment&),
+              (override));
+};
+
 }  // namespace test
 }  // namespace webrtc
 

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",

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<MockAudioProcessingFactory>();
+  auto audio_processing = make_ref_counted<NiceMock<MockAudioProcessing>>();
+  // 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<FILE*>(), 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<PeerConnectionFactoryInterface> 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<NiceMock<MockAudioProcessing>>();
+  EXPECT_CALL(*audio_processing, CreateAndAttachAecDump(A<FILE*>(), 24'242, _));
+
+  PeerConnectionFactoryDependencies pcf_dependencies;
+  pcf_dependencies.adm = FakeAudioCaptureModule::Create();
+  pcf_dependencies.audio_processing = std::move(audio_processing);
+  EnableMediaWithDefaults(pcf_dependencies);
+
+  scoped_refptr<PeerConnectionFactoryInterface> pcf =
+      CreateModularPeerConnectionFactory(std::move(pcf_dependencies));
+  pcf->StartAecDump(nullptr, 24'242);
+}
+
 }  // namespace
 }  // namespace webrtc

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.