Bug Fix: WebRTC Receiver Timestamp Jump Detection

webrtc/system_wrappers/source/timestamp_extrapolator.cc

Index: webrtc/system_wrappers/source/timestamp_extrapolator.cc
diff --git a/webrtc/system_wrappers/source/timestamp_extrapolator.cc b/webrtc/system_wrappers/source/timestamp_extrapolator.cc
index c7ed856a54742222434e898780d46393981dbbb1..b030799b1a96a0eed9386edd557d49fb61993326 100644
--- a/webrtc/system_wrappers/source/timestamp_extrapolator.cc
+++ b/webrtc/system_wrappers/source/timestamp_extrapolator.cc
@@ -77,15 +77,26 @@ TimestampExtrapolator::Update(int64_t tMs, uint32_t ts90khz)
         _prevMs = tMs;
     }
 
-    // Remove offset to prevent badly scaled matrices
-    tMs -= _startMs;
-
     CheckForWrapArounds(ts90khz);
 
     int64_t unwrapped_ts90khz = static_cast<int64_t>(ts90khz) +
         _wrapArounds * ((static_cast<int64_t>(1) << 32) - 1);
 
     if (_prevUnwrappedTimestamp >= 0 &&
+        std::abs(unwrapped_ts90khz - _prevUnwrappedTimestamp) >
+            90 * 1000 * 10)

[nisse-webrtc, 2017/03/30 06:37:07]

This limit deserves a symbolic name.

And I think it makes sense to log a message when reset happens, since it should
be rare and indicates a problem somewhere in the system.

[stefan-webrtc, 2017/03/30 06:55:14]

I think this will trigger cases where we may not want to reset. For instance if
there was some network interruption for 10 seconds and then it resumed fine. But
maybe it's still fine to reset in that case anyway?

Maybe we should use the existing DelayChangeDetection() method instead and reset
after looking for a consistent offset in timestamps?

[qiangchen, 2017/03/30 17:47:38]

That's a good idea, and I do see the following code (line 109) does it. In that
case, _pP[1][1] is set to a large number, which  essentially means we throw away
the historical sample data, and in turn w will quickly be adjusted to proper
value by the following several updates. 

But line 89 is before this check, which means that if we have a large timestamp
jump back, line 109 can never be triggered for long time, further the
extrapolater stops updating at all, which in turn results in wrong clock time
conversion for a long time.

Do you think it is proper to 
1. remove my current resetting fix, and
2. put the block (ln88~94) after the block (ln106~116)?

+    {
+        // Time stamp jump more than 10s.
+        // We took this as a sign of a different stream, reset the extrapolator.
+        _rwLock->ReleaseLockExclusive();
+        Reset(tMs);
+        _rwLock->AcquireLockExclusive();
+    }
+
+    // Remove offset to prevent badly scaled matrices
+    tMs -= _startMs;
+
+    if (_prevUnwrappedTimestamp >= 0 &&
         unwrapped_ts90khz < _prevUnwrappedTimestamp)
     {
         // Drop reordered frames.