From 4f1af1156f416767a430b482ca8a4b7a47f52ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Tue, 5 Jul 2022 08:52:23 +0000 Subject: [PATCH] Revert "When VP9 SVC is used, use SvcConfig to set max bitrate for the stream." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 3afb8e24311dc1297150d4011894b6cb00841735. Reason for revert: Causes some unexpected perf regressions. Original change's description: > When VP9 SVC is used, use SvcConfig to set max bitrate for the stream. > > Currently, a default max bitrate is determined within WebRtcVideoEngine, > which maxes out at 2.5Mbps - and that limits the max bitrate deteremined > by SvcConfig for resolutions above 720p. > > This does not affect simulcast, as WebRtcVideoEngine already knows to > trust the rate allocation in simulcast.cc instead. > > Bug: webrtc:14017 > Change-Id: I0c310a6fd496e9e5a10eae45838900068aa1ae2d > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267160 > Reviewed-by: Ilya Nikolaevskiy > Commit-Queue: Erik Språng > Cr-Commit-Position: refs/heads/main@{#37370} Bug: webrtc:14017 Change-Id: I1e45ee3f78deb50a9057d648146b1a6360782aa3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267800 Commit-Queue: Ilya Nikolaevskiy Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Erik Språng Auto-Submit: Erik Språng Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#37438} --- media/BUILD.gn | 1 - media/engine/webrtc_video_engine.cc | 44 ++++-------------- media/engine/webrtc_video_engine_unittest.cc | 48 -------------------- 3 files changed, 9 insertions(+), 84 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index ae252b35ae..0481e3f0fe 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -304,7 +304,6 @@ rtc_library("rtc_audio_video") { "../modules/video_coding", "../modules/video_coding:video_codec_interface", "../modules/video_coding:video_coding_utility", - "../modules/video_coding:webrtc_vp9_helpers", "../modules/video_coding/svc:scalability_mode_util", "../rtc_base", "../rtc_base:audio_format_to_string", diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index f089881f67..26ea4ee30d 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -32,7 +32,6 @@ #include "media/engine/webrtc_media_engine.h" #include "media/engine/webrtc_voice_engine.h" #include "modules/rtp_rtcp/source/rtp_util.h" -#include "modules/video_coding/codecs/vp9/svc_config.h" #include "modules/video_coding/svc/scalability_mode_util.h" #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/experiments/field_trial_parser.h" @@ -3639,40 +3638,6 @@ EncoderStreamFactory::CreateDefaultVideoStreams( kMinLayerSize); } - if (absl::EqualsIgnoreCase(codec_name_, kVp9CodecName)) { - RTC_DCHECK(encoder_config.encoder_specific_settings); - // Use VP9 SVC layering from codec settings which might be initialized - // though field trial in ConfigureVideoEncoderSettings. - webrtc::VideoCodecVP9 vp9_settings; - encoder_config.encoder_specific_settings->FillVideoCodecVp9(&vp9_settings); - layer.num_temporal_layers = vp9_settings.numberOfTemporalLayers; - - // Spatial layers are currently signalled as simulcast layers. - int num_spatial_layers = std::max(encoder_config.simulcast_layers.size(), - encoder_config.spatial_layers.size()); - if (width * height > 0 && - (layer.num_temporal_layers > 1u || num_spatial_layers > 1)) { - // In SVC mode, the VP9 max bitrate is determined by SvcConfig, instead of - // GetMaxDefaultVideoBitrateKbps(). - std::vector svc_layers = - webrtc::GetSvcConfig(width, height, max_framerate, - /*first_active_layer=*/0, num_spatial_layers, - *layer.num_temporal_layers, is_screenshare_); - int sum_max_bitrates_kbps = 0; - for (const webrtc::SpatialLayer& spatial_layer : svc_layers) { - sum_max_bitrates_kbps += spatial_layer.maxBitrate; - } - RTC_DCHECK_GE(sum_max_bitrates_kbps, 0); - if (encoder_config.max_bitrate_bps <= 0) { - max_bitrate_bps = sum_max_bitrates_kbps * 1000; - } else { - max_bitrate_bps = - std::min(max_bitrate_bps, sum_max_bitrates_kbps * 1000); - } - max_bitrate_bps = std::max(min_bitrate_bps, max_bitrate_bps); - } - } - // In the case that the application sets a max bitrate that's lower than the // min bitrate, we adjust it down (see bugs.webrtc.org/9141). layer.min_bitrate_bps = std::min(min_bitrate_bps, max_bitrate_bps); @@ -3686,6 +3651,15 @@ EncoderStreamFactory::CreateDefaultVideoStreams( layer.max_qp = max_qp_; layer.bitrate_priority = encoder_config.bitrate_priority; + if (absl::EqualsIgnoreCase(codec_name_, kVp9CodecName)) { + RTC_DCHECK(encoder_config.encoder_specific_settings); + // Use VP9 SVC layering from codec settings which might be initialized + // though field trial in ConfigureVideoEncoderSettings. + webrtc::VideoCodecVP9 vp9_settings; + encoder_config.encoder_specific_settings->FillVideoCodecVp9(&vp9_settings); + layer.num_temporal_layers = vp9_settings.numberOfTemporalLayers; + } + if (IsTemporalLayersSupported(codec_name_)) { // Use configured number of temporal layers if set. if (encoder_config.simulcast_layers[0].num_temporal_layers) { diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index b54443103c..d84c32323e 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -70,11 +70,9 @@ using ::testing::_; using ::testing::Contains; using ::testing::Each; -using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::Field; -using ::testing::Gt; using ::testing::IsEmpty; using ::testing::Pair; using ::testing::Return; @@ -3734,52 +3732,6 @@ TEST_F(Vp9SettingsTest, AllEncodingParametersCopied) { EXPECT_TRUE(encoder_config.simulcast_layers[2].active); } -TEST_F(Vp9SettingsTest, MaxBitrateDeterminedBySvcResolutions) { - cricket::VideoSendParameters parameters; - parameters.codecs.push_back(GetEngineCodec("VP9")); - ASSERT_TRUE(channel_->SetSendParameters(parameters)); - - std::vector ssrcs = MAKE_VECTOR(kSsrcs3); - - FakeVideoSendStream* stream = - AddSendStream(CreateSimStreamParams("cname", ssrcs)); - - webrtc::VideoSendStream::Config config = stream->GetConfig().Copy(); - - webrtc::test::FrameForwarder frame_forwarder; - EXPECT_TRUE(channel_->SetVideoSend(ssrcs[0], nullptr, &frame_forwarder)); - channel_->SetSend(true); - - // Send frame at 1080p@30fps. - frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame( - 1920, 1080, webrtc::VideoRotation::kVideoRotation_0, - /*duration_us=*/33000)); - - webrtc::VideoCodecVP9 vp9_settings; - ASSERT_TRUE(stream->GetVp9Settings(&vp9_settings)) << "No VP9 config set."; - - const size_t kNumSpatialLayers = ssrcs.size(); - const size_t kNumTemporalLayers = 3; - EXPECT_EQ(vp9_settings.numberOfSpatialLayers, kNumSpatialLayers); - EXPECT_EQ(vp9_settings.numberOfTemporalLayers, kNumTemporalLayers); - - EXPECT_TRUE(channel_->SetVideoSend(ssrcs[0], nullptr, nullptr)); - - // VideoStream max bitrate should be more than legacy 2.5Mbps default stream - // cap. - EXPECT_THAT( - stream->GetVideoStreams(), - ElementsAre(Field(&webrtc::VideoStream::max_bitrate_bps, Gt(2500000)))); - - // Update send parameters to 2Mbps, this should cap the max bitrate of the - // stream. - parameters.max_bandwidth_bps = 2000000; - channel_->SetSendParameters(parameters); - EXPECT_THAT( - stream->GetVideoStreams(), - ElementsAre(Field(&webrtc::VideoStream::max_bitrate_bps, Eq(2000000)))); -} - class Vp9SettingsTestWithFieldTrial : public Vp9SettingsTest, public ::testing::WithParamInterface<