From b65aa01a90beca980a0cad7fea83fdc4f9778e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 24 Sep 2018 11:35:19 +0200 Subject: [PATCH] Revert "Reland "Enable simulcast screenshare by default"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 89b2963810b4cea0f95abdce011cb4e12fcdf1a1. Reason for revert: Make experiment default off to not mess up data in re-launch. Original change's description: > Reland "Enable simulcast screenshare by default" > > This is a reland of d43c692ba7f53b5576a494c0343bc7a4bb36831b after fixes > to failing chromium tests. No change to the original CL were done. > Original CL reviewed on: https://webrtc-review.googlesource.com/87560 > > TBR=stefan@webrtc.org > > Bug: chromium:690537 > Change-Id: I6b59ffc90d789aff21c7e52b118d3dfbe756c8a9 > Reviewed-on: https://webrtc-review.googlesource.com/89081 > Reviewed-by: Ilya Nikolaevskiy > Commit-Queue: Ilya Nikolaevskiy > Cr-Commit-Position: refs/heads/master@{#24013} TBR=ilnik@webrtc.org,sprang@webrtc.org,stefan@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:690537, b:116052898 Change-Id: I429677de5547ce3a7badfb4414231ff9589e7414 Reviewed-on: https://webrtc-review.googlesource.com/101560 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#24798} --- media/engine/simulcast.cc | 2 +- media/engine/simulcast_unittest.cc | 4 ++-- media/engine/webrtcvideoengine_unittest.cc | 12 ++++-------- video/full_stack_tests.cc | 19 +++++-------------- 4 files changed, 12 insertions(+), 25 deletions(-) diff --git a/media/engine/simulcast.cc b/media/engine/simulcast.cc index a8dc1430b7..215ad9889a 100644 --- a/media/engine/simulcast.cc +++ b/media/engine/simulcast.cc @@ -382,7 +382,7 @@ std::vector GetScreenshareLayers( } bool ScreenshareSimulcastFieldTrialEnabled() { - return !webrtc::field_trial::IsDisabled(kSimulcastScreenshareFieldTrialName); + return webrtc::field_trial::IsEnabled(kSimulcastScreenshareFieldTrialName); } } // namespace cricket diff --git a/media/engine/simulcast_unittest.cc b/media/engine/simulcast_unittest.cc index 187691faee..f8d6e4db93 100644 --- a/media/engine/simulcast_unittest.cc +++ b/media/engine/simulcast_unittest.cc @@ -165,8 +165,6 @@ TEST(SimulcastTest, GetConfigWithNormalizedResolution) { } 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, @@ -186,6 +184,8 @@ TEST(SimulcastTest, GetConfigForScreenshare) { } TEST(SimulcastTest, GetConfigForScreenshareSimulcast) { + test::ScopedFieldTrials field_trials("WebRTC-SimulcastScreenshare/Enabled/"); + const size_t kMaxLayers = 3; std::vector streams = cricket::GetSimulcastConfig( kMaxLayers, 1400, 800, kMaxBitrateBps, kBitratePriority, kQpMax, kMaxFps, diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index d825078ab3..a5a4f98a74 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -6670,32 +6670,28 @@ TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsWithOddSizeInSimulcast) { true); } -// TODO(ilnik): Remove this test once Simulcast Screenshare is launched. TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsForScreenshare) { - webrtc::test::ScopedFieldTrials override_field_trials_( - "WebRTC-SimulcastScreenshare/Disabled/"); - VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true, false); } -// TODO(ilnik): Remove this test once Simulcast Screenshare is launched. TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsForConferenceModeScreenshare) { - webrtc::test::ScopedFieldTrials override_field_trials_( - "WebRTC-SimulcastScreenshare/Disabled/"); - VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true, true); } TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsForSimulcastScreenshare) { + webrtc::test::ScopedFieldTrials override_field_trials_( + "WebRTC-SimulcastScreenshare/Enabled/"); VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 2, true, true); } TEST_F(WebRtcVideoChannelSimulcastTest, NoSimulcastScreenshareWithoutConference) { + webrtc::test::ScopedFieldTrials override_field_trials_( + "WebRTC-SimulcastScreenshare/Enabled/"); VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true, false); } diff --git a/video/full_stack_tests.cc b/video/full_stack_tests.cc index a27f0f6f5e..32625128b6 100644 --- a/video/full_stack_tests.cc +++ b/video/full_stack_tests.cc @@ -46,8 +46,8 @@ namespace webrtc { namespace { static const int kFullStackTestDurationSecs = 45; -const char kNotScreenshareSimulcastExperiment[] = - "WebRTC-SimulcastScreenshare/Disabled/"; +const char kScreenshareSimulcastExperiment[] = + "WebRTC-SimulcastScreenshare/Enabled/"; const char kPacerPushBackExperiment[] = "WebRTC-PacerPushbackExperiment/Enabled/"; @@ -597,7 +597,6 @@ TEST(FullStackTest, ConferenceMotionHd2000kbps100msLimitedQueueVP9) { #endif TEST(FullStackTest, ScreenshareSlidesVP8_2TL) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -612,6 +611,7 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL) { TEST(FullStackTest, ScreenshareSlidesVP8_3TL_Simulcast) { auto fixture = CreateVideoQualityTestFixture(); + test::ScopedFieldTrials field_trial(kScreenshareSimulcastExperiment); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; screenshare.screenshare[0] = {true, false, 10}; @@ -639,8 +639,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_3TL_Simulcast) { } TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Scroll) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); - auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging config; config.call.send_side_bwe = true; @@ -654,8 +652,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Scroll) { } TEST(FullStackTest, ScreenshareSlidesVP8_2TL_LossyNet) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); - auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -672,8 +668,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_LossyNet) { } TEST(FullStackTest, ScreenshareSlidesVP8_2TL_VeryLossyNet) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); - auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -690,8 +684,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_VeryLossyNet) { } TEST(FullStackTest, ScreenshareSlidesVP8_2TL_LossyNetRestrictedQueue) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); - auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -709,8 +701,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_LossyNetRestrictedQueue) { } TEST(FullStackTest, ScreenshareSlidesVP8_2TL_ModeratelyRestricted) { - test::ScopedFieldTrials field_trial(kNotScreenshareSimulcastExperiment); - auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; @@ -1070,7 +1060,8 @@ class DualStreamsTest : public ::testing::TestWithParam {}; TEST_P(DualStreamsTest, ModeratelyRestricted_SlidesVp8_3TL_Simulcast_Video_Simulcast_High) { test::ScopedFieldTrials field_trial( - AppendFieldTrials(std::string(kPacerPushBackExperiment))); + AppendFieldTrials(std::string(kPacerPushBackExperiment) + + std::string(kScreenshareSimulcastExperiment))); const int first_stream = GetParam(); ParamsWithLogging dual_streams;