Cast: Deduplicate packets in paced sender and always send older packets first

media/cast/transport/pacing/paced_sender.h

Index: media/cast/transport/pacing/paced_sender.h
diff --git a/media/cast/transport/pacing/paced_sender.h b/media/cast/transport/pacing/paced_sender.h
index 238a9517f08fbc7d4481005f0b393802fbb95e28..a884edaa981a50d3bab3b7b743770b4300649f0b 100644
--- a/media/cast/transport/pacing/paced_sender.h
+++ b/media/cast/transport/pacing/paced_sender.h
@@ -26,15 +26,23 @@ class LoggingImpl;
 
 namespace transport {
 
+// Use std::pair for free comparison operators.
+// { capture_time, ssrc, packet_id }
+typedef std::pair<base::TimeTicks, std::pair<unsigned int, uint16> > PacketKey;

[miu, 2014/04/24 00:09:06]

ssrc is defined as uint32 in other parts of the code.

[miu, 2014/04/24 00:09:06]

I'm wondering whether PacketKey should be in cast_transport_defines.h, since it
is used in other modules.

[hubbe, 2014/04/24 18:36:42]

I don't think so. The sort order is specific to the pacer, anything else that
uses it is doing so because the pacer needs it.

[hubbe, 2014/04/24 18:36:42]

Done.

+typedef std::vector<std::pair<PacketKey, PacketRef> > SendPacketVector;
+
 // We have this pure virtual class to enable mocking.
 class PacedPacketSender {
  public:
-  // Inform the pacer / sender of the total number of packets.
-  virtual bool SendPackets(const PacketList& packets) = 0;
-  virtual bool ResendPackets(const PacketList& packets) = 0;
-  virtual bool SendRtcpPacket(PacketRef packet) = 0;
+  virtual bool SendPackets(const SendPacketVector& packets) = 0;
+  virtual bool ResendPackets(const SendPacketVector& packets) = 0;
+  virtual bool SendRtcpPacket(unsigned int ssrc, PacketRef packet) = 0;
 
   virtual ~PacedPacketSender() {}
+
+  static PacketKey MakePacketKey(const base::TimeTicks& ticks,
+                                 unsigned int ssrc,
+                                 uint16 packet_id);
 };
 
 class PacedSender : public PacedPacketSender,
@@ -56,9 +64,9 @@ class PacedSender : public PacedPacketSender,
   void RegisterVideoSsrc(uint32 video_ssrc);
 
   // PacedPacketSender implementation.
-  virtual bool SendPackets(const PacketList& packets) OVERRIDE;
-  virtual bool ResendPackets(const PacketList& packets) OVERRIDE;
-  virtual bool SendRtcpPacket(PacketRef packet) OVERRIDE;
+  virtual bool SendPackets(const SendPacketVector& packets) OVERRIDE;
+  virtual bool ResendPackets(const SendPacketVector& packets) OVERRIDE;
+  virtual bool SendRtcpPacket(unsigned int ssrc, PacketRef packet) OVERRIDE;
 
  private:
   // Actually sends the packets to the transport.
@@ -100,11 +108,7 @@ class PacedSender : public PacedPacketSender,
   scoped_refptr<base::SingleThreadTaskRunner> transport_task_runner_;
   uint32 audio_ssrc_;
   uint32 video_ssrc_;
-  // Note: We can't combine the |packet_list_| and the |resend_packet_list_|
-  // since then we might get reordering of the retransmitted packets.
-  std::deque<PacketRef> rtcp_packet_list_;
-  std::deque<PacketRef> resend_packet_list_;
-  std::deque<PacketRef> packet_list_;
+  std::map<PacketKey, std::pair<PacketType, PacketRef> > packet_list_;

[miu, 2014/04/24 00:09:06]

nit: Maybe comment here (or above, where PacketKey is defined) why the sort
order of the map causes packets to be emitted in the desired priority order.

[hubbe, 2014/04/24 18:36:42]

Done.

 
   // Maximum burst size for the next three bursts.
   size_t max_burst_size_;