From a8ae407a480a2a9982eecf9e3a9b10da5373cd9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CMichael?= Date: Wed, 24 Apr 2019 08:04:34 -0500 Subject: [PATCH] Add ability to cap the video jitter estimate to a max value. Bug: webrtc:10572 Change-Id: I21112824dc02afa71db61bb8c2f02723e8b325b6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133963 Commit-Queue: Michael Horowitz Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#27744} --- modules/video_coding/frame_buffer2.cc | 6 +++++- modules/video_coding/frame_buffer2_unittest.cc | 7 ++++--- modules/video_coding/jitter_buffer.cc | 3 ++- modules/video_coding/jitter_estimator.cc | 7 +++++-- modules/video_coding/jitter_estimator.h | 2 +- modules/video_coding/jitter_estimator_tests.cc | 4 ++-- 6 files changed, 19 insertions(+), 10 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index c846477067..a327c1bf23 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; + float jitter_est_cap_ms = 300.0; if (RttMultExperiment::RttMultEnabled()) { rtt_mult = RttMultExperiment::GetRttMultValue(); + // TODO(mhoro): add RttMultExperiment::GetJitterEstCapValue(); + jitter_est_cap_ms = 300.0; } - timing_->SetJitterDelay(jitter_estimator_->GetJitterEstimate(rtt_mult)); + timing_->SetJitterDelay( + jitter_estimator_->GetJitterEstimate(rtt_mult, jitter_est_cap_ms)); timing_->UpdateCurrentDelay(render_time_ms, now_ms); } else { if (RttMultExperiment::RttMultEnabled() || add_rtt_to_playout_delay_) diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc index 0fb2572523..fe0123e061 100644 --- a/modules/video_coding/frame_buffer2_unittest.cc +++ b/modules/video_coding/frame_buffer2_unittest.cc @@ -83,7 +83,8 @@ class VCMJitterEstimatorMock : public VCMJitterEstimator { void(int64_t frameDelayMs, uint32_t frameSizeBytes, bool incompleteFrame)); - MOCK_METHOD1(GetJitterEstimate, int(double rttMultiplier)); + MOCK_METHOD2(GetJitterEstimate, + int(double rttMultiplier, double jitterEstCapMs)); }; class FrameObjectFake : public EncodedFrame { @@ -403,12 +404,12 @@ TEST_F(TestFrameBuffer2, ProtectionMode) { uint16_t pid = Rand(); uint32_t ts = Rand(); - EXPECT_CALL(jitter_estimator_, GetJitterEstimate(1.0)); + EXPECT_CALL(jitter_estimator_, GetJitterEstimate(1.0, 300.0)); InsertFrame(pid, 0, ts, false, true); ExtractFrame(); buffer_->SetProtectionMode(kProtectionNackFEC); - EXPECT_CALL(jitter_estimator_, GetJitterEstimate(0.0)); + EXPECT_CALL(jitter_estimator_, GetJitterEstimate(0.0, 300.0)); InsertFrame(pid + 1, 0, ts, false, true); ExtractFrame(); } diff --git a/modules/video_coding/jitter_buffer.cc b/modules/video_coding/jitter_buffer.cc index 3d3b967ba1..7a7f632618 100644 --- a/modules/video_coding/jitter_buffer.cc +++ b/modules/video_coding/jitter_buffer.cc @@ -582,7 +582,8 @@ void VCMJitterBuffer::FindAndInsertContinuousFramesWithState( uint32_t VCMJitterBuffer::EstimatedJitterMs() { rtc::CritScope cs(&crit_sect_); const double rtt_mult = 1.0f; - return jitter_estimate_.GetJitterEstimate(rtt_mult); + const double jitter_est_cap_ms = 300.0f; + return jitter_estimate_.GetJitterEstimate(rtt_mult, jitter_est_cap_ms); } 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 e380f9ea96..a4fb696986 100644 --- a/modules/video_coding/jitter_estimator.cc +++ b/modules/video_coding/jitter_estimator.cc @@ -384,7 +384,8 @@ void VCMJitterEstimator::UpdateMaxFrameSize(uint32_t frameSizeBytes) { // Returns the current filtered estimate if available, // otherwise tries to calculate an estimate. -int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { +int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier, + double jitterEstCapMs) { double jitterMS = CalculateEstimate() + OPERATING_SYSTEM_JITTER; uint64_t now = clock_->TimeInMicroseconds(); @@ -393,8 +394,10 @@ int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { if (_filterJitterEstimate > jitterMS) jitterMS = _filterJitterEstimate; - if (_nackCount >= _nackLimit) + if (_nackCount >= _nackLimit) { jitterMS += _rttFilter.RttMs() * rttMultiplier; + jitterMS = std::min(jitterMS, jitterEstCapMs); + } 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 8bffa05f4a..60c1780e7a 100644 --- a/modules/video_coding/jitter_estimator.h +++ b/modules/video_coding/jitter_estimator.h @@ -47,7 +47,7 @@ class VCMJitterEstimator { // - rttMultiplier : RTT param multiplier (when applicable). // // Return value : Jitter estimate in milliseconds. - virtual int GetJitterEstimate(double rttMultiplier); + virtual int GetJitterEstimate(double rttMultiplier, double jitterEstCapMs); // 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..3f5221aaab 100644 --- a/modules/video_coding/jitter_estimator_tests.cc +++ b/modules/video_coding/jitter_estimator_tests.cc @@ -67,7 +67,7 @@ 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, 300), 0); gen.Advance(); } } @@ -98,7 +98,7 @@ TEST_F(TestVCMJitterEstimator, TestUpperBound) { estimator_->UpdateEstimate(gen.Delay(), gen.FrameSize()); AdvanceClock(time_delta_us); context.percentiles.Add( - static_cast(estimator_->GetJitterEstimate(0))); + static_cast(estimator_->GetJitterEstimate(0, 300))); gen.Advance(); } }