diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index b92f775ddc..0e4805d94c 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -171,6 +171,7 @@ rtc_static_library("video_coding") { "../../rtc_base:rtc_task_queue", "../../rtc_base:sequenced_task_checker", "../../rtc_base/experiments:alr_experiment", + "../../rtc_base/experiments:jitter_upper_bound_experiment", "../../rtc_base/experiments:rtt_mult_experiment", "../../rtc_base/system:fallthrough", "../../rtc_base/third_party/base64", @@ -909,6 +910,7 @@ if (rtc_include_tests) { "../../rtc_base:rtc_numerics", "../../rtc_base:rtc_task_queue", "../../rtc_base:rtc_task_queue_for_test", + "../../rtc_base/experiments:jitter_upper_bound_experiment", "../../system_wrappers", "../../system_wrappers:field_trial", "../../system_wrappers:metrics", @@ -920,6 +922,7 @@ if (rtc_include_tests) { "../../test:video_test_support", "../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/memory", + "//third_party/abseil-cpp/absl/types:optional", ] if (rtc_build_libvpx) { deps += [ rtc_libvpx_dir ] diff --git a/modules/video_coding/jitter_estimator.cc b/modules/video_coding/jitter_estimator.cc index dfe7f46551..0d0fac7291 100644 --- a/modules/video_coding/jitter_estimator.cc +++ b/modules/video_coding/jitter_estimator.cc @@ -14,19 +14,24 @@ #include #include #include +#include #include +#include "absl/types/optional.h" #include "modules/video_coding/internal_defines.h" #include "modules/video_coding/rtt_filter.h" +#include "rtc_base/experiments/jitter_upper_bound_experiment.h" #include "system_wrappers/include/clock.h" #include "system_wrappers/include/field_trial.h" namespace webrtc { - -enum { kStartupDelaySamples = 30 }; -enum { kFsAccuStartupSamples = 5 }; -enum { kMaxFramerateEstimate = 200 }; -enum { kNackCountTimeoutMs = 60000 }; +namespace { +static constexpr uint32_t kStartupDelaySamples = 30; +static constexpr int64_t kFsAccuStartupSamples = 5; +static constexpr double kMaxFramerateEstimate = 200.0; +static constexpr int64_t kNackCountTimeoutMs = 60000; +static constexpr double kDefaultMaxTimestampDeviationInSigmas = 3.5; +} // namespace VCMJitterEstimator::VCMJitterEstimator(const Clock* clock, int32_t vcmId, @@ -46,7 +51,9 @@ VCMJitterEstimator::VCMJitterEstimator(const Clock* clock, _rttFilter(), fps_counter_(30), // TODO(sprang): Use an estimator with limit based on // time, rather than number of samples. - low_rate_experiment_(kInit), + time_deviation_upper_bound_( + JitterUpperBoundExperiment::GetUpperBoundSigmas().value_or( + kDefaultMaxTimestampDeviationInSigmas)), clock_(clock) { Reset(); } @@ -76,6 +83,7 @@ VCMJitterEstimator& VCMJitterEstimator::operator=( _latestNackTimestamp = rhs._latestNackTimestamp; _nackCount = rhs._nackCount; _rttFilter = rhs._rttFilter; + clock_ = rhs.clock_; } return *this; } @@ -157,6 +165,12 @@ void VCMJitterEstimator::UpdateEstimate(int64_t frameDelayMS, } _prevFrameSize = frameSizeBytes; + // Cap frameDelayMS based on the current time deviation noise. + int64_t max_time_deviation_ms = + static_cast(time_deviation_upper_bound_ * sqrt(_varNoise) + 0.5); + frameDelayMS = std::max(std::min(frameDelayMS, max_time_deviation_ms), + -max_time_deviation_ms); + // Only update the Kalman filter if the sample is not considered // an extreme outlier. Even if it is an extreme outlier from a // delay point of view, if the frame size also is large the @@ -306,22 +320,20 @@ void VCMJitterEstimator::EstimateRandomJitter(double d_dT, if (_alphaCount > _alphaCountMax) _alphaCount = _alphaCountMax; - if (LowRateExperimentEnabled()) { - // In order to avoid a low frame rate stream to react slower to changes, - // scale the alpha weight relative a 30 fps stream. - double fps = GetFrameRate(); - if (fps > 0.0) { - double rate_scale = 30.0 / fps; - // At startup, there can be a lot of noise in the fps estimate. - // Interpolate rate_scale linearly, from 1.0 at sample #1, to 30.0 / fps - // at sample #kStartupDelaySamples. - if (_alphaCount < kStartupDelaySamples) { - rate_scale = - (_alphaCount * rate_scale + (kStartupDelaySamples - _alphaCount)) / - kStartupDelaySamples; - } - alpha = pow(alpha, rate_scale); + // In order to avoid a low frame rate stream to react slower to changes, + // scale the alpha weight relative a 30 fps stream. + double fps = GetFrameRate(); + if (fps > 0.0) { + double rate_scale = 30.0 / fps; + // At startup, there can be a lot of noise in the fps estimate. + // Interpolate rate_scale linearly, from 1.0 at sample #1, to 30.0 / fps + // at sample #kStartupDelaySamples. + if (_alphaCount < kStartupDelaySamples) { + rate_scale = + (_alphaCount * rate_scale + (kStartupDelaySamples - _alphaCount)) / + kStartupDelaySamples; } + alpha = pow(alpha, rate_scale); } double avgNoise = alpha * _avgNoise + (1 - alpha) * d_dT; @@ -393,41 +405,25 @@ int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { if (_nackCount >= _nackLimit) jitterMS += _rttFilter.RttMs() * rttMultiplier; - if (LowRateExperimentEnabled()) { - static const double kJitterScaleLowThreshold = 5.0; - static const double kJitterScaleHighThreshold = 10.0; - double fps = GetFrameRate(); - // Ignore jitter for very low fps streams. - if (fps < kJitterScaleLowThreshold) { - if (fps == 0.0) { - return jitterMS; - } - return 0; - } - - // Semi-low frame rate; scale by factor linearly interpolated from 0.0 at - // kJitterScaleLowThreshold to 1.0 at kJitterScaleHighThreshold. - if (fps < kJitterScaleHighThreshold) { - jitterMS = - (1.0 / (kJitterScaleHighThreshold - kJitterScaleLowThreshold)) * - (fps - kJitterScaleLowThreshold) * jitterMS; + static const double kJitterScaleLowThreshold = 5.0; + static const double kJitterScaleHighThreshold = 10.0; + double fps = GetFrameRate(); + // Ignore jitter for very low fps streams. + if (fps < kJitterScaleLowThreshold) { + if (fps == 0.0) { + return rtc::checked_cast(std::max(0.0, jitterMS) + 0.5); } + return 0; } - return static_cast(jitterMS + 0.5); -} - -bool VCMJitterEstimator::LowRateExperimentEnabled() { - if (low_rate_experiment_ == kInit) { - std::string group = - webrtc::field_trial::FindFullName("WebRTC-ReducedJitterDelay"); - if (group == "Disabled") { - low_rate_experiment_ = kDisabled; - } else { - low_rate_experiment_ = kEnabled; - } + // Semi-low frame rate; scale by factor linearly interpolated from 0.0 at + // kJitterScaleLowThreshold to 1.0 at kJitterScaleHighThreshold. + if (fps < kJitterScaleHighThreshold) { + jitterMS = (1.0 / (kJitterScaleHighThreshold - kJitterScaleLowThreshold)) * + (fps - kJitterScaleLowThreshold) * jitterMS; } - return low_rate_experiment_ == kEnabled ? true : false; + + return rtc::checked_cast(std::max(0.0, jitterMS) + 0.5); } double VCMJitterEstimator::GetFrameRate() const { diff --git a/modules/video_coding/jitter_estimator.h b/modules/video_coding/jitter_estimator.h index aeba3eded3..56b532851d 100644 --- a/modules/video_coding/jitter_estimator.h +++ b/modules/video_coding/jitter_estimator.h @@ -72,8 +72,6 @@ class VCMJitterEstimator { double _theta[2]; // Estimated line parameters (slope, offset) double _varNoise; // Variance of the time-deviation from the line - virtual bool LowRateExperimentEnabled(); - private: // Updates the Kalman filter for the line describing // the frame size dependent jitter. @@ -159,8 +157,7 @@ class VCMJitterEstimator { VCMRttFilter _rttFilter; rtc::RollingAccumulator fps_counter_; - enum ExperimentFlag { kInit, kEnabled, kDisabled }; - ExperimentFlag low_rate_experiment_; + const double time_deviation_upper_bound_; const Clock* clock_; }; diff --git a/modules/video_coding/jitter_estimator_tests.cc b/modules/video_coding/jitter_estimator_tests.cc index f00e1ef6da..45262ef836 100644 --- a/modules/video_coding/jitter_estimator_tests.cc +++ b/modules/video_coding/jitter_estimator_tests.cc @@ -9,38 +9,30 @@ #include "modules/video_coding/jitter_estimator.h" +#include "rtc_base/experiments/jitter_upper_bound_experiment.h" +#include "rtc_base/logging.h" +#include "rtc_base/numerics/histogram_percentile_counter.h" +#include "rtc_base/timeutils.h" #include "system_wrappers/include/clock.h" +#include "test/field_trial.h" #include "test/gtest.h" namespace webrtc { -class TestEstimator : public VCMJitterEstimator { - public: - explicit TestEstimator(bool exp_enabled) - : VCMJitterEstimator(&fake_clock_, 0, 0), - fake_clock_(0), - exp_enabled_(exp_enabled) {} +class TestVCMJitterEstimator : public ::testing::Test { + protected: + TestVCMJitterEstimator() : fake_clock_(0) {} - virtual bool LowRateExperimentEnabled() { return exp_enabled_; } + virtual void SetUp() { + estimator_ = absl::make_unique(&fake_clock_, 0, 0); + } void AdvanceClock(int64_t microseconds) { fake_clock_.AdvanceTimeMicroseconds(microseconds); } - private: SimulatedClock fake_clock_; - const bool exp_enabled_; -}; - -class TestVCMJitterEstimator : public ::testing::Test { - protected: - TestVCMJitterEstimator() - : regular_estimator_(false), low_rate_estimator_(true) {} - - virtual void SetUp() { regular_estimator_.Reset(); } - - TestEstimator regular_estimator_; - TestEstimator low_rate_estimator_; + std::unique_ptr estimator_; }; // Generates some simple test data in the form of a sawtooth wave. @@ -64,98 +56,56 @@ class ValueGenerator { // 5 fps, disable jitter delay altogether. TEST_F(TestVCMJitterEstimator, TestLowRate) { ValueGenerator gen(10); - uint64_t time_delta = 1000000 / 5; + uint64_t time_delta_us = rtc::kNumMicrosecsPerSec / 5; for (int i = 0; i < 60; ++i) { - regular_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - regular_estimator_.AdvanceClock(time_delta); - low_rate_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - low_rate_estimator_.AdvanceClock(time_delta); - EXPECT_GT(regular_estimator_.GetJitterEstimate(0), 0); + estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); + AdvanceClock(time_delta_us); if (i > 2) - EXPECT_EQ(low_rate_estimator_.GetJitterEstimate(0), 0); + EXPECT_EQ(estimator_->GetJitterEstimate(0), 0); gen.Advance(); } } -// 8 fps, steady state estimate should be in interpolated interval between 0 -// and value of previous method. -TEST_F(TestVCMJitterEstimator, TestMidRate) { - ValueGenerator gen(10); - uint64_t time_delta = 1000000 / 8; - for (int i = 0; i < 60; ++i) { - regular_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - regular_estimator_.AdvanceClock(time_delta); - low_rate_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - low_rate_estimator_.AdvanceClock(time_delta); - EXPECT_GT(regular_estimator_.GetJitterEstimate(0), 0); - EXPECT_GT(low_rate_estimator_.GetJitterEstimate(0), 0); - EXPECT_GE(regular_estimator_.GetJitterEstimate(0), - low_rate_estimator_.GetJitterEstimate(0)); - gen.Advance(); - } -} +TEST_F(TestVCMJitterEstimator, TestUpperBound) { + struct TestContext { + TestContext() : upper_bound(0.0), percentiles(1000) {} + double upper_bound; + rtc::HistogramPercentileCounter percentiles; + }; + std::vector test_cases(2); -// 30 fps, steady state estimate should be same as previous method. -TEST_F(TestVCMJitterEstimator, TestHighRate) { - ValueGenerator gen(10); - uint64_t time_delta = 1000000 / 30; - for (int i = 0; i < 60; ++i) { - regular_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - regular_estimator_.AdvanceClock(time_delta); - low_rate_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - low_rate_estimator_.AdvanceClock(time_delta); - EXPECT_EQ(regular_estimator_.GetJitterEstimate(0), - low_rate_estimator_.GetJitterEstimate(0)); - gen.Advance(); - } -} + test_cases[0].upper_bound = 100.0; // First use essentially no cap. + test_cases[1].upper_bound = 3.5; // Second, reasonably small cap. -// 10 fps, high jitter then low jitter. Low rate estimator should converge -// faster to low noise estimate. -TEST_F(TestVCMJitterEstimator, TestConvergence) { - // Reach a steady state with high noise. - ValueGenerator gen(50); - uint64_t time_delta = 1000000 / 10; - for (int i = 0; i < 100; ++i) { - regular_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - regular_estimator_.AdvanceClock(time_delta * 2); - low_rate_estimator_.UpdateEstimate(gen.Delay(), gen.FrameSize()); - low_rate_estimator_.AdvanceClock(time_delta * 2); - gen.Advance(); - } + 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::ScopedFieldTrials field_trials(ssb.str()); + SetUp(); - int threshold = regular_estimator_.GetJitterEstimate(0) / 2; - - // New generator with zero noise. - ValueGenerator low_gen(0); - int regular_iterations = 0; - int low_rate_iterations = 0; - for (int i = 0; i < 500; ++i) { - if (regular_iterations == 0) { - regular_estimator_.UpdateEstimate(low_gen.Delay(), low_gen.FrameSize()); - regular_estimator_.AdvanceClock(time_delta); - if (regular_estimator_.GetJitterEstimate(0) < threshold) { - regular_iterations = i; - } + ValueGenerator gen(50); + uint64_t time_delta_us = rtc::kNumMicrosecsPerSec / 30; + for (int i = 0; i < 100; ++i) { + estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); + AdvanceClock(time_delta_us); + context.percentiles.Add( + static_cast(estimator_->GetJitterEstimate(0))); + gen.Advance(); } - - if (low_rate_iterations == 0) { - low_rate_estimator_.UpdateEstimate(low_gen.Delay(), low_gen.FrameSize()); - low_rate_estimator_.AdvanceClock(time_delta); - if (low_rate_estimator_.GetJitterEstimate(0) < threshold) { - low_rate_iterations = i; - } - } - - if (regular_iterations != 0 && low_rate_iterations != 0) { - break; - } - - gen.Advance(); } - EXPECT_NE(regular_iterations, 0); - EXPECT_NE(low_rate_iterations, 0); - EXPECT_LE(low_rate_iterations, regular_iterations); + // 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)); } + } // namespace webrtc diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index 3afaa85a23..924b490eb6 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -86,6 +86,18 @@ rtc_static_library("rtt_mult_experiment") { ] } +rtc_static_library("jitter_upper_bound_experiment") { + sources = [ + "jitter_upper_bound_experiment.cc", + "jitter_upper_bound_experiment.h", + ] + deps = [ + "../:rtc_base_approved", + "../../system_wrappers:field_trial", + "//third_party/abseil-cpp/absl/types:optional", + ] +} + if (rtc_include_tests) { rtc_source_set("experiments_unittests") { testonly = true diff --git a/rtc_base/experiments/jitter_upper_bound_experiment.cc b/rtc_base/experiments/jitter_upper_bound_experiment.cc new file mode 100644 index 0000000000..b3e923019b --- /dev/null +++ b/rtc_base/experiments/jitter_upper_bound_experiment.cc @@ -0,0 +1,46 @@ +/* + * 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 new file mode 100644 index 0000000000..262cd79efa --- /dev/null +++ b/rtc_base/experiments/jitter_upper_bound_experiment.h @@ -0,0 +1,31 @@ +/* + * 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_