Add support of negotiating multiple codecs in PC framework

Bug: webrtc:10138
Change-Id: Iec7df60a4185a039bd81de200c0691747e92c10c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166601
Reviewed-by: Patrik Höglund <phoglund@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30318}

api/test/peerconnection_quality_test_fixture.h

@@ -325,6 +325,27 @@ class PeerConnectionE2EQualityTestFixture {
     TimeDelta echo_delay = TimeDelta::ms(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 {
@@ -335,12 +356,14 @@ class PeerConnectionE2EQualityTestFixture {
     // it will be shut downed.
     TimeDelta run_duration;
 
+    // Deprecated. Use |video_codecs| instead.
     // 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 video_codec_name = cricket::kVp8CodecName;
+    // Deprecated. Use |video_codecs| instead.
     // 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 |video_codec_required_params| there
@@ -348,6 +371,16 @@ class PeerConnectionE2EQualityTestFixture {
     // be equal to the value from |video_codec_required_params| for this key.
     // If empty then only name will be used to match the codec.
     std::map<std::string, std::string> video_codec_required_params;
+    // 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
+    // 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.
+    // TODO(titovartem) replace with Vp8 will be used as default after cleanup.
+    // If list is empty |video_codec_name| and |video_codec_required_params|
+    // will be used.
+    std::vector<VideoCodecConfig> video_codecs;
     bool use_ulp_fec = false;
     bool use_flex_fec = false;
     // Specifies how much video encoder target bitrate should be different than

test/pc/e2e/BUILD.gn

@@ -44,9 +44,7 @@ if (rtc_include_tests) {
 rtc_library("peer_connection_quality_test_params") {
   visibility = [ "*" ]
   testonly = true
-  sources = [
+  sources = [ "peer_connection_quality_test_params.h" ]
-    "peer_connection_quality_test_params.h",
-  ]
 
   deps = [
     "../../../api:callfactory_api",
@@ -66,13 +64,9 @@ rtc_library("peer_connection_quality_test_params") {
 rtc_library("encoded_image_data_injector_api") {
   visibility = [ "*" ]
   testonly = true
-  sources = [
+  sources = [ "analyzer/video/encoded_image_data_injector.h" ]
-    "analyzer/video/encoded_image_data_injector.h",
-  ]
 
-  deps = [
+  deps = [ "../../../api/video:encoded_image" ]
-    "../../../api/video:encoded_image",
-  ]
 }
 
 rtc_library("default_encoded_image_data_injector") {
@@ -324,9 +318,8 @@ if (rtc_include_tests) {
 
   rtc_library("default_encoded_image_data_injector_unittest") {
     testonly = true
-    sources = [
+    sources =
-      "analyzer/video/default_encoded_image_data_injector_unittest.cc",
+        [ "analyzer/video/default_encoded_image_data_injector_unittest.cc" ]
-    ]
     deps = [
       ":default_encoded_image_data_injector",
       "../../../api/video:encoded_image",
@@ -343,18 +336,14 @@ if (rtc_include_tests) {
     bundle_data("peer_connection_e2e_smoke_test_resources_bundle_data") {
       testonly = true
       sources = peer_connection_e2e_smoke_test_resources
-      outputs = [
+      outputs = [ "{{bundle_resources_dir}}/{{source_file_part}}" ]
-        "{{bundle_resources_dir}}/{{source_file_part}}",
-      ]
     }
   }
 
   rtc_library("peer_connection_e2e_smoke_test") {
     testonly = true
 
-    sources = [
+    sources = [ "peer_connection_e2e_smoke_test.cc" ]
-      "peer_connection_e2e_smoke_test.cc",
-    ]
     deps = [
       ":default_audio_quality_analyzer",
       ":default_video_quality_analyzer",
@@ -512,6 +501,7 @@ rtc_library("sdp_changer") {
   deps = [
     "../../../api:array_view",
     "../../../api:libjingle_peerconnection_api",
+    "../../../api:peer_connection_quality_test_fixture_api",
     "../../../api:rtp_parameters",
     "../../../media:rtc_media_base",
     "../../../p2p:rtc_p2p",

test/pc/e2e/peer_connection_e2e_smoke_test.cc

@@ -33,6 +33,8 @@ class PeerConnectionE2EQualityTestSmokeTest : public ::testing::Test {
   using PeerConfigurer = PeerConnectionE2EQualityTestFixture::PeerConfigurer;
   using RunParams = PeerConnectionE2EQualityTestFixture::RunParams;
   using VideoConfig = PeerConnectionE2EQualityTestFixture::VideoConfig;
+  using VideoCodecConfig =
+      PeerConnectionE2EQualityTestFixture::VideoCodecConfig;
   using AudioConfig = PeerConnectionE2EQualityTestFixture::AudioConfig;
   using ScreenShareConfig =
       PeerConnectionE2EQualityTestFixture::ScreenShareConfig;
@@ -133,8 +135,8 @@ class PeerConnectionE2EQualityTestSmokeTest : public ::testing::Test {
 #endif
 TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Smoke) {
   RunParams run_params(TimeDelta::seconds(7));
-  run_params.video_codec_name = cricket::kVp9CodecName;
+  run_params.video_codecs = {
-  run_params.video_codec_required_params = {{"profile-id", "0"}};
+      VideoCodecConfig(cricket::kVp9CodecName, {{"profile-id", "0"}})};
   run_params.use_flex_fec = true;
   run_params.use_ulp_fec = true;
   run_params.video_encoder_bitrate_multiplier = 1.1;
@@ -217,7 +219,7 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Echo) {
 #endif
 TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Simulcast) {
   RunParams run_params(TimeDelta::seconds(7));
-  run_params.video_codec_name = cricket::kVp8CodecName;
+  run_params.video_codecs = {VideoCodecConfig(cricket::kVp8CodecName)};
   RunTest(
       "simulcast", run_params,
       [](PeerConfigurer* alice) {
@@ -255,7 +257,7 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Simulcast) {
 #endif
 TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Svc) {
   RunParams run_params(TimeDelta::seconds(7));
-  run_params.video_codec_name = cricket::kVp9CodecName;
+  run_params.video_codecs = {VideoCodecConfig(cricket::kVp9CodecName)};
   RunTest(
       "simulcast", run_params,
       [](PeerConfigurer* alice) {
@@ -295,8 +297,8 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Svc) {
 #endif
 TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_HighBitrate) {
   RunParams run_params(TimeDelta::seconds(7));
-  run_params.video_codec_name = cricket::kVp9CodecName;
+  run_params.video_codecs = {
-  run_params.video_codec_required_params = {{"profile-id", "0"}};
+      VideoCodecConfig(cricket::kVp9CodecName, {{"profile-id", "0"}})};
 
   RunTest(
       "smoke", run_params,

test/pc/e2e/peer_connection_quality_test.cc

@@ -44,6 +44,7 @@ namespace webrtc_pc_e2e {
 namespace {
 
 using VideoConfig = PeerConnectionE2EQualityTestFixture::VideoConfig;
+using VideoCodecConfig = PeerConnectionE2EQualityTestFixture::VideoCodecConfig;
 
 constexpr int kDefaultTimeoutMs = 10000;
 constexpr char kSignalThreadName[] = "signaling_thread";
@@ -250,7 +251,7 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
   peer_configurations_.clear();
 
   SetDefaultValuesForMissingParams(
-      {alice_params.get(), bob_params.get()},
+      &run_params, {alice_params.get(), bob_params.get()},
       {&alice_video_generators, &bob_video_generators});
   ValidateParams(run_params, {alice_params.get(), bob_params.get()},
                  {&alice_video_generators, &bob_video_generators});
@@ -452,6 +453,7 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) {
 }
 
 void PeerConnectionE2EQualityTest::SetDefaultValuesForMissingParams(
+    RunParams* run_params,
     std::vector<Params*> params,
     std::vector<std::vector<std::unique_ptr<test::FrameGeneratorInterface>>*>
         video_generators) {
@@ -490,6 +492,11 @@ void PeerConnectionE2EQualityTest::SetDefaultValuesForMissingParams(
       }
     }
   }
+
+  if (run_params->video_codecs.empty()) {
+    run_params->video_codecs.push_back(VideoCodecConfig(
+        run_params->video_codec_name, run_params->video_codec_required_params));
+  }
 }
 
 void PeerConnectionE2EQualityTest::ValidateParams(
@@ -888,15 +895,15 @@ void PeerConnectionE2EQualityTest::SetPeerCodecPreferences(
     const RunParams& run_params) {
   std::vector<RtpCodecCapability> with_rtx_video_capabilities =
       FilterVideoCodecCapabilities(
-          run_params.video_codec_name, run_params.video_codec_required_params,
+          run_params.video_codecs, true, run_params.use_ulp_fec,
-          true, run_params.use_ulp_fec, run_params.use_flex_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_codec_name, run_params.video_codec_required_params,
+          run_params.video_codecs, false, run_params.use_ulp_fec,
-          false, run_params.use_ulp_fec, run_params.use_flex_fec,
+          run_params.use_flex_fec,
           peer->pc_factory()
               ->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO)
               .codecs);

test/pc/e2e/peer_connection_quality_test.h

@@ -270,7 +270,9 @@ class PeerConnectionE2EQualityTest
   //  * Generate video stream labels if some of them missed
   //  * Generate audio stream labels if some of them missed
   //  * Set video source generation mode if it is not specified
+  //  * Video codecs under test
   void SetDefaultValuesForMissingParams(
+      RunParams* run_params,
       std::vector<Params*> params,
       std::vector<std::vector<std::unique_ptr<test::FrameGeneratorInterface>>*>
           video_sources);

test/pc/e2e/sdp/sdp_changer.cc

@@ -23,6 +23,8 @@ namespace webrtc {
 namespace webrtc_pc_e2e {
 namespace {
 
+using VideoCodecConfig = PeerConnectionE2EQualityTestFixture::VideoCodecConfig;
+
 std::string CodecRequiredParamsToString(
     const std::map<std::string, std::string>& codec_required_params) {
   rtc::StringBuilder out;
@@ -35,40 +37,42 @@ std::string CodecRequiredParamsToString(
 }  // namespace
 
 std::vector<RtpCodecCapability> FilterVideoCodecCapabilities(
-    absl::string_view codec_name,
+    rtc::ArrayView<const VideoCodecConfig> video_codecs,
-    const std::map<std::string, std::string>& codec_required_params,
     bool use_rtx,
     bool use_ulpfec,
     bool use_flexfec,
-    std::vector<RtpCodecCapability> supported_codecs) {
+    rtc::ArrayView<const RtpCodecCapability> supported_codecs) {
   std::vector<RtpCodecCapability> output_codecs;
-  // Find main requested codecs among supported and add them to output.
+  // Find requested codecs among supported and add them to output in the order
-  for (auto& codec : supported_codecs) {
+  // they were requested.
-    if (codec.name != codec_name) {
+  for (auto& codec_request : video_codecs) {
-      continue;
+    size_t size_before = output_codecs.size();
-    }
+    for (auto& codec : supported_codecs) {
-    bool parameters_matched = true;
+      if (codec.name != codec_request.name) {
-    for (auto item : codec_required_params) {
+        continue;
-      auto it = codec.parameters.find(item.first);
-      if (it == codec.parameters.end()) {
-        parameters_matched = false;
-        break;
       }
-      if (item.second != it->second) {
+      bool parameters_matched = true;
-        parameters_matched = false;
+      for (auto item : codec_request.required_params) {
-        break;
+        auto it = codec.parameters.find(item.first);
+        if (it == codec.parameters.end()) {
+          parameters_matched = false;
+          break;
+        }
+        if (item.second != it->second) {
+          parameters_matched = false;
+          break;
+        }
+      }
+      if (parameters_matched) {
+        output_codecs.push_back(codec);
       }
     }
-    if (parameters_matched) {
+    RTC_CHECK_GT(output_codecs.size(), size_before)
-      output_codecs.push_back(codec);
+        << "Codec with name=" << codec_request.name << " and params {"
-    }
+        << CodecRequiredParamsToString(codec_request.required_params)
+        << "} is unsupported for this peer connection";
   }
 
-  RTC_CHECK_GT(output_codecs.size(), 0)
-      << "Codec with name=" << codec_name << " and params {"
-      << CodecRequiredParamsToString(codec_required_params)
-      << "} is unsupported for this peer connection";
-
   // Add required FEC and RTX codecs to output.
   for (auto& codec : supported_codecs) {
     if (codec.name == cricket::kRtxCodecName && use_rtx) {

test/pc/e2e/sdp/sdp_changer.h

@@ -20,6 +20,7 @@
 #include "api/array_view.h"
 #include "api/jsep.h"
 #include "api/rtp_parameters.h"
+#include "api/test/peerconnection_quality_test_fixture.h"
 #include "media/base/rid_description.h"
 #include "pc/session_description.h"
 #include "pc/simulcast_description.h"
@@ -28,24 +29,23 @@ namespace webrtc {
 namespace webrtc_pc_e2e {
 
 // Creates list of capabilities, which can be set on RtpTransceiverInterface via
-// RtpTransceiverInterface::SetCodecPreferences(...) to negotiate use of codec
+// RtpTransceiverInterface::SetCodecPreferences(...) to negotiate use of codecs
-// from list of |supported_codecs| with specified |codec_name| and parameters,
+// from list of |supported_codecs| which will match |video_codecs|. If flags
-// which contains all of |codec_required_params|. If flags |ulpfec| or |flexfec|
+// |ulpfec| or |flexfec| set to true corresponding FEC codec will be added.
-// set to true corresponding FEC codec will be added. FEC and RTX codecs will be
+// FEC and RTX codecs will be added after required codecs.
-// added after required codecs.
 //
 // All codecs will be added only if they exists in the list of
-// |supported_codecs|. If multiple codecs from this list will have |codec_name|
+// |supported_codecs|. If multiple codecs from this list will match
-// and |codec_required_params|, then all of them will be added to the output
+// |video_codecs|, then all of them will be added to the output
 // vector and they will be added in the same order, as they were in
 // |supported_codecs|.
 std::vector<RtpCodecCapability> FilterVideoCodecCapabilities(
-    absl::string_view codec_name,
+    rtc::ArrayView<const PeerConnectionE2EQualityTestFixture::VideoCodecConfig>
-    const std::map<std::string, std::string>& codec_required_params,
+        video_codecs,
     bool use_rtx,
     bool use_ulpfec,
     bool use_flexfec,
-    std::vector<RtpCodecCapability> supported_codecs);
+    rtc::ArrayView<const RtpCodecCapability> supported_codecs);
 
 struct LocalAndRemoteSdp {
   LocalAndRemoteSdp(std::unique_ptr<SessionDescriptionInterface> local_sdp,