From c401923f3ee3b4ec5423a3ede7e91550e2acd52d Mon Sep 17 00:00:00 2001 From: Yun Zhang Date: Fri, 2 Oct 2020 11:23:03 -0700 Subject: [PATCH] Take max bitrate into account for target bitrate decision when min bitrate is empty Currently, when only max bitrate available and min bitratea & target bitrate are missing from encoding config, the target bitrate is decided by the calculation from GetSimulcastConfig() according to width/height/qp. The max bitrate doesn't play a role here other than ensure target < max. This will make the target bitrate cap at some calculated number even when control message gives much larger allocation through max bitrate. In our cases, the L0 (at 180p) is capped at 80-90kbps even control message gives L0's max bitrate over 300kbps. This under-use of bandwidth happens to all layer other than top layer. Top layer will be compensated with all the left bandwidth up to max at last. Since in web api, we cannot pass down either min bitrate or target bitrate (https://developer.mozilla.org/en-US/docs/Web/API/RTCRtpEncodingParameters). We propose a new logic to take max bitrate into consideration in this case, use 3/4 max bitrate or calculated target bitrate whichever is larger. Bug: None Change-Id: I2234b4636daa379fd47d4bbe764cf8307b9a1ea4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186161 Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Rasmus Brandt Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#32308} --- media/engine/webrtc_video_engine.cc | 12 ++++++++++-- media/engine/webrtc_video_engine_unittest.cc | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index e56905f0c6..43ecf9dc4b 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3569,10 +3569,18 @@ EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams( std::max(layers[i].max_bitrate_bps, layers[i].min_bitrate_bps); } else if (encoder_config.simulcast_layers[i].max_bitrate_bps > 0) { // Only max bitrate is configured, make sure min/target are below max. + // Keep target bitrate if it is set explicitly in encoding config. + // Otherwise set target bitrate to 3/4 of the max bitrate + // or the one calculated from GetSimulcastConfig() which is larger. layers[i].min_bitrate_bps = std::min(layers[i].min_bitrate_bps, layers[i].max_bitrate_bps); - layers[i].target_bitrate_bps = - std::min(layers[i].target_bitrate_bps, layers[i].max_bitrate_bps); + if (encoder_config.simulcast_layers[i].target_bitrate_bps <= 0) { + layers[i].target_bitrate_bps = std::max( + layers[i].target_bitrate_bps, layers[i].max_bitrate_bps * 3 / 4); + } + layers[i].target_bitrate_bps = std::max( + std::min(layers[i].target_bitrate_bps, layers[i].max_bitrate_bps), + layers[i].min_bitrate_bps); } if (i == layers.size() - 1) { is_highest_layer_max_bitrate_configured = diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 9353f19da6..ccac755134 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -10,6 +10,7 @@ #include "media/engine/webrtc_video_engine.h" +#include #include #include #include @@ -7545,10 +7546,13 @@ TEST_F(WebRtcVideoChannelTest, MinOrMaxSimulcastBitratePropagatedToEncoder) { EXPECT_EQ(kDefault[0].max_bitrate_bps, stream->GetVideoStreams()[0].max_bitrate_bps); // Layer 1: max configured bitrate should overwrite max default. + // And target bitrate should be 3/4 * max bitrate or default target + // which is larger. EXPECT_EQ(kDefault[1].min_bitrate_bps, stream->GetVideoStreams()[1].min_bitrate_bps); - EXPECT_EQ(kDefault[1].target_bitrate_bps, - stream->GetVideoStreams()[1].target_bitrate_bps); + const int kTargetBpsLayer1 = + std::max(kDefault[1].target_bitrate_bps, kMaxBpsLayer1 * 3 / 4); + EXPECT_EQ(kTargetBpsLayer1, stream->GetVideoStreams()[1].target_bitrate_bps); EXPECT_EQ(kMaxBpsLayer1, stream->GetVideoStreams()[1].max_bitrate_bps); // Layer 2: min and max bitrate not configured, default expected. EXPECT_EQ(kDefault[2].min_bitrate_bps,