diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index cd01302c66..2293540c42 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -219,7 +219,6 @@ rtc_library("stable_target_rate_experiment") { ] deps = [ ":field_trial_parser", - ":rate_control_settings", "../../api:field_trials_view", "../../api/transport:field_trial_based_config", ] diff --git a/rtc_base/experiments/rate_control_settings.cc b/rtc_base/experiments/rate_control_settings.cc index 91b475a04a..ea5f90ab39 100644 --- a/rtc_base/experiments/rate_control_settings.cc +++ b/rtc_base/experiments/rate_control_settings.cc @@ -34,27 +34,11 @@ const char kCongestionWindowDefaultFieldTrialString[] = const char kUseBaseHeavyVp8Tl3RateAllocationFieldTrialName[] = "WebRTC-UseBaseHeavyVP8TL3RateAllocation"; -const char* kVideoHysteresisFieldTrialname = - "WebRTC-SimulcastUpswitchHysteresisPercent"; -const char* kScreenshareHysteresisFieldTrialname = - "WebRTC-SimulcastScreenshareUpswitchHysteresisPercent"; - bool IsEnabled(const FieldTrialsView* const key_value_config, absl::string_view key) { return absl::StartsWith(key_value_config->Lookup(key), "Enabled"); } -void ParseHysteresisFactor(const FieldTrialsView* const key_value_config, - absl::string_view key, - double* output_value) { - std::string group_name = key_value_config->Lookup(key); - int percent = 0; - if (!group_name.empty() && sscanf(group_name.c_str(), "%d", &percent) == 1 && - percent >= 0) { - *output_value = 1.0 + (percent / 100.0); - } -} - } // namespace constexpr char CongestionWindowConfig::kKey[]; @@ -78,18 +62,16 @@ constexpr char VideoRateControlConfig::kKey[]; std::unique_ptr VideoRateControlConfig::Parser() { // The empty comments ensures that each pair is on a separate line. return StructParametersParser::Create( - "pacing_factor", &pacing_factor, // - "alr_probing", &alr_probing, // - "vp8_qp_max", &vp8_qp_max, // - "vp8_min_pixels", &vp8_min_pixels, // - "trust_vp8", &trust_vp8, // - "trust_vp9", &trust_vp9, // - "video_hysteresis", &video_hysteresis, // - "screenshare_hysteresis", &screenshare_hysteresis, // - "probe_max_allocation", &probe_max_allocation, // - "bitrate_adjuster", &bitrate_adjuster, // - "adjuster_use_headroom", &adjuster_use_headroom, // - "vp8_s0_boost", &vp8_s0_boost, // + "pacing_factor", &pacing_factor, // + "alr_probing", &alr_probing, // + "vp8_qp_max", &vp8_qp_max, // + "vp8_min_pixels", &vp8_min_pixels, // + "trust_vp8", &trust_vp8, // + "trust_vp9", &trust_vp9, // + "probe_max_allocation", &probe_max_allocation, // + "bitrate_adjuster", &bitrate_adjuster, // + "adjuster_use_headroom", &adjuster_use_headroom, // + "vp8_s0_boost", &vp8_s0_boost, // "vp8_base_heavy_tl3_alloc", &vp8_base_heavy_tl3_alloc); } @@ -103,10 +85,6 @@ RateControlSettings::RateControlSettings( CongestionWindowConfig::Parse(congestion_window_config); video_config_.vp8_base_heavy_tl3_alloc = IsEnabled( key_value_config, kUseBaseHeavyVp8Tl3RateAllocationFieldTrialName); - ParseHysteresisFactor(key_value_config, kVideoHysteresisFieldTrialname, - &video_config_.video_hysteresis); - ParseHysteresisFactor(key_value_config, kScreenshareHysteresisFieldTrialname, - &video_config_.screenshare_hysteresis); video_config_.Parser()->Parse( key_value_config->Lookup(VideoRateControlConfig::kKey)); } @@ -191,22 +169,6 @@ bool RateControlSettings::LibvpxVp9TrustedRateController() const { return video_config_.trust_vp9; } -double RateControlSettings::GetSimulcastHysteresisFactor( - VideoCodecMode mode) const { - if (mode == VideoCodecMode::kScreensharing) { - return video_config_.screenshare_hysteresis; - } - return video_config_.video_hysteresis; -} - -double RateControlSettings::GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType content_type) const { - if (content_type == VideoEncoderConfig::ContentType::kScreen) { - return video_config_.screenshare_hysteresis; - } - return video_config_.video_hysteresis; -} - bool RateControlSettings::Vp8BaseHeavyTl3RateAllocation() const { return video_config_.vp8_base_heavy_tl3_alloc; } diff --git a/rtc_base/experiments/rate_control_settings.h b/rtc_base/experiments/rate_control_settings.h index bad16d2eee..656eeb6dc3 100644 --- a/rtc_base/experiments/rate_control_settings.h +++ b/rtc_base/experiments/rate_control_settings.h @@ -38,9 +38,6 @@ struct VideoRateControlConfig { absl::optional vp8_min_pixels; bool trust_vp8 = true; bool trust_vp9 = true; - double video_hysteresis = 1.2; - // Default to 35% hysteresis for simulcast screenshare. - double screenshare_hysteresis = 1.35; bool probe_max_allocation = true; bool bitrate_adjuster = true; bool adjuster_use_headroom = true; @@ -80,12 +77,6 @@ class RateControlSettings final { bool LibvpxVp9TrustedRateController() const; bool Vp9DynamicRateSettings() const; - // TODO(bugs.webrtc.org/10272): Remove one of these when we have merged - // VideoCodecMode and VideoEncoderConfig::ContentType. - double GetSimulcastHysteresisFactor(VideoCodecMode mode) const; - double GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType content_type) const; - bool Vp8BaseHeavyTl3RateAllocation() const; bool TriggerProbeOnMaxAllocatedBitrateChange() const; diff --git a/rtc_base/experiments/rate_control_settings_unittest.cc b/rtc_base/experiments/rate_control_settings_unittest.cc index 84e5825224..1946f38fd4 100644 --- a/rtc_base/experiments/rate_control_settings_unittest.cc +++ b/rtc_base/experiments/rate_control_settings_unittest.cc @@ -172,42 +172,6 @@ TEST(RateControlSettingsTest, EXPECT_FALSE(settings_after.Vp8BaseHeavyTl3RateAllocation()); } -TEST(RateControlSettingsTest, GetSimulcastHysteresisFactor) { - const RateControlSettings settings_before = - RateControlSettings::ParseFromFieldTrials(); - EXPECT_DOUBLE_EQ(settings_before.GetSimulcastHysteresisFactor( - VideoCodecMode::kRealtimeVideo), - 1.2); - EXPECT_DOUBLE_EQ(settings_before.GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType::kRealtimeVideo), - 1.2); - EXPECT_DOUBLE_EQ(settings_before.GetSimulcastHysteresisFactor( - VideoCodecMode::kScreensharing), - 1.35); - EXPECT_DOUBLE_EQ(settings_before.GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType::kScreen), - 1.35); - - test::ScopedFieldTrials field_trials( - "WebRTC-VideoRateControl/" - "video_hysteresis:1.0,screenshare_hysteresis:1.4/"); - const RateControlSettings settings_after = - RateControlSettings::ParseFromFieldTrials(); - - EXPECT_DOUBLE_EQ(settings_after.GetSimulcastHysteresisFactor( - VideoCodecMode::kRealtimeVideo), - 1.0); - EXPECT_DOUBLE_EQ(settings_after.GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType::kRealtimeVideo), - 1.0); - EXPECT_DOUBLE_EQ(settings_after.GetSimulcastHysteresisFactor( - VideoCodecMode::kScreensharing), - 1.4); - EXPECT_DOUBLE_EQ(settings_after.GetSimulcastHysteresisFactor( - VideoEncoderConfig::ContentType::kScreen), - 1.4); -} - TEST(RateControlSettingsTest, TriggerProbeOnMaxAllocatedBitrateChange) { EXPECT_TRUE(RateControlSettings::ParseFromFieldTrials() .TriggerProbeOnMaxAllocatedBitrateChange()); diff --git a/rtc_base/experiments/stable_target_rate_experiment.cc b/rtc_base/experiments/stable_target_rate_experiment.cc index b7246d4a12..fa04fa35b4 100644 --- a/rtc_base/experiments/stable_target_rate_experiment.cc +++ b/rtc_base/experiments/stable_target_rate_experiment.cc @@ -11,7 +11,6 @@ #include "rtc_base/experiments/stable_target_rate_experiment.h" #include "api/transport/field_trial_based_config.h" -#include "rtc_base/experiments/rate_control_settings.h" namespace webrtc { namespace { @@ -44,13 +43,9 @@ StableTargetRateExperiment StableTargetRateExperiment::ParseFromFieldTrials() { StableTargetRateExperiment StableTargetRateExperiment::ParseFromKeyValueConfig( const FieldTrialsView* const key_value_config) { - RateControlSettings rate_control = - RateControlSettings::ParseFromKeyValueConfig(key_value_config); - return StableTargetRateExperiment( - key_value_config, - rate_control.GetSimulcastHysteresisFactor(VideoCodecMode::kRealtimeVideo), - rate_control.GetSimulcastHysteresisFactor( - VideoCodecMode::kScreensharing)); + return StableTargetRateExperiment(key_value_config, + /*default_video_hysteresis=*/1.2, + /*default_screenshare_hysteresis=*/1.35); } bool StableTargetRateExperiment::IsEnabled() const { diff --git a/rtc_base/experiments/stable_target_rate_experiment_unittest.cc b/rtc_base/experiments/stable_target_rate_experiment_unittest.cc index dbd841840d..854398e910 100644 --- a/rtc_base/experiments/stable_target_rate_experiment_unittest.cc +++ b/rtc_base/experiments/stable_target_rate_experiment_unittest.cc @@ -48,19 +48,6 @@ TEST(StableBweExperimentTest, EnabledWithHysteresis) { EXPECT_EQ(config.GetScreenshareHysteresisFactor(), 1.2); } -TEST(StableBweExperimentTest, OnNoHysteresisPropagatesVideoRateHystersis) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-StableTargetRate/enabled:true/" - "WebRTC-VideoRateControl/video_hysteresis:1.3," - "screenshare_hysteresis:1.4/"); - - StableTargetRateExperiment config = - StableTargetRateExperiment::ParseFromFieldTrials(); - EXPECT_TRUE(config.IsEnabled()); - EXPECT_EQ(config.GetVideoHysteresisFactor(), 1.3); - EXPECT_EQ(config.GetScreenshareHysteresisFactor(), 1.4); -} - TEST(StableBweExperimentTest, HysteresisOverrideVideoRateHystersis) { webrtc::test::ScopedFieldTrials field_trials( "WebRTC-StableTargetRate/" diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 3954ef0fed..a9e2544011 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -47,6 +47,9 @@ static constexpr int64_t kMaxVbaThrottleTimeMs = 500; constexpr TimeDelta kEncoderTimeOut = TimeDelta::Seconds(2); +constexpr double kVideoHysteresis = 1.2; +constexpr double kScreenshareHysteresis = 1.35; + // When send-side BWE is used a stricter 1.1x pacing factor is used, rather than // the 2.5x which is used with receive-side BWE. Provides a more careful // bandwidth rampup with less risk of overshoots causing adverse effects like @@ -95,8 +98,9 @@ int CalculateMaxPadBitrateBps(const std::vector& streams, // Without alr probing, pad up to start bitrate of the // highest active stream. const double hysteresis_factor = - RateControlSettings::ParseFromFieldTrials() - .GetSimulcastHysteresisFactor(content_type); + content_type == VideoEncoderConfig::ContentType::kScreen + ? kScreenshareHysteresis + : kVideoHysteresis; if (is_svc) { // For SVC, since there is only one "stream", the padding bitrate // needed to enable the top spatial layer is stored in the diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 52593a8272..ac7ea2159d 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -1002,10 +1002,7 @@ TEST_F(VideoSendStreamImplTest, ConfiguresBitratesForSvc) { using_alr ? stream.min_bitrate_bps : static_cast(stream.target_bitrate_bps * - trials.GetSimulcastHysteresisFactor( - test_config.screenshare - ? VideoCodecMode::kScreensharing - : VideoCodecMode::kRealtimeVideo)); + (test_config.screenshare ? 1.35 : 1.2)); // Min padding bitrate may override padding target. expected_padding = std::max(expected_padding, test_config.min_padding_bitrate_bps);