Smarter algorithm for extending 8-bit frame IDs to 32-bit frame IDs.

media/cast/net/cast_transport_defines.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef MEDIA_CAST_NET_CAST_TRANSPORT_DEFINES_H_
 #define MEDIA_CAST_NET_CAST_TRANSPORT_DEFINES_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 48 matching lines @@
 // Each uint16 represents one packet id within a cast frame.
 typedef std::set<uint16> PacketIdSet;
 // Each uint8 represents one cast frame.
 typedef std::map<uint8, PacketIdSet> MissingFramesAndPacketsMap;
 
 // TODO(miu): UGLY IN-LINE DEFINITION IN HEADER FILE!  Move to appropriate
 // location, separated into .h and .cc files.
 class FrameIdWrapHelper {
  public:
   FrameIdWrapHelper()
-      : first_(true), frame_id_wrap_count_(0), range_(kLowRange) {}
+      : largest_frame_id_seen_(kStartFrameId) {}

[Alpha Left Google, 2014/08/13 18:47:24]

Please add another ctor for testing edge cases.

[hubbe, 2014/08/13 21:18:29]

Added setter instead.

 
   uint32 MapTo32bitsFrameId(const uint8 over_the_wire_frame_id) {
-    if (first_) {
-      first_ = false;
-      if (over_the_wire_frame_id == 0xff) {
-        // Special case for startup.
-        return kStartFrameId;
-      }
+    int32 ret = (largest_frame_id_seen_ & ~0xff) | over_the_wire_frame_id;
+    if (ret - largest_frame_id_seen_ > 127) {
+      ret -= 0x100;
     }
-
-    uint32 wrap_count = frame_id_wrap_count_;
-    switch (range_) {
-      case kLowRange:
-        if (over_the_wire_frame_id > kLowRangeThreshold &&
-            over_the_wire_frame_id < kHighRangeThreshold) {
-          range_ = kMiddleRange;
-        }
-        if (over_the_wire_frame_id >= kHighRangeThreshold) {
-          // Wrap count was incremented in High->Low transition, but this frame
-          // is 'old', actually from before the wrap count got incremented.
-          --wrap_count;
-        }
-        break;
-      case kMiddleRange:
-        if (over_the_wire_frame_id >= kHighRangeThreshold) {
-          range_ = kHighRange;
-        }
-        break;
-      case kHighRange:
-        if (over_the_wire_frame_id <= kLowRangeThreshold) {
-          // Wrap-around detected.
-          range_ = kLowRange;
-          ++frame_id_wrap_count_;
-          // Frame triggering wrap-around so wrap count should be incremented as
-          // as well to match |frame_id_wrap_count_|.
-          ++wrap_count;
-        }
-        break;
+    if (ret - largest_frame_id_seen_ < -128) {

[miu, 2014/08/13 18:38:57]

s/if/else if/

[Alpha Left Google, 2014/08/13 18:47:24]

I think so too.

+      ret += 0x100;
     }
-    return (wrap_count << 8) + over_the_wire_frame_id;
+    if (ret - largest_frame_id_seen_ > 0) {

[miu, 2014/08/13 18:38:57]

I think this is wrong.  Shouldn't it be:

  if (static_cast<uint32>(ret) - static_cast<uint32>(largest_frame_id_seen_) >
0) {
    ...

+      largest_frame_id_seen_ = ret;
+    }
+    return static_cast<uint32>(ret);
   }
 
  private:
-  enum Range { kLowRange, kMiddleRange, kHighRange, };
-
-  static const uint8 kLowRangeThreshold = 63;
-  static const uint8 kHighRangeThreshold = 192;
   static const uint32 kStartFrameId = UINT32_C(0xffffffff);
 
-  bool first_;
-  uint32 frame_id_wrap_count_;
-  Range range_;
+  int32 largest_frame_id_seen_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameIdWrapHelper);
 };
 
 inline uint32 GetVideoRtpTimestamp(const base::TimeTicks& time_ticks) {
   base::TimeTicks zero_time;
   base::TimeDelta recorded_delta = time_ticks - zero_time;
   // Timestamp is in 90 KHz for video.
   return static_cast<uint32>(recorded_delta.InMilliseconds() * 90);
 }
 
 }  // namespace cast
 }  // namespace media
 
 #endif  // MEDIA_CAST_NET_CAST_TRANSPORT_DEFINES_H_