[PCLF] Extract configurable params from params

Bug: b/213863770
Change-Id: I6d4f5f3910c315da7fe5dfa7088da769b9f87212
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260109
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36658}

test/pc/e2e/media/media_helper.cc

@@ -52,9 +52,10 @@ std::vector<rtc::scoped_refptr<TestVideoCapturerVideoTrackSource>>
 MediaHelper::MaybeAddVideo(TestPeer* peer) {
   // Params here valid because of pre-run validation.
   const Params& params = peer->params();
+  const ConfigurableParams& configurable_params = peer->configurable_params();
   std::vector<rtc::scoped_refptr<TestVideoCapturerVideoTrackSource>> out;
-  for (size_t i = 0; i < params.video_configs.size(); ++i) {
+  for (size_t i = 0; i < configurable_params.video_configs.size(); ++i) {
-    auto video_config = params.video_configs[i];
+    const VideoConfig& video_config = configurable_params.video_configs[i];
     // Setup input video source into peer connection.
     std::unique_ptr<test::TestVideoCapturer> capturer = CreateVideoCapturer(
         video_config, peer->ReleaseVideoSource(i),

test/pc/e2e/peer_configurer.cc

@@ -65,21 +65,27 @@ PeerParamsPreprocessor::PeerParamsPreprocessor()
 
 void PeerParamsPreprocessor::SetDefaultValuesForMissingParams(
     PeerConfigurerImpl& peer) {
-  auto* p = peer.params();
+  Params* params = peer.params();
-  peer_names_provider.MaybeSetName(p->name);
+  ConfigurableParams* configurable_params = peer.configurable_params();
-  DefaultNamesProvider video_stream_names_provider(*p->name +
+  peer_names_provider.MaybeSetName(params->name);
+  DefaultNamesProvider video_stream_names_provider(*params->name +
                                                    "_auto_video_stream_label_");
-  for (VideoConfig& video_config : p->video_configs) {
+  for (size_t i = 0; i < configurable_params->video_configs.size(); ++i) {
-    video_stream_names_provider.MaybeSetName(video_config.stream_label);
+    video_stream_names_provider.MaybeSetName(
+        configurable_params->video_configs[i].stream_label);
+    // TODO(titovartem): remove when downstream will migrate on new API.
+    params->video_configs[i].stream_label =
+        configurable_params->video_configs[i].stream_label;
   }
-  if (p->audio_config) {
+  if (params->audio_config) {
     DefaultNamesProvider audio_stream_names_provider(
-        *p->name + "_auto_audio_stream_label_");
+        *params->name + "_auto_audio_stream_label_");
-    audio_stream_names_provider.MaybeSetName(p->audio_config->stream_label);
+    audio_stream_names_provider.MaybeSetName(
+        params->audio_config->stream_label);
   }
 
-  if (p->video_codecs.empty()) {
+  if (params->video_codecs.empty()) {
-    p->video_codecs.push_back(
+    params->video_codecs.push_back(
         PeerConnectionE2EQualityTestFixture::VideoCodecConfig(
             cricket::kVp8CodecName));
   }
@@ -100,11 +106,12 @@ void PeerParamsPreprocessor::ValidateParams(const PeerConfigurerImpl& peer) {
   if (p.audio_config) {
     media_streams_count++;
   }
-  media_streams_count += p.video_configs.size();
+  media_streams_count += peer.configurable_params().video_configs.size();
 
   // Validate that all video stream labels are unique and sync groups are
   // valid.
-  for (const VideoConfig& video_config : p.video_configs) {
+  for (const VideoConfig& video_config :
+       peer.configurable_params().video_configs) {
     RTC_CHECK(video_config.stream_label);
     bool inserted =
         video_labels.insert(video_config.stream_label.value()).second;

test/pc/e2e/peer_configurer.h

@@ -51,7 +51,8 @@ class PeerConfigurerImpl final
             std::make_unique<InjectableComponents>(network_thread,
                                                    network_manager,
                                                    packet_socket_factory)),
-        params_(std::make_unique<Params>()) {}
+        params_(std::make_unique<Params>()),
+        configurable_params_(std::make_unique<ConfigurableParams>()) {}
 
   PeerConfigurer* SetName(absl::string_view name) override {
     params_->name = std::string(name);
@@ -127,13 +128,17 @@ class PeerConfigurerImpl final
       PeerConnectionE2EQualityTestFixture::VideoConfig config) override {
     video_sources_.push_back(
         CreateSquareFrameGenerator(config, /*type=*/absl::nullopt));
-    params_->video_configs.push_back(std::move(config));
+    // TODO(titovartem): remove when downstream will migrate on new API.
+    params_->video_configs.push_back(config);
+    configurable_params_->video_configs.push_back(std::move(config));
     return this;
   }
   PeerConfigurer* AddVideoConfig(
       PeerConnectionE2EQualityTestFixture::VideoConfig config,
       std::unique_ptr<test::FrameGeneratorInterface> generator) override {
-    params_->video_configs.push_back(std::move(config));
+    // TODO(titovartem): remove when downstream will migrate on new API.
+    params_->video_configs.push_back(config);
+    configurable_params_->video_configs.push_back(std::move(config));
     video_sources_.push_back(std::move(generator));
     return this;
   }
@@ -141,14 +146,18 @@ class PeerConfigurerImpl final
       PeerConnectionE2EQualityTestFixture::VideoConfig config,
       PeerConnectionE2EQualityTestFixture::CapturingDeviceIndex index)
       override {
-    params_->video_configs.push_back(std::move(config));
+    // TODO(titovartem): remove when downstream will migrate on new API.
+    params_->video_configs.push_back(config);
+    configurable_params_->video_configs.push_back(std::move(config));
     video_sources_.push_back(index);
     return this;
   }
   PeerConfigurer* SetVideoSubscription(
       PeerConnectionE2EQualityTestFixture::VideoSubscription subscription)
       override {
-    params_->video_subscription = std::move(subscription);
+    // TODO(titovartem): remove when downstream will migrate on new API.
+    params_->video_subscription = subscription;
+    configurable_params_->video_subscription = std::move(subscription);
     return this;
   }
   PeerConfigurer* SetAudioConfig(
@@ -227,7 +236,13 @@ class PeerConfigurerImpl final
 
   InjectableComponents* components() { return components_.get(); }
   Params* params() { return params_.get(); }
+  ConfigurableParams* configurable_params() {
+    return configurable_params_.get();
+  }
   const Params& params() const { return *params_; }
+  const ConfigurableParams& configurable_params() const {
+    return *configurable_params_;
+  }
   std::vector<VideoSource>* video_sources() { return &video_sources_; }
 
   // Returns InjectableComponents and transfer ownership to the caller.
@@ -238,6 +253,7 @@ class PeerConfigurerImpl final
     components_ = nullptr;
     return components;
   }
+
   // Returns Params and transfer ownership to the caller.
   // Can be called once.
   std::unique_ptr<Params> ReleaseParams() {
@@ -246,6 +262,16 @@ class PeerConfigurerImpl final
     params_ = nullptr;
     return params;
   }
+
+  // Returns ConfigurableParams and transfer ownership to the caller.
+  // Can be called once.
+  std::unique_ptr<ConfigurableParams> ReleaseConfigurableParams() {
+    RTC_CHECK(configurable_params_);
+    auto configurable_params = std::move(configurable_params_);
+    configurable_params_ = nullptr;
+    return configurable_params;
+  }
+
   // Returns video sources and transfer frame generators ownership to the
   // caller. Can be called once.
   std::vector<VideoSource> ReleaseVideoSources() {
@@ -257,6 +283,7 @@ class PeerConfigurerImpl final
  private:
   std::unique_ptr<InjectableComponents> components_;
   std::unique_ptr<Params> params_;
+  std::unique_ptr<ConfigurableParams> configurable_params_;
   std::vector<VideoSource> video_sources_;
 };
 

test/pc/e2e/peer_connection_quality_test.cc

@@ -106,11 +106,12 @@ class FixturePeerConnectionObserver : public MockPeerConnectionObserver {
 void ValidateP2PSimulcastParams(
     const std::vector<std::unique_ptr<PeerConfigurerImpl>>& peers) {
   for (size_t i = 0; i < peers.size(); ++i) {
-    Params* p = peers[i]->params();
+    Params* params = peers[i]->params();
-    for (const VideoConfig& video_config : p->video_configs) {
+    ConfigurableParams* configurable_params = peers[i]->configurable_params();
+    for (const VideoConfig& video_config : configurable_params->video_configs) {
       if (video_config.simulcast_config) {
         // When we simulate SFU we support only one video codec.
-        RTC_CHECK_EQ(p->video_codecs.size(), 1)
+        RTC_CHECK_EQ(params->video_codecs.size(), 1)
             << "Only 1 video codec is supported when simulcast is enabled in "
             << "at least 1 video config";
       }
@@ -196,27 +197,28 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
       std::move(peer_configurations_[1]);
   peer_configurations_.clear();
 
-  for (size_t i = 0; i < bob_configurer->params()->video_configs.size(); ++i) {
+  for (size_t i = 0;
+       i < bob_configurer->configurable_params()->video_configs.size(); ++i) {
     // We support simulcast only from caller.
-    RTC_CHECK(!bob_configurer->params()->video_configs[i].simulcast_config)
+    RTC_CHECK(!bob_configurer->configurable_params()
+                   ->video_configs[i]
+                   .simulcast_config)
         << "Only simulcast stream from first peer is supported";
   }
 
   test::ScopedFieldTrials field_trials(GetFieldTrials(run_params));
 
   // Print test summary
-  RTC_LOG(LS_INFO) << "Media quality test: "
+  RTC_LOG(LS_INFO)
-                   << *alice_configurer->params()->name
+      << "Media quality test: " << *alice_configurer->params()->name
-                   << " will make a call to " << *bob_configurer->params()->name
+      << " will make a call to " << *bob_configurer->params()->name
-                   << " with media video="
+      << " with media video="
-                   << !alice_configurer->params()->video_configs.empty()
+      << !alice_configurer->configurable_params()->video_configs.empty()
-                   << "; audio="
+      << "; audio=" << alice_configurer->params()->audio_config.has_value()
-                   << alice_configurer->params()->audio_config.has_value()
+      << ". " << *bob_configurer->params()->name
-                   << ". " << *bob_configurer->params()->name
+      << " will respond with media video="
-                   << " will respond with media video="
+      << !bob_configurer->configurable_params()->video_configs.empty()
-                   << !bob_configurer->params()->video_configs.empty()
+      << "; audio=" << bob_configurer->params()->audio_config.has_value();
-                   << "; audio="
-                   << bob_configurer->params()->audio_config.has_value();
 
   const std::unique_ptr<rtc::Thread> signaling_thread =
       time_controller_.CreateThread(kSignalThreadName);
@@ -240,10 +242,10 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
   // Copy Alice and Bob video configs and names to correctly pass them into
   // lambdas.
   std::vector<VideoConfig> alice_video_configs =
-      alice_configurer->params()->video_configs;
+      alice_configurer->configurable_params()->video_configs;
   std::string alice_name = alice_configurer->params()->name.value();
   std::vector<VideoConfig> bob_video_configs =
-      bob_configurer->params()->video_configs;
+      bob_configurer->configurable_params()->video_configs;
   std::string bob_name = bob_configurer->params()->name.value();
 
   TestPeerFactory test_peer_factory(
@@ -468,7 +470,7 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
   }
 
   size_t alice_video_transceivers_non_simulcast_counter = 0;
-  for (auto& video_config : alice_->params().video_configs) {
+  for (auto& video_config : alice_->configurable_params().video_configs) {
     RtpTransceiverInit transceiver_params;
     if (video_config.simulcast_config) {
       transceiver_params.direction = RtpTransceiverDirection::kSendOnly;
@@ -510,7 +512,7 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
   // Add receive only transceivers in case Bob has more video_configs than
   // Alice.
   for (size_t i = alice_video_transceivers_non_simulcast_counter;
-       i < bob_->params().video_configs.size(); ++i) {
+       i < bob_->configurable_params().video_configs.size(); ++i) {
     RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>> result =
         alice_->AddTransceiver(cricket::MediaType::MEDIA_TYPE_VIDEO,
                                receive_only_transceiver_init);
@@ -572,7 +574,7 @@ PeerConnectionE2EQualityTest::CreateSignalingInterceptor(
   std::map<std::string, int> stream_label_to_simulcast_streams_count;
   // We add only Alice here, because simulcast/svc is supported only from the
   // first peer.
-  for (auto& video_config : alice_->params().video_configs) {
+  for (auto& video_config : alice_->configurable_params().video_configs) {
     if (video_config.simulcast_config) {
       stream_label_to_simulcast_streams_count.insert(
           {*video_config.stream_label,

test/pc/e2e/peer_connection_quality_test_params.h

@@ -115,6 +115,7 @@ struct Params {
   // Peer name. If empty - default one will be set by the fixture.
   absl::optional<std::string> name;
   // If `video_configs` is empty - no video should be added to the test call.
+  // TODO(titovartem): remove when downstream will migrate on new API.
   std::vector<PeerConnectionE2EQualityTestFixture::VideoConfig> video_configs;
   // If `audio_config` is set audio stream will be configured
   absl::optional<PeerConnectionE2EQualityTestFixture::AudioConfig> audio_config;
@@ -143,6 +144,18 @@ struct Params {
   BitrateSettings bitrate_settings;
   std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
       video_codecs;
+
+  // TODO(titovartem): remove when downstream will migrate on new API.
+  PeerConnectionE2EQualityTestFixture::VideoSubscription video_subscription =
+      PeerConnectionE2EQualityTestFixture::VideoSubscription()
+          .SubscribeToAllPeers();
+};
+
+// Contains parameters that maybe changed by test writer during the test call.
+struct ConfigurableParams {
+  // If `video_configs` is empty - no video should be added to the test call.
+  std::vector<PeerConnectionE2EQualityTestFixture::VideoConfig> video_configs;
+
   PeerConnectionE2EQualityTestFixture::VideoSubscription video_subscription =
       PeerConnectionE2EQualityTestFixture::VideoSubscription()
           .SubscribeToAllPeers();

test/pc/e2e/test_peer.cc

@@ -91,11 +91,13 @@ TestPeer::TestPeer(
     rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory,
     rtc::scoped_refptr<PeerConnectionInterface> pc,
     std::unique_ptr<MockPeerConnectionObserver> observer,
-    std::unique_ptr<Params> params,
+    Params params,
+    ConfigurableParams configurable_params,
     std::vector<PeerConfigurerImpl::VideoSource> video_sources,
     rtc::scoped_refptr<AudioProcessing> audio_processing,
     std::unique_ptr<rtc::Thread> worker_thread)
-    : params_(std::move(*params)),
+    : params_(std::move(params)),
+      configurable_params_(std::move(configurable_params)),
       worker_thread_(std::move(worker_thread)),
       wrapper_(std::make_unique<PeerConnectionWrapper>(std::move(pc_factory),
                                                        std::move(pc),

test/pc/e2e/test_peer.h

@@ -34,6 +34,9 @@ namespace webrtc_pc_e2e {
 class TestPeer final {
  public:
   const Params& params() const { return params_; }
+  ConfigurableParams configurable_params() const {
+    return configurable_params_;
+  }
 
   // TODO(titovartem): delete when downstreams will migrate to the new method.
   const Params& params2() const { return params_; }
@@ -143,13 +146,15 @@ class TestPeer final {
   TestPeer(rtc::scoped_refptr<PeerConnectionFactoryInterface> pc_factory,
            rtc::scoped_refptr<PeerConnectionInterface> pc,
            std::unique_ptr<MockPeerConnectionObserver> observer,
-           std::unique_ptr<Params> params,
+           Params params,
+           ConfigurableParams configurable_params,
            std::vector<PeerConfigurerImpl::VideoSource> video_sources,
            rtc::scoped_refptr<AudioProcessing> audio_processing,
            std::unique_ptr<rtc::Thread> worker_thread);
 
  private:
   Params params_;
+  ConfigurableParams configurable_params_;
   // Keeps ownership of worker thread. It has to be destroyed after `wrapper_`.
   std::unique_ptr<rtc::Thread> worker_thread_;
   std::unique_ptr<PeerConnectionWrapper> wrapper_;

test/pc/e2e/test_peer_factory.cc

@@ -306,11 +306,15 @@ std::unique_ptr<TestPeer> TestPeerFactory::CreateTestPeer(
   std::unique_ptr<InjectableComponents> components =
       configurer->ReleaseComponents();
   std::unique_ptr<Params> params = configurer->ReleaseParams();
+  std::unique_ptr<ConfigurableParams> configurable_params =
+      configurer->ReleaseConfigurableParams();
   std::vector<PeerConfigurerImpl::VideoSource> video_sources =
       configurer->ReleaseVideoSources();
   RTC_DCHECK(components);
   RTC_DCHECK(params);
-  RTC_DCHECK_EQ(params->video_configs.size(), video_sources.size());
+  RTC_DCHECK(configurable_params);
+  RTC_DCHECK_EQ(configurable_params->video_configs.size(),
+                video_sources.size());
   SetMandatoryEntities(components.get(), time_controller_);
   params->rtc_configuration.sdp_semantics = SdpSemantics::kUnifiedPlan;
 
@@ -329,7 +333,8 @@ std::unique_ptr<TestPeer> TestPeerFactory::CreateTestPeer(
           components->pcf_dependencies->task_queue_factory.get());
   WrapVideoEncoderFactory(
       params->name.value(), params->video_encoder_bitrate_multiplier,
-      CalculateRequiredSpatialIndexPerStream(params->video_configs),
+      CalculateRequiredSpatialIndexPerStream(
+          configurable_params->video_configs),
       components->pcf_dependencies.get(), video_analyzer_helper_);
   WrapVideoDecoderFactory(params->name.value(),
                           components->pcf_dependencies.get(),
@@ -363,8 +368,8 @@ std::unique_ptr<TestPeer> TestPeerFactory::CreateTestPeer(
 
   return absl::WrapUnique(new TestPeer(
       peer_connection_factory, peer_connection, std::move(observer),
-      std::move(params), std::move(video_sources), audio_processing,
+      std::move(*params), std::move(*configurable_params),
-      std::move(worker_thread)));
+      std::move(video_sources), audio_processing, std::move(worker_thread)));
 }
 
 }  // namespace webrtc_pc_e2e