diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index a72e6347fd..6b370f5541 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -253,7 +253,6 @@ rtc_library("video_coding") { "../../rtc_base:timeutils", "../../rtc_base/experiments:alr_experiment", "../../rtc_base/experiments:field_trial_parser", - "../../rtc_base/experiments:jitter_upper_bound_experiment", "../../rtc_base/experiments:min_video_bitrate_experiment", "../../rtc_base/experiments:rate_control_settings", "../../rtc_base/experiments:rtt_mult_experiment", @@ -1195,7 +1194,6 @@ if (rtc_include_tests) { "../../rtc_base:task_queue_for_test", "../../rtc_base:timeutils", "../../rtc_base/experiments:encoder_info_settings", - "../../rtc_base/experiments:jitter_upper_bound_experiment", "../../rtc_base/synchronization:mutex", "../../rtc_base/system:unused", "../../system_wrappers", diff --git a/modules/video_coding/timing/BUILD.gn b/modules/video_coding/timing/BUILD.gn index b6415f5ca6..17f716f254 100644 --- a/modules/video_coding/timing/BUILD.gn +++ b/modules/video_coding/timing/BUILD.gn @@ -44,7 +44,6 @@ rtc_library("jitter_estimator") { "../../../api/units:timestamp", "../../../rtc_base", "../../../rtc_base:safe_conversions", - "../../../rtc_base/experiments:jitter_upper_bound_experiment", "../../../system_wrappers", ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] @@ -105,7 +104,6 @@ rtc_library("timing_unittests") { "../../../rtc_base:histogram_percentile_counter", "../../../rtc_base:stringutils", "../../../rtc_base:timeutils", - "../../../rtc_base/experiments:jitter_upper_bound_experiment", "../../../system_wrappers:system_wrappers", "../../../test:scoped_key_value_config", "../../../test:test_support", diff --git a/modules/video_coding/timing/jitter_estimator.cc b/modules/video_coding/timing/jitter_estimator.cc index 168bfc2b4b..aaccf3d808 100644 --- a/modules/video_coding/timing/jitter_estimator.cc +++ b/modules/video_coding/timing/jitter_estimator.cc @@ -23,7 +23,6 @@ #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/video_coding/timing/rtt_filter.h" -#include "rtc_base/experiments/jitter_upper_bound_experiment.h" #include "rtc_base/numerics/safe_conversions.h" #include "system_wrappers/include/clock.h" @@ -53,9 +52,6 @@ JitterEstimator::JitterEstimator(Clock* clock, const FieldTrialsView& field_trials) : fps_counter_(30), // TODO(sprang): Use an estimator with limit based on // time, rather than number of samples. - time_deviation_upper_bound_( - JitterUpperBoundExperiment::GetUpperBoundSigmas().value_or( - kDefaultMaxTimestampDeviationInSigmas)), enable_reduced_delay_( !field_trials.IsEnabled("WebRTC-ReducedJitterDelayKillSwitch")), clock_(clock) { @@ -133,8 +129,8 @@ void JitterEstimator::UpdateEstimate(TimeDelta frame_delay, prev_frame_size_ = frame_size; // Cap frame_delay based on the current time deviation noise. - TimeDelta max_time_deviation = - TimeDelta::Millis(time_deviation_upper_bound_ * sqrt(var_noise_) + 0.5); + TimeDelta max_time_deviation = TimeDelta::Millis( + kDefaultMaxTimestampDeviationInSigmas * sqrt(var_noise_) + 0.5); frame_delay.Clamp(-max_time_deviation, max_time_deviation); // Only update the Kalman filter if the sample is not considered an extreme diff --git a/modules/video_coding/timing/jitter_estimator.h b/modules/video_coding/timing/jitter_estimator.h index 87a4f980e9..5150ef568d 100644 --- a/modules/video_coding/timing/jitter_estimator.h +++ b/modules/video_coding/timing/jitter_estimator.h @@ -148,7 +148,6 @@ class JitterEstimator { // Tracks frame rates in microseconds. rtc::RollingAccumulator fps_counter_; - const double time_deviation_upper_bound_; const bool enable_reduced_delay_; Clock* clock_; }; diff --git a/modules/video_coding/timing/jitter_estimator_unittest.cc b/modules/video_coding/timing/jitter_estimator_unittest.cc index 29098cdf45..2001e1670a 100644 --- a/modules/video_coding/timing/jitter_estimator_unittest.cc +++ b/modules/video_coding/timing/jitter_estimator_unittest.cc @@ -12,6 +12,7 @@ #include #include +#include #include #include "absl/types/optional.h" @@ -19,7 +20,6 @@ #include "api/units/data_size.h" #include "api/units/frequency.h" #include "api/units/time_delta.h" -#include "rtc_base/experiments/jitter_upper_bound_experiment.h" #include "rtc_base/numerics/histogram_percentile_counter.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/time_utils.h" @@ -96,45 +96,15 @@ TEST_F(TestJitterEstimator, TestLowRateDisabled) { } } -TEST_F(TestJitterEstimator, TestUpperBound) { - struct TestContext { - TestContext() - : upper_bound(0.0), - rtt_mult(0), - rtt_mult_add_cap_ms(absl::nullopt), - percentiles(1000) {} - double upper_bound; - double rtt_mult; - absl::optional rtt_mult_add_cap_ms; - rtc::HistogramPercentileCounter percentiles; - }; - std::vector test_cases(4); +TEST_F(TestJitterEstimator, RttMultAddCap) { + std::vector> + jitter_by_rtt_mult_cap; + jitter_by_rtt_mult_cap.emplace_back( + /*rtt_mult_add_cap=*/TimeDelta::Millis(10), /*long_tail_boundary=*/1000); + jitter_by_rtt_mult_cap.emplace_back( + /*rtt_mult_add_cap=*/TimeDelta::Millis(200), /*long_tail_boundary=*/1000); - // Large upper bound, rtt_mult = 0, and nullopt for rtt_mult addition cap. - test_cases[0].upper_bound = 100.0; - test_cases[0].rtt_mult = 0; - test_cases[0].rtt_mult_add_cap_ms = absl::nullopt; - // Small upper bound, rtt_mult = 0, and nullopt for rtt_mult addition cap. - test_cases[1].upper_bound = 3.5; - test_cases[1].rtt_mult = 0; - test_cases[1].rtt_mult_add_cap_ms = absl::nullopt; - // Large upper bound, rtt_mult = 1, and large rtt_mult addition cap value. - test_cases[2].upper_bound = 1000.0; - test_cases[2].rtt_mult = 1.0; - test_cases[2].rtt_mult_add_cap_ms = TimeDelta::Millis(200); - // Large upper bound, rtt_mult = 1, and small rtt_mult addition cap value. - test_cases[3].upper_bound = 1000.0; - test_cases[3].rtt_mult = 1.0; - test_cases[3].rtt_mult_add_cap_ms = TimeDelta::Millis(10); - - // Test jitter buffer upper_bound and rtt_mult addition cap sizes. - for (TestContext& context : test_cases) { - // Set up field trial and reset jitter estimator. - char string_buf[64]; - rtc::SimpleStringBuilder ssb(string_buf); - ssb << JitterUpperBoundExperiment::kJitterUpperBoundExperimentName - << "/Enabled-" << context.upper_bound << "/"; - test::ScopedKeyValueConfig field_trials(field_trials_, ssb.str()); + for (auto& [rtt_mult_add_cap, jitter] : jitter_by_rtt_mult_cap) { SetUp(); ValueGenerator gen(50); @@ -143,30 +113,18 @@ TEST_F(TestJitterEstimator, TestUpperBound) { for (int i = 0; i < 100; ++i) { estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); fake_clock_.AdvanceTime(time_delta); - estimator_->FrameNacked(); // To test rtt_mult. - estimator_->UpdateRtt(kRtt); // To test rtt_mult. - context.percentiles.Add( - estimator_ - ->GetJitterEstimate(context.rtt_mult, context.rtt_mult_add_cap_ms) + estimator_->FrameNacked(); + estimator_->UpdateRtt(kRtt); + jitter.Add( + estimator_->GetJitterEstimate(/*rtt_mult=*/1.0, rtt_mult_add_cap) .ms()); gen.Advance(); } } - // Median should be similar after three seconds. Allow 5% error margin. - uint32_t median_unbound = *test_cases[0].percentiles.GetPercentile(0.5); - uint32_t median_bounded = *test_cases[1].percentiles.GetPercentile(0.5); - EXPECT_NEAR(median_unbound, median_bounded, (median_unbound * 5) / 100); - - // Max should be lower for the bounded case. - uint32_t max_unbound = *test_cases[0].percentiles.GetPercentile(1.0); - uint32_t max_bounded = *test_cases[1].percentiles.GetPercentile(1.0); - EXPECT_GT(max_unbound, static_cast(max_bounded * 1.25)); - - // With rtt_mult = 1, max should be lower with small rtt_mult add cap value. - max_unbound = *test_cases[2].percentiles.GetPercentile(1.0); - max_bounded = *test_cases[3].percentiles.GetPercentile(1.0); - EXPECT_GT(max_unbound, static_cast(max_bounded * 1.25)); + // 200ms cap should result in at least 25% higher max compared to 10ms. + EXPECT_GT(*jitter_by_rtt_mult_cap[1].second.GetPercentile(1.0), + *jitter_by_rtt_mult_cap[0].second.GetPercentile(1.0) * 1.25); } } // namespace webrtc diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index 213377218e..cd01302c66 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -178,18 +178,6 @@ rtc_library("rtt_mult_experiment") { absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } -rtc_library("jitter_upper_bound_experiment") { - sources = [ - "jitter_upper_bound_experiment.cc", - "jitter_upper_bound_experiment.h", - ] - deps = [ - "..:logging", - "../../system_wrappers:field_trial", - ] - absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] -} - rtc_library("rate_control_settings") { sources = [ "rate_control_settings.cc", diff --git a/rtc_base/experiments/OWNERS b/rtc_base/experiments/OWNERS index f057a5ddc7..0a3b89533d 100644 --- a/rtc_base/experiments/OWNERS +++ b/rtc_base/experiments/OWNERS @@ -2,15 +2,12 @@ asapersson@webrtc.org sprang@webrtc.org srte@webrtc.org -per-file alr_experiment*=sprang@webrtc.org per-file audio_allocation_settings*=srte@webrtc.org per-file congestion_controller_experiment*=srte@webrtc.org per-file cpu_speed_experiment*=asapersson@webrtc.org per-file field_trial*=srte@webrtc.org -per-file jitter_upper_bound_experiment*=sprang@webrtc.org per-file keyframe_interval_settings*=brandtr@webrtc.org per-file normalize_simulcast_size_experiment*=asapersson@webrtc.org per-file quality_scaling_experiment*=asapersson@webrtc.org per-file rtt_mult_experiment*=mhoro@webrtc.org -per-file rate_control_settings*=sprang@webrtc.org per-file rate_control_settings*=srte@webrtc.org diff --git a/rtc_base/experiments/jitter_upper_bound_experiment.cc b/rtc_base/experiments/jitter_upper_bound_experiment.cc deleted file mode 100644 index ea95e84d15..0000000000 --- a/rtc_base/experiments/jitter_upper_bound_experiment.cc +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (c) 2018 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 "rtc_base/experiments/jitter_upper_bound_experiment.h" - -#include - -#include - -#include "rtc_base/logging.h" -#include "system_wrappers/include/field_trial.h" - -namespace webrtc { - -const char JitterUpperBoundExperiment::kJitterUpperBoundExperimentName[] = - "WebRTC-JitterUpperBound"; - -absl::optional JitterUpperBoundExperiment::GetUpperBoundSigmas() { - if (!field_trial::IsEnabled(kJitterUpperBoundExperimentName)) { - return absl::nullopt; - } - const std::string group = - webrtc::field_trial::FindFullName(kJitterUpperBoundExperimentName); - - double upper_bound_sigmas; - if (sscanf(group.c_str(), "Enabled-%lf", &upper_bound_sigmas) != 1) { - RTC_LOG(LS_WARNING) << "Invalid number of parameters provided."; - return absl::nullopt; - } - - if (upper_bound_sigmas < 0) { - RTC_LOG(LS_WARNING) << "Invalid jitter upper bound sigmas, must be >= 0.0: " - << upper_bound_sigmas; - return absl::nullopt; - } - - return upper_bound_sigmas; -} - -} // namespace webrtc diff --git a/rtc_base/experiments/jitter_upper_bound_experiment.h b/rtc_base/experiments/jitter_upper_bound_experiment.h deleted file mode 100644 index 262cd79efa..0000000000 --- a/rtc_base/experiments/jitter_upper_bound_experiment.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (c) 2018 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. - */ - -#ifndef RTC_BASE_EXPERIMENTS_JITTER_UPPER_BOUND_EXPERIMENT_H_ -#define RTC_BASE_EXPERIMENTS_JITTER_UPPER_BOUND_EXPERIMENT_H_ - -#include "absl/types/optional.h" - -namespace webrtc { - -class JitterUpperBoundExperiment { - public: - // Returns nullopt if experiment is not on, otherwise returns the configured - // upper bound for frame delay delta used in jitter estimation, expressed as - // number of standard deviations of the current deviation from the expected - // delay. - static absl::optional GetUpperBoundSigmas(); - - static const char kJitterUpperBoundExperimentName[]; -}; - -} // namespace webrtc - -#endif // RTC_BASE_EXPERIMENTS_JITTER_UPPER_BOUND_EXPERIMENT_H_