Avoid adding duplicates in pacer lists.

Review URL: https://webrtc-codereview.appspot.com/1329007

git-svn-id: http://webrtc.googlecode.com/svn/trunk@3899 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/pacing/include/paced_sender.h

@@ -12,6 +12,7 @@
 #define WEBRTC_MODULES_PACED_SENDER_H_
 
 #include <list>
+#include <set>
 
 #include "webrtc/modules/interface/module.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
@@ -89,14 +90,27 @@ class PacedSender : public Module {
     int bytes_;
   };
 
-  typedef std::list<Packet> PacketList;
+  // STL list style class which prevents duplicates in the list.
+  class PacketList {
+   public:
+    PacketList() {};
+
+    bool empty() const;
+    Packet front() const;
+    void pop_front();
+    void push_back(const Packet& packet);
+
+   private:
+    std::list<Packet> packet_list_;
+    std::set<uint16_t> sequence_number_set_;
+  };
 
   // Checks if next packet in line can be transmitted. Returns true on success.
   bool GetNextPacket(uint32_t* ssrc, uint16_t* sequence_number,
                      int64_t* capture_time_ms);
 
   // Local helper function to GetNextPacket.
-  void GetNextPacketFromList(std::list<Packet>* list,
+  void GetNextPacketFromList(PacketList* list,
       uint32_t* ssrc, uint16_t* sequence_number, int64_t* capture_time_ms);
 
   // Updates the number of bytes that can be sent for the next time interval.

webrtc/modules/pacing/paced_sender.cc

@@ -36,6 +36,29 @@ const int kMaxQueueTimeWithoutSendingMs = 30;
 
 namespace webrtc {
 
+bool PacedSender::PacketList::empty() const {
+  return packet_list_.empty();
+}
+
+PacedSender::Packet PacedSender::PacketList::front() const {
+  return packet_list_.front();
+}
+
+void PacedSender::PacketList::pop_front() {
+  PacedSender::Packet& packet = packet_list_.front();
+  packet_list_.pop_front();
+  sequence_number_set_.erase(packet.sequence_number_);
+}
+
+void PacedSender::PacketList::push_back(const PacedSender::Packet& packet) {
+  if (sequence_number_set_.find(packet.sequence_number_) ==
+      sequence_number_set_.end()) {
+    // Don't insert duplicates.
+    packet_list_.push_back(packet);
+    sequence_number_set_.insert(packet.sequence_number_);
+  }
+}
+
 PacedSender::PacedSender(Callback* callback, int target_bitrate_kbps)
     : callback_(callback),
       enable_(false),
@@ -49,9 +72,6 @@ PacedSender::PacedSender(Callback* callback, int target_bitrate_kbps)
 }
 
 PacedSender::~PacedSender() {
-  high_priority_packets_.clear();
-  normal_priority_packets_.clear();
-  low_priority_packets_.clear();
 }
 
 void PacedSender::Pause() {
@@ -262,7 +282,7 @@ bool PacedSender::GetNextPacket(uint32_t* ssrc, uint16_t* sequence_number,
   return false;
 }
 
-void PacedSender::GetNextPacketFromList(std::list<Packet>* list,
+void PacedSender::GetNextPacketFromList(PacketList* list,
     uint32_t* ssrc, uint16_t* sequence_number, int64_t* capture_time_ms) {
   Packet packet = list->front();
   UpdateState(packet.bytes_);

webrtc/modules/pacing/paced_sender_unittest.cc

@@ -111,6 +111,52 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) {
       sequence_number++, capture_time_ms, 250));
 }
 
+TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
+  uint32_t ssrc = 12345;
+  uint16_t sequence_number = 1234;
+  int64_t capture_time_ms = 56789;
+  uint16_t queued_sequence_number;
+
+  // Due to the multiplicative factor we can send 3 packets not 2 packets.
+  for (int i = 0; i < 3; ++i) {
+    EXPECT_TRUE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+        sequence_number++, capture_time_ms, 250));
+  }
+  queued_sequence_number = sequence_number;
+
+  for (int j = 0; j < 30; ++j) {
+    // Send in duplicate packets.
+    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+        sequence_number, capture_time_ms, 250));
+    EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+        sequence_number++, capture_time_ms, 250));
+  }
+  EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
+  for (int k = 0; k < 10; ++k) {
+    EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
+    TickTime::AdvanceFakeClock(5);
+
+    for (int i = 0; i < 3; ++i) {
+      EXPECT_CALL(callback_, TimeToSendPacket(ssrc, queued_sequence_number++,
+                                              capture_time_ms)).Times(1);
+   }
+    EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
+    EXPECT_EQ(0, send_bucket_->Process());
+  }
+  EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
+  TickTime::AdvanceFakeClock(5);
+  EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
+  EXPECT_EQ(0, send_bucket_->Process());
+  EXPECT_TRUE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+      sequence_number++, capture_time_ms, 250));
+  EXPECT_TRUE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+      sequence_number++, capture_time_ms, 250));
+  EXPECT_TRUE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+      sequence_number++, capture_time_ms, 250));
+  EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc,
+      sequence_number++, capture_time_ms, 250));
+}
+
 TEST_F(PacedSenderTest, Padding) {
   uint32_t ssrc = 12345;
   uint16_t sequence_number = 1234;