From 4ed7e511f6ab0e2dd938b3dc9319a2767926abd3 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Wed, 22 May 2019 16:06:14 +0200 Subject: [PATCH] Revert "Add ability to cap the video jitter estimate to a max value." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit a8ae407a480a2a9982eecf9e3a9b10da5373cd9a. Reason for revert: This CL incorrectly affects non-experiment branch. A new CL affecting only the experiment will be uploaded. Original change's description: > 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} TBR=stefan@webrtc.org,mhoro@webrtc.org Bug: webrtc:10572 Change-Id: I4af334168ca70ecfae7fd18fc7c852819a98d866 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138063 Reviewed-by: Stefan Holmer Reviewed-by: Michael Horowitz Reviewed-by: Erik Språng Reviewed-by: Björn Terelius Commit-Queue: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#28025} --- modules/video_coding/frame_buffer2.cc | 6 +----- 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 ++-- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 000152fda0..a4676623f7 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -299,14 +299,10 @@ 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, jitter_est_cap_ms)); + timing_->SetJitterDelay(jitter_estimator_.GetJitterEstimate(rtt_mult)); 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 7a7f632618..3d3b967ba1 100644 --- a/modules/video_coding/jitter_buffer.cc +++ b/modules/video_coding/jitter_buffer.cc @@ -582,8 +582,7 @@ void VCMJitterBuffer::FindAndInsertContinuousFramesWithState( uint32_t VCMJitterBuffer::EstimatedJitterMs() { rtc::CritScope cs(&crit_sect_); const double rtt_mult = 1.0f; - const double jitter_est_cap_ms = 300.0f; - return jitter_estimate_.GetJitterEstimate(rtt_mult, jitter_est_cap_ms); + return jitter_estimate_.GetJitterEstimate(rtt_mult); } 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 d4899a2803..052d3063f6 100644 --- a/modules/video_coding/jitter_estimator.cc +++ b/modules/video_coding/jitter_estimator.cc @@ -374,8 +374,7 @@ void VCMJitterEstimator::UpdateRtt(int64_t rttMs) { // Returns the current filtered estimate if available, // otherwise tries to calculate an estimate. -int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier, - double jitterEstCapMs) { +int VCMJitterEstimator::GetJitterEstimate(double rttMultiplier) { double jitterMS = CalculateEstimate() + OPERATING_SYSTEM_JITTER; uint64_t now = clock_->TimeInMicroseconds(); @@ -384,10 +383,8 @@ 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 a58d667ad2..4ce43f9d23 100644 --- a/modules/video_coding/jitter_estimator.h +++ b/modules/video_coding/jitter_estimator.h @@ -46,7 +46,7 @@ class VCMJitterEstimator { // - rttMultiplier : RTT param multiplier (when applicable). // // Return value : Jitter estimate in milliseconds. - virtual int GetJitterEstimate(double rttMultiplier, double jitterEstCapMs); + virtual int GetJitterEstimate(double rttMultiplier); // 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 3f5221aaab..fba2bf7fec 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, 300), 0); + EXPECT_EQ(estimator_->GetJitterEstimate(0), 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, 300))); + static_cast(estimator_->GetJitterEstimate(0))); gen.Advance(); } }