From b27ac6bc83e58fdd28a09becaf0127c8cf144594 Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Thu, 18 Jul 2024 16:54:12 +0200 Subject: [PATCH] Set min bitrate equal to kDefaultMinVideoBitrateBps If experimental min bitrate value is not configured, set the min bitrate for the first simulcast stream equal to kDefaultMinVideoBitrateBps (=30kbps). Min bitrate depends on resolution. At absence of the experimental min bitrate override, we got high min bitrate values for high resolutions (600kbps for VP8 720p, for example) before. That led to encode pauses [1] which is an undesired behavior. [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/utility/simulcast_rate_allocator.cc;l=173;drc=f317f7106a7a15a04da7cd30c2e2ddb1b3025bc6 Bug: webrtc:351644568, b/352504711 Change-Id: Ifc93cc230fb194d2c9a739368d415f24385939fd No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/357420 Reviewed-by: Danil Chapovalov Commit-Queue: Sergey Silkin Cr-Commit-Position: refs/heads/main@{#42649} --- video/config/BUILD.gn | 1 + video/config/encoder_stream_factory.cc | 19 ++++++------ .../config/encoder_stream_factory_unittest.cc | 31 ++++++++++++++++++- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/video/config/BUILD.gn b/video/config/BUILD.gn index aef6c7453c..4c20e02f55 100644 --- a/video/config/BUILD.gn +++ b/video/config/BUILD.gn @@ -71,6 +71,7 @@ if (rtc_include_tests) { ":streams_config", "../../call/adaptation:resource_adaptation", "../../media:media_constants", + "../../rtc_base/experiments:min_video_bitrate_experiment", "../../test:explicit_key_value_config", "../../test:test_support", ] diff --git a/video/config/encoder_stream_factory.cc b/video/config/encoder_stream_factory.cc index 54dff93cc8..7dd53ab389 100644 --- a/video/config/encoder_stream_factory.cc +++ b/video/config/encoder_stream_factory.cc @@ -320,23 +320,24 @@ EncoderStreamFactory::CreateSimulcastOrConferenceModeScreenshareStreams( const absl::optional& experimental_min_bitrate) const { bool is_screencast = encoder_config.content_type == webrtc::VideoEncoderConfig::ContentType::kScreen; + const bool is_legacy_screencast = + webrtc::SimulcastUtility::IsConferenceModeScreenshare(encoder_config); std::vector layers; const bool temporal_layers_supported = IsTemporalLayersSupported(encoder_config.codec_type); // Use legacy simulcast screenshare if conference mode is explicitly enabled // or use the regular simulcast configuration path which is generic. - layers = GetSimulcastConfig( - FindRequiredActiveLayers(encoder_config), - encoder_config.number_of_streams, width, height, - webrtc::SimulcastUtility::IsConferenceModeScreenshare(encoder_config), - temporal_layers_supported, trials, encoder_config.codec_type); + layers = GetSimulcastConfig(FindRequiredActiveLayers(encoder_config), + encoder_config.number_of_streams, width, height, + is_legacy_screencast, temporal_layers_supported, + trials, encoder_config.codec_type); // Allow an experiment to override the minimum bitrate for the lowest // spatial layer. The experiment's configuration has the lowest priority. - if (experimental_min_bitrate) { - layers[0].min_bitrate_bps = - rtc::saturated_cast(experimental_min_bitrate->bps()); - } + layers[0].min_bitrate_bps = experimental_min_bitrate + .value_or(webrtc::DataRate::BitsPerSec( + webrtc::kDefaultMinVideoBitrateBps)) + .bps(); const bool has_scale_resolution_down_by = absl::c_any_of( encoder_config.simulcast_layers, [](const webrtc::VideoStream& layer) { diff --git a/video/config/encoder_stream_factory_unittest.cc b/video/config/encoder_stream_factory_unittest.cc index f095a086d6..7daf0b2eea 100644 --- a/video/config/encoder_stream_factory_unittest.cc +++ b/video/config/encoder_stream_factory_unittest.cc @@ -11,14 +11,17 @@ #include "video/config/encoder_stream_factory.h" #include "call/adaptation/video_source_restrictions.h" +#include "rtc_base/experiments/min_video_bitrate_experiment.h" #include "test/explicit_key_value_config.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { namespace { - using ::cricket::EncoderStreamFactory; using test::ExplicitKeyValueConfig; +using ::testing::IsEmpty; +using ::testing::Not; std::vector GetStreamResolutions( const std::vector& streams) { @@ -96,4 +99,30 @@ TEST(EncoderStreamFactory, BitratePriority) { EXPECT_EQ(streams[0].bitrate_priority, kBitratePriority); EXPECT_FALSE(streams[1].bitrate_priority); } + +TEST(EncoderStreamFactory, SetsMinBitrateToDefaultValue) { + VideoEncoder::EncoderInfo encoder_info; + auto factory = rtc::make_ref_counted(encoder_info); + VideoEncoderConfig encoder_config; + encoder_config.number_of_streams = 2; + encoder_config.simulcast_layers.resize(encoder_config.number_of_streams); + auto streams = factory->CreateEncoderStreams(ExplicitKeyValueConfig(""), 1920, + 1080, encoder_config); + ASSERT_THAT(streams, Not(IsEmpty())); + EXPECT_EQ(streams[0].min_bitrate_bps, kDefaultMinVideoBitrateBps); +} + +TEST(EncoderStreamFactory, SetsMinBitrateToExperimentalValue) { + VideoEncoder::EncoderInfo encoder_info; + auto factory = rtc::make_ref_counted(encoder_info); + VideoEncoderConfig encoder_config; + encoder_config.number_of_streams = 2; + encoder_config.simulcast_layers.resize(encoder_config.number_of_streams); + auto streams = factory->CreateEncoderStreams( + ExplicitKeyValueConfig("WebRTC-Video-MinVideoBitrate/Enabled,br:1kbps/"), + 1920, 1080, encoder_config); + ASSERT_THAT(streams, Not(IsEmpty())); + EXPECT_NE(streams[0].min_bitrate_bps, kDefaultMinVideoBitrateBps); + EXPECT_EQ(streams[0].min_bitrate_bps, 1000); +} } // namespace webrtc