From a63b6b7d4003464351a99d6b117d509f3a45529a Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Mon, 25 Apr 2022 17:27:15 +0200 Subject: [PATCH] [PCLF] Allow configuring RtpEncodingParameters with singlecast With the encoding parameters in the SimulcastConfig objects, it wasn't possible to configure explicit encoding parameters when using singlecast, required for example to use the spec standard SVC API. Bug: webrtc:11607 Change-Id: I92b1446e772e2ecec93379dc91a3da159b8bc209 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260002 Commit-Queue: Florent Castelli Reviewed-by: Mirko Bonadei Reviewed-by: Artem Titov Cr-Commit-Position: refs/heads/main@{#36731} --- .../peerconnection_quality_test_fixture.h | 15 ++++++++- test/pc/e2e/peer_configurer.cc | 17 ++++++++-- test/pc/e2e/peer_connection_e2e_smoke_test.cc | 33 ++++++++++--------- test/pc/e2e/peer_connection_quality_test.cc | 6 ++++ 4 files changed, 52 insertions(+), 19 deletions(-) diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h index 4961fb59e4..7e1c0e4fb2 100644 --- a/api/test/peerconnection_quality_test_fixture.h +++ b/api/test/peerconnection_quality_test_fixture.h @@ -127,7 +127,12 @@ class PeerConnectionE2EQualityTestFixture { std::vector slides_yuv_file_names; }; - // Config for Vp8 simulcast or Vp9 SVC testing. + // Config for Vp8 simulcast or non-standard Vp9 SVC testing. + // + // To configure standard SVC setting, use `scalability_mode` in the + // `encoding_params` array. + // This configures Vp9 SVC by requesting simulcast layers, the request is + // internally converted to a request for SVC layers. // // SVC support is limited: // During SVC testing there is no SFU, so framework will try to emulate SFU @@ -256,6 +261,14 @@ class PeerConnectionE2EQualityTestFixture { // but only on non-lossy networks. See more in documentation to // VideoSimulcastConfig. absl::optional simulcast_config; + // Encoding parameters for both singlecast and per simulcast layer. + // If singlecast is used, if not empty, a single value can be provided. + // If simulcast is used, if not empty, `encoding_params` size have to be + // equal to `simulcast_config.simulcast_streams_count`. Will be used to set + // transceiver send encoding params for each layer. + // RtpEncodingParameters::rid may be changed by fixture implementation to + // ensure signaling correctness. + std::vector encoding_params; // Count of temporal layers for video stream. This value will be set into // each RtpEncodingParameters of RtpParameters of corresponding // RtpSenderInterface for this video stream. diff --git a/test/pc/e2e/peer_configurer.cc b/test/pc/e2e/peer_configurer.cc index d7dd94be6d..c316c68c12 100644 --- a/test/pc/e2e/peer_configurer.cc +++ b/test/pc/e2e/peer_configurer.cc @@ -139,13 +139,26 @@ void PeerParamsPreprocessor::ValidateParams(const PeerConfigurerImpl& peer) { << "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 (p.video_codecs[0].name == cricket::kVp8CodecName && - !video_config.simulcast_config->encoding_params.empty()) { + RTC_CHECK(video_config.simulcast_config->encoding_params.empty() || + video_config.encoding_params.empty()) + << "Can't provide |encoding_params| in both |simulcast_config| and " + "|video_config|."; + if (!video_config.simulcast_config->encoding_params.empty()) { RTC_CHECK_EQ(video_config.simulcast_config->simulcast_streams_count, video_config.simulcast_config->encoding_params.size()) << "|encoding_params| have to be specified for each simulcast " << "stream in |simulcast_config|."; } + if (!video_config.encoding_params.empty()) { + RTC_CHECK_EQ(video_config.simulcast_config->simulcast_streams_count, + video_config.encoding_params.size()) + << "|encoding_params| have to be specified for each simulcast " + << "stream in |video_config|."; + } + } else { + RTC_CHECK_LE(video_config.encoding_params.size(), 1) + << "|encoding_params| has multiple values but simulcast is not " + "enabled."; } } if (p.audio_config) { diff --git a/test/pc/e2e/peer_connection_e2e_smoke_test.cc b/test/pc/e2e/peer_connection_e2e_smoke_test.cc index 09c40a6d75..4ee6590419 100644 --- a/test/pc/e2e/peer_connection_e2e_smoke_test.cc +++ b/test/pc/e2e/peer_connection_e2e_smoke_test.cc @@ -322,20 +322,19 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_ChangeNetworkConditions) { TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_Screenshare) { std::pair network_links = CreateNetwork(); - AddPeer( - network_links.first, [](PeerConfigurer* alice) { - VideoConfig screenshare(320, 180, 30); - screenshare.stream_label = "alice-screenshare"; - screenshare.content_hint = VideoTrackInterface::ContentHint::kText; - ScreenShareConfig screen_share_config = - ScreenShareConfig(TimeDelta::Seconds(2)); - screen_share_config.scrolling_params = ScrollingParams( - TimeDelta::Millis(1800), kDefaultSlidesWidth, kDefaultSlidesHeight); - auto screen_share_frame_generator = - CreateScreenShareFrameGenerator(screenshare, screen_share_config); - alice->AddVideoConfig(std::move(screenshare), - std::move(screen_share_frame_generator)); - }); + AddPeer(network_links.first, [](PeerConfigurer* alice) { + VideoConfig screenshare(320, 180, 30); + screenshare.stream_label = "alice-screenshare"; + screenshare.content_hint = VideoTrackInterface::ContentHint::kText; + ScreenShareConfig screen_share_config = + ScreenShareConfig(TimeDelta::Seconds(2)); + screen_share_config.scrolling_params = ScrollingParams( + TimeDelta::Millis(1800), kDefaultSlidesWidth, kDefaultSlidesHeight); + auto screen_share_frame_generator = + CreateScreenShareFrameGenerator(screenshare, screen_share_config); + alice->AddVideoConfig(std::move(screenshare), + std::move(screen_share_frame_generator)); + }); AddPeer(network_links.second, [](PeerConfigurer* bob) {}); RunAndCheckEachVideoStreamReceivedFrames(RunParams(TimeDelta::Seconds(2))); } @@ -444,8 +443,10 @@ TEST_F(PeerConnectionE2EQualityTestSmokeTest, MAYBE_HighBitrate) { alice->SetBitrateSettings(bitrate_settings); VideoConfig video(800, 600, 15); video.stream_label = "alice-video"; - video.min_encode_bitrate_bps = 500'000; - video.max_encode_bitrate_bps = 3'000'000; + RtpEncodingParameters encoding_parameters; + encoding_parameters.min_bitrate_bps = 500'000; + encoding_parameters.max_bitrate_bps = 3'000'000; + video.encoding_params.push_back(std::move(encoding_parameters)); alice->AddVideoConfig(std::move(video)); AudioConfig audio; diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index b72369b285..89b595db15 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -486,6 +486,9 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread( if (!video_config.simulcast_config->encoding_params.empty()) { enc_params = video_config.simulcast_config->encoding_params[i]; } + if (!video_config.encoding_params.empty()) { + enc_params = video_config.encoding_params[i]; + } // We need to be sure, that all rids will be unique with all mids. enc_params.rid = std::to_string(alice_transceivers_counter) + "000" + std::to_string(i); @@ -495,6 +498,9 @@ void PeerConnectionE2EQualityTest::SetupCallOnSignalingThread( } else { transceiver_params.direction = RtpTransceiverDirection::kSendRecv; RtpEncodingParameters enc_params; + if (video_config.encoding_params.size() == 1) { + enc_params = video_config.encoding_params[0]; + } enc_params.max_bitrate_bps = video_config.max_encode_bitrate_bps; enc_params.min_bitrate_bps = video_config.min_encode_bitrate_bps; transceiver_params.send_encodings.push_back(enc_params);