Reset InterArrival if arrival time clock makes a jump.

webrtc/modules/remote_bitrate_estimator/inter_arrival.cc

Index: webrtc/modules/remote_bitrate_estimator/inter_arrival.cc
diff --git a/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc b/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc
index 1b7ce07583d4250083873f78c3fc92dd05a9093d..7389f35eb7b8c1e86e362d70f6416c66359ec831 100644
--- a/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc
+++ b/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc
@@ -27,10 +27,12 @@ InterArrival::InterArrival(uint32_t timestamp_group_length_ticks,
       current_timestamp_group_(),
       prev_timestamp_group_(),
       timestamp_to_ms_coeff_(timestamp_to_ms_coeff),
-      burst_grouping_(enable_burst_grouping) {}
+      burst_grouping_(enable_burst_grouping),
+      num_consecutive_reordered_packets_(0) {}
 
 bool InterArrival::ComputeDeltas(uint32_t timestamp,
                                  int64_t arrival_time_ms,
+                                 int64_t system_time_ms,
                                  size_t packet_size,
                                  uint32_t* timestamp_delta,
                                  int64_t* arrival_time_delta_ms,
@@ -53,13 +55,31 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
                          prev_timestamp_group_.timestamp;
       *arrival_time_delta_ms = current_timestamp_group_.complete_time_ms -
                                prev_timestamp_group_.complete_time_ms;
+      // Check system time differences to see if we have an unproportional jump
+      // in arrival time. In that case reset the inter-arrival computations.
+      int64_t system_time_delta_ms =
+          current_timestamp_group_.last_system_time_ms -
+          prev_timestamp_group_.last_system_time_ms;
+      if (*arrival_time_delta_ms > system_time_delta_ms) {

[sprang_webrtc, 2016/07/06 12:46:36]

I'm not sure I follow. arrival_time_delta_ms is the arrival time delta between
the last two packets. Since this is a new timestamp group, system_time_delta_ms
is the wall clock delta between the last two packets arriving in the
ComputeDeltas() call? Shouldn't these be in the same ballpark, possibly falsely
triggering this? 

[stefan-webrtc, 2016/07/06 14:53:24]

You are right, changed after offline discussion.

+        LOG(LS_WARNING) << "The arrival time clock offset has changed (diff = "
+                        << *arrival_time_delta_ms - system_time_delta_ms
+                        << " ms), resetting.";
+        Reset();
+        return false;
+      }
       if (*arrival_time_delta_ms < 0) {
         // The group of packets has been reordered since receiving its local
         // arrival timestamp.
-        LOG(LS_WARNING) << "Packets are being reordered on the path from the "
-                           "socket to the bandwidth estimator. Ignoring this "
-                           "packet for bandwidth estimation.";
+        ++num_consecutive_reordered_packets_;
+        if (num_consecutive_reordered_packets_ >= 2) {

[sprang_webrtc, 2016/07/06 12:46:36]

Is 2 enough?
Make this a public named constant so it can be used in a unit test.

[stefan-webrtc, 2016/07/06 14:53:24]

3 should be enough...

+          LOG(LS_WARNING) << "Packets are being reordered on the path from the "
+                             "socket to the bandwidth estimator. Ignoring this "
+                             "packet for bandwidth estimation, resetting.";
+          Reset();
+        }
         return false;
+      } else {
+        num_consecutive_reordered_packets_ = 0;
       }
       assert(*arrival_time_delta_ms >= 0);
       *packet_size_delta = static_cast<int>(current_timestamp_group_.size) -
@@ -78,6 +98,7 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp,
   // Accumulate the frame size.
   current_timestamp_group_.size += packet_size;
   current_timestamp_group_.complete_time_ms = arrival_time_ms;
+  current_timestamp_group_.last_system_time_ms = system_time_ms;
 
   return calculated_deltas;
 }
@@ -126,4 +147,10 @@ bool InterArrival::BelongsToBurst(int64_t arrival_time_ms,
   return propagation_delta_ms < 0 &&
       arrival_time_delta_ms <= kBurstDeltaThresholdMs;
 }
+
+void InterArrival::Reset() {
+  num_consecutive_reordered_packets_ = 0;
+  current_timestamp_group_ = TimestampGroup();
+  prev_timestamp_group_ = TimestampGroup();
+}
 }  // namespace webrtc