From e0f370471ab2597ec06cb42e91d3e1f2986e1a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CMichael?= Date: Tue, 4 Jun 2019 10:04:12 -0500 Subject: [PATCH] Add cap to video jitter buffer size/latency in experiment branches only. Bug: webrtc:10664 Change-Id: I03762c8b318f26f2689e89545aa8cc8e5b4a4329 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138081 Commit-Queue: Michael Horowitz Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#28155} --- modules/video_coding/frame_buffer2.cc | 6 ++- modules/video_coding/jitter_buffer.cc | 2 +- modules/video_coding/jitter_estimator.cc | 14 +++++-- modules/video_coding/jitter_estimator.h | 3 +- .../video_coding/jitter_estimator_tests.cc | 42 ++++++++++++++++--- 5 files changed, 55 insertions(+), 12 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 80d1aecb39..10bc711332 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -299,10 +299,14 @@ EncodedFrame* FrameBuffer::GetNextFrame() { } float rtt_mult = protection_mode_ == kProtectionNackFEC ? 0.0 : 1.0; + absl::optional rtt_mult_add_cap_ms = absl::nullopt; if (RttMultExperiment::RttMultEnabled()) { rtt_mult = RttMultExperiment::GetRttMultValue(); + // TODO(mhoro): add RttMultExperiment::GetJitterEstCapValue(); + rtt_mult_add_cap_ms = 200.0; } - timing_->SetJitterDelay(jitter_estimator_.GetJitterEstimate(rtt_mult)); + timing_->SetJitterDelay( + jitter_estimator_.GetJitterEstimate(rtt_mult, rtt_mult_add_cap_ms)); timing_->UpdateCurrentDelay(render_time_ms, now_ms); } else { if (RttMultExperiment::RttMultEnabled() || add_rtt_to_playout_delay_) diff --git a/modules/video_coding/jitter_buffer.cc b/modules/video_coding/jitter_buffer.cc index 3d3b967ba1..27b228f049 100644 --- a/modules/video_coding/jitter_buffer.cc +++ b/modules/video_coding/jitter_buffer.cc @@ -582,7 +582,7 @@ void VCMJitterBuffer::FindAndInsertContinuousFramesWithState( uint32_t VCMJitterBuffer::EstimatedJitterMs() { rtc::CritScope cs(&crit_sect_); const double rtt_mult = 1.0f; - return jitter_estimate_.GetJitterEstimate(rtt_mult); + return jitter_estimate_.GetJitterEstimate(rtt_mult, absl::nullopt); } void VCMJitterBuffer::SetNackSettings(size_t max_nack_list_size, diff --git a/modules/video_coding/jitter_estimator.cc b/modules/video_coding/jitter_estimator.cc index 052d3063f6..55efdf83f6 100644 --- a/modules/video_coding/jitter_estimator.cc +++ b/modules/video_coding/jitter_estimator.cc @@ -374,7 +374,9 @@ void VCMJitterEstimator::UpdateRtt(int64_t rttMs) { // Returns the current filtered estimate if available, // otherwise tries to calculate an estimate. -int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { +int VCMJitterEstimator::GetJitterEstimate( + double rttMultiplier, + absl::optional rttMultAddCapMs) { double jitterMS = CalculateEstimate() + OPERATING_SYSTEM_JITTER; uint64_t now = clock_->TimeInMicroseconds(); @@ -383,8 +385,14 @@ int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { if (_filterJitterEstimate > jitterMS) jitterMS = _filterJitterEstimate; - if (_nackCount >= _nackLimit) - jitterMS += _rttFilter.RttMs() * rttMultiplier; + if (_nackCount >= _nackLimit) { + if (rttMultAddCapMs.has_value()) { + jitterMS += + std::min(_rttFilter.RttMs() * rttMultiplier, rttMultAddCapMs.value()); + } else { + jitterMS += _rttFilter.RttMs() * rttMultiplier; + } + } static const double kJitterScaleLowThreshold = 5.0; static const double kJitterScaleHighThreshold = 10.0; diff --git a/modules/video_coding/jitter_estimator.h b/modules/video_coding/jitter_estimator.h index 4ce43f9d23..d9798b40a1 100644 --- a/modules/video_coding/jitter_estimator.h +++ b/modules/video_coding/jitter_estimator.h @@ -46,7 +46,8 @@ class VCMJitterEstimator { // - rttMultiplier : RTT param multiplier (when applicable). // // Return value : Jitter estimate in milliseconds. - virtual int GetJitterEstimate(double rttMultiplier); + virtual int GetJitterEstimate(double rttMultiplier, + absl::optional rttMultAddCapMs); // Updates the nack counter. void FrameNacked(); diff --git a/modules/video_coding/jitter_estimator_tests.cc b/modules/video_coding/jitter_estimator_tests.cc index fba2bf7fec..edf7f10a68 100644 --- a/modules/video_coding/jitter_estimator_tests.cc +++ b/modules/video_coding/jitter_estimator_tests.cc @@ -67,22 +67,43 @@ TEST_F(TestVCMJitterEstimator, TestLowRate) { estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); AdvanceClock(time_delta_us); if (i > 2) - EXPECT_EQ(estimator_->GetJitterEstimate(0), 0); + EXPECT_EQ(estimator_->GetJitterEstimate(0, absl::nullopt), 0); gen.Advance(); } } TEST_F(TestVCMJitterEstimator, TestUpperBound) { struct TestContext { - TestContext() : upper_bound(0.0), percentiles(1000) {} + 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(2); + std::vector test_cases(4); - test_cases[0].upper_bound = 100.0; // First use essentially no cap. - test_cases[1].upper_bound = 3.5; // Second, reasonably small cap. + // 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 = 200.0; + // 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 = 10.0; + // 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]; @@ -94,11 +115,15 @@ TEST_F(TestVCMJitterEstimator, TestUpperBound) { ValueGenerator gen(50); uint64_t time_delta_us = rtc::kNumMicrosecsPerSec / 30; + constexpr int64_t kRttMs = 250; for (int i = 0; i < 100; ++i) { estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); AdvanceClock(time_delta_us); + estimator_->FrameNacked(); // To test rtt_mult. + estimator_->UpdateRtt(kRttMs); // To test rtt_mult. context.percentiles.Add( - static_cast(estimator_->GetJitterEstimate(0))); + static_cast(estimator_->GetJitterEstimate( + context.rtt_mult, context.rtt_mult_add_cap_ms))); gen.Advance(); } } @@ -112,6 +137,11 @@ TEST_F(TestVCMJitterEstimator, TestUpperBound) { 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)); } } // namespace webrtc