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

media/cast/transport/pacing/paced_sender.h

 // Copyright 2013 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_TRANSPORT_PACING_PACED_SENDER_H_
 #define MEDIA_CAST_TRANSPORT_PACING_PACED_SENDER_H_
 
 #include <list>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time/default_tick_clock.h"
 #include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "media/cast/transport/cast_transport_config.h"
 #include "media/cast/transport/transport/udp_transport.h"
 
 namespace media {
 namespace cast {
 
 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,
                     public base::NonThreadSafe,
                     public base::SupportsWeakPtr<PacedSender> {
  public:
   // The |external_transport| should only be used by the Cast receiver and for
   // testing.
   PacedSender(
       base::TickClock* clock,
       LoggingImpl* logging,
       PacketSender* external_transport,
       const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner);
 
   virtual ~PacedSender();
 
   // These must be called before non-RTCP packets are sent.
   void RegisterAudioSsrc(uint32 audio_ssrc);
   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.
   void SendStoredPackets();
   void LogPacketEvent(const Packet& packet, bool retransmit);
 
   enum PacketType {
     PacketType_RTCP,
     PacketType_Resend,
     PacketType_Normal
@@ skipping 21 matching lines @@
   // resend packets have second highest priority and then comes everything
   // else.
   PacketRef GetNextPacket(PacketType* packet_type);
 
   base::TickClock* const clock_;  // Not owned by this class.
   LoggingImpl* const logging_;    // Not owned by this class.
   PacketSender* transport_;       // Not owned by this class.
   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_;
   size_t next_max_burst_size_;
   size_t next_next_max_burst_size_;
   // Number of packets already sent in the current burst.
   size_t current_burst_size_;
   // This is when the current burst ends.
   base::TimeTicks burst_end_;
 
   State state_;
 
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<PacedSender> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PacedSender);
 };
 
 }  // namespace transport
 }  // namespace cast
 }  // namespace media
 
 #endif  // MEDIA_CAST_TRANSPORT_PACING_PACED_SENDER_H_