Remove WebRTC-SimulcastScreenshare and enable it by default

As per the spec, you should be able to use simulcast with screenshare.
We remove the field trial for it and keep the old behavior only for
screenshare sources with conference flag on.

Bug: webrtc:8785
Change-Id: I1d6d4e18256fb5cfe0195620706de068f25b8d9b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144785
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28543}
This commit is contained in:
Florent Castelli 2019-07-10 16:57:57 +02:00 committed by Commit Bot
parent ac6c09634f
commit 66b3860fc9
8 changed files with 32 additions and 99 deletions

View File

@ -43,8 +43,6 @@ constexpr int kScreenshareDefaultTl1BitrateKbps = 1000;
// screen content.
constexpr int kScreenshareHighStreamMinBitrateBps = 600000;
constexpr int kScreenshareHighStreamMaxBitrateBps = 1250000;
static const char* kSimulcastScreenshareFieldTrialName =
"WebRTC-SimulcastScreenshare";
} // namespace
@ -183,12 +181,12 @@ std::vector<webrtc::VideoStream> GetSimulcastConfig(
int height,
double bitrate_priority,
int max_qp,
bool is_screenshare,
bool is_screenshare_with_conference_mode,
bool temporal_layers_supported) {
if (is_screenshare) {
RTC_DCHECK(max_layers > 1 || is_screenshare_with_conference_mode);
if (is_screenshare_with_conference_mode) {
return GetScreenshareLayers(max_layers, width, height, bitrate_priority,
max_qp, ScreenshareSimulcastFieldTrialEnabled(),
temporal_layers_supported);
max_qp, temporal_layers_supported);
} else {
return GetNormalSimulcastLayers(max_layers, width, height, bitrate_priority,
max_qp, temporal_layers_supported);
@ -270,10 +268,8 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
int height,
double bitrate_priority,
int max_qp,
bool screenshare_simulcast_enabled,
bool temporal_layers_supported) {
auto max_screenshare_layers =
screenshare_simulcast_enabled ? kMaxScreenshareSimulcastLayers : 1;
auto max_screenshare_layers = kMaxScreenshareSimulcastLayers;
size_t num_simulcast_layers =
std::min<int>(max_layers, max_screenshare_layers);
@ -347,8 +343,4 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
return layers;
}
bool ScreenshareSimulcastFieldTrialEnabled() {
return !webrtc::field_trial::IsDisabled(kSimulcastScreenshareFieldTrialName);
}
} // namespace cricket

View File

@ -38,7 +38,7 @@ std::vector<webrtc::VideoStream> GetSimulcastConfig(
int height,
double bitrate_priority,
int max_qp,
bool is_screenshare,
bool is_screenshare_with_conference_mode,
bool temporal_layers_supported = true);
// Gets the simulcast config layers for a non-screensharing case.
@ -57,11 +57,8 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
int height,
double bitrate_priority,
int max_qp,
bool screenshare_simulcast_enabled,
bool temporal_layers_supported = true);
bool ScreenshareSimulcastFieldTrialEnabled();
} // namespace cricket
#endif // MEDIA_ENGINE_SIMULCAST_H_

View File

@ -212,25 +212,6 @@ TEST(SimulcastTest, GetConfigWithNormalizedResolutionDivisibleBy8) {
EXPECT_EQ(496u, streams[1].height);
}
TEST(SimulcastTest, GetConfigForScreenshare) {
test::ScopedFieldTrials field_trials("WebRTC-SimulcastScreenshare/Disabled/");
const size_t kMaxLayers = 3;
std::vector<VideoStream> streams = cricket::GetSimulcastConfig(
kMaxLayers, 1400, 800, kBitratePriority, kQpMax, kScreenshare);
EXPECT_EQ(1u, streams.size()) << "No simulcast.";
EXPECT_EQ(1400u, streams[0].width);
EXPECT_EQ(800u, streams[0].height);
EXPECT_EQ(kQpMax, streams[0].max_qp);
EXPECT_EQ(kBitratePriority, streams[0].bitrate_priority);
EXPECT_TRUE(streams[0].active);
EXPECT_GT(streams[0].num_temporal_layers, size_t{1});
EXPECT_GT(streams[0].max_framerate, 0);
EXPECT_EQ(cricket::kMinVideoBitrateBps, streams[0].min_bitrate_bps);
EXPECT_GT(streams[0].target_bitrate_bps, streams[0].min_bitrate_bps);
EXPECT_GT(streams[0].max_bitrate_bps, streams[0].target_bitrate_bps);
}
TEST(SimulcastTest, GetConfigForScreenshareSimulcast) {
const size_t kMaxLayers = 3;
std::vector<VideoStream> streams = cricket::GetSimulcastConfig(

View File

@ -2149,9 +2149,7 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig(
// or a screencast (and not in simulcast screenshare experiment), only
// configure a single stream.
encoder_config.number_of_streams = parameters_.config.rtp.ssrcs.size();
if (IsCodecBlacklistedForSimulcast(codec.name) ||
(is_screencast && (!ScreenshareSimulcastFieldTrialEnabled() ||
!parameters_.conference_mode))) {
if (IsCodecBlacklistedForSimulcast(codec.name)) {
encoder_config.number_of_streams = 1;
}
@ -2976,28 +2974,20 @@ WebRtcVideoChannel::MapCodecs(const std::vector<VideoCodec>& codecs) {
// TODO(bugs.webrtc.org/8785): Consider removing max_qp as member of
// EncoderStreamFactory and instead set this value individually for each stream
// in the VideoEncoderConfig.simulcast_layers.
EncoderStreamFactory::EncoderStreamFactory(
std::string codec_name,
int max_qp,
bool is_screenshare,
bool screenshare_config_explicitly_enabled)
EncoderStreamFactory::EncoderStreamFactory(std::string codec_name,
int max_qp,
bool is_screenshare,
bool conference_mode)
: codec_name_(codec_name),
max_qp_(max_qp),
is_screenshare_(is_screenshare),
screenshare_config_explicitly_enabled_(
screenshare_config_explicitly_enabled) {}
conference_mode_(conference_mode) {}
std::vector<webrtc::VideoStream> EncoderStreamFactory::CreateEncoderStreams(
int width,
int height,
const webrtc::VideoEncoderConfig& encoder_config) {
bool screenshare_simulcast_enabled =
screenshare_config_explicitly_enabled_ &&
cricket::ScreenshareSimulcastFieldTrialEnabled();
if (is_screenshare_ && !screenshare_simulcast_enabled) {
RTC_DCHECK_EQ(1, encoder_config.number_of_streams);
}
RTC_DCHECK_GT(encoder_config.number_of_streams, 0);
RTC_DCHECK_GE(encoder_config.simulcast_layers.size(),
encoder_config.number_of_streams);
@ -3006,13 +2996,16 @@ std::vector<webrtc::VideoStream> EncoderStreamFactory::CreateEncoderStreams(
if (encoder_config.number_of_streams > 1 ||
((absl::EqualsIgnoreCase(codec_name_, kVp8CodecName) ||
absl::EqualsIgnoreCase(codec_name_, kH264CodecName)) &&
is_screenshare_ && screenshare_config_explicitly_enabled_)) {
is_screenshare_ && conference_mode_)) {
const bool temporal_layers_supported =
absl::EqualsIgnoreCase(codec_name_, kVp8CodecName) ||
absl::EqualsIgnoreCase(codec_name_, kH264CodecName);
// Use legacy simulcast screenshare if conference mode is explicitly enabled
// or use the regular simulcast configuration path which is generic.
layers = GetSimulcastConfig(encoder_config.number_of_streams, width, height,
encoder_config.bitrate_priority, max_qp_,
is_screenshare_, temporal_layers_supported);
is_screenshare_ && conference_mode_,
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.

View File

@ -575,7 +575,7 @@ class EncoderStreamFactory
EncoderStreamFactory(std::string codec_name,
int max_qp,
bool is_screenshare,
bool screenshare_config_explicitly_enabled);
bool conference_mode);
private:
std::vector<webrtc::VideoStream> CreateEncoderStreams(
@ -587,8 +587,8 @@ class EncoderStreamFactory
const int max_qp_;
const bool is_screenshare_;
// Allows a screenshare specific configuration, which enables temporal
// layering and allows simulcast.
const bool screenshare_config_explicitly_enabled_;
// layering and various settings.
const bool conference_mode_;
};
} // namespace cricket

View File

@ -3123,14 +3123,14 @@ TEST_F(WebRtcVideoChannelTest, VerifyVp8SpecificSettings) {
EXPECT_FALSE(vp8_settings.automaticResizeOn);
EXPECT_TRUE(vp8_settings.frameDroppingOn);
// In screen-share mode, denoising is forced off and simulcast disabled.
// In screen-share mode, denoising is forced off.
VideoOptions options;
options.is_screencast = true;
EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder));
stream = SetDenoisingOption(last_ssrc_, &frame_forwarder, false);
EXPECT_EQ(1u, stream->GetVideoStreams().size());
EXPECT_EQ(3u, stream->GetVideoStreams().size());
ASSERT_TRUE(stream->GetVp8Settings(&vp8_settings)) << "No VP8 config set.";
EXPECT_FALSE(vp8_settings.denoisingOn);
// Resizing and frame dropping always off for screen sharing.
@ -7425,11 +7425,12 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test {
EXPECT_LE(expected_num_streams, stream->GetConfig().rtp.ssrcs.size());
std::vector<webrtc::VideoStream> expected_streams;
if (conference_mode) {
if (num_configured_streams > 1 || conference_mode) {
expected_streams = GetSimulcastConfig(
num_configured_streams, capture_width, capture_height,
webrtc::kDefaultBitratePriority, kDefaultQpMax, screenshare, true);
if (screenshare) {
webrtc::kDefaultBitratePriority, kDefaultQpMax,
screenshare && conference_mode, true);
if (screenshare && conference_mode) {
for (const webrtc::VideoStream& stream : expected_streams) {
// Never scale screen content.
EXPECT_EQ(stream.width, rtc::checked_cast<size_t>(capture_width));
@ -7475,7 +7476,7 @@ class WebRtcVideoChannelSimulcastTest : public ::testing::Test {
EXPECT_GT(video_streams[i].max_qp, 0);
EXPECT_EQ(expected_streams[i].max_qp, video_streams[i].max_qp);
EXPECT_EQ(conference_mode,
EXPECT_EQ(num_configured_streams > 1 || conference_mode,
expected_streams[i].num_temporal_layers.has_value());
if (conference_mode) {
@ -7551,26 +7552,17 @@ TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsWithOddSizeInSimulcast) {
}
TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsForScreenshare) {
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true,
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 3, true,
false);
}
TEST_F(WebRtcVideoChannelSimulcastTest,
SetSendCodecsForConferenceModeScreenshare) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-SimulcastScreenshare/Disabled/");
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true,
true);
}
TEST_F(WebRtcVideoChannelSimulcastTest, SetSendCodecsForSimulcastScreenshare) {
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 2, true,
true);
}
TEST_F(WebRtcVideoChannelSimulcastTest,
NoSimulcastScreenshareWithoutConference) {
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 1, true,
TEST_F(WebRtcVideoChannelSimulcastTest, SimulcastScreenshareWithoutConference) {
VerifySimulcastSettings(cricket::VideoCodec("VP8"), 1280, 720, 3, 3, true,
false);
}

View File

@ -833,19 +833,12 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL) {
fixture->RunWithAnalyzer(screenshare);
}
#if !defined(WEBRTC_MAC)
// All the tests using this constant are disabled on Mac.
const char kScreenshareSimulcastExperiment[] =
"WebRTC-SimulcastScreenshare/Enabled/";
#if !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN)
// TODO(bugs.webrtc.org/9840): Investigate why is this test flaky on Win/Mac.
#if !defined(WEBRTC_WIN)
const char kScreenshareSimulcastVariableFramerateExperiment[] =
"WebRTC-SimulcastScreenshare/Enabled/"
"WebRTC-VP8VariableFramerateScreenshare/"
"Enabled,min_fps:5.0,min_qp:15,undershoot:30/";
TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Simulcast) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(kScreenshareSimulcastExperiment));
auto fixture = CreateVideoQualityTestFixture();
ParamsWithLogging screenshare;
screenshare.call.send_side_bwe = true;
@ -904,8 +897,6 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Simulcast_Variable_Framerate) {
}
TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Simulcast_low) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(kScreenshareSimulcastExperiment));
auto fixture = CreateVideoQualityTestFixture();
ParamsWithLogging screenshare;
screenshare.call.send_side_bwe = true;
@ -933,8 +924,7 @@ TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Simulcast_low) {
fixture->RunWithAnalyzer(screenshare);
}
#endif // !defined(WEBRTC_WIN)
#endif // !defined(WEBRTC_MAC)
#endif // !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN)
TEST(FullStackTest, ScreenshareSlidesVP8_2TL_Scroll) {
auto fixture = CreateVideoQualityTestFixture();
@ -1480,8 +1470,6 @@ class DualStreamsTest : public ::testing::TestWithParam<int> {};
#if !defined(WEBRTC_ANDROID) && !defined(WEBRTC_IOS) && !defined(WEBRTC_MAC)
TEST_P(DualStreamsTest,
ModeratelyRestricted_SlidesVp8_2TL_Simulcast_Video_Simulcast_High) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(std::string(kScreenshareSimulcastExperiment)));
const int first_stream = GetParam();
ParamsWithLogging dual_streams;

View File

@ -1091,19 +1091,13 @@ TEST(PCFullStackTest, ScreenshareSlidesVP8_2TL) {
}
#if !defined(WEBRTC_MAC)
// All the tests using this constant are disabled on Mac.
const char kScreenshareSimulcastExperiment[] =
"WebRTC-SimulcastScreenshare/Enabled/";
// TODO(bugs.webrtc.org/9840): Investigate why is this test flaky on Win/Mac.
#if !defined(WEBRTC_WIN)
const char kScreenshareSimulcastVariableFramerateExperiment[] =
"WebRTC-SimulcastScreenshare/Enabled/"
"WebRTC-VP8VariableFramerateScreenshare/"
"Enabled,min_fps:5.0,min_qp:15,undershoot:30/";
// TODO(bugs.webrtc.org/10639) requires simulcast/SVC support in PC framework
TEST(PCFullStackTest, ScreenshareSlidesVP8_2TL_Simulcast) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(kScreenshareSimulcastExperiment));
auto fixture = CreateVideoQualityTestFixture();
ParamsWithLogging screenshare;
screenshare.call.send_side_bwe = true;
@ -1164,8 +1158,6 @@ TEST(PCFullStackTest, ScreenshareSlidesVP8_2TL_Simulcast_Variable_Framerate) {
// TODO(bugs.webrtc.org/10639) requires simulcast/SVC support in PC framework
TEST(PCFullStackTest, ScreenshareSlidesVP8_2TL_Simulcast_low) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(kScreenshareSimulcastExperiment));
auto fixture = CreateVideoQualityTestFixture();
ParamsWithLogging screenshare;
screenshare.call.send_side_bwe = true;
@ -1754,8 +1746,6 @@ class PCDualStreamsTest : public ::testing::TestWithParam<int> {};
// TODO(bugs.webrtc.org/10639) requires simulcast/SVC support in PC framework
TEST_P(PCDualStreamsTest,
ModeratelyRestricted_SlidesVp8_2TL_Simulcast_Video_Simulcast_High) {
test::ScopedFieldTrials field_trial(
AppendFieldTrials(std::string(kScreenshareSimulcastExperiment)));
const int first_stream = GetParam();
ParamsWithLogging dual_streams;