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 <titovartem@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38608}

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

api/test/peerconnection_quality_test_fixture.h

@@ -20,6 +20,7 @@
 #include <utility>
 #include <vector>
 
+#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<void(PeerConfigurer*)> configurer) = 0;
+      rtc::FunctionView<void(PeerConfigurer*)> 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<PeerConfigurer> 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

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

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 =
+using ::webrtc::webrtc_pc_e2e::AudioConfig;
-    webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::PeerConfigurer;
+using ::webrtc::webrtc_pc_e2e::PeerConfigurer;
-using RunParams = webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::RunParams;
+using ::webrtc::webrtc_pc_e2e::RunParams;
-using AudioConfig =
-    webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::AudioConfig;
 
 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(),
+  auto alice = std::make_unique<PeerConfigurer>(
-                   alice_configurer);
+      network_links.first->network_dependencies());
-  fixture->AddPeer(network_links.second->network_dependencies(),
+  auto bob = std::make_unique<PeerConfigurer>(
-                   bob_configurer);
+      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<webrtc_pc_e2e::NetworkQualityMetricsReporter>(
           network_links.first, network_links.second,

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

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<webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture>
 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(),
+  auto alice = std::make_unique<PeerConfigurer>(
-                   alice_configurer);
+      network_links.first->network_dependencies());
-  fixture->AddPeer(network_links.second->network_dependencies(),
+  auto bob = std::make_unique<PeerConfigurer>(
-                   bob_configurer);
+      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;
 }
 

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

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<DefaultVideoQualityAnalyzer>(
@@ -81,8 +80,11 @@ class PeerConnectionE2EQualityTestSmokeTest : public ::testing::Test {
   }
 
   void AddPeer(EmulatedNetworkManagerInterface* network,
-               rtc::FunctionView<void(PeerConfigurer*)> configurer) {
+               rtc::FunctionView<void(PeerConfigurer*)> update_configurer) {
-    fixture_->AddPeer(network->network_dependencies(), configurer);
+    auto configurer =
+        std::make_unique<PeerConfigurer>(network->network_dependencies());
+    update_configurer(configurer.get());
+    fixture_->AddPeer(std::move(configurer));
   }
 
   void RunAndCheckEachVideoStreamReceivedFrames(const RunParams& run_params) {

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<PeerConfigurer> 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_) {

test/pc/e2e/peer_connection_quality_test.h

@@ -74,6 +74,7 @@ class PeerConnectionE2EQualityTest
   PeerHandle* AddPeer(
       const PeerNetworkDependencies& network_dependencies,
       rtc::FunctionView<void(PeerConfigurer*)> configurer) override;
+  PeerHandle* AddPeer(std::unique_ptr<PeerConfigurer> configurer) override;
   void Run(RunParams run_params) override;
 
   TimeDelta GetRealTestDuration() const override {

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::
+using ::webrtc::webrtc_pc_e2e::PeerConfigurer;
-    PeerConnectionE2EQualityTestFixture::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) {
+  AudioConfig audio{std::string(audio_stream_label)};
-    peer->SetName(peer_name);
+  audio.sync_group = std::string(peer_name);
-    AudioConfig audio{std::string(audio_stream_label)};
+  VideoConfig video(std::string(video_stream_label), 320, 180, 15);
-    audio.sync_group = std::string(peer_name);
+  video.sync_group = std::string(peer_name);
-    peer->SetAudioConfig(std::move(audio));
+  auto peer = std::make_unique<PeerConfigurer>(network_dependencies);
-    VideoConfig video(std::string(video_stream_label), 320, 180, 15);
+  peer->SetName(peer_name);
-    video.sync_group = std::string(peer_name);
+  peer->SetAudioConfig(std::move(audio));
-    peer->AddVideoConfig(std::move(video));
+  peer->AddVideoConfig(std::move(video));
-    peer->SetVideoCodecs({VideoCodecConfig(cricket::kVp8CodecName)});
+  peer->SetVideoCodecs({VideoCodecConfig(cricket::kVp8CodecName)});
-  });
+  fixture.AddPeer(std::move(peer));
 }
 
 // Metric fields to assert on

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(
+  VideoConfig alice_video("alice_video", 320, 180, 15);
-      alice_network->network_dependencies(), [&](PeerConfigurer* peer) {
+  alice_video.output_dump_options = VideoDumpOptions(test_directory_);
-        peer->SetName("alice");
+  PeerConfigurer alice(alice_network->network_dependencies());
-        VideoConfig video("alice_video", 320, 180, 15);
+  alice.SetName("alice");
-        video.output_dump_options = VideoDumpOptions(test_directory_);
+  alice.AddVideoConfig(std::move(alice_video));
-        peer->AddVideoConfig(std::move(video));
+  fixture.AddPeer(std::make_unique<PeerConfigurer>(std::move(alice)));
-      });
+
-  fixture.AddPeer(bob_network->network_dependencies(),
+  PeerConfigurer bob(bob_network->network_dependencies());
-                  [&](PeerConfigurer* peer) { peer->SetName("bob"); });
+  bob.SetName("bob");
+  fixture.AddPeer(std::make_unique<PeerConfigurer>(std::move(bob)));
 
   fixture.Run(RunParams(TimeDelta::Seconds(2)));
 

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

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 =
+using ::webrtc::webrtc_pc_e2e::AudioConfig;
-    webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::EmulatedSFUConfig;
+using ::webrtc::webrtc_pc_e2e::EmulatedSFUConfig;
-using PeerConfigurer =
+using ::webrtc::webrtc_pc_e2e::PeerConfigurer;
-    webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::PeerConfigurer;
+using ::webrtc::webrtc_pc_e2e::RunParams;
-using RunParams = webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::RunParams;
+using ::webrtc::webrtc_pc_e2e::ScreenShareConfig;
-using VideoConfig =
+using ::webrtc::webrtc_pc_e2e::VideoCodecConfig;
-    webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig;
+using ::webrtc::webrtc_pc_e2e::VideoConfig;
-using AudioConfig =
+using ::webrtc::webrtc_pc_e2e::VideoSimulcastConfig;
-    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;
 
 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(),
+  auto alice = std::make_unique<PeerConfigurer>(
-                   alice_configurer);
+      network_links.first->network_dependencies());
-  fixture->AddPeer(network_links.second->network_dependencies(),
+  auto bob = std::make_unique<PeerConfigurer>(
-                   bob_configurer);
+      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<webrtc_pc_e2e::NetworkQualityMetricsReporter>(
           network_links.first, network_links.second,