From 4fd1cc70da13b1ea6bab9bcca3115f7a32f4cf41 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 3 Nov 2023 12:51:31 +0100 Subject: [PATCH] Add EnableMediaWithDefaultsAndTimeController To replace CreateTimeControllerBasedCallFactory Update webrtc tests to use this new function Bug: webrtc:15574 Change-Id: I2b74cd930ecc4f72dd1e7aa853764ca298b66ad8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325527 Reviewed-by: Artem Titov Reviewed-by: Harald Alvestrand Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41076} --- api/BUILD.gn | 6 +++ api/enable_media.cc | 5 +++ api/test/create_time_controller.cc | 39 ++++++++++++++++ api/test/create_time_controller.h | 12 ++++- api/test/pclf/media_quality_test_params.h | 2 - test/pc/e2e/BUILD.gn | 3 +- test/pc/e2e/test_peer_factory.cc | 52 +++++++--------------- test/peer_scenario/peer_scenario_client.cc | 24 ++++------ 8 files changed, 85 insertions(+), 58 deletions(-) diff --git a/api/BUILD.gn b/api/BUILD.gn index 5bc33eb2bc..a163315440 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -1459,12 +1459,18 @@ if (rtc_include_tests) { deps = [ ":callfactory_api", + ":enable_media_with_defaults", + ":libjingle_peerconnection_api", ":time_controller", "../call", "../call:call_interfaces", "../call:rtp_interfaces", + "../pc:media_factory", + "../rtc_base:checks", + "../system_wrappers", "../test/time_controller", ] + absl_deps = [ "//third_party/abseil-cpp/absl/base:nullability" ] } rtc_library("rtc_api_unittests") { diff --git a/api/enable_media.cc b/api/enable_media.cc index 4c97acef56..ce1aa61a92 100644 --- a/api/enable_media.cc +++ b/api/enable_media.cc @@ -69,6 +69,11 @@ class MediaFactoryImpl : public MediaFactory { } // namespace void EnableMedia(PeerConnectionFactoryDependencies& deps) { + if (deps.media_factory != nullptr) { + // Do nothing if media is already enabled. Overwriting media_factory can be + // harmful when a different (e.g. test-only) implementation is used. + return; + } deps.media_factory = std::make_unique(); } diff --git a/api/test/create_time_controller.cc b/api/test/create_time_controller.cc index 2c356cb887..3378805088 100644 --- a/api/test/create_time_controller.cc +++ b/api/test/create_time_controller.cc @@ -11,10 +11,17 @@ #include "api/test/create_time_controller.h" #include +#include +#include "absl/base/nullability.h" +#include "api/enable_media_with_defaults.h" +#include "api/peer_connection_interface.h" #include "call/call.h" #include "call/rtp_transport_config.h" #include "call/rtp_transport_controller_send_factory_interface.h" +#include "pc/media_factory.h" +#include "rtc_base/checks.h" +#include "system_wrappers/include/clock.h" #include "test/time_controller/external_time_controller.h" #include "test/time_controller/simulated_time_controller.h" @@ -50,4 +57,36 @@ std::unique_ptr CreateTimeControllerBasedCallFactory( return std::make_unique(time_controller); } +void EnableMediaWithDefaultsAndTimeController( + TimeController& time_controller, + PeerConnectionFactoryDependencies& deps) { + class TimeControllerBasedFactory : public MediaFactory { + public: + TimeControllerBasedFactory( + absl::Nonnull clock, + absl::Nonnull> media_factory) + : clock_(clock), media_factory_(std::move(media_factory)) {} + + std::unique_ptr CreateCall(const CallConfig& config) override { + return Call::Create(config, clock_, + config.rtp_transport_controller_send_factory->Create( + config.ExtractTransportConfig(), clock_)); + } + + std::unique_ptr CreateMediaEngine( + PeerConnectionFactoryDependencies& dependencies) override { + return media_factory_->CreateMediaEngine(dependencies); + } + + private: + absl::Nonnull clock_; + absl::Nonnull> media_factory_; + }; + + EnableMediaWithDefaults(deps); + RTC_CHECK(deps.media_factory); + deps.media_factory = std::make_unique( + time_controller.GetClock(), std::move(deps.media_factory)); +} + } // namespace webrtc diff --git a/api/test/create_time_controller.h b/api/test/create_time_controller.h index e7bc9cb465..9d0e7dde21 100644 --- a/api/test/create_time_controller.h +++ b/api/test/create_time_controller.h @@ -13,6 +13,7 @@ #include #include "api/call/call_factory_interface.h" +#include "api/peer_connection_interface.h" #include "api/test/time_controller.h" namespace webrtc { @@ -26,8 +27,15 @@ std::unique_ptr CreateSimulatedTimeController(); // This is creates a call factory that creates Call instances that are backed by // a time controller. -std::unique_ptr CreateTimeControllerBasedCallFactory( - TimeController* time_controller); +[[deprecated("bugs.webrtc.org/15574")]] std::unique_ptr +CreateTimeControllerBasedCallFactory(TimeController* time_controller); + +// Adjusts media `deps` to use clock `time_controller` provides, fills media +// related dependencies, and enables media support for a PeerConnectionFactory +// created from `deps`. +void EnableMediaWithDefaultsAndTimeController( + TimeController& time_controller, + PeerConnectionFactoryDependencies& deps); } // namespace webrtc diff --git a/api/test/pclf/media_quality_test_params.h b/api/test/pclf/media_quality_test_params.h index b2ccdf18c5..aad04c3eb6 100644 --- a/api/test/pclf/media_quality_test_params.h +++ b/api/test/pclf/media_quality_test_params.h @@ -49,8 +49,6 @@ struct PeerConnectionFactoryComponents { std::unique_ptr network_controller_factory; std::unique_ptr neteq_factory; - // Will be passed to MediaEngineInterface, that will be used in - // PeerConnectionFactory. std::unique_ptr video_encoder_factory; std::unique_ptr video_decoder_factory; rtc::scoped_refptr audio_encoder_factory; diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 86a9110f58..9d1c1d437f 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -96,6 +96,7 @@ if (!build_with_chromium) { ":test_peer", "../..:copy_to_file_audio_capturer", "../../../api:create_time_controller", + "../../../api:enable_media_with_defaults", "../../../api:time_controller", "../../../api/rtc_event_log:rtc_event_log_factory", "../../../api/task_queue:default_task_queue_factory", @@ -105,8 +106,6 @@ if (!build_with_chromium) { "../../../api/transport:field_trial_based_config", "../../../api/video_codecs:builtin_video_decoder_factory", "../../../api/video_codecs:builtin_video_encoder_factory", - "../../../media:rtc_audio_video", - "../../../media:rtc_media_engine_defaults", "../../../modules/audio_device:test_audio_device_module", "../../../modules/audio_processing/aec_dump", "../../../p2p:rtc_p2p", diff --git a/test/pc/e2e/test_peer_factory.cc b/test/pc/e2e/test_peer_factory.cc index 281dac5c55..dd900027ee 100644 --- a/test/pc/e2e/test_peer_factory.cc +++ b/test/pc/e2e/test_peer_factory.cc @@ -13,6 +13,7 @@ #include "absl/memory/memory.h" #include "absl/strings/string_view.h" +#include "api/enable_media_with_defaults.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/test/create_time_controller.h" #include "api/test/pclf/media_configuration.h" @@ -21,8 +22,6 @@ #include "api/transport/field_trial_based_config.h" #include "api/video_codecs/builtin_video_decoder_factory.h" #include "api/video_codecs/builtin_video_encoder_factory.h" -#include "media/engine/webrtc_media_engine.h" -#include "media/engine/webrtc_media_engine_defaults.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "p2p/client/basic_port_allocator.h" #include "rtc_base/thread.h" @@ -146,30 +145,6 @@ rtc::scoped_refptr CreateAudioDeviceModule( std::move(renderer), /*speed=*/1.f); } -std::unique_ptr CreateMediaEngine( - TaskQueueFactory* task_queue_factory, - PeerConnectionFactoryComponents* pcf_dependencies, - rtc::scoped_refptr audio_device_module) { - cricket::MediaEngineDependencies media_deps; - media_deps.task_queue_factory = task_queue_factory; - media_deps.adm = audio_device_module; - media_deps.audio_processing = pcf_dependencies->audio_processing; - media_deps.audio_mixer = pcf_dependencies->audio_mixer; - media_deps.video_encoder_factory = - std::move(pcf_dependencies->video_encoder_factory); - media_deps.video_decoder_factory = - std::move(pcf_dependencies->video_decoder_factory); - media_deps.audio_encoder_factory = pcf_dependencies->audio_encoder_factory; - media_deps.audio_decoder_factory = pcf_dependencies->audio_decoder_factory; - // TODO(bugs.webrtc.org/15574): Migrate to enabling media with EnableMedia - // kind of helper when TimeController aware test helper is implemented. - webrtc::DeprecatedSetMediaEngineDefaults(&media_deps); - RTC_DCHECK(pcf_dependencies->trials); - media_deps.trials = pcf_dependencies->trials.get(); - - return cricket::CreateMediaEngine(std::move(media_deps)); -} - void WrapVideoEncoderFactory( absl::string_view peer_name, double bitrate_multiplier, @@ -208,7 +183,7 @@ void WrapVideoDecoderFactory( PeerConnectionFactoryDependencies CreatePCFDependencies( std::unique_ptr pcf_dependencies, TimeController& time_controller, - std::unique_ptr media_engine, + rtc::scoped_refptr audio_device_module, rtc::Thread* signaling_thread, rtc::Thread* worker_thread, rtc::Thread* network_thread) { @@ -216,10 +191,7 @@ PeerConnectionFactoryDependencies CreatePCFDependencies( pcf_deps.signaling_thread = signaling_thread; pcf_deps.worker_thread = worker_thread; pcf_deps.network_thread = network_thread; - pcf_deps.media_engine = std::move(media_engine); - pcf_deps.call_factory = - CreateTimeControllerBasedCallFactory(&time_controller); pcf_deps.event_log_factory = std::move(pcf_dependencies->event_log_factory); pcf_deps.task_queue_factory = time_controller.CreateTaskQueueFactory(); @@ -238,6 +210,18 @@ PeerConnectionFactoryDependencies CreatePCFDependencies( pcf_deps.trials = std::move(pcf_dependencies->trials); } + // Media dependencies + pcf_deps.adm = std::move(audio_device_module); + pcf_deps.audio_processing = pcf_dependencies->audio_processing; + pcf_deps.audio_mixer = pcf_dependencies->audio_mixer; + pcf_deps.video_encoder_factory = + std::move(pcf_dependencies->video_encoder_factory); + pcf_deps.video_decoder_factory = + std::move(pcf_dependencies->video_decoder_factory); + pcf_deps.audio_encoder_factory = pcf_dependencies->audio_encoder_factory; + pcf_deps.audio_decoder_factory = pcf_dependencies->audio_decoder_factory; + EnableMediaWithDefaultsAndTimeController(time_controller, pcf_deps); + return pcf_deps; } @@ -326,10 +310,6 @@ std::unique_ptr TestPeerFactory::CreateTestPeer( WrapVideoDecoderFactory(params->name.value(), components->pcf_dependencies.get(), video_analyzer_helper_); - std::unique_ptr media_engine = - CreateMediaEngine(time_controller_.GetTaskQueueFactory(), - components->pcf_dependencies.get(), - audio_device_module); std::unique_ptr owned_worker_thread = components->worker_thread != nullptr @@ -345,8 +325,8 @@ std::unique_ptr TestPeerFactory::CreateTestPeer( components->pcf_dependencies->audio_processing; PeerConnectionFactoryDependencies pcf_deps = CreatePCFDependencies( std::move(components->pcf_dependencies), time_controller_, - std::move(media_engine), signaling_thread_, components->worker_thread, - components->network_thread); + std::move(audio_device_module), signaling_thread_, + components->worker_thread, components->network_thread); rtc::scoped_refptr peer_connection_factory = CreateModularPeerConnectionFactory(std::move(pcf_deps)); diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc index 697bf055a7..60f2ea7f2e 100644 --- a/test/peer_scenario/peer_scenario_client.cc +++ b/test/peer_scenario/peer_scenario_client.cc @@ -246,17 +246,13 @@ PeerScenarioClient::PeerScenarioClient( pcf_deps.network_thread = manager->network_thread(); pcf_deps.signaling_thread = signaling_thread_; pcf_deps.worker_thread = worker_thread_.get(); - pcf_deps.call_factory = - CreateTimeControllerBasedCallFactory(net->time_controller()); pcf_deps.task_queue_factory = net->time_controller()->CreateTaskQueueFactory(); pcf_deps.event_log_factory = std::make_unique(task_queue_factory_); pcf_deps.trials = std::make_unique(); - cricket::MediaEngineDependencies media_deps; - media_deps.task_queue_factory = task_queue_factory_; - media_deps.adm = TestAudioDeviceModule::Create( + pcf_deps.adm = TestAudioDeviceModule::Create( task_queue_factory_, TestAudioDeviceModule::CreatePulsedNoiseCapturer( config.audio.pulsed_noise->amplitude * @@ -264,28 +260,24 @@ PeerScenarioClient::PeerScenarioClient( config.audio.sample_rate, config.audio.channels), TestAudioDeviceModule::CreateDiscardRenderer(config.audio.sample_rate)); - media_deps.audio_processing = AudioProcessingBuilder().Create(); if (config.video.use_fake_codecs) { - media_deps.video_encoder_factory = - std::make_unique( - net->time_controller()->GetClock()); - media_deps.video_decoder_factory = + pcf_deps.video_encoder_factory = std::make_unique( + net->time_controller()->GetClock()); + pcf_deps.video_decoder_factory = std::make_unique(); } else { - media_deps.video_encoder_factory = + pcf_deps.video_encoder_factory = std::make_unique>(); - media_deps.video_decoder_factory = + pcf_deps.video_decoder_factory = std::make_unique>(); } - media_deps.audio_encoder_factory = CreateBuiltinAudioEncoderFactory(); - media_deps.audio_decoder_factory = CreateBuiltinAudioDecoderFactory(); - media_deps.trials = pcf_deps.trials.get(); - pcf_deps.media_engine = cricket::CreateMediaEngine(std::move(media_deps)); + EnableMediaWithDefaultsAndTimeController(*net->time_controller(), pcf_deps); + pcf_deps.fec_controller_factory = nullptr; pcf_deps.network_controller_factory = nullptr; pcf_deps.network_state_predictor_factory = nullptr;