From a3e51df5f3377bee71b4dcdd1ee8f50c520a9ea8 Mon Sep 17 00:00:00 2001 From: Jeremy Leconte Date: Thu, 10 Nov 2022 15:42:53 +0100 Subject: [PATCH] Add a new PeerConnectionE2EQualityTestFixture::AddPeer method. Change-Id: Ic5879613db51a00e3e958931f5eda19fda1ae94a Bug: webrtc:14627 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282640 Reviewed-by: Artem Titov Commit-Queue: Jeremy Leconte Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#38608} --- api/BUILD.gn | 1 + .../peerconnection_quality_test_fixture.h | 14 ++++++- audio/BUILD.gn | 3 ++ audio/test/pc_low_bandwidth_audio_test.cc | 23 ++++++----- pc/BUILD.gn | 3 ++ pc/test/svc_e2e_tests.cc | 33 ++++++++-------- test/pc/e2e/BUILD.gn | 2 + test/pc/e2e/peer_connection_e2e_smoke_test.cc | 10 +++-- test/pc/e2e/peer_connection_quality_test.cc | 7 ++++ test/pc/e2e/peer_connection_quality_test.h | 1 + ...nnection_quality_test_metric_names_test.cc | 24 ++++++------ .../e2e/peer_connection_quality_test_test.cc | 19 +++++----- video/BUILD.gn | 3 ++ video/pc_full_stack_tests.cc | 38 +++++++++---------- 14 files changed, 110 insertions(+), 71 deletions(-) diff --git a/api/BUILD.gn b/api/BUILD.gn index d6745c998e..56e92379d1 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -547,6 +547,7 @@ rtc_source_set("peer_connection_quality_test_fixture_api") { "video_codecs:video_codecs_api", ] absl_deps = [ + "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h index d031bcd16f..9db209301a 100644 --- a/api/test/peerconnection_quality_test_fixture.h +++ b/api/test/peerconnection_quality_test_fixture.h @@ -20,6 +20,7 @@ #include #include +#include "absl/base/macros.h" #include "absl/memory/memory.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -132,9 +133,18 @@ class PeerConnectionE2EQualityTestFixture { // `network_dependencies` are used to provide networking for peer's peer // connection. Members must be non-null. // `configurer` function will be used to configure peer in the call. - virtual PeerHandle* AddPeer( + [[deprecated("bugs.webrtc.org/14627")]] virtual PeerHandle* AddPeer( const PeerNetworkDependencies& network_dependencies, - rtc::FunctionView configurer) = 0; + rtc::FunctionView configurer) { + RTC_CHECK_NOTREACHED(); + return nullptr; + } + // TODO(bugs.webrtc.org/14627): make pure virtual once all subclasses + // implement it. + virtual PeerHandle* AddPeer(std::unique_ptr configurer) { + RTC_CHECK_NOTREACHED(); + return nullptr; + } // Runs the media quality test, which includes setting up the call with // configured participants, running it according to provided `run_params` and diff --git a/audio/BUILD.gn b/audio/BUILD.gn index 33cacf856e..91d66d4f8b 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -233,6 +233,9 @@ if (rtc_include_tests) { "../api/test/metrics:global_metrics_logger_and_exporter", "../api/test/metrics:metrics_exporter", "../api/test/metrics:stdout_metrics_exporter", + "../api/test/pclf:media_configuration", + "../api/test/pclf:media_quality_test_params", + "../api/test/pclf:peer_configurer", "../call:simulated_network", "../common_audio", "../system_wrappers", diff --git a/audio/test/pc_low_bandwidth_audio_test.cc b/audio/test/pc_low_bandwidth_audio_test.cc index 4fab15b9dd..8b733d578d 100644 --- a/audio/test/pc_low_bandwidth_audio_test.cc +++ b/audio/test/pc_low_bandwidth_audio_test.cc @@ -20,6 +20,9 @@ #include "api/test/metrics/metrics_exporter.h" #include "api/test/metrics/stdout_metrics_exporter.h" #include "api/test/network_emulation_manager.h" +#include "api/test/pclf/media_configuration.h" +#include "api/test/pclf/media_quality_test_params.h" +#include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "api/test/simulated_network.h" #include "api/test/time_controller.h" @@ -35,11 +38,9 @@ ABSL_DECLARE_FLAG(bool, quick); namespace webrtc { namespace test { -using PeerConfigurer = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::PeerConfigurer; -using RunParams = webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::RunParams; -using AudioConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::AudioConfig; +using ::webrtc::webrtc_pc_e2e::AudioConfig; +using ::webrtc::webrtc_pc_e2e::PeerConfigurer; +using ::webrtc::webrtc_pc_e2e::RunParams; namespace { @@ -67,10 +68,14 @@ CreateTestFixture(absl::string_view test_case_name, std::string(test_case_name), time_controller, /*audio_quality_analyzer=*/nullptr, /*video_quality_analyzer=*/nullptr); - fixture->AddPeer(network_links.first->network_dependencies(), - alice_configurer); - fixture->AddPeer(network_links.second->network_dependencies(), - bob_configurer); + auto alice = std::make_unique( + network_links.first->network_dependencies()); + auto bob = std::make_unique( + network_links.second->network_dependencies()); + alice_configurer(alice.get()); + bob_configurer(bob.get()); + fixture->AddPeer(std::move(alice)); + fixture->AddPeer(std::move(bob)); fixture->AddQualityMetricsReporter( std::make_unique( network_links.first, network_links.second, diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 2621d57ea4..8126fedb83 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2714,6 +2714,9 @@ if (rtc_include_tests && !build_with_chromium) { "../api:simulated_network_api", "../api:time_controller", "../api/test/metrics:global_metrics_logger_and_exporter", + "../api/test/pclf:media_configuration", + "../api/test/pclf:media_quality_test_params", + "../api/test/pclf:peer_configurer", "../api/video_codecs:video_codecs_api", "../call:simulated_network", "../modules/video_coding:webrtc_vp9", diff --git a/pc/test/svc_e2e_tests.cc b/pc/test/svc_e2e_tests.cc index 264b990e1b..5d98f7313d 100644 --- a/pc/test/svc_e2e_tests.cc +++ b/pc/test/svc_e2e_tests.cc @@ -19,6 +19,9 @@ #include "api/test/frame_generator_interface.h" #include "api/test/metrics/global_metrics_logger_and_exporter.h" #include "api/test/network_emulation_manager.h" +#include "api/test/pclf/media_configuration.h" +#include "api/test/pclf/media_quality_test_params.h" +#include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "api/test/simulated_network.h" #include "api/test/time_controller.h" @@ -38,18 +41,6 @@ namespace webrtc { namespace { -using PeerConfigurer = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::PeerConfigurer; -using RunParams = - ::webrtc::webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::RunParams; -using VideoConfig = - ::webrtc::webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig; -using ScreenShareConfig = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::ScreenShareConfig; -using VideoCodecConfig = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::VideoCodecConfig; -using EmulatedSFUConfig = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::EmulatedSFUConfig; using ::cricket::kAv1CodecName; using ::cricket::kH264CodecName; using ::cricket::kVp8CodecName; @@ -58,6 +49,12 @@ using ::testing::Combine; using ::testing::UnitTest; using ::testing::Values; using ::testing::ValuesIn; +using ::webrtc::webrtc_pc_e2e::EmulatedSFUConfig; +using ::webrtc::webrtc_pc_e2e::PeerConfigurer; +using ::webrtc::webrtc_pc_e2e::RunParams; +using ::webrtc::webrtc_pc_e2e::ScreenShareConfig; +using ::webrtc::webrtc_pc_e2e::VideoCodecConfig; +using ::webrtc::webrtc_pc_e2e::VideoConfig; std::unique_ptr CreateTestFixture(absl::string_view test_case_name, @@ -71,10 +68,14 @@ CreateTestFixture(absl::string_view test_case_name, auto fixture = webrtc_pc_e2e::CreatePeerConnectionE2EQualityTestFixture( std::string(test_case_name), time_controller, nullptr, std::move(video_quality_analyzer)); - fixture->AddPeer(network_links.first->network_dependencies(), - alice_configurer); - fixture->AddPeer(network_links.second->network_dependencies(), - bob_configurer); + auto alice = std::make_unique( + network_links.first->network_dependencies()); + auto bob = std::make_unique( + network_links.second->network_dependencies()); + alice_configurer(alice.get()); + bob_configurer(bob.get()); + fixture->AddPeer(std::move(alice)); + fixture->AddPeer(std::move(bob)); return fixture; } diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 27e1e71134..807599961b 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -613,6 +613,7 @@ if (!build_with_chromium) { "../../../api/test/metrics:global_metrics_logger_and_exporter", "../../../api/test/pclf:media_configuration", "../../../api/test/pclf:media_quality_test_params", + "../../../api/test/pclf:peer_configurer", "../../../api/video_codecs:builtin_video_decoder_factory", "../../../api/video_codecs:builtin_video_encoder_factory", "../../../call:simulated_network", @@ -655,6 +656,7 @@ if (!build_with_chromium) { "../../../api/test/metrics:stdout_metrics_exporter", "../../../api/test/pclf:media_configuration", "../../../api/test/pclf:media_quality_test_params", + "../../../api/test/pclf:peer_configurer", "../../../api/units:time_delta", ] } diff --git a/test/pc/e2e/peer_connection_e2e_smoke_test.cc b/test/pc/e2e/peer_connection_e2e_smoke_test.cc index 52c525f494..0e7993e5be 100644 --- a/test/pc/e2e/peer_connection_e2e_smoke_test.cc +++ b/test/pc/e2e/peer_connection_e2e_smoke_test.cc @@ -20,6 +20,7 @@ #include "api/test/network_emulation_manager.h" #include "api/test/pclf/media_configuration.h" #include "api/test/pclf/media_quality_test_params.h" +#include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "call/simulated_network.h" #include "system_wrappers/include/field_trial.h" @@ -41,8 +42,6 @@ namespace { class PeerConnectionE2EQualityTestSmokeTest : public ::testing::Test { public: - using PeerConfigurer = PeerConnectionE2EQualityTestFixture::PeerConfigurer; - void SetUp() override { network_emulation_ = CreateNetworkEmulationManager(); auto video_quality_analyzer = std::make_unique( @@ -81,8 +80,11 @@ class PeerConnectionE2EQualityTestSmokeTest : public ::testing::Test { } void AddPeer(EmulatedNetworkManagerInterface* network, - rtc::FunctionView configurer) { - fixture_->AddPeer(network->network_dependencies(), configurer); + rtc::FunctionView update_configurer) { + auto configurer = + std::make_unique(network->network_dependencies()); + update_configurer(configurer.get()); + fixture_->AddPeer(std::move(configurer)); } void RunAndCheckEachVideoStreamReceivedFrames(const RunParams& run_params) { diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 90bc5c2d1f..fe4114c4ed 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -205,6 +205,13 @@ PeerConnectionE2EQualityTest::PeerHandle* PeerConnectionE2EQualityTest::AddPeer( return &peer_handles_.back(); } +PeerConnectionE2EQualityTest::PeerHandle* PeerConnectionE2EQualityTest::AddPeer( + std::unique_ptr configurer) { + peer_configurations_.push_back(std::move(configurer)); + peer_handles_.push_back(PeerHandleImpl()); + return &peer_handles_.back(); +} + void PeerConnectionE2EQualityTest::Run(RunParams run_params) { webrtc::webrtc_pc_e2e::PeerParamsPreprocessor params_preprocessor; for (auto& peer_configuration : peer_configurations_) { diff --git a/test/pc/e2e/peer_connection_quality_test.h b/test/pc/e2e/peer_connection_quality_test.h index 9ee59b7f9c..e077673f09 100644 --- a/test/pc/e2e/peer_connection_quality_test.h +++ b/test/pc/e2e/peer_connection_quality_test.h @@ -74,6 +74,7 @@ class PeerConnectionE2EQualityTest PeerHandle* AddPeer( const PeerNetworkDependencies& network_dependencies, rtc::FunctionView configurer) override; + PeerHandle* AddPeer(std::unique_ptr configurer) override; void Run(RunParams run_params) override; TimeDelta GetRealTestDuration() const override { diff --git a/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc b/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc index 195cbc8baa..cf359448b9 100644 --- a/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc +++ b/test/pc/e2e/peer_connection_quality_test_metric_names_test.cc @@ -19,6 +19,7 @@ #include "api/test/network_emulation_manager.h" #include "api/test/pclf/media_configuration.h" #include "api/test/pclf/media_quality_test_params.h" +#include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "api/units/time_delta.h" #include "test/gmock.h" @@ -39,8 +40,7 @@ using ::webrtc::test::Metric; using ::webrtc::test::MetricsExporter; using ::webrtc::test::StdoutMetricsExporter; using ::webrtc::test::Unit; -using PeerConfigurer = ::webrtc::webrtc_pc_e2e:: - PeerConnectionE2EQualityTestFixture::PeerConfigurer; +using ::webrtc::webrtc_pc_e2e::PeerConfigurer; // Adds a peer with some audio and video (the client should not care about // details about audio and video configs). @@ -50,16 +50,16 @@ void AddDefaultAudioVideoPeer( absl::string_view video_stream_label, const PeerNetworkDependencies& network_dependencies, PeerConnectionE2EQualityTestFixture& fixture) { - fixture.AddPeer(network_dependencies, [&](PeerConfigurer* peer) { - peer->SetName(peer_name); - AudioConfig audio{std::string(audio_stream_label)}; - audio.sync_group = std::string(peer_name); - peer->SetAudioConfig(std::move(audio)); - VideoConfig video(std::string(video_stream_label), 320, 180, 15); - video.sync_group = std::string(peer_name); - peer->AddVideoConfig(std::move(video)); - peer->SetVideoCodecs({VideoCodecConfig(cricket::kVp8CodecName)}); - }); + AudioConfig audio{std::string(audio_stream_label)}; + audio.sync_group = std::string(peer_name); + VideoConfig video(std::string(video_stream_label), 320, 180, 15); + video.sync_group = std::string(peer_name); + auto peer = std::make_unique(network_dependencies); + peer->SetName(peer_name); + peer->SetAudioConfig(std::move(audio)); + peer->AddVideoConfig(std::move(video)); + peer->SetVideoCodecs({VideoCodecConfig(cricket::kVp8CodecName)}); + fixture.AddPeer(std::move(peer)); } // Metric fields to assert on diff --git a/test/pc/e2e/peer_connection_quality_test_test.cc b/test/pc/e2e/peer_connection_quality_test_test.cc index 2511bd2bdf..f39b4f5421 100644 --- a/test/pc/e2e/peer_connection_quality_test_test.cc +++ b/test/pc/e2e/peer_connection_quality_test_test.cc @@ -114,15 +114,16 @@ TEST_F(PeerConnectionE2EQualityTestTest, OutputVideoIsDumpedWhenRequested) { EmulatedNetworkManagerInterface* bob_network = network_emulation->CreateEmulatedNetworkManagerInterface({bob_endpoint}); - fixture.AddPeer( - alice_network->network_dependencies(), [&](PeerConfigurer* peer) { - peer->SetName("alice"); - VideoConfig video("alice_video", 320, 180, 15); - video.output_dump_options = VideoDumpOptions(test_directory_); - peer->AddVideoConfig(std::move(video)); - }); - fixture.AddPeer(bob_network->network_dependencies(), - [&](PeerConfigurer* peer) { peer->SetName("bob"); }); + VideoConfig alice_video("alice_video", 320, 180, 15); + alice_video.output_dump_options = VideoDumpOptions(test_directory_); + PeerConfigurer alice(alice_network->network_dependencies()); + alice.SetName("alice"); + alice.AddVideoConfig(std::move(alice_video)); + fixture.AddPeer(std::make_unique(std::move(alice))); + + PeerConfigurer bob(bob_network->network_dependencies()); + bob.SetName("bob"); + fixture.AddPeer(std::make_unique(std::move(bob))); fixture.Run(RunParams(TimeDelta::Seconds(2))); diff --git a/video/BUILD.gn b/video/BUILD.gn index 169b485eb3..7990d207ef 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -625,6 +625,9 @@ if (rtc_include_tests) { "../api:simulated_network_api", "../api:time_controller", "../api/test/metrics:global_metrics_logger_and_exporter", + "../api/test/pclf:media_configuration", + "../api/test/pclf:media_quality_test_params", + "../api/test/pclf:peer_configurer", "../api/video_codecs:video_codecs_api", "../call:simulated_network", "../modules/video_coding:webrtc_vp9", diff --git a/video/pc_full_stack_tests.cc b/video/pc_full_stack_tests.cc index d1a1ff6c92..83b06830e0 100644 --- a/video/pc_full_stack_tests.cc +++ b/video/pc_full_stack_tests.cc @@ -19,6 +19,9 @@ #include "api/test/frame_generator_interface.h" #include "api/test/metrics/global_metrics_logger_and_exporter.h" #include "api/test/network_emulation_manager.h" +#include "api/test/pclf/media_configuration.h" +#include "api/test/pclf/media_quality_test_params.h" +#include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "api/test/simulated_network.h" #include "api/test/time_controller.h" @@ -33,21 +36,14 @@ namespace webrtc { -using EmulatedSFUConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::EmulatedSFUConfig; -using PeerConfigurer = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::PeerConfigurer; -using RunParams = webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::RunParams; -using VideoConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig; -using AudioConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::AudioConfig; -using ScreenShareConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::ScreenShareConfig; -using VideoSimulcastConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoSimulcastConfig; -using VideoCodecConfig = - webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoCodecConfig; +using ::webrtc::webrtc_pc_e2e::AudioConfig; +using ::webrtc::webrtc_pc_e2e::EmulatedSFUConfig; +using ::webrtc::webrtc_pc_e2e::PeerConfigurer; +using ::webrtc::webrtc_pc_e2e::RunParams; +using ::webrtc::webrtc_pc_e2e::ScreenShareConfig; +using ::webrtc::webrtc_pc_e2e::VideoCodecConfig; +using ::webrtc::webrtc_pc_e2e::VideoConfig; +using ::webrtc::webrtc_pc_e2e::VideoSimulcastConfig; namespace { @@ -63,10 +59,14 @@ CreateTestFixture(const std::string& test_case_name, auto fixture = webrtc_pc_e2e::CreatePeerConnectionE2EQualityTestFixture( test_case_name, time_controller, /*audio_quality_analyzer=*/nullptr, /*video_quality_analyzer=*/nullptr); - fixture->AddPeer(network_links.first->network_dependencies(), - alice_configurer); - fixture->AddPeer(network_links.second->network_dependencies(), - bob_configurer); + auto alice = std::make_unique( + network_links.first->network_dependencies()); + auto bob = std::make_unique( + network_links.second->network_dependencies()); + alice_configurer(alice.get()); + bob_configurer(bob.get()); + fixture->AddPeer(std::move(alice)); + fixture->AddPeer(std::move(bob)); fixture->AddQualityMetricsReporter( std::make_unique( network_links.first, network_links.second,