From 51f2453d980bf0a4739df4e24dd0362d74be1418 Mon Sep 17 00:00:00 2001 From: "holmer@google.com" Date: Wed, 15 Jun 2011 07:37:08 +0000 Subject: [PATCH] Fixed a Flush/Start initialization bug in the jitter buffer. Also cleaned up "Nack estimate". Review URL: http://webrtc-codereview.appspot.com/32009 git-svn-id: http://webrtc.googlecode.com/svn/trunk@78 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../video_coding/main/source/jitter_buffer.cc | 33 +++++++++++------ .../main/source/jitter_estimator.cc | 35 +++++-------------- .../main/source/jitter_estimator.h | 9 ++--- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/modules/video_coding/main/source/jitter_buffer.cc b/modules/video_coding/main/source/jitter_buffer.cc index b74adb06bd..90075191c2 100644 --- a/modules/video_coding/main/source/jitter_buffer.cc +++ b/modules/video_coding/main/source/jitter_buffer.cc @@ -213,7 +213,9 @@ VCMJitterBuffer::Start() _waitingForCompletion.latestPacketTime = -1; _missingMarkerBits = false; _firstPacket = true; + _NACKSeqNumLength = 0; _rttMs = 0; + WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, VCMId(_vcmId, _receiverId), "JB(0x%x): Jitter buffer: start", this); } @@ -286,6 +288,8 @@ VCMJitterBuffer::FlushInternal() _missingMarkerBits = false; _firstPacket = true; + _NACKSeqNumLength = 0; + WEBRTC_TRACE(webrtc::kTraceDebug, webrtc::kTraceVideoCoding, VCMId(_vcmId, _receiverId), "JB(0x%x): Jitter buffer: flush", this); } @@ -846,17 +850,20 @@ VCMJitterBuffer::GetCompleteFrameForDecoding(WebRtc_UWord32 maxWaitTimeMS) } // we have a frame - // store seqnum + // store seqnum _lastDecodedSeqNum = oldestFrame->GetHighSeqNum(); // store current timestamp _lastDecodedTimeStamp = oldestFrame->TimeStamp(); // Update jitter estimate const bool retransmitted = (oldestFrame->GetNackCount() > 0); - _jitterEstimate.UpdateNackEstimate(retransmitted); - // Ignore retransmitted frames also ignore empty frames. - if (!retransmitted && oldestFrame->Length() > 0) + if (retransmitted) { + _jitterEstimate.FrameNacked(); + } + else if (oldestFrame->Length() > 0) + { + // Ignore retransmitted and empty frames. UpdateJitterAndDelayEstimates(*oldestFrame, false); } @@ -1061,10 +1068,13 @@ VCMJitterBuffer::GetFrameForDecoding() // This frame shouldn't have been retransmitted, but if we recently // turned off NACK this might still happen. const bool retransmitted = (oldestFrame->GetNackCount() > 0); - _jitterEstimate.UpdateNackEstimate(retransmitted); - // Ignore retransmitted frames also ignore empty frames. - if (!retransmitted && oldestFrame->Length() > 0) + if (retransmitted) { + _jitterEstimate.FrameNacked(); + } + else if (oldestFrame->Length() > 0) + { + // Ignore retransmitted and empty frames. // Update with the previous incomplete frame first if (_waitingForCompletion.latestPacketTime >= 0) { @@ -1137,10 +1147,13 @@ VCMJitterBuffer::GetFrameForDecodingNACK() // Update jitter estimate const bool retransmitted = (oldestFrame->GetNackCount() > 0); - _jitterEstimate.UpdateNackEstimate(retransmitted); - // Ignore retransmitted frames also ignore empty frames. - if (!retransmitted && oldestFrame->Length() > 0) + if (retransmitted) { + _jitterEstimate.FrameNacked(); + } + else if (oldestFrame->Length() > 0) + { + // Ignore retransmitted and empty frames. UpdateJitterAndDelayEstimates(*oldestFrame, false); } diff --git a/modules/video_coding/main/source/jitter_estimator.cc b/modules/video_coding/main/source/jitter_estimator.cc index 59c71f14b5..233fad4cd6 100644 --- a/modules/video_coding/main/source/jitter_estimator.cc +++ b/modules/video_coding/main/source/jitter_estimator.cc @@ -28,9 +28,7 @@ _psi(0.9999), _alphaCountMax(400), _beta(0.9994), _thetaLow(0.000001), -_nackLimit(3), // This should be 1 if the old - // retransmition estimate is used. -_nackWindowMS(15000), +_nackLimit(3), _numStdDevDelayOutlier(15), _numStdDevFrameSizeOutlier(3), _noiseStdDevs(2.33), // ~Less than 1% chance @@ -213,33 +211,18 @@ VCMJitterEstimator::UpdateEstimate(WebRtc_Word64 frameDelayMS, WebRtc_UWord32 fr // Updates the nack/packet ratio void -VCMJitterEstimator::UpdateNackEstimate(bool retransmitted, WebRtc_Word64 /*wallClockMS = -1*/) +VCMJitterEstimator::FrameNacked() { - // Simplified since it seems to be hard to be sure if a - // packet actually has been retransmitted or not, resulting - // in a delay which varies up and down with one RTT. - // The solution is to wait until _nackLimit retransmitts - // has been received, then always add an RTT to the estimate. - if (retransmitted && _nackCount < _nackLimit) + // Wait until _nackLimit retransmissions has been received, + // then always add ~1 RTT delay. + // TODO(holmer): Should we ever remove the additional delay if the + // the packet losses seem to have stopped? We could for instance scale + // the number of RTTs to add with the amount of retransmissions in a given + // time interval, or similar. + if (_nackCount < _nackLimit) { _nackCount++; } - //if (wallClockMS == -1) - //{ - // wallClockMS = VCMTickTime::MillisecondTimestamp(); - //} - //if (retransmitted) - //{ - // if (_nackCount < _nackLimit) - // { - // _nackCount++; - // } - // _latestNackTimestamp = wallClockMS; - //} - //else if (_nackCount > 0 && wallClockMS - _latestNackTimestamp > _nackWindowMS) - //{ - // _nackCount = 0; - //} } // Updates Kalman estimate of the channel diff --git a/modules/video_coding/main/source/jitter_estimator.h b/modules/video_coding/main/source/jitter_estimator.h index 1c5b0715c3..6fc47030a2 100644 --- a/modules/video_coding/main/source/jitter_estimator.h +++ b/modules/video_coding/main/source/jitter_estimator.h @@ -47,12 +47,8 @@ public: // Return value : Jitter estimate in milliseconds double GetJitterEstimate(double rttMultiplier); - // Updates the nack counter/timer. - // - // Input: - // - retransmitted : True for a nacked frames, false otherwise - // - wallClockMS : Used for testing - void UpdateNackEstimate(bool retransmitted, WebRtc_Word64 wallClockMS = -1); + // Updates the nack counter. + void FrameNacked(); // Updates the RTT filter. // @@ -122,7 +118,6 @@ private: const double _beta; const double _thetaLow; const WebRtc_UWord32 _nackLimit; - const WebRtc_UWord32 _nackWindowMS; const WebRtc_Word32 _numStdDevDelayOutlier; const WebRtc_Word32 _numStdDevFrameSizeOutlier; const double _noiseStdDevs;