From 54ed3ad524caf45cb043d7dbb59914a1167290f2 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Wed, 13 Nov 2024 15:56:37 +0000 Subject: [PATCH] Revert "Set default scalability mode for H.265 to L1T1." This reverts commit 775639e930f14a619974944594b40c633cc574a3. Reason for revert: Breaks internal tests. Original change's description: > Set default scalability mode for H.265 to L1T1. > > H.265 does not have software fallback, and it may have issue supporting > more than 1 temporal layers on some devices. Set default to L1T1 when > scalability is not configured, or if a scalability mode is reported as > not supported by encoder. > > Bug: chromium:41480904 > Change-Id: I53895c45ec821d65774ffe2db5f418184e3fb02a > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367835 > Reviewed-by: Sergey Silkin > Reviewed-by: Ilya Nikolaevskiy > Commit-Queue: Jianlin Qiu > Cr-Commit-Position: refs/heads/main@{#43389} Bug: chromium:41480904 No-Try: true Change-Id: I5485b1abfd5f586ec187cc57817940aa2efd72af Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368200 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Auto-Submit: Ilya Nikolaevskiy Commit-Queue: Mirko Bonadei Reviewed-by: Mirko Bonadei Reviewed-by: Jeremy Leconte Owners-Override: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#43396} --- media/engine/webrtc_video_engine.cc | 9 +- media/engine/webrtc_video_engine_unittest.cc | 86 +------------------ .../video_coding/svc/scalability_mode_util.h | 4 - 3 files changed, 5 insertions(+), 94 deletions(-) diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 69b71fff7e..781e997cc4 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -553,17 +553,10 @@ void FallbackToDefaultScalabilityModeIfNotSupported( // scalability mode of the first encoding when the others are inactive. continue; } - if (!encoding.scalability_mode.has_value() || !IsScalabilityModeSupportedByCodec(codec, *encoding.scalability_mode, config)) { - encoding.scalability_mode = - (encoding.scalability_mode != - std::string(webrtc::kDefaultScalabilityModeStr) && - IsScalabilityModeSupportedByCodec( - codec, webrtc::kDefaultScalabilityModeStr, config)) - ? webrtc::kDefaultScalabilityModeStr - : webrtc::kNoLayeringScalabilityModeStr; + encoding.scalability_mode = webrtc::kDefaultScalabilityModeStr; RTC_LOG(LS_INFO) << " -> " << *encoding.scalability_mode; } } diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 11f2ec6dfb..3aff1a6ac3 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -8481,90 +8481,12 @@ TEST_F(WebRtcVideoChannelTest, FallbackForUnsetOrUnsupportedScalabilityMode) { EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); } -#ifdef RTC_ENABLE_H265 -TEST_F( - WebRtcVideoChannelTest, - NoLayeringValueUsedIfModeIsUnsetOrUnsupportedByH265AndDefaultUnsupported) { - const absl::InlinedVector - kSupportedModes = {ScalabilityMode::kL1T1, ScalabilityMode::kL1T3}; - - encoder_factory_->AddSupportedVideoCodec(webrtc::SdpVideoFormat( - "H265", webrtc::CodecParameterMap(), kSupportedModes)); - cricket::VideoSenderParameters send_parameters; - send_parameters.codecs.push_back(GetEngineCodec("H265")); - EXPECT_TRUE(send_channel_->SetSenderParameters(send_parameters)); - - FakeVideoSendStream* stream = SetUpSimulcast(true, /*with_rtx=*/false); - - // Send a full size frame so all simulcast layers are used when reconfiguring. - webrtc::test::FrameForwarder frame_forwarder; - VideoOptions options; - EXPECT_TRUE( - send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); - send_channel_->SetSend(true); - frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame()); - - // Set scalability mode. - webrtc::RtpParameters parameters = - send_channel_->GetRtpSendParameters(last_ssrc_); - EXPECT_EQ(kNumSimulcastStreams, parameters.encodings.size()); - parameters.encodings[0].scalability_mode = std::nullopt; - parameters.encodings[1].scalability_mode = "L1T3"; // Supported. - parameters.encodings[2].scalability_mode = "L3T3"; // Unsupported. - EXPECT_TRUE(send_channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); - - // Verify that the new value is propagated down to the encoder. - // Check that WebRtcVideoSendStream updates VideoEncoderConfig correctly. - const std::optional kDefaultScalabilityMode = - webrtc::ScalabilityModeFromString(webrtc::kNoLayeringScalabilityModeStr); - EXPECT_EQ(2, stream->num_encoder_reconfigurations()); - webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig().Copy(); - EXPECT_EQ(kNumSimulcastStreams, encoder_config.number_of_streams); - EXPECT_THAT(encoder_config.simulcast_layers, - ElementsAre(Field(&webrtc::VideoStream::scalability_mode, - kDefaultScalabilityMode), - Field(&webrtc::VideoStream::scalability_mode, - ScalabilityMode::kL1T3), - Field(&webrtc::VideoStream::scalability_mode, - kDefaultScalabilityMode))); - - // FakeVideoSendStream calls CreateEncoderStreams, test that the vector of - // VideoStreams are created appropriately for the simulcast case. - EXPECT_THAT(stream->GetVideoStreams(), - ElementsAre(Field(&webrtc::VideoStream::scalability_mode, - kDefaultScalabilityMode), - Field(&webrtc::VideoStream::scalability_mode, - ScalabilityMode::kL1T3), - Field(&webrtc::VideoStream::scalability_mode, - kDefaultScalabilityMode))); - - // GetParameters. - parameters = send_channel_->GetRtpSendParameters(last_ssrc_); - EXPECT_THAT( - parameters.encodings, - ElementsAre( - Field(&webrtc::RtpEncodingParameters::scalability_mode, - webrtc::kNoLayeringScalabilityModeStr), - Field(&webrtc::RtpEncodingParameters::scalability_mode, "L1T3"), - Field(&webrtc::RtpEncodingParameters::scalability_mode, - webrtc::kNoLayeringScalabilityModeStr))); - - // No parameters changed, encoder should not be reconfigured. - EXPECT_TRUE(send_channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); - EXPECT_EQ(2, stream->num_encoder_reconfigurations()); - - EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); -} -#endif - TEST_F(WebRtcVideoChannelTest, DefaultValueUsedIfScalabilityModeIsUnsupportedByCodec) { - encoder_factory_->AddSupportedVideoCodec( - webrtc::SdpVideoFormat("VP8", webrtc::CodecParameterMap(), - {ScalabilityMode::kL1T1, ScalabilityMode::kL1T2})); - encoder_factory_->AddSupportedVideoCodec( - webrtc::SdpVideoFormat("VP9", webrtc::CodecParameterMap(), - {ScalabilityMode::kL1T2, ScalabilityMode::kL3T3})); + encoder_factory_->AddSupportedVideoCodec(webrtc::SdpVideoFormat( + "VP8", webrtc::CodecParameterMap(), {ScalabilityMode::kL1T1})); + encoder_factory_->AddSupportedVideoCodec(webrtc::SdpVideoFormat( + "VP9", webrtc::CodecParameterMap(), {ScalabilityMode::kL3T3})); cricket::VideoSenderParameters send_parameters; send_parameters.codecs.push_back(GetEngineCodec("VP9")); diff --git a/modules/video_coding/svc/scalability_mode_util.h b/modules/video_coding/svc/scalability_mode_util.h index f7689988bc..80c7283c25 100644 --- a/modules/video_coding/svc/scalability_mode_util.h +++ b/modules/video_coding/svc/scalability_mode_util.h @@ -26,10 +26,6 @@ enum class ScalabilityModeResolutionRatio { static constexpr char kDefaultScalabilityModeStr[] = "L1T2"; -// Scalability mode to be used if falling back to default scalability mode is -// unsupported. -static constexpr char kNoLayeringScalabilityModeStr[] = "L1T1"; - RTC_EXPORT std::optional MakeScalabilityMode( int num_spatial_layers, int num_temporal_layers,