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

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);
+    if (retransmitted)
-    // Ignore retransmitted frames also ignore empty frames.
-    if (!retransmitted && oldestFrame->Length() > 0)
     {
+        _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);
+    if (retransmitted)
-    // Ignore retransmitted frames also ignore empty frames.
-    if (!retransmitted && oldestFrame->Length() > 0)
     {
+        _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);
+    if (retransmitted)
-    // Ignore retransmitted frames also ignore empty frames.
-    if (!retransmitted && oldestFrame->Length() > 0)
     {
+        _jitterEstimate.FrameNacked();
+    }
+    else if (oldestFrame->Length() > 0)
+    {
+        // Ignore retransmitted and empty frames.
         UpdateJitterAndDelayEstimates(*oldestFrame, false);
     }
 

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
+_nackLimit(3),
-               // retransmition estimate is used.
-_nackWindowMS(15000),
 _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
+    // Wait until _nackLimit retransmissions has been received,
-    // packet actually has been retransmitted or not, resulting
+    // then always add ~1 RTT delay.
-    // in a delay which varies up and down with one RTT.
+    // TODO(holmer): Should we ever remove the additional delay if the
-    // The solution is to wait until _nackLimit retransmitts
+    // the packet losses seem to have stopped? We could for instance scale
-    // has been received, then always add an RTT to the estimate.
+    // the number of RTTs to add with the amount of retransmissions in a given
-    if (retransmitted && _nackCount < _nackLimit)
+    // 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

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.
+    // Updates the nack counter.
-    //
+    void FrameNacked();
-    // Input:
-    //          - retransmitted       : True for a nacked frames, false otherwise
-    //          - wallClockMS         : Used for testing
-    void UpdateNackEstimate(bool retransmitted, WebRtc_Word64 wallClockMS = -1);
 
     // 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;