From 668ce0c7fa8f261dacdba45149d10a23f772a9b6 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Thu, 4 Jul 2019 17:06:04 +0200 Subject: [PATCH] Remove trial WebRTC-SimulcastMaxLayers and make its behavior default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also cleans up the unused parameters from GetSimulcastConfig. Bug: webrtc:8785, webrtc:8486 Change-Id: I1aea8f6c9e6590211ec5ee5cafc0ec2a2100d68f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144627 Reviewed-by: Rasmus Brandt Reviewed-by: Erik Språng Reviewed-by: Sergey Silkin Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/master@{#28496} --- media/engine/simulcast.cc | 75 +++---------------- media/engine/simulcast.h | 2 - media/engine/simulcast_unittest.cc | 69 +++++++---------- media/engine/webrtc_video_engine.cc | 5 +- media/engine/webrtc_video_engine_unittest.cc | 5 +- .../test/videocodec_test_fixture_impl.cc | 5 +- 6 files changed, 45 insertions(+), 116 deletions(-) diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc index f0936755fe..bfd7a2ac5c 100644 --- a/media/engine/simulcast.cc +++ b/media/engine/simulcast.cc @@ -49,9 +49,6 @@ static const char* kSimulcastScreenshareFieldTrialName = struct SimulcastFormat { int width; int height; - // The maximum number of simulcast layers can be used for - // resolutions at |widthxheigh|. - size_t max_layers; // The maximum bitrate for encoding stream at |widthxheight|, when we are // not sending the next higher spatial stream. int max_bitrate_kbps; @@ -68,13 +65,13 @@ struct SimulcastFormat { // Important!! Keep this table from high resolution to low resolution. // clang-format off const SimulcastFormat kSimulcastFormats[] = { - {1920, 1080, 3, 5000, 4000, 800}, - {1280, 720, 3, 2500, 2500, 600}, - {960, 540, 3, 900, 900, 450}, - {640, 360, 2, 700, 500, 150}, - {480, 270, 2, 450, 350, 150}, - {320, 180, 1, 200, 150, 30}, - {0, 0, 1, 200, 150, 30} + {1920, 1080, 5000, 4000, 800}, + {1280, 720, 2500, 2500, 600}, + {960, 540, 900, 900, 450}, + {640, 360, 700, 500, 150}, + {480, 270, 450, 350, 150}, + {320, 180, 200, 150, 30}, + {0, 0, 200, 150, 30} }; // clang-format on @@ -126,21 +123,6 @@ int FindSimulcastFormatIndex(int width, int height) { return -1; } -int FindSimulcastFormatIndex(int width, int height, size_t max_layers) { - RTC_DCHECK_GE(width, 0); - RTC_DCHECK_GE(height, 0); - RTC_DCHECK_GT(max_layers, 0); - for (uint32_t i = 0; i < arraysize(kSimulcastFormats); ++i) { - if (width * height >= - kSimulcastFormats[i].width * kSimulcastFormats[i].height && - max_layers == kSimulcastFormats[i].max_layers) { - return i; - } - } - RTC_NOTREACHED(); - return -1; -} - // Simulcast stream width and height must both be dividable by // |2 ^ (simulcast_layers - 1)|. int NormalizeSimulcastSize(int size, size_t simulcast_layers) { @@ -154,11 +136,6 @@ int NormalizeSimulcastSize(int size, size_t simulcast_layers) { return ((size >> base2_exponent) << base2_exponent); } -size_t FindSimulcastMaxLayers(int width, int height) { - int index = FindSimulcastFormatIndex(width, height); - return kSimulcastFormats[index].max_layers; -} - int FindSimulcastMaxBitrateBps(int width, int height) { const int format_index = FindSimulcastFormatIndex(width, height); return kSimulcastFormats[format_index].max_bitrate_kbps * 1000; @@ -174,14 +151,6 @@ int FindSimulcastMinBitrateBps(int width, int height) { return kSimulcastFormats[format_index].min_bitrate_kbps * 1000; } -void SlotSimulcastMaxResolution(size_t max_layers, int* width, int* height) { - int index = FindSimulcastFormatIndex(*width, *height, max_layers); - *width = kSimulcastFormats[index].width; - *height = kSimulcastFormats[index].height; - RTC_LOG(LS_INFO) << "SlotSimulcastMaxResolution to width:" << *width - << " height:" << *height; -} - void BoostMaxSimulcastLayer(int max_bitrate_bps, std::vector* layers) { if (layers->empty()) @@ -210,10 +179,8 @@ std::vector GetSimulcastConfig( size_t max_layers, int width, int height, - int /*max_bitrate_bps*/, double bitrate_priority, int max_qp, - int /*max_framerate*/, bool is_screenshare, bool temporal_layers_supported) { if (is_screenshare) { @@ -227,39 +194,21 @@ std::vector GetSimulcastConfig( } std::vector GetNormalSimulcastLayers( - size_t max_layers, + size_t layer_count, int width, int height, double bitrate_priority, int max_qp, bool temporal_layers_supported) { - // TODO(bugs.webrtc.org/8785): Currently if the resolution isn't large enough - // (defined in kSimulcastFormats) we scale down the number of simulcast - // layers. Consider changing this so that the application can have more - // control over exactly how many simulcast layers are used. - size_t num_simulcast_layers = FindSimulcastMaxLayers(width, height); - if (webrtc::field_trial::IsEnabled("WebRTC-SimulcastMaxLayers")) { - num_simulcast_layers = max_layers; - } - if (num_simulcast_layers > max_layers) { - // TODO(bugs.webrtc.org/8486): This scales down the resolution if the - // number of simulcast layers created by the application isn't sufficient - // (defined in kSimulcastFormats). For example if the input frame's - // resolution is HD, but there are only 2 simulcast layers, the - // resolution gets scaled down to VGA. Consider taking this logic out to - // allow the application more control over the resolutions. - SlotSimulcastMaxResolution(max_layers, &width, &height); - num_simulcast_layers = max_layers; - } - std::vector layers(num_simulcast_layers); + std::vector layers(layer_count); // Format width and height has to be divisible by |2 ^ num_simulcast_layers - // 1|. - width = NormalizeSimulcastSize(width, num_simulcast_layers); - height = NormalizeSimulcastSize(height, num_simulcast_layers); + width = NormalizeSimulcastSize(width, layer_count); + height = NormalizeSimulcastSize(height, layer_count); // Add simulcast streams, from highest resolution (|s| = num_simulcast_layers // -1) to lowest resolution at |s| = 0. - for (size_t s = num_simulcast_layers - 1;; --s) { + for (size_t s = layer_count - 1;; --s) { layers[s].width = width; layers[s].height = height; // TODO(pbos): Fill actual temporal-layer bitrate thresholds. diff --git a/media/engine/simulcast.h b/media/engine/simulcast.h index 0e5afc2455..1f3d2e4df2 100644 --- a/media/engine/simulcast.h +++ b/media/engine/simulcast.h @@ -35,10 +35,8 @@ std::vector GetSimulcastConfig( size_t max_layers, int width, int height, - int /*max_bitrate_bps*/, double bitrate_priority, int max_qp, - int /*max_framerate*/, bool is_screenshare, bool temporal_layers_supported = true); diff --git a/media/engine/simulcast_unittest.cc b/media/engine/simulcast_unittest.cc index ec4d0d0d6a..41958cb894 100644 --- a/media/engine/simulcast_unittest.cc +++ b/media/engine/simulcast_unittest.cc @@ -19,8 +19,6 @@ namespace webrtc { namespace { constexpr int kQpMax = 55; constexpr double kBitratePriority = 2.0; -constexpr int kMaxFps = 33; -constexpr int kMaxBitrateBps = 0; constexpr bool kScreenshare = true; constexpr int kDefaultTemporalLayers = 3; // Value from simulcast.cc. @@ -82,8 +80,7 @@ TEST(SimulcastTest, GetConfig) { const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1280, 720, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 1280, 720, kBitratePriority, kQpMax, !kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); EXPECT_EQ(320u, streams[0].width); @@ -116,8 +113,7 @@ TEST(SimulcastTest, GetConfigWithBaseHeavyVP8TL3RateAllocation) { const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1280, 720, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 1280, 720, kBitratePriority, kQpMax, !kScreenshare); EXPECT_EQ(kExpected[0].min_bitrate_bps, streams[0].min_bitrate_bps); EXPECT_EQ(static_cast(0.4 * kExpected[0].target_bitrate_bps / 0.6), @@ -134,35 +130,33 @@ TEST(SimulcastTest, GetConfigWithBaseHeavyVP8TL3RateAllocation) { TEST(SimulcastTest, GetConfigWithLimitedMaxLayers) { const size_t kMaxLayers = 2; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1280, 720, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 1280, 720, kBitratePriority, kQpMax, !kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); - EXPECT_EQ(320u, streams[0].width); - EXPECT_EQ(180u, streams[0].height); - EXPECT_EQ(640u, streams[1].width); - EXPECT_EQ(360u, streams[1].height); + EXPECT_EQ(640u, streams[0].width); + EXPECT_EQ(360u, streams[0].height); + EXPECT_EQ(1280u, streams[1].width); + EXPECT_EQ(720u, streams[1].height); } TEST(SimulcastTest, GetConfigWithLimitedMaxLayersForResolution) { const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 800, 600, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 800, 600, kBitratePriority, kQpMax, !kScreenshare); - EXPECT_EQ(2u, streams.size()); - EXPECT_EQ(400u, streams[0].width); - EXPECT_EQ(300u, streams[0].height); - EXPECT_EQ(800u, streams[1].width); - EXPECT_EQ(600u, streams[1].height); + EXPECT_EQ(3u, streams.size()); + EXPECT_EQ(200u, streams[0].width); + EXPECT_EQ(150u, streams[0].height); + EXPECT_EQ(400u, streams[1].width); + EXPECT_EQ(300u, streams[1].height); + EXPECT_EQ(800u, streams[2].width); + EXPECT_EQ(600u, streams[2].height); } TEST(SimulcastTest, GetConfigWithNotLimitedMaxLayersForResolution) { - test::ScopedFieldTrials field_trials("WebRTC-SimulcastMaxLayers/Enabled/"); const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 800, 600, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 800, 600, kBitratePriority, kQpMax, !kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); EXPECT_EQ(200u, streams[0].width); @@ -176,8 +170,7 @@ TEST(SimulcastTest, GetConfigWithNotLimitedMaxLayersForResolution) { TEST(SimulcastTest, GetConfigWithNormalizedResolution) { const size_t kMaxLayers = 2; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 640 + 1, 360 + 1, kMaxBitrateBps, kBitratePriority, kQpMax, - kMaxFps, !kScreenshare); + kMaxLayers, 640 + 1, 360 + 1, kBitratePriority, kQpMax, !kScreenshare); // Must be divisible by |2 ^ (num_layers - 1)|. EXPECT_EQ(kMaxLayers, streams.size()); @@ -193,8 +186,7 @@ TEST(SimulcastTest, GetConfigWithNormalizedResolutionDivisibleBy4) { const size_t kMaxLayers = 2; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 709, 501, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 709, 501, kBitratePriority, kQpMax, !kScreenshare); // Must be divisible by |2 ^ 2|. EXPECT_EQ(kMaxLayers, streams.size()); @@ -210,8 +202,7 @@ TEST(SimulcastTest, GetConfigWithNormalizedResolutionDivisibleBy8) { const size_t kMaxLayers = 2; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 709, 501, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - !kScreenshare); + kMaxLayers, 709, 501, kBitratePriority, kQpMax, !kScreenshare); // Must be divisible by |2 ^ 3|. EXPECT_EQ(kMaxLayers, streams.size()); @@ -225,8 +216,7 @@ TEST(SimulcastTest, GetConfigForScreenshare) { test::ScopedFieldTrials field_trials("WebRTC-SimulcastScreenshare/Disabled/"); const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1400, 800, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - kScreenshare); + kMaxLayers, 1400, 800, kBitratePriority, kQpMax, kScreenshare); EXPECT_EQ(1u, streams.size()) << "No simulcast."; EXPECT_EQ(1400u, streams[0].width); @@ -244,8 +234,7 @@ TEST(SimulcastTest, GetConfigForScreenshare) { TEST(SimulcastTest, GetConfigForScreenshareSimulcast) { const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1400, 800, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - kScreenshare); + kMaxLayers, 1400, 800, kBitratePriority, kQpMax, kScreenshare); EXPECT_GT(streams.size(), 1u); for (size_t i = 0; i < streams.size(); ++i) { @@ -264,8 +253,7 @@ TEST(SimulcastTest, GetConfigForScreenshareSimulcast) { TEST(SimulcastTest, GetConfigForScreenshareSimulcastWithLimitedMaxLayers) { const size_t kMaxLayers = 1; std::vector streams = cricket::GetSimulcastConfig( - kMaxLayers, 1400, 800, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, - kScreenshare); + kMaxLayers, 1400, 800, kBitratePriority, kQpMax, kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); } @@ -278,17 +266,15 @@ TEST(SimulcastTest, SimulcastScreenshareMaxBitrateAdjustedForResolution) { // 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); + kMaxLayers, 1920, 1080, kBitratePriority, kQpMax, 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); + streams = cricket::GetSimulcastConfig(kMaxLayers, 960, 540, kBitratePriority, + kQpMax, kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); EXPECT_EQ(streams[1].max_bitrate_bps, kMaxBitrate960_540); EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps); @@ -296,9 +282,8 @@ TEST(SimulcastTest, SimulcastScreenshareMaxBitrateAdjustedForResolution) { // 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); + streams = cricket::GetSimulcastConfig(kMaxLayers, 480, 270, kBitratePriority, + kQpMax, kScreenshare); EXPECT_EQ(kMaxLayers, streams.size()); EXPECT_EQ(streams[1].max_bitrate_bps, kScreenshareHighStreamMinBitrateBps); EXPECT_EQ(streams[1].min_bitrate_bps, kScreenshareHighStreamMinBitrateBps); diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 83366836e3..9770b1b225 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3011,9 +3011,8 @@ std::vector EncoderStreamFactory::CreateEncoderStreams( absl::EqualsIgnoreCase(codec_name_, kVp8CodecName) || absl::EqualsIgnoreCase(codec_name_, kH264CodecName); layers = GetSimulcastConfig(encoder_config.number_of_streams, width, height, - 0 /*not used*/, encoder_config.bitrate_priority, - max_qp_, 0 /*not_used*/, is_screenshare_, - temporal_layers_supported); + encoder_config.bitrate_priority, max_qp_, + is_screenshare_, temporal_layers_supported); // The maximum |max_framerate| is currently used for video. const int max_framerate = GetMaxFramerate(encoder_config, layers.size()); // Update the active simulcast layers and configured bitrates. diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 1c2cda87cd..0f09960f4c 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -7426,9 +7426,8 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test { std::vector expected_streams; if (conference_mode) { expected_streams = GetSimulcastConfig( - num_configured_streams, capture_width, capture_height, 0, - webrtc::kDefaultBitratePriority, kDefaultQpMax, - kDefaultVideoMaxFramerate, screenshare, true); + num_configured_streams, capture_width, capture_height, + webrtc::kDefaultBitratePriority, kDefaultQpMax, screenshare, true); if (screenshare) { for (const webrtc::VideoStream& stream : expected_streams) { // Never scale screen content. diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc index 129c596a6f..8586ee8a56 100644 --- a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc +++ b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc @@ -57,7 +57,6 @@ using VideoStatistics = VideoCodecTestStats::VideoStatistics; namespace { const int kBaseKeyFrameInterval = 3000; -const int kMaxBitrateBps = 5000 * 1000; // From kSimulcastFormats. const double kBitratePriority = 1.0; const int kMaxFramerateFps = 30; const int kMaxQp = 56; @@ -65,8 +64,8 @@ const int kMaxQp = 56; void ConfigureSimulcast(VideoCodec* codec_settings) { const std::vector streams = cricket::GetSimulcastConfig( codec_settings->numberOfSimulcastStreams, codec_settings->width, - codec_settings->height, kMaxBitrateBps, kBitratePriority, kMaxQp, - kMaxFramerateFps, /* is_screenshare = */ false, true); + codec_settings->height, kBitratePriority, kMaxQp, + /* is_screenshare = */ false, true); for (size_t i = 0; i < streams.size(); ++i) { SimulcastStream* ss = &codec_settings->simulcastStream[i];