From d3811947e6f37da67c682d9d3c51ddeb344df2b6 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Thu, 26 Nov 2020 17:24:47 +0100 Subject: [PATCH] Adjust min bitrate for the first active stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this change, if the user disables QVGA and VGA streams via |active| flags in SetParamters, the resulting stream would have too high min bitrate. This would lead to bad performance and low quality adaptation rate. Bug: none Change-Id: I919a30bfb248c06747c989afe6965b3afaef2260 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195325 Reviewed-by: Åsa Persson Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#32706} --- media/engine/webrtc_video_engine.cc | 26 ++++++++++ media/engine/webrtc_video_engine_unittest.cc | 53 ++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 8a916c4c7e..5837899bf3 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3625,6 +3625,32 @@ EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams( BoostMaxSimulcastLayer( webrtc::DataRate::BitsPerSec(encoder_config.max_bitrate_bps), &layers); } + + // Sort the layers by max_bitrate_bps, they might not always be from + // smallest to biggest + std::vector index(layers.size()); + std::iota(index.begin(), index.end(), 0); + std::stable_sort(index.begin(), index.end(), [&layers](size_t a, size_t b) { + return layers[a].max_bitrate_bps < layers[b].max_bitrate_bps; + }); + + if (!layers[index[0]].active) { + // Adjust min bitrate of the first active layer to allow it to go as low as + // the lowest (now inactive) layer could. + // Otherwise, if e.g. a single HD stream is active, it would have 600kbps + // min bitrate, which would always be allocated to the stream. + // This would lead to congested network, dropped frames and overall bad + // experience. + + const int min_configured_bitrate = layers[index[0]].min_bitrate_bps; + for (size_t i = 0; i < layers.size(); ++i) { + if (layers[index[i]].active) { + layers[index[i]].min_bitrate_bps = min_configured_bitrate; + break; + } + } + } + return layers; } diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 44984c5ed4..b940cbf091 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -7878,6 +7878,59 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersMultipleEncodingsActive) { EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, nullptr, nullptr)); } +// Tests that when some streams are disactivated then the lowest +// stream min_bitrate would be reused for the first active stream. +TEST_F(WebRtcVideoChannelTest, + SetRtpSendParametersSetsMinBitrateForFirstActiveStream) { + // Create the stream params with multiple ssrcs for simulcast. + const size_t kNumSimulcastStreams = 3; + std::vector ssrcs = MAKE_VECTOR(kSsrcs3); + StreamParams stream_params = CreateSimStreamParams("cname", ssrcs); + FakeVideoSendStream* fake_video_send_stream = AddSendStream(stream_params); + uint32_t primary_ssrc = stream_params.first_ssrc(); + + // Using the FrameForwarder, we manually send a full size + // frame. This allows us to test that ReconfigureEncoder is called + // appropriately. + webrtc::test::FrameForwarder frame_forwarder; + VideoOptions options; + EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, &options, &frame_forwarder)); + channel_->SetSend(true); + frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame( + 1920, 1080, webrtc::VideoRotation::kVideoRotation_0, + rtc::kNumMicrosecsPerSec / 30)); + + // Check that all encodings are initially active. + webrtc::RtpParameters parameters = + channel_->GetRtpSendParameters(primary_ssrc); + EXPECT_EQ(kNumSimulcastStreams, parameters.encodings.size()); + EXPECT_TRUE(parameters.encodings[0].active); + EXPECT_TRUE(parameters.encodings[1].active); + EXPECT_TRUE(parameters.encodings[2].active); + EXPECT_TRUE(fake_video_send_stream->IsSending()); + + // Only turn on the highest stream. + parameters.encodings[0].active = false; + parameters.encodings[1].active = false; + parameters.encodings[2].active = true; + EXPECT_TRUE(channel_->SetRtpSendParameters(primary_ssrc, parameters).ok()); + + // Check that the VideoSendStream is updated appropriately. This means its + // send state was updated and it was reconfigured. + EXPECT_TRUE(fake_video_send_stream->IsSending()); + std::vector simulcast_streams = + fake_video_send_stream->GetVideoStreams(); + EXPECT_EQ(kNumSimulcastStreams, simulcast_streams.size()); + EXPECT_FALSE(simulcast_streams[0].active); + EXPECT_FALSE(simulcast_streams[1].active); + EXPECT_TRUE(simulcast_streams[2].active); + + EXPECT_EQ(simulcast_streams[2].min_bitrate_bps, + simulcast_streams[0].min_bitrate_bps); + + EXPECT_TRUE(channel_->SetVideoSend(primary_ssrc, nullptr, nullptr)); +} + // Test that if a stream is reconfigured (due to a codec change or other // change) while its encoding is still inactive, it doesn't start sending. TEST_F(WebRtcVideoChannelTest,