From 195d1d77ea4d8aa13994a5ddd173dea16645f027 Mon Sep 17 00:00:00 2001 From: Rasmus Brandt Date: Wed, 9 May 2018 11:28:01 +0200 Subject: [PATCH] Remove ScreenshareLayerConfig. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: I7fe020f9985fa5ca1d9873a126a8518a991ded8e Reviewed-on: https://webrtc-review.googlesource.com/75509 Reviewed-by: Per Kjellander Reviewed-by: Erik Språng Commit-Queue: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#23321} --- media/BUILD.gn | 1 - media/engine/simulcast.cc | 67 ++++------------------ media/engine/simulcast.h | 19 ------ media/engine/simulcast_unittest.cc | 56 ------------------ media/engine/webrtcvideoengine_unittest.cc | 3 +- 5 files changed, 13 insertions(+), 133 deletions(-) delete mode 100644 media/engine/simulcast_unittest.cc diff --git a/media/BUILD.gn b/media/BUILD.gn index 4fdb185fb0..10ecd68e24 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -571,7 +571,6 @@ if (rtc_include_tests) { "engine/nullwebrtcvideoengine_unittest.cc", "engine/payload_type_mapper_unittest.cc", "engine/simulcast_encoder_adapter_unittest.cc", - "engine/simulcast_unittest.cc", "engine/videodecodersoftwarefallbackwrapper_unittest.cc", "engine/videoencodersoftwarefallbackwrapper_unittest.cc", "engine/vp8_encoder_simulcast_proxy_unittest.cc", diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc index 58ca9e8ecd..f8cb083434 100644 --- a/media/engine/simulcast.cc +++ b/media/engine/simulcast.cc @@ -21,6 +21,16 @@ namespace cricket { +namespace { + +constexpr int kScreenshareDefaultTl0BitrateKbps = 200; +constexpr int kScreenshareDefaultTl1BitrateKbps = 1000; + +static const char* kSimulcastScreenshareFieldTrialName = + "WebRTC-SimulcastScreenshare"; + +} // namespace + struct SimulcastFormat { int width; int height; @@ -253,7 +263,6 @@ std::vector GetScreenshareLayers( std::min(max_layers, max_screenshare_layers); std::vector layers(num_simulcast_layers); - ScreenshareLayerConfig config = ScreenshareLayerConfig::GetDefault(); // For legacy screenshare in conference mode, tl0 and tl1 bitrates are // piggybacked on the VideoCodec struct as target and max bitrates, // respectively. See eg. webrtc::LibvpxVp8Encoder::SetRates(). @@ -262,8 +271,8 @@ std::vector GetScreenshareLayers( layers[0].max_qp = max_qp; layers[0].max_framerate = 5; layers[0].min_bitrate_bps = kMinVideoBitrateBps; - layers[0].target_bitrate_bps = config.tl0_bitrate_kbps * 1000; - layers[0].max_bitrate_bps = config.tl1_bitrate_kbps * 1000; + layers[0].target_bitrate_bps = kScreenshareDefaultTl0BitrateKbps * 1000; + layers[0].max_bitrate_bps = kScreenshareDefaultTl1BitrateKbps * 1000; layers[0].num_temporal_layers = 2; // With simulcast enabled, add another spatial layer. This one will have a @@ -297,58 +306,6 @@ std::vector GetScreenshareLayers( return layers; } -static const int kScreenshareMinBitrateKbps = 50; -static const int kScreenshareMaxBitrateKbps = 6000; -static const int kScreenshareDefaultTl0BitrateKbps = 200; -static const int kScreenshareDefaultTl1BitrateKbps = 1000; - -static const char* kScreenshareLayerFieldTrialName = - "WebRTC-ScreenshareLayerRates"; -static const char* kSimulcastScreenshareFieldTrialName = - "WebRTC-SimulcastScreenshare"; - -ScreenshareLayerConfig::ScreenshareLayerConfig(int tl0_bitrate, int tl1_bitrate) - : tl0_bitrate_kbps(tl0_bitrate), tl1_bitrate_kbps(tl1_bitrate) { -} - -ScreenshareLayerConfig ScreenshareLayerConfig::GetDefault() { - std::string group = - webrtc::field_trial::FindFullName(kScreenshareLayerFieldTrialName); - - ScreenshareLayerConfig config(kScreenshareDefaultTl0BitrateKbps, - kScreenshareDefaultTl1BitrateKbps); - if (!group.empty() && !FromFieldTrialGroup(group, &config)) { - RTC_LOG(LS_WARNING) << "Unable to parse WebRTC-ScreenshareLayerRates" - " field trial group: '" - << group << "'."; - } - return config; -} - -bool ScreenshareLayerConfig::FromFieldTrialGroup( - const std::string& group, - ScreenshareLayerConfig* config) { - // Parse field trial group name, containing bitrates for tl0 and tl1. - int tl0_bitrate; - int tl1_bitrate; - if (sscanf(group.c_str(), "%d-%d", &tl0_bitrate, &tl1_bitrate) != 2) { - return false; - } - - // Sanity check. - if (tl0_bitrate < kScreenshareMinBitrateKbps || - tl0_bitrate > kScreenshareMaxBitrateKbps || - tl1_bitrate < kScreenshareMinBitrateKbps || - tl1_bitrate > kScreenshareMaxBitrateKbps || tl0_bitrate > tl1_bitrate) { - return false; - } - - config->tl0_bitrate_kbps = tl0_bitrate; - config->tl1_bitrate_kbps = tl1_bitrate; - - return true; -} - bool ScreenshareSimulcastFieldTrialEnabled() { return webrtc::field_trial::IsEnabled(kSimulcastScreenshareFieldTrialName); } diff --git a/media/engine/simulcast.h b/media/engine/simulcast.h index 7ab5854fd1..b47a5c15de 100644 --- a/media/engine/simulcast.h +++ b/media/engine/simulcast.h @@ -18,25 +18,6 @@ #include "rtc_base/basictypes.h" namespace cricket { -struct StreamParams; - -// TODO(sprang): Remove this, as we're moving away from temporal layer mode. -// Config for use with screen cast when temporal layers are enabled. -struct ScreenshareLayerConfig { - public: - ScreenshareLayerConfig(int tl0_bitrate, int tl1_bitrate); - - // Bitrates, for temporal layers 0 and 1. - int tl0_bitrate_kbps; - int tl1_bitrate_kbps; - - static ScreenshareLayerConfig GetDefault(); - - // Parse bitrate from group name on format "(tl0_bitrate)-(tl1_bitrate)", - // eg. "100-1000" for the default rates. - static bool FromFieldTrialGroup(const std::string& group, - ScreenshareLayerConfig* config); -}; // TODO(pthatcher): Write unit tests just for these functions, // independent of WebrtcVideoEngine. diff --git a/media/engine/simulcast_unittest.cc b/media/engine/simulcast_unittest.cc deleted file mode 100644 index 1433c5c335..0000000000 --- a/media/engine/simulcast_unittest.cc +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright (c) 2014 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include - -#include "media/engine/simulcast.h" -#include "test/gtest.h" - -namespace cricket { - -class ScreenshareLayerConfigTest : public testing::Test, - protected ScreenshareLayerConfig { - public: - ScreenshareLayerConfigTest() : ScreenshareLayerConfig(0, 0) {} - - void ExpectParsingFails(const std::string& group) { - ScreenshareLayerConfig config(100, 1000); - EXPECT_FALSE(FromFieldTrialGroup(group, &config)); - } -}; - -TEST_F(ScreenshareLayerConfigTest, UsesDefaultBitrateConfigForDefaultGroup) { - ExpectParsingFails(""); -} - -TEST_F(ScreenshareLayerConfigTest, UsesDefaultConfigForInvalidBitrates) { - ExpectParsingFails("-"); - ExpectParsingFails("1-"); - ExpectParsingFails("-1"); - ExpectParsingFails("-12"); - ExpectParsingFails("12-"); - ExpectParsingFails("booh!"); - ExpectParsingFails("1-b"); - ExpectParsingFails("a-2"); - ExpectParsingFails("49-1000"); - ExpectParsingFails("50-6001"); - ExpectParsingFails("100-99"); - ExpectParsingFails("1002003004005006-99"); - ExpectParsingFails("99-1002003004005006"); -} - -TEST_F(ScreenshareLayerConfigTest, ParsesValidBitrateConfig) { - ScreenshareLayerConfig config(100, 1000); - EXPECT_TRUE(ScreenshareLayerConfig::FromFieldTrialGroup("101-1001", &config)); - EXPECT_EQ(101, config.tl0_bitrate_kbps); - EXPECT_EQ(1001, config.tl1_bitrate_kbps); -} - -} // namespace cricket diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index b368abe9ce..6ecf750ec3 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -2725,8 +2725,7 @@ TEST_F(WebRtcVideoChannelTest, UsesCorrectSettingsForScreencast) { TEST_F(WebRtcVideoChannelTest, ConferenceModeScreencastConfiguresTemporalLayer) { - static const int kConferenceScreencastTemporalBitrateBps = - ScreenshareLayerConfig::GetDefault().tl0_bitrate_kbps * 1000; + static const int kConferenceScreencastTemporalBitrateBps = 200 * 1000; send_parameters_.conference_mode = true; channel_->SetSendParameters(send_parameters_);