Cast: Logging estimates clock difference based on packets

media/cast/logging/receiver_time_offset_estimator_impl.h

Index: media/cast/logging/receiver_time_offset_estimator_impl.h
diff --git a/media/cast/logging/receiver_time_offset_estimator_impl.h b/media/cast/logging/receiver_time_offset_estimator_impl.h
index f0abd297f633571fa420e9ed5b26ce32751e5dd9..641cf075e83337d892182dfe6267fdcb2266634b 100644
--- a/media/cast/logging/receiver_time_offset_estimator_impl.h
+++ b/media/cast/logging/receiver_time_offset_estimator_impl.h
@@ -5,30 +5,36 @@
 #ifndef MEDIA_CAST_LOGGING_RECEIVER_TIME_OFFSET_ESTIMATOR_IMPL_H_
 #define MEDIA_CAST_LOGGING_RECEIVER_TIME_OFFSET_ESTIMATOR_IMPL_H_
 
+#include <map>
+
 #include "base/time/time.h"
 #include "base/threading/thread_checker.h"
+#include "media/cast/common/mod_util.h"
 #include "media/cast/logging/logging_defines.h"
 #include "media/cast/logging/receiver_time_offset_estimator.h"
 
-namespace base {
-class TickClock;
-}
-
 namespace media {
 namespace cast {
 
-// This implementation listens to three types of video events:
-// 1. FRAME_ENCODED (sender side)
-// 2. FRAME_ACK_SENT (receiver side)
-// 3. FRAME_ACK_RECEIVED (sender side)
+
+// This should be large enough so that we can collect all 3 events before
+// the entry gets removed from the map.
+const size_t kMaxEventTimesMapSize = 500;
+
+// The lower, this is, the faster we adjusts to clock drift.
+// (But with more jitter.)
+const size_t kClockDriftSpeed = 500;
+
+
+// This implementation listens to two pair of events
+// 1. FRAME_ACK_SENT / FRAME_ACK_RECEIVED  (receiver->sender)
+// 2. PACKET_SENT_TO_NETWORK / PACKET_RECEIVED (sender->receiver)
 // There is a causal relationship between these events in that these events
 // must happen in order. This class obtains the lower and upper bounds for
-// the offset by taking the difference of timestamps (2) - (1) and (2) - (3),
-// respectively.
-// The bound will become better as the latency between the events decreases.
+// the offset by taking the difference of timestamps.
 class ReceiverTimeOffsetEstimatorImpl : public ReceiverTimeOffsetEstimator {
  public:
-  ReceiverTimeOffsetEstimatorImpl(base::TickClock* clock);
+  ReceiverTimeOffsetEstimatorImpl();
 
   virtual ~ReceiverTimeOffsetEstimatorImpl();
 
@@ -41,28 +47,39 @@ class ReceiverTimeOffsetEstimatorImpl : public ReceiverTimeOffsetEstimator {
                                        base::TimeDelta* upper_bound) OVERRIDE;
 
  private:
-  struct EventTimes {
-    base::TimeTicks event_a_time;
-    base::TimeTicks event_b_time;
-    base::TimeTicks event_c_time;
+  class BoundCalculator {
+   public:
+    typedef std::pair<base::TimeTicks, base::TimeTicks> TimeTickPair;

[imcheng, 2014/09/09 01:04:23]

would it be possible to define such that the first element is always the sender
event timestamp, and second is receiver event timestamp? It seems strange that
the lower_bound_.bounds() has to be inverted before it is returned.

[hubbe, 2014/09/09 19:06:15]

I find it far less confusing to have the events ordered in time.
(a always occurs before b)
Changing it means making BoundCalculator into a template which takes a
comparison function as argument, which also confuses the code I think.

+    typedef std::map<uint64, TimeTickPair> EventMap;
+
+    BoundCalculator();
+    ~BoundCalculator();
+    bool has_bound() const { return has_bound_; }
+    base::TimeDelta bound() const { return bound_; }
+
+    void SetSent(uint32 rtp,
+                 uint32 packet_id,
+                 bool audio,
+                 base::TimeTicks t);
+
+    void SetReceived(uint32 rtp,
+                     uint16 packet_id,
+                     bool audio,
+                     base::TimeTicks t);
+
+   private:
+    void UpdateBound(base::TimeTicks a, base::TimeTicks b);
+    void CheckUpdate(uint64 key);
+
+   private:
+    EventMap events_;
+    bool has_bound_;
+    base::TimeDelta bound_;
   };
 
-  typedef std::map<RtpTimestamp, EventTimes> EventTimesMap;
-
-  void UpdateOffsetBounds(const EventTimes& event);
-
   // Fixed size storage to store event times for recent frames.
-  EventTimesMap event_times_map_;
-
-  bool bounded_;
-  base::TickClock* clock_;  // Not owned by this class.
-
-  bool offset_bounds_valid_;
-  base::TimeDelta offset_lower_bound_;
-  base::TimeDelta offset_upper_bound_;
-  base::TimeDelta prev_offset_lower_bound_;
-  base::TimeDelta prev_offset_upper_bound_;
-  base::TimeTicks last_reset_time_;
+  BoundCalculator upper_bound_;
+  BoundCalculator lower_bound_;
 
   base::ThreadChecker thread_checker_;
   DISALLOW_COPY_AND_ASSIGN(ReceiverTimeOffsetEstimatorImpl);