From 9fbe9ae1c109dfd276fa99d7f79861ceeab015ed Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Mon, 20 Jan 2020 11:53:26 +0100 Subject: [PATCH] Add support of negotiating multiple codecs in PC framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10138 Change-Id: Iec7df60a4185a039bd81de200c0691747e92c10c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166601 Reviewed-by: Patrik Höglund Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#30318} --- .../peerconnection_quality_test_fixture.h | 33 ++++++++++++ test/pc/e2e/BUILD.gn | 26 +++------ test/pc/e2e/peer_connection_e2e_smoke_test.cc | 14 ++--- test/pc/e2e/peer_connection_quality_test.cc | 17 ++++-- test/pc/e2e/peer_connection_quality_test.h | 2 + test/pc/e2e/sdp/sdp_changer.cc | 54 ++++++++++--------- test/pc/e2e/sdp/sdp_changer.h | 20 +++---- 7 files changed, 102 insertions(+), 64 deletions(-) diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h index 87d3288394..7e9282b2ad 100644 --- a/api/test/peerconnection_quality_test_fixture.h +++ b/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 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 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 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 video_codecs; bool use_ulp_fec = false; bool use_flex_fec = false; // Specifies how much video encoder target bitrate should be different than diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 15475dba71..9aef78a35d 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -44,9 +44,7 @@ if (rtc_include_tests) { rtc_library("peer_connection_quality_test_params") { visibility = [ "*" ] testonly = true - sources = [ - "peer_connection_quality_test_params.h", - ] + sources = [ "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 = [ - "analyzer/video/encoded_image_data_injector.h", - ] + sources = [ "analyzer/video/encoded_image_data_injector.h" ] - deps = [ - "../../../api/video:encoded_image", - ] + deps = [ "../../../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 = [ - "analyzer/video/default_encoded_image_data_injector_unittest.cc", - ] + sources = + [ "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 = [ - "{{bundle_resources_dir}}/{{source_file_part}}", - ] + outputs = [ "{{bundle_resources_dir}}/{{source_file_part}}" ] } } rtc_library("peer_connection_e2e_smoke_test") { testonly = true - sources = [ - "peer_connection_e2e_smoke_test.cc", - ] + sources = [ "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", diff --git a/test/pc/e2e/peer_connection_e2e_smoke_test.cc b/test/pc/e2e/peer_connection_e2e_smoke_test.cc index c7fad1e7a5..6c78bbf6a0 100644 --- a/test/pc/e2e/peer_connection_e2e_smoke_test.cc +++ b/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_codec_required_params = {{"profile-id", "0"}}; + run_params.video_codecs = { + 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_codec_required_params = {{"profile-id", "0"}}; + run_params.video_codecs = { + VideoCodecConfig(cricket::kVp9CodecName, {{"profile-id", "0"}})}; RunTest( "smoke", run_params, diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 2921a7e4bd..595060d919 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/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, std::vector>*> 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 with_rtx_video_capabilities = FilterVideoCodecCapabilities( - run_params.video_codec_name, run_params.video_codec_required_params, - true, run_params.use_ulp_fec, run_params.use_flex_fec, + run_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 without_rtx_video_capabilities = FilterVideoCodecCapabilities( - run_params.video_codec_name, run_params.video_codec_required_params, - false, run_params.use_ulp_fec, run_params.use_flex_fec, + run_params.video_codecs, false, run_params.use_ulp_fec, + run_params.use_flex_fec, peer->pc_factory() ->GetRtpSenderCapabilities(cricket::MediaType::MEDIA_TYPE_VIDEO) .codecs); diff --git a/test/pc/e2e/peer_connection_quality_test.h b/test/pc/e2e/peer_connection_quality_test.h index bbeb946a7e..894e78f6b0 100644 --- a/test/pc/e2e/peer_connection_quality_test.h +++ b/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, std::vector>*> video_sources); diff --git a/test/pc/e2e/sdp/sdp_changer.cc b/test/pc/e2e/sdp/sdp_changer.cc index 3fa2e9fcd1..5536c26896 100644 --- a/test/pc/e2e/sdp/sdp_changer.cc +++ b/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& codec_required_params) { rtc::StringBuilder out; @@ -35,40 +37,42 @@ std::string CodecRequiredParamsToString( } // namespace std::vector FilterVideoCodecCapabilities( - absl::string_view codec_name, - const std::map& codec_required_params, + rtc::ArrayView video_codecs, bool use_rtx, bool use_ulpfec, bool use_flexfec, - std::vector supported_codecs) { + rtc::ArrayView supported_codecs) { std::vector output_codecs; - // Find main requested codecs among supported and add them to output. - for (auto& codec : supported_codecs) { - if (codec.name != codec_name) { - continue; - } - bool parameters_matched = true; - for (auto item : codec_required_params) { - auto it = codec.parameters.find(item.first); - if (it == codec.parameters.end()) { - parameters_matched = false; - break; + // Find requested codecs among supported and add them to output in the order + // they were requested. + for (auto& codec_request : video_codecs) { + size_t size_before = output_codecs.size(); + for (auto& codec : supported_codecs) { + if (codec.name != codec_request.name) { + continue; } - if (item.second != it->second) { - parameters_matched = false; - break; + bool parameters_matched = true; + for (auto item : codec_request.required_params) { + 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) { - output_codecs.push_back(codec); - } + RTC_CHECK_GT(output_codecs.size(), size_before) + << "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) { diff --git a/test/pc/e2e/sdp/sdp_changer.h b/test/pc/e2e/sdp/sdp_changer.h index aea72b062f..ca3de7e9f4 100644 --- a/test/pc/e2e/sdp/sdp_changer.h +++ b/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 -// from list of |supported_codecs| with specified |codec_name| and parameters, -// which contains all of |codec_required_params|. If flags |ulpfec| or |flexfec| -// set to true corresponding FEC codec will be added. FEC and RTX codecs will be -// added after required codecs. +// RtpTransceiverInterface::SetCodecPreferences(...) to negotiate use of codecs +// from list of |supported_codecs| which will match |video_codecs|. If flags +// |ulpfec| or |flexfec| set to true corresponding FEC codec will be added. +// FEC and RTX codecs will be 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| -// and |codec_required_params|, then all of them will be added to the output +// |supported_codecs|. If multiple codecs from this list will match +// |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 FilterVideoCodecCapabilities( - absl::string_view codec_name, - const std::map& codec_required_params, + rtc::ArrayView + video_codecs, bool use_rtx, bool use_ulpfec, bool use_flexfec, - std::vector supported_codecs); + rtc::ArrayView supported_codecs); struct LocalAndRemoteSdp { LocalAndRemoteSdp(std::unique_ptr local_sdp,