Adjust min bitrate for the first active stream
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 <asapersson@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32706}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 8a916c4..5837899 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -3625,6 +3625,32 @@
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<size_t> 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 44984c5..b940cbf 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -7878,6 +7878,59 @@
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<uint32_t> 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<webrtc::VideoStream> 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,