Add ability to configure video codecs at peer level (PC level framework)

Bug: b/192821182
Change-Id: Ic1b55028102fbd331f0fb6a3a8c758c311267cbc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226220
Reviewed-by: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34477}

api/test/peerconnection_quality_test_fixture.h

@@ -268,6 +268,27 @@ class PeerConnectionE2EQualityTestFixture {
     absl::optional<std::string> sync_group;
   };
 
+  struct VideoCodecConfig {
+    explicit VideoCodecConfig(std::string name)
+        : name(std::move(name)), required_params() {}
+    VideoCodecConfig(std::string name,
+                     std::map<std::string, std::string> required_params)
+        : name(std::move(name)), required_params(std::move(required_params)) {}
+    // Next two fields are used to specify concrete video codec, that should be
+    // used in the test. Video code will be negotiated in SDP during offer/
+    // answer exchange.
+    // Video codec name. You can find valid names in
+    // media/base/media_constants.h
+    std::string name = cricket::kVp8CodecName;
+    // Map of parameters, that have to be specified on SDP codec. Each parameter
+    // is described by key and value. Codec parameters will match the specified
+    // map if and only if for each key from |required_params| there will be
+    // a parameter with name equal to this key and parameter value will be equal
+    // to the value from |required_params| for this key.
+    // If empty then only name will be used to match the codec.
+    std::map<std::string, std::string> required_params;
+  };
+
   // This class is used to fully configure one peer inside the call.
   class PeerConfigurer {
    public:
@@ -343,6 +364,14 @@ class PeerConnectionE2EQualityTestFixture {
     // applied to all summed RTP streams for this peer.
     virtual PeerConfigurer* SetBitrateSettings(
         BitrateSettings bitrate_settings) = 0;
+    // Set the list of video codecs used by the peer during the test. These
+    // codecs will be negotiated in SDP during offer/answer exchange. The order
+    // of these codecs during negotiation will be the same as in |video_codecs|.
+    // Codecs have to be available in codecs list provided by peer connection to
+    // be negotiated. If some of specified codecs won't be found, the test will
+    // crash.
+    virtual PeerConfigurer* SetVideoCodecs(
+        std::vector<VideoCodecConfig> video_codecs) = 0;
   };
 
   // Contains configuration for echo emulator.
@@ -352,27 +381,6 @@ class PeerConnectionE2EQualityTestFixture {
     TimeDelta echo_delay = TimeDelta::Millis(50);
   };
 
-  struct VideoCodecConfig {
-    explicit VideoCodecConfig(std::string name)
-        : name(std::move(name)), required_params() {}
-    VideoCodecConfig(std::string name,
-                     std::map<std::string, std::string> required_params)
-        : name(std::move(name)), required_params(std::move(required_params)) {}
-    // Next two fields are used to specify concrete video codec, that should be
-    // used in the test. Video code will be negotiated in SDP during offer/
-    // answer exchange.
-    // Video codec name. You can find valid names in
-    // media/base/media_constants.h
-    std::string name = cricket::kVp8CodecName;
-    // Map of parameters, that have to be specified on SDP codec. Each parameter
-    // is described by key and value. Codec parameters will match the specified
-    // map if and only if for each key from |required_params| there will be
-    // a parameter with name equal to this key and parameter value will be equal
-    // to the value from |required_params| for this key.
-    // If empty then only name will be used to match the codec.
-    std::map<std::string, std::string> required_params;
-  };
-
   // Contains parameters, that describe how long framework should run quality
   // test.
   struct RunParams {
@@ -383,6 +391,11 @@ class PeerConnectionE2EQualityTestFixture {
     // it will be shut downed.
     TimeDelta run_duration;
 
+    // DEPRECATED: Instead of setting the codecs in RunParams (which apply to
+    //             all the participants in the call, please set them with
+    //             PeerConfigurer, this will allow more flexibility and let
+    //             different Peers support different codecs.
+    //
     // List of video codecs to use during the test. These codecs will be
     // negotiated in SDP during offer/answer exchange. The order of these codecs
     // during negotiation will be the same as in |video_codecs|. Codecs have

test/pc/e2e/peer_configurer.cc

@@ -90,11 +90,22 @@ void SetDefaultValuesForMissingParams(
           *p->name + "_auto_audio_stream_label_");
       audio_stream_names_provider.MaybeSetName(&p->audio_config->stream_label);
     }
-  }
 
-  if (run_params->video_codecs.empty()) {
+    if (p->video_codecs.empty()) {
-    run_params->video_codecs.push_back(
+      // TODO(mbonadei): Remove the usage of RunParams to set codecs, this is
-        VideoCodecConfig(cricket::kVp8CodecName));
+      // only needed for backwards compatibility.
+      if (!run_params->video_codecs.empty()) {
+        p->video_codecs = run_params->video_codecs;
+      } else {
+        p->video_codecs.push_back(
+            PeerConnectionE2EQualityTestFixture::VideoCodecConfig(
+                cricket::kVp8CodecName));
+      }
+    } else {
+      RTC_CHECK(run_params->video_codecs.empty())
+          << "Setting video_codecs in both PeerConfigurer and RunParams is not "
+             "supported.";
+    }
   }
 }
 
@@ -102,7 +113,6 @@ void ValidateParams(
     const RunParams& run_params,
     const std::vector<std::unique_ptr<PeerConfigurerImpl>>& peers) {
   RTC_CHECK_GT(run_params.video_encoder_bitrate_multiplier, 0.0);
-  RTC_CHECK_GE(run_params.video_codecs.size(), 1);
 
   std::set<std::string> peer_names;
   std::set<std::string> video_labels;
@@ -113,6 +123,8 @@ void ValidateParams(
 
   for (size_t i = 0; i < peers.size(); ++i) {
     Params* p = peers[i]->params();
+    // Each peer should at least support 1 video codec.
+    RTC_CHECK_GE(p->video_codecs.size(), 1);
 
     {
       RTC_CHECK(p->name);
@@ -160,14 +172,14 @@ void ValidateParams(
           RTC_CHECK_LT(*video_config.simulcast_config->target_spatial_index,
                        video_config.simulcast_config->simulcast_streams_count);
         }
-        RTC_CHECK_EQ(run_params.video_codecs.size(), 1)
+        RTC_CHECK_EQ(p->video_codecs.size(), 1)
             << "Only 1 video codec is supported when simulcast is enabled in "
             << "at least 1 video config";
         RTC_CHECK(!video_config.max_encode_bitrate_bps)
             << "Setting max encode bitrate is not implemented for simulcast.";
         RTC_CHECK(!video_config.min_encode_bitrate_bps)
             << "Setting min encode bitrate is not implemented for simulcast.";
-        if (run_params.video_codecs[0].name == cricket::kVp8CodecName &&
+        if (p->video_codecs[0].name == cricket::kVp8CodecName &&
             !video_config.simulcast_config->encoding_params.empty()) {
           RTC_CHECK_EQ(video_config.simulcast_config->simulcast_streams_count,
                        video_config.simulcast_config->encoding_params.size())

test/pc/e2e/peer_configurer.h

@@ -168,6 +168,12 @@ class PeerConfigurerImpl final
     params_->bitrate_settings = bitrate_settings;
     return this;
   }
+  PeerConfigurer* SetVideoCodecs(
+      std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
+          video_codecs) override {
+    params_->video_codecs = std::move(video_codecs);
+    return this;
+  }
 
   PeerConfigurer* SetIceTransportFactory(
       std::unique_ptr<IceTransportFactory> factory) override {

test/pc/e2e/peer_connection_quality_test.cc

@@ -444,8 +444,9 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread(
     RtpTransceiverInit transceiver_params;
     if (video_config.simulcast_config) {
       transceiver_params.direction = RtpTransceiverDirection::kSendOnly;
-      // Because simulcast enabled |run_params.video_codecs| has only 1 element.
+      // Because simulcast enabled |alice_->params()->video_codecs| has only 1
-      if (run_params.video_codecs[0].name == cricket::kVp8CodecName) {
+      // element.
+      if (alice_->params()->video_codecs[0].name == cricket::kVp8CodecName) {
         // For Vp8 simulcast we need to add as many RtpEncodingParameters to the
         // track as many simulcast streams requested. If they specified in
         // |video_config.simulcast_config| it should be copied from there.
@@ -508,14 +509,14 @@ void PeerConnectionE2EQualityTest::SetPeerCodecPreferences(
     const RunParams& run_params) {
   std::vector<RtpCodecCapability> with_rtx_video_capabilities =
       FilterVideoCodecCapabilities(
-          run_params.video_codecs, true, run_params.use_ulp_fec,
+          peer->params()->video_codecs, true, run_params.use_ulp_fec,
           run_params.use_flex_fec,
           peer->pc_factory()
               ->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
               .codecs);
   std::vector<RtpCodecCapability> without_rtx_video_capabilities =
       FilterVideoCodecCapabilities(
-          run_params.video_codecs, false, run_params.use_ulp_fec,
+          peer->params()->video_codecs, false, run_params.use_ulp_fec,
           run_params.use_flex_fec,
           peer->pc_factory()
               ->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
@@ -552,8 +553,7 @@ PeerConnectionE2EQualityTest::CreateSignalingInterceptor(
            video_config.simulcast_config->simulcast_streams_count});
     }
   }
-  PatchingParams patching_params(run_params.video_codecs,
+  PatchingParams patching_params(run_params.use_conference_mode,
-                                 run_params.use_conference_mode,
                                  stream_label_to_simulcast_streams_count);
   return std::make_unique<SignalingInterceptor>(patching_params);
 }
@@ -594,8 +594,8 @@ void PeerConnectionE2EQualityTest::ExchangeOfferAnswer(
   RTC_CHECK(offer);
   offer->ToString(&log_output);
   RTC_LOG(INFO) << "Original offer: " << log_output;
-  LocalAndRemoteSdp patch_result =
+  LocalAndRemoteSdp patch_result = signaling_interceptor->PatchOffer(
-      signaling_interceptor->PatchOffer(std::move(offer));
+      std::move(offer), alice_->params()->video_codecs[0]);
   patch_result.local_sdp->ToString(&log_output);
   RTC_LOG(INFO) << "Offer to set as local description: " << log_output;
   patch_result.remote_sdp->ToString(&log_output);
@@ -611,7 +611,8 @@ void PeerConnectionE2EQualityTest::ExchangeOfferAnswer(
   RTC_CHECK(answer);
   answer->ToString(&log_output);
   RTC_LOG(INFO) << "Original answer: " << log_output;
-  patch_result = signaling_interceptor->PatchAnswer(std::move(answer));
+  patch_result = signaling_interceptor->PatchAnswer(
+      std::move(answer), bob_->params()->video_codecs[0]);
   patch_result.local_sdp->ToString(&log_output);
   RTC_LOG(INFO) << "Answer to set as local description: " << log_output;
   patch_result.remote_sdp->ToString(&log_output);

test/pc/e2e/peer_connection_quality_test_params.h

@@ -116,6 +116,8 @@ struct Params {
 
   PeerConnectionInterface::RTCConfiguration rtc_configuration;
   BitrateSettings bitrate_settings;
+  std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
+      video_codecs;
 };
 
 }  // namespace webrtc_pc_e2e

test/pc/e2e/sdp/sdp_changer.cc

@@ -166,14 +166,15 @@ void SignalingInterceptor::FillSimulcastContext(
 }
 
 LocalAndRemoteSdp SignalingInterceptor::PatchOffer(
-    std::unique_ptr<SessionDescriptionInterface> offer) {
+    std::unique_ptr<SessionDescriptionInterface> offer,
+    const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec) {
   for (auto& content : offer->description()->contents()) {
     context_.mids_order.push_back(content.mid());
     cricket::MediaContentDescription* media_desc = content.media_description();
     if (media_desc->type() != cricket::MediaType::MEDIA_TYPE_VIDEO) {
       continue;
     }
-    if (content.media_description()->streams().size() == 0) {
+    if (content.media_description()->streams().empty()) {
       // It means that this media section describes receive only media section
       // in SDP.
       RTC_CHECK_EQ(content.media_description()->direction(),
@@ -183,13 +184,13 @@ LocalAndRemoteSdp SignalingInterceptor::PatchOffer(
     media_desc->set_conference_mode(params_.use_conference_mode);
   }
 
-  if (params_.stream_label_to_simulcast_streams_count.size() > 0) {
+  if (!params_.stream_label_to_simulcast_streams_count.empty()) {
     // Because simulcast enabled |params_.video_codecs| has only 1 element.
-    if (params_.video_codecs[0].name == cricket::kVp8CodecName) {
+    if (first_codec.name == cricket::kVp8CodecName) {
       return PatchVp8Offer(std::move(offer));
     }
 
-    if (params_.video_codecs[0].name == cricket::kVp9CodecName) {
+    if (first_codec.name == cricket::kVp9CodecName) {
       return PatchVp9Offer(std::move(offer));
     }
   }
@@ -362,7 +363,8 @@ LocalAndRemoteSdp SignalingInterceptor::PatchVp9Offer(
 }
 
 LocalAndRemoteSdp SignalingInterceptor::PatchAnswer(
-    std::unique_ptr<SessionDescriptionInterface> answer) {
+    std::unique_ptr<SessionDescriptionInterface> answer,
+    const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec) {
   for (auto& content : answer->description()->contents()) {
     cricket::MediaContentDescription* media_desc = content.media_description();
     if (media_desc->type() != cricket::MediaType::MEDIA_TYPE_VIDEO) {
@@ -375,13 +377,13 @@ LocalAndRemoteSdp SignalingInterceptor::PatchAnswer(
     media_desc->set_conference_mode(params_.use_conference_mode);
   }
 
-  if (params_.stream_label_to_simulcast_streams_count.size() > 0) {
+  if (!params_.stream_label_to_simulcast_streams_count.empty()) {
     // Because simulcast enabled |params_.video_codecs| has only 1 element.
-    if (params_.video_codecs[0].name == cricket::kVp8CodecName) {
+    if (first_codec.name == cricket::kVp8CodecName) {
       return PatchVp8Answer(std::move(answer));
     }
 
-    if (params_.video_codecs[0].name == cricket::kVp9CodecName) {
+    if (first_codec.name == cricket::kVp9CodecName) {
       return PatchVp9Answer(std::move(answer));
     }
   }
@@ -534,7 +536,7 @@ SignalingInterceptor::PatchOffererIceCandidates(
       // This is candidate for simulcast section, so it should be transformed
       // into candidates for replicated sections. The sdpMLineIndex is set to
       // -1 and ignored if the rid is present.
-      for (auto rid : simulcast_info_it->second->rids) {
+      for (const std::string& rid : simulcast_info_it->second->rids) {
         out.push_back(CreateIceCandidate(rid, -1, candidate->candidate()));
       }
     } else {
@@ -560,7 +562,7 @@ SignalingInterceptor::PatchAnswererIceCandidates(
       // section.
       out.push_back(CreateIceCandidate(simulcast_info_it->second->mid, 0,
                                        candidate->candidate()));
-    } else if (context_.simulcast_infos_by_rid.size()) {
+    } else if (!context_.simulcast_infos_by_rid.empty()) {
       // When using simulcast and bundle, put everything on the first m-line.
       out.push_back(CreateIceCandidate("", 0, candidate->candidate()));
     } else {

test/pc/e2e/sdp/sdp_changer.h

@@ -61,17 +61,12 @@ struct LocalAndRemoteSdp {
 
 struct PatchingParams {
   PatchingParams(
-      std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
-          video_codecs,
       bool use_conference_mode,
       std::map<std::string, int> stream_label_to_simulcast_streams_count)
-      : video_codecs(std::move(video_codecs)),
+      : use_conference_mode(use_conference_mode),
-        use_conference_mode(use_conference_mode),
         stream_label_to_simulcast_streams_count(
             stream_label_to_simulcast_streams_count) {}
 
-  std::vector<PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
-      video_codecs;
   bool use_conference_mode;
   std::map<std::string, int> stream_label_to_simulcast_streams_count;
 };
@@ -81,9 +76,11 @@ class SignalingInterceptor {
   explicit SignalingInterceptor(PatchingParams params) : params_(params) {}
 
   LocalAndRemoteSdp PatchOffer(
-      std::unique_ptr<SessionDescriptionInterface> offer);
+      std::unique_ptr<SessionDescriptionInterface> offer,
+      const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec);
   LocalAndRemoteSdp PatchAnswer(
-      std::unique_ptr<SessionDescriptionInterface> offer);
+      std::unique_ptr<SessionDescriptionInterface> answer,
+      const PeerConnectionE2EQualityTestFixture::VideoCodecConfig& first_codec);
 
   std::vector<std::unique_ptr<IceCandidateInterface>> PatchOffererIceCandidates(
       rtc::ArrayView<const IceCandidateInterface* const> candidates);
@@ -132,9 +129,9 @@ class SignalingInterceptor {
   LocalAndRemoteSdp PatchVp9Offer(
       std::unique_ptr<SessionDescriptionInterface> offer);
   LocalAndRemoteSdp PatchVp8Answer(
-      std::unique_ptr<SessionDescriptionInterface> offer);
+      std::unique_ptr<SessionDescriptionInterface> answer);
   LocalAndRemoteSdp PatchVp9Answer(
-      std::unique_ptr<SessionDescriptionInterface> offer);
+      std::unique_ptr<SessionDescriptionInterface> answer);
 
   void FillSimulcastContext(SessionDescriptionInterface* offer);
   std::unique_ptr<cricket::SessionDescription> RestoreMediaSectionsOrder(