Cast: Implement priority packets in PacedSender

This change implements a high priority packet list in PacedSender. In
practice the order of packet priority is:
1. RTCP
2. Audio
3. Video

The reason for doing this such that audio packets can interleave between
video packets even if the audio packets has a greater value of
timestamp. This characteristic of interleaving packets will facilitate
better bandwidth estimation by looking at audio send and ACK pairs.

cast_simulator shows no ill effect but a slight improvement:
Before
Audio frame count received: 17978
Video frame count received: 5169

After
Audio frame count received: 17989
Video frame count received: 5174

BUG=394191

Review URL: https://codereview.chromium.org/399743002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283911 0039d316-1c4b-4281-b951-d872f2087c98

media/cast/net/cast_transport_sender_impl.cc

@@ -89,7 +89,9 @@ void CastTransportSenderImpl::InitializeAudio(
   }
   audio_sender_.reset(new RtpSender(clock_, transport_task_runner_, &pacer_));
   if (audio_sender_->Initialize(config)) {
+    // Audio packets have a higher priority.
     pacer_.RegisterAudioSsrc(config.ssrc);
+    pacer_.RegisterPrioritySsrc(config.ssrc);
     status_callback_.Run(TRANSPORT_AUDIO_INITIALIZED);
   } else {
     audio_sender_.reset();

media/cast/net/pacing/paced_sender.cc

@@ -60,14 +60,25 @@ void PacedSender::RegisterVideoSsrc(uint32 video_ssrc) {
   video_ssrc_ = video_ssrc;
 }
 
+void PacedSender::RegisterPrioritySsrc(uint32 ssrc) {
+  priority_ssrcs_.push_back(ssrc);
+}
+
 bool PacedSender::SendPackets(const SendPacketVector& packets) {
   if (packets.empty()) {
     return true;
   }
+  const bool high_priority = IsHighPriority(packets.begin()->first);
   for (size_t i = 0; i < packets.size(); i++) {
+    DCHECK(IsHighPriority(packets[i].first) == high_priority);
+    if (high_priority) {
+      priority_packet_list_[packets[i].first] =
+          make_pair(PacketType_Normal, packets[i].second);
+    } else {
       packet_list_[packets[i].first] =
           make_pair(PacketType_Normal, packets[i].second);
     }
+  }
   if (state_ == State_Unblocked) {
     SendStoredPackets();
   }
@@ -79,6 +90,7 @@ bool PacedSender::ResendPackets(const SendPacketVector& packets,
   if (packets.empty()) {
     return true;
   }
+  const bool high_priority = IsHighPriority(packets.begin()->first);
   base::TimeTicks now = clock_->NowTicks();
   for (size_t i = 0; i < packets.size(); i++) {
     std::map<PacketKey, base::TimeTicks>::const_iterator j =
@@ -89,9 +101,15 @@ bool PacedSender::ResendPackets(const SendPacketVector& packets,
       continue;
     }
 
+    DCHECK(IsHighPriority(packets[i].first) == high_priority);
+    if (high_priority) {
+      priority_packet_list_[packets[i].first] =
+          make_pair(PacketType_Resend, packets[i].second);
+    } else {
       packet_list_[packets[i].first] =
           make_pair(PacketType_Resend, packets[i].second);
     }
+  }
   if (state_ == State_Unblocked) {
     SendStoredPackets();
   }
@@ -100,7 +118,8 @@ bool PacedSender::ResendPackets(const SendPacketVector& packets,
 
 bool PacedSender::SendRtcpPacket(uint32 ssrc, PacketRef packet) {
   if (state_ == State_TransportBlocked) {
-    packet_list_[PacedPacketSender::MakePacketKey(base::TimeTicks(), ssrc, 0)] =
+    priority_packet_list_[
+        PacedPacketSender::MakePacketKey(base::TimeTicks(), ssrc, 0)] =
         make_pair(PacketType_RTCP, packet);
   } else {
     // We pass the RTCP packets straight through.
@@ -110,33 +129,39 @@ bool PacedSender::SendRtcpPacket(uint32 ssrc, PacketRef packet) {
                        weak_factory_.GetWeakPtr()))) {
       state_ = State_TransportBlocked;
     }
-
   }
   return true;
 }
 
 void PacedSender::CancelSendingPacket(const PacketKey& packet_key) {
   packet_list_.erase(packet_key);
+  priority_packet_list_.erase(packet_key);
 }
 
-PacketRef PacedSender::GetNextPacket(PacketType* packet_type,
+PacketRef PacedSender::PopNextPacket(PacketType* packet_type,
                                      PacketKey* packet_key) {
-  std::map<PacketKey, std::pair<PacketType, PacketRef> >::iterator i;
-  i = packet_list_.begin();
-  DCHECK(i != packet_list_.end());
+  PacketList* list = !priority_packet_list_.empty() ?
+      &priority_packet_list_ : &packet_list_;
+  DCHECK(!list->empty());
+  PacketList::iterator i = list->begin();
   *packet_type = i->second.first;
   *packet_key = i->first;
   PacketRef ret = i->second.second;
-  packet_list_.erase(i);
+  list->erase(i);
   return ret;
 }
 
+bool PacedSender::IsHighPriority(const PacketKey& packet_key) const {
+  return std::find(priority_ssrcs_.begin(), priority_ssrcs_.end(),
+                   packet_key.second.first) != priority_ssrcs_.end();
+}
+
 bool PacedSender::empty() const {
-  return packet_list_.empty();
+  return packet_list_.empty() && priority_packet_list_.empty();
 }
 
 size_t PacedSender::size() const {
-  return packet_list_.size();
+  return packet_list_.size() + priority_packet_list_.size();
 }
 
 // This function can be called from three places:
@@ -199,7 +224,7 @@ void PacedSender::SendStoredPackets() {
     }
     PacketType packet_type;
     PacketKey packet_key;
-    PacketRef packet = GetNextPacket(&packet_type, &packet_key);
+    PacketRef packet = PopNextPacket(&packet_type, &packet_key);
     sent_time_[packet_key] = now;
     sent_time_buffer_[packet_key] = now;
 

media/cast/net/pacing/paced_sender.h

@@ -5,7 +5,7 @@
 #ifndef MEDIA_CAST_NET_PACING_PACED_SENDER_H_
 #define MEDIA_CAST_NET_PACING_PACED_SENDER_H_
 
-#include <list>
+#include <map>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -69,6 +69,12 @@ class PacedSender : public PacedPacketSender,
   void RegisterAudioSsrc(uint32 audio_ssrc);
   void RegisterVideoSsrc(uint32 video_ssrc);
 
+  // Register SSRC that has a higher priority for sending. Multiple SSRCs can
+  // be registered.
+  // Note that it is not expected to register many SSRCs with this method.
+  // Because IsHigherPriority() is determined in linear time.
+  void RegisterPrioritySsrc(uint32 ssrc);
+
   // PacedPacketSender implementation.
   virtual bool SendPackets(const SendPacketVector& packets) OVERRIDE;
   virtual bool ResendPackets(const SendPacketVector& packets,
@@ -108,16 +114,25 @@ class PacedSender : public PacedPacketSender,
   // Returns the next packet to send. RTCP packets have highest priority,
   // resend packets have second highest priority and then comes everything
   // else.
-  PacketRef GetNextPacket(PacketType* packet_type,
+  PacketRef PopNextPacket(PacketType* packet_type,
                           PacketKey* packet_key);
 
+  // Returns true if the packet should have a higher priority.
+  bool IsHighPriority(const PacketKey& packet_key) const;
+
   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_;
-  std::map<PacketKey, std::pair<PacketType, PacketRef> > packet_list_;
+
+  // Set of SSRCs that have higher priority. This is a vector instead of a
+  // set because there's only very few in it (most likely 1).
+  std::vector<uint32> priority_ssrcs_;
+  typedef std::map<PacketKey, std::pair<PacketType, PacketRef> > PacketList;
+  PacketList packet_list_;
+  PacketList priority_packet_list_;
   std::map<PacketKey, base::TimeTicks> sent_time_;
   std::map<PacketKey, base::TimeTicks> sent_time_buffer_;
 

media/cast/net/pacing/paced_sender_unittest.cc

@@ -19,11 +19,11 @@ namespace cast {
 namespace {
 
 static const uint8 kValue = 123;
-static const size_t kSize1 = 100;
-static const size_t kSize2 = 101;
-static const size_t kSize3 = 102;
-static const size_t kSize4 = 103;
-static const size_t kNackSize = 104;
+static const size_t kSize1 = 101;
+static const size_t kSize2 = 102;
+static const size_t kSize3 = 103;
+static const size_t kSize4 = 104;
+static const size_t kNackSize = 105;
 static const int64 kStartMillisecond = INT64_C(12345678900000);
 static const uint32 kVideoSsrc = 0x1234;
 static const uint32 kAudioSsrc = 0x5678;
@@ -349,5 +349,56 @@ TEST_F(PacedSenderTest, PaceWith60fps) {
   EXPECT_TRUE(RunUntilEmpty(5));
 }
 
+TEST_F(PacedSenderTest, SendPriority) {
+  // Actual order to the network is:
+  // 1. Video packets x 10.
+  // 2. RTCP packet x 1.
+  // 3. Audio packet x 1.
+  // 4. Video retransmission packet x 10.
+  // 5. Video packet x 10.
+  mock_transport_.AddExpectedSize(kSize2, 10);  // Normal video packets.
+  mock_transport_.AddExpectedSize(kSize3, 1);  // RTCP packet.
+  mock_transport_.AddExpectedSize(kSize1, 1);  // Audio packet.
+  mock_transport_.AddExpectedSize(kSize4, 10);  // Resend video packets.
+  mock_transport_.AddExpectedSize(kSize2, 10);  // Normal video packets.
+
+  paced_sender_->RegisterPrioritySsrc(kAudioSsrc);
+
+  // Retransmission packets with the earlier timestamp.
+  SendPacketVector resend_packets =
+      CreateSendPacketVector(kSize4, 10, false);
+  testing_clock_.Advance(base::TimeDelta::FromMilliseconds(10));
+
+  // Send 20 normal video packets. Only 10 will be sent in this
+  // call, the rest will be sitting in the queue waiting for pacing.
+  EXPECT_TRUE(paced_sender_->SendPackets(
+      CreateSendPacketVector(kSize2, 20, false)));
+
+  testing_clock_.Advance(base::TimeDelta::FromMilliseconds(10));
+
+  // Send normal audio packet. This is queued and will be sent
+  // earlier than video packets.
+  EXPECT_TRUE(paced_sender_->SendPackets(
+      CreateSendPacketVector(kSize1, 1, true)));
+
+  // Send RTCP packet. This is queued and will be sent first.
+  EXPECT_TRUE(paced_sender_->SendRtcpPacket(
+      kVideoSsrc,
+      new base::RefCountedData<Packet>(Packet(kSize3, kValue))));
+
+  // Resend video packets. This is queued and will be sent
+  // earlier than normal video packets.
+  EXPECT_TRUE(paced_sender_->ResendPackets(
+      resend_packets, base::TimeDelta()));
+
+  // Roll the clock. Queued packets will be sent in this order:
+  // 1. RTCP packet x 1.
+  // 2. Audio packet x 1.
+  // 3. Video retransmission packet x 10.
+  // 4. Video packet x 10.
+  task_runner_->RunTasks();
+  EXPECT_TRUE(RunUntilEmpty(4));
+}
+
 }  // namespace cast
 }  // namespace media