From cf919422ce32733d415462edaf75c8a7aacead6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 17 Sep 2018 16:18:57 +0200 Subject: [PATCH] Prevent resolution limited max bitrate from going below min MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When simulcast screenshare is enabled, the max bitrate for the high quality stream can be limited based on the resolution. This CL fixes a bug where that limit could get so low that it is below the min bitrate of the top layer, which in turn could cause the encoder to fail initialization. Bug: webrtc:9761 Change-Id: I093bd0ba68fe0165e8982d169daf02cdf912c924 Reviewed-on: https://webrtc-review.googlesource.com/100682 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#24767} --- media/engine/simulcast.cc | 19 +++++++++------ media/engine/simulcast_unittest.cc | 37 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc index 0cb86fb08fc..ccda6c2c92d 100644 --- a/media/engine/simulcast.cc +++ b/media/engine/simulcast.cc @@ -329,6 +329,7 @@ std::vector GetScreenshareLayers( // Add optional upper simulcast layer. const int num_temporal_layers = DefaultNumberOfTemporalLayers(1, true); int max_bitrate_bps; + bool using_boosted_bitrate = false; if (!temporal_layers_supported) { // Set the max bitrate to where the base layer would have been if temporal // layers were enabled. @@ -340,6 +341,7 @@ std::vector GetScreenshareLayers( webrtc::field_trial::IsEnabled("WebRTC-UseShortVP8TL3Pattern")) { // Experimental temporal layer mode used, use increased max bitrate. max_bitrate_bps = kScreenshareHighStreamMaxBitrateBps; + using_boosted_bitrate = true; } else { // Keep current bitrates with default 3tl/8 frame settings. // Lowest temporal layers of a 3 layer setup will have 40% of the total @@ -350,19 +352,22 @@ std::vector GetScreenshareLayers( max_bitrate_bps = 2 * ((layers[0].target_bitrate_bps * 10) / 4); } - // Cap max bitrate so it isn't overly high for the given resolution. - max_bitrate_bps = std::min(max_bitrate_bps, - FindSimulcastMaxBitrateBps(width, height)); layers[1].width = width; layers[1].height = height; layers[1].max_qp = max_qp; layers[1].max_framerate = kDefaultVideoMaxFramerate; layers[1].num_temporal_layers = temporal_layers_supported ? DefaultNumberOfTemporalLayers(1, true) : 0; - layers[1].min_bitrate_bps = - max_bitrate_bps == kScreenshareHighStreamMaxBitrateBps - ? kScreenshareHighStreamMinBitrateBps - : layers[0].target_bitrate_bps * 2; + layers[1].min_bitrate_bps = using_boosted_bitrate + ? kScreenshareHighStreamMinBitrateBps + : layers[0].target_bitrate_bps * 2; + + // Cap max bitrate so it isn't overly high for the given resolution. + int resolution_limited_bitrate = std::max( + FindSimulcastMaxBitrateBps(width, height), layers[1].min_bitrate_bps); + max_bitrate_bps = + std::min(max_bitrate_bps, resolution_limited_bitrate); + layers[1].target_bitrate_bps = max_bitrate_bps; layers[1].max_bitrate_bps = max_bitrate_bps; } diff --git a/media/engine/simulcast_unittest.cc b/media/engine/simulcast_unittest.cc index d4cc624ba0c..207284f33b3 100644 --- a/media/engine/simulcast_unittest.cc +++ b/media/engine/simulcast_unittest.cc @@ -187,4 +187,41 @@ TEST(SimulcastTest, GetConfigForScreenshareSimulcastWithLimitedMaxLayers) { EXPECT_EQ(kMaxLayers, streams.size()); } +TEST(SimulcastTest, SimulcastScreenshareMaxBitrateAdjustedForResolution) { + test::ScopedFieldTrials field_trials("WebRTC-SimulcastScreenshare/Enabled/"); + + constexpr int kScreenshareHighStreamMinBitrateBps = 600000; + constexpr int kScreenshareHighStreamMaxBitrateBps = 1250000; + constexpr int kMaxBirate960_540 = 900000; + + // Normal case, max bitrate not limited by resolution. + const size_t kMaxLayers = 2; + std::vector streams = cricket::GetSimulcastConfig( + kMaxLayers, 1920, 1080, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, + kScreenshare); + EXPECT_EQ(kMaxLayers, streams.size()); + EXPECT_EQ(streams[1].max_bitrate_bps, kScreenshareHighStreamMaxBitrateBps); + EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps); + EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps); + + // At 960x540, the max bitrate is limited to 900kbps. + streams = cricket::GetSimulcastConfig(kMaxLayers, 960, 540, kMaxBitrateBps, + kBitratePriority, kQpMax, kMaxFps, + kScreenshare); + EXPECT_EQ(kMaxLayers, streams.size()); + EXPECT_EQ(streams[1].max_bitrate_bps, kMaxBirate960_540); + EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps); + EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps); + + // At 480x270, the max bitrate is limited to 450kbps. This is lower than + // the min bitrate, so use that as a lower bound. + streams = cricket::GetSimulcastConfig(kMaxLayers, 480, 270, kMaxBitrateBps, + kBitratePriority, kQpMax, kMaxFps, + kScreenshare); + EXPECT_EQ(kMaxLayers, streams.size()); + EXPECT_EQ(streams[1].max_bitrate_bps, kScreenshareHighStreamMinBitrateBps); + EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps); + EXPECT_GE(streams[1].max_bitrate_bps, streams[1].min_bitrate_bps); +} + } // namespace webrtc