[Cast] Limit frames in flight by duration, and not by number of frames.

media/cast/net/rtp/packet_storage.cc

Index: media/cast/net/rtp/packet_storage.cc
diff --git a/media/cast/net/rtp/packet_storage.cc b/media/cast/net/rtp/packet_storage.cc
index 59ac9ce44eef87cf768a1b807584631647c0e0ef..190e7a3f3a0e24fc7c97ef86a91cfd62f1767565 100644
--- a/media/cast/net/rtp/packet_storage.cc
+++ b/media/cast/net/rtp/packet_storage.cc
@@ -4,47 +4,60 @@
 
 #include "media/cast/net/rtp/packet_storage.h"
 
-#include <string>
-
 #include "base/logging.h"
 #include "media/cast/cast_defines.h"
 
 namespace media {
 namespace cast {
 
-PacketStorage::PacketStorage(size_t stored_frames)
-    : max_stored_frames_(stored_frames),
-      first_frame_id_in_list_(0),
-      last_frame_id_in_list_(0) {
+PacketStorage::PacketStorage()
+    : first_frame_id_in_list_(0),
+      zombie_count_(0) {
 }
 
 PacketStorage::~PacketStorage() {
 }
 
-bool PacketStorage::IsValid() const {
-  return max_stored_frames_ > 0 &&
-      static_cast<int>(max_stored_frames_) <= kMaxUnackedFrames;
-}
-
 size_t PacketStorage::GetNumberOfStoredFrames() const {
-  return frames_.size();
+  return frames_.size() - zombie_count_;
 }
 
 void PacketStorage::StoreFrame(uint32 frame_id,
                                const SendPacketVector& packets) {
+  if (packets.empty()) {
+    NOTREACHED();
+    return;
+  }
+
   if (frames_.empty()) {
     first_frame_id_in_list_ = frame_id;
   } else {
     // Make sure frame IDs are consecutive.
-    DCHECK_EQ(last_frame_id_in_list_ + 1, frame_id);
+    DCHECK_EQ(first_frame_id_in_list_ + static_cast<uint32>(frames_.size()),
+              frame_id);
+
+    // Evict the oldest frame if the list will become too long.
+    if (frames_.size() == kMaxUnackedFrames)

[hubbe, 2014/09/11 23:05:05]

This really should never happen, right?
LOG(WARNING) or something?

[miu, 2014/09/18 01:10:52]

Done.  Made it a DCHECK.

Also, got rid of the unit test that was testing PacketStorage capacity overflow,
since this functionality is now erroneous.

+      ReleaseFrame(first_frame_id_in_list_);
   }
 
   // Save new frame to the end of the list.
-  last_frame_id_in_list_ = frame_id;
   frames_.push_back(packets);
+}
+
+void PacketStorage::ReleaseFrame(uint32 frame_id) {
+  const uint32 offset = frame_id - first_frame_id_in_list_;
+  if (static_cast<int32>(offset) < 0 || offset >= frames_.size() ||
+      frames_[offset].empty()) {
+    return;
+  }
+
+  frames_[offset].clear();
+  ++zombie_count_;
 
-  // Evict the oldest frame if the list is too long.
-  if (frames_.size() > max_stored_frames_) {
+  while (!frames_.empty() && frames_.front().empty()) {
+    DCHECK_GT(zombie_count_, 0u);
+    --zombie_count_;
     frames_.pop_front();
     ++first_frame_id_in_list_;
   }
@@ -57,7 +70,8 @@ const SendPacketVector* PacketStorage::GetFrame8(uint8 frame_id_8bits) const {
   index_8bits = frame_id_8bits - index_8bits;
   if (index_8bits >= frames_.size())
     return NULL;
-  return &(frames_[index_8bits]);
+  const SendPacketVector& packets = frames_[index_8bits];
+  return packets.empty() ? NULL : &packets;
 }
 
 }  // namespace cast