Cast: Avoid retransmit if we sent the same packet recently (less than RTT)

When transmitting 4Mbit over the "bad" udp_proxy profile, this reduces the
amount of data sent from 12Mbit/s to 7Mbit/s, and seems to make it able to
actually send more frames across the wire and recover faster.

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

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

18 files changed, 102 insertions, 36 deletions

diff --git a/media/cast/audio_sender/audio_sender.cc b/media/cast/audio_sender/audio_sender.cc
index ea44218..878f345 100644
--- a/media/cast/audio_sender/audio_sender.cc
+++ b/media/cast/audio_sender/audio_sender.cc
@@ -252,9 +252,15 @@ void AudioSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
     // This is to avoid aggresive resend.
     duplicate_ack_counter_ = 0;
 
+    base::TimeDelta rtt;
+    base::TimeDelta avg_rtt;
+    base::TimeDelta min_rtt;
+    base::TimeDelta max_rtt;
+    rtcp_.Rtt(&rtt, &avg_rtt, &min_rtt, &max_rtt);
+
     // A NACK is also used to cancel pending re-transmissions.
     transport_sender_->ResendPackets(
-        true, cast_feedback.missing_frames_and_packets_, true);
+        true, cast_feedback.missing_frames_and_packets_, false, min_rtt);
   }
 
   const base::TimeTicks now = cast_environment_->Clock()->NowTicks();
@@ -280,7 +286,8 @@ void AudioSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
       latest_acked_frame_id_++;
       missing_frames_and_packets[latest_acked_frame_id_] = missing;
     }
-    transport_sender_->ResendPackets(true, missing_frames_and_packets, true);
+    transport_sender_->ResendPackets(
+        true, missing_frames_and_packets, true, base::TimeDelta());
     latest_acked_frame_id_ = cast_feedback.ack_frame_id_;
   }
 }
@@ -313,10 +320,16 @@ void AudioSender::ResendForKickstart() {
       std::make_pair(last_sent_frame_id_, missing));
   last_send_time_ = cast_environment_->Clock()->NowTicks();
 
+  base::TimeDelta rtt;
+  base::TimeDelta avg_rtt;
+  base::TimeDelta min_rtt;
+  base::TimeDelta max_rtt;
+  rtcp_.Rtt(&rtt, &avg_rtt, &min_rtt, &max_rtt);
+
   // Sending this extra packet is to kick-start the session. There is
   // no need to optimize re-transmission for this case.
-  transport_sender_->ResendPackets(true, missing_frames_and_packets,
-                                   false);
+  transport_sender_->ResendPackets(
+      true, missing_frames_and_packets, false, min_rtt);
 }
 
 }  // namespace cast
diff --git a/media/cast/logging/logging_defines.cc b/media/cast/logging/logging_defines.cc
index d0dd5c8..05ceeb9 100644
--- a/media/cast/logging/logging_defines.cc
+++ b/media/cast/logging/logging_defines.cc
@@ -25,6 +25,7 @@ const char* CastLoggingToString(CastLoggingEvent event) {
     ENUM_TO_STRING(FRAME_PLAYOUT);
     ENUM_TO_STRING(PACKET_SENT_TO_NETWORK);
     ENUM_TO_STRING(PACKET_RETRANSMITTED);
+    ENUM_TO_STRING(PACKET_RTX_REJECTED);
     ENUM_TO_STRING(PACKET_RECEIVED);
   }
   NOTREACHED();
diff --git a/media/cast/logging/logging_defines.h b/media/cast/logging/logging_defines.h
index b3f3841..021a3c9 100644
--- a/media/cast/logging/logging_defines.h
+++ b/media/cast/logging/logging_defines.h
@@ -32,6 +32,7 @@ enum CastLoggingEvent {
   // Sender side packet events.
   PACKET_SENT_TO_NETWORK,
   PACKET_RETRANSMITTED,
+  PACKET_RTX_REJECTED,
   // Receiver side packet events.
   PACKET_RECEIVED,
   kNumOfLoggingEvents = PACKET_RECEIVED
diff --git a/media/cast/logging/proto/proto_utils.cc b/media/cast/logging/proto/proto_utils.cc
index 1f05616..03251e6 100644
--- a/media/cast/logging/proto/proto_utils.cc
+++ b/media/cast/logging/proto/proto_utils.cc
@@ -25,6 +25,7 @@ proto::EventType ToProtoEventType(CastLoggingEvent event) {
     TO_PROTO_ENUM(FRAME_PLAYOUT);
     TO_PROTO_ENUM(PACKET_SENT_TO_NETWORK);
     TO_PROTO_ENUM(PACKET_RETRANSMITTED);
+    TO_PROTO_ENUM(PACKET_RTX_REJECTED);
     TO_PROTO_ENUM(PACKET_RECEIVED);
   }
   NOTREACHED();
diff --git a/media/cast/logging/proto/raw_events.proto b/media/cast/logging/proto/raw_events.proto
index 08bf53d..1d2c537 100644
--- a/media/cast/logging/proto/raw_events.proto
+++ b/media/cast/logging/proto/raw_events.proto
@@ -66,6 +66,7 @@ enum EventType {
   PACKET_SENT_TO_NETWORK = 36;
   PACKET_RETRANSMITTED = 37;
   PACKET_RECEIVED = 38;
+  PACKET_RTX_REJECTED = 39;
 }
 
 // Contains information independent of the stream that describes the system
diff --git a/media/cast/rtcp/rtcp_sender_unittest.cc b/media/cast/rtcp/rtcp_sender_unittest.cc
index 2bd807f..0b0c7d3 100644
--- a/media/cast/rtcp/rtcp_sender_unittest.cc
+++ b/media/cast/rtcp/rtcp_sender_unittest.cc
@@ -59,7 +59,8 @@ class TestRtcpTransport : public transport::PacedPacketSender {
     return false;
   }
   virtual bool ResendPackets(
-      const transport::SendPacketVector& packets) OVERRIDE {
+      const transport::SendPacketVector& packets,
+      base::TimeDelta dedupe_window) OVERRIDE {
     return false;
   }
 
diff --git a/media/cast/rtcp/rtcp_unittest.cc b/media/cast/rtcp/rtcp_unittest.cc
index d5bf312..095e6d2 100644
--- a/media/cast/rtcp/rtcp_unittest.cc
+++ b/media/cast/rtcp/rtcp_unittest.cc
@@ -104,7 +104,8 @@ class LocalRtcpTransport : public transport::PacedPacketSender {
   }
 
   virtual bool ResendPackets(
-      const transport::SendPacketVector& packets) OVERRIDE {
+      const transport::SendPacketVector& packets,
+      base::TimeDelta dedupe_window) OVERRIDE {
     return false;
   }
 
diff --git a/media/cast/test/cast_benchmarks.cc b/media/cast/test/cast_benchmarks.cc
index c3468a2..6625762 100644
--- a/media/cast/test/cast_benchmarks.cc
+++ b/media/cast/test/cast_benchmarks.cc
@@ -212,9 +212,10 @@ class CastTransportSenderWrapper : public transport::CastTransportSender {
   virtual void ResendPackets(
       bool is_audio,
       const MissingFramesAndPacketsMap& missing_packets,
-      bool cancel_rtx_if_not_in_list) OVERRIDE {
+      bool cancel_rtx_if_not_in_list,
+      base::TimeDelta dedupe_window) OVERRIDE {
     transport_->ResendPackets(
-        is_audio, missing_packets, cancel_rtx_if_not_in_list);
+        is_audio, missing_packets, cancel_rtx_if_not_in_list, dedupe_window);
   }
 
  private:
diff --git a/media/cast/transport/cast_transport_sender.h b/media/cast/transport/cast_transport_sender.h
index 2556a8b..e88f2f4 100644
--- a/media/cast/transport/cast_transport_sender.h
+++ b/media/cast/transport/cast_transport_sender.h
@@ -96,11 +96,14 @@ class CastTransportSender : public base::NonThreadSafe {
   // frame to be re-transmitted.
   // If |cancel_rtx_if_not_in_list| is used as an optimization to cancel
   // pending re-transmission requests of packets not listed in
-  // |missing_packets|.
+  // |missing_packets|. If the requested packet(s) were sent recently
+  // (how long is specified by |dedupe_window|) then this re-transmit
+  // will be ignored.
   virtual void ResendPackets(
       bool is_audio,
       const MissingFramesAndPacketsMap& missing_packets,
-      bool cancel_rtx_if_not_in_list) = 0;
+      bool cancel_rtx_if_not_in_list,
+      base::TimeDelta dedupe_window) = 0;
 };
 
 }  // namespace transport
diff --git a/media/cast/transport/cast_transport_sender_impl.cc b/media/cast/transport/cast_transport_sender_impl.cc
index c440c6c..6fd848f 100644
--- a/media/cast/transport/cast_transport_sender_impl.cc
+++ b/media/cast/transport/cast_transport_sender_impl.cc
@@ -184,15 +184,18 @@ void CastTransportSenderImpl::SendRtcpFromRtpSender(
 void CastTransportSenderImpl::ResendPackets(
     bool is_audio,
     const MissingFramesAndPacketsMap& missing_packets,
-    bool cancel_rtx_if_not_in_list) {
+    bool cancel_rtx_if_not_in_list,
+    base::TimeDelta dedupe_window) {
   if (is_audio) {
     DCHECK(audio_sender_) << "Audio sender uninitialized";
     audio_sender_->ResendPackets(missing_packets,
-                                 cancel_rtx_if_not_in_list);
+                                 cancel_rtx_if_not_in_list,
+                                 dedupe_window);
   } else {
     DCHECK(video_sender_) << "Video sender uninitialized";
     video_sender_->ResendPackets(missing_packets,
-                                 cancel_rtx_if_not_in_list);
+                                 cancel_rtx_if_not_in_list,
+                                 dedupe_window);
   }
 }
 
diff --git a/media/cast/transport/cast_transport_sender_impl.h b/media/cast/transport/cast_transport_sender_impl.h
index 4fc074c..035ef84 100644
--- a/media/cast/transport/cast_transport_sender_impl.h
+++ b/media/cast/transport/cast_transport_sender_impl.h
@@ -67,7 +67,8 @@ class CastTransportSenderImpl : public CastTransportSender {
 
   virtual void ResendPackets(bool is_audio,
                              const MissingFramesAndPacketsMap& missing_packets,
-                             bool cancel_rtx_if_not_in_list)
+                             bool cancel_rtx_if_not_in_list,
+                             base::TimeDelta dedupe_window)
       OVERRIDE;
 
  private:
diff --git a/media/cast/transport/pacing/mock_paced_packet_sender.h b/media/cast/transport/pacing/mock_paced_packet_sender.h
index 9f6d204..20b7647 100644
--- a/media/cast/transport/pacing/mock_paced_packet_sender.h
+++ b/media/cast/transport/pacing/mock_paced_packet_sender.h
@@ -18,7 +18,8 @@ class MockPacedPacketSender : public PacedPacketSender {
   virtual ~MockPacedPacketSender();
 
   MOCK_METHOD1(SendPackets, bool(const SendPacketVector& packets));
-  MOCK_METHOD1(ResendPackets, bool(const SendPacketVector& packets));
+  MOCK_METHOD2(ResendPackets, bool(const SendPacketVector& packets,
+                                   base::TimeDelta dedupe_window));
   MOCK_METHOD2(SendRtcpPacket, bool(unsigned int ssrc, PacketRef packet));
   MOCK_METHOD1(CancelSendingPacket, void(const PacketKey& packet_key));
 };
diff --git a/media/cast/transport/pacing/paced_sender.cc b/media/cast/transport/pacing/paced_sender.cc
index 10b4224..20cbde8 100644
--- a/media/cast/transport/pacing/paced_sender.cc
+++ b/media/cast/transport/pacing/paced_sender.cc
@@ -20,6 +20,7 @@ static const int64 kPacingIntervalMs = 10;
 static const size_t kPacingMaxBurstsPerFrame = 3;
 static const size_t kTargetBurstSize = 10;
 static const size_t kMaxBurstSize = 20;
+static const size_t kMaxDedupeWindowMs = 500;
 
 }  // namespace
 
@@ -73,11 +74,21 @@ bool PacedSender::SendPackets(const SendPacketVector& packets) {
   return true;
 }
 
-bool PacedSender::ResendPackets(const SendPacketVector& packets) {
+bool PacedSender::ResendPackets(const SendPacketVector& packets,
+                                base::TimeDelta dedupe_window) {
   if (packets.empty()) {
     return true;
   }
+  base::TimeTicks now = clock_->NowTicks();
   for (size_t i = 0; i < packets.size(); i++) {
+    std::map<PacketKey, base::TimeTicks>::const_iterator j =
+        sent_time_.find(packets[i].first);
+
+    if (j != sent_time_.end() && now - j->second < dedupe_window) {
+      LogPacketEvent(packets[i].second->data, PACKET_RTX_REJECTED);
+      continue;
+    }
+
     packet_list_[packets[i].first] =
         make_pair(PacketType_Resend, packets[i].second);
   }
@@ -108,11 +119,13 @@ void PacedSender::CancelSendingPacket(const PacketKey& packet_key) {
   packet_list_.erase(packet_key);
 }
 
-PacketRef PacedSender::GetNextPacket(PacketType* packet_type) {
+PacketRef PacedSender::GetNextPacket(PacketType* packet_type,
+                                     PacketKey* packet_key) {
   std::map<PacketKey, std::pair<PacketType, PacketRef> >::iterator i;
   i = packet_list_.begin();
   DCHECK(i != packet_list_.end());
   *packet_type = i->second.first;
+  *packet_key = i->first;
   PacketRef ret = i->second.second;
   packet_list_.erase(i);
   return ret;
@@ -185,14 +198,17 @@ void PacedSender::SendStoredPackets() {
       return;
     }
     PacketType packet_type;
-    PacketRef packet = GetNextPacket(&packet_type);
+    PacketKey packet_key;
+    PacketRef packet = GetNextPacket(&packet_type, &packet_key);
+    sent_time_[packet_key] = now;
+    sent_time_buffer_[packet_key] = now;
 
     switch (packet_type) {
       case PacketType_Resend:
-        LogPacketEvent(packet->data, true);
+        LogPacketEvent(packet->data, PACKET_RETRANSMITTED);
         break;
       case PacketType_Normal:
-        LogPacketEvent(packet->data, false);
+        LogPacketEvent(packet->data, PACKET_SENT_TO_NETWORK);
         break;
       case PacketType_RTCP:
         break;
@@ -203,10 +219,20 @@ void PacedSender::SendStoredPackets() {
     }
     current_burst_size_++;
   }
+  // Keep ~0.5 seconds of data (1000 packets)
+  if (sent_time_buffer_.size() >=
+      kMaxBurstSize * kMaxDedupeWindowMs / kPacingIntervalMs) {
+    sent_time_.swap(sent_time_buffer_);
+    sent_time_buffer_.clear();
+  }
+  DCHECK_LE(sent_time_buffer_.size(),
+            kMaxBurstSize * kMaxDedupeWindowMs / kPacingIntervalMs);
+  DCHECK_LE(sent_time_.size(),
+            2 * kMaxBurstSize * kMaxDedupeWindowMs / kPacingIntervalMs);
   state_ = State_Unblocked;
 }
 
-void PacedSender::LogPacketEvent(const Packet& packet, bool retransmit) {
+void PacedSender::LogPacketEvent(const Packet& packet, CastLoggingEvent event) {
   // Get SSRC from packet and compare with the audio_ssrc / video_ssrc to see
   // if the packet is audio or video.
   DCHECK_GE(packet.size(), 12u);
@@ -224,8 +250,6 @@ void PacedSender::LogPacketEvent(const Packet& packet, bool retransmit) {
     return;
   }
 
-  CastLoggingEvent event = retransmit ?
-      PACKET_RETRANSMITTED : PACKET_SENT_TO_NETWORK;
   EventMediaType media_type = is_audio ? AUDIO_EVENT : VIDEO_EVENT;
   logging_->InsertSinglePacketEvent(clock_->NowTicks(), event, media_type,
       packet);
diff --git a/media/cast/transport/pacing/paced_sender.h b/media/cast/transport/pacing/paced_sender.h
index 2373fb5..9fc0c8b 100644
--- a/media/cast/transport/pacing/paced_sender.h
+++ b/media/cast/transport/pacing/paced_sender.h
@@ -41,7 +41,8 @@ typedef std::vector<std::pair<PacketKey, PacketRef> > SendPacketVector;
 class PacedPacketSender {
  public:
   virtual bool SendPackets(const SendPacketVector& packets) = 0;
-  virtual bool ResendPackets(const SendPacketVector& packets) = 0;
+  virtual bool ResendPackets(const SendPacketVector& packets,
+                             base::TimeDelta dedupe_window) = 0;
   virtual bool SendRtcpPacket(uint32 ssrc, PacketRef packet) = 0;
   virtual void CancelSendingPacket(const PacketKey& packet_key) = 0;
 
@@ -72,14 +73,15 @@ class PacedSender : public PacedPacketSender,
 
   // PacedPacketSender implementation.
   virtual bool SendPackets(const SendPacketVector& packets) OVERRIDE;
-  virtual bool ResendPackets(const SendPacketVector& packets) OVERRIDE;
+  virtual bool ResendPackets(const SendPacketVector& packets,
+                             base::TimeDelta dedupe_window) OVERRIDE;
   virtual bool SendRtcpPacket(uint32 ssrc, PacketRef packet) OVERRIDE;
   virtual void CancelSendingPacket(const PacketKey& packet_key) OVERRIDE;
 
  private:
   // Actually sends the packets to the transport.
   void SendStoredPackets();
-  void LogPacketEvent(const Packet& packet, bool retransmit);
+  void LogPacketEvent(const Packet& packet, CastLoggingEvent event);
 
   enum PacketType {
     PacketType_RTCP,
@@ -108,7 +110,8 @@ 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 GetNextPacket(PacketType* packet_type,
+                          PacketKey* packet_key);
 
   base::TickClock* const clock_;  // Not owned by this class.
   LoggingImpl* const logging_;    // Not owned by this class.
@@ -117,6 +120,8 @@ class PacedSender : public PacedPacketSender,
   uint32 audio_ssrc_;
   uint32 video_ssrc_;
   std::map<PacketKey, std::pair<PacketType, PacketRef> > packet_list_;
+  std::map<PacketKey, base::TimeTicks> sent_time_;
+  std::map<PacketKey, base::TimeTicks> sent_time_buffer_;
 
   // Maximum burst size for the next three bursts.
   size_t max_burst_size_;
diff --git a/media/cast/transport/pacing/paced_sender_unittest.cc b/media/cast/transport/pacing/paced_sender_unittest.cc
index ef9d89b..5e24fca 100644
--- a/media/cast/transport/pacing/paced_sender_unittest.cc
+++ b/media/cast/transport/pacing/paced_sender_unittest.cc
@@ -129,7 +129,7 @@ TEST_F(PacedSenderTest, PassThroughRtcp) {
   SendPacketVector packets = CreateSendPacketVector(kSize1, 1, true);
 
   EXPECT_TRUE(paced_sender_->SendPackets(packets));
-  EXPECT_TRUE(paced_sender_->ResendPackets(packets));
+  EXPECT_TRUE(paced_sender_->ResendPackets(packets, base::TimeDelta()));
 
   mock_transport_.AddExpectedSize(kSize2, 1);
   Packet tmp(kSize2, kValue);
@@ -202,7 +202,7 @@ TEST_F(PacedSenderTest, PaceWithNack) {
   EXPECT_TRUE(paced_sender_->SendPackets(first_frame_packets));
 
   // Add first NACK request.
-  EXPECT_TRUE(paced_sender_->ResendPackets(nack_packets));
+  EXPECT_TRUE(paced_sender_->ResendPackets(nack_packets, base::TimeDelta()));
 
   // Check that we get the first NACK burst.
   mock_transport_.AddExpectedSize(kNackSize, 10);
@@ -211,7 +211,7 @@ TEST_F(PacedSenderTest, PaceWithNack) {
   task_runner_->RunTasks();
 
   // Add second NACK request.
-  EXPECT_TRUE(paced_sender_->ResendPackets(nack_packets));
+  EXPECT_TRUE(paced_sender_->ResendPackets(nack_packets, base::TimeDelta()));
 
   // Check that we get the next NACK burst.
   mock_transport_.AddExpectedSize(kNackSize, 10);
diff --git a/media/cast/transport/rtp_sender/rtp_sender.cc b/media/cast/transport/rtp_sender/rtp_sender.cc
index 511d21c..b807b34 100644
--- a/media/cast/transport/rtp_sender/rtp_sender.cc
+++ b/media/cast/transport/rtp_sender/rtp_sender.cc
@@ -76,7 +76,8 @@ void RtpSender::SendFrame(const EncodedFrame& frame) {
 
 void RtpSender::ResendPackets(
     const MissingFramesAndPacketsMap& missing_frames_and_packets,
-    bool cancel_rtx_if_not_in_list) {
+    bool cancel_rtx_if_not_in_list,
+    base::TimeDelta dedupe_window) {
   DCHECK(storage_);
   // Iterate over all frames in the list.
   for (MissingFramesAndPacketsMap::const_iterator it =
@@ -130,7 +131,7 @@ void RtpSender::ResendPackets(
         transport_->CancelSendingPacket(it->first);
       }
     }
-    transport_->ResendPackets(packets_to_resend);
+    transport_->ResendPackets(packets_to_resend, dedupe_window);
   }
 }
 
diff --git a/media/cast/transport/rtp_sender/rtp_sender.h b/media/cast/transport/rtp_sender/rtp_sender.h
index bfb46cb..e65326a 100644
--- a/media/cast/transport/rtp_sender/rtp_sender.h
+++ b/media/cast/transport/rtp_sender/rtp_sender.h
@@ -51,7 +51,8 @@ class RtpSender {
   void SendFrame(const EncodedFrame& frame);
 
   void ResendPackets(const MissingFramesAndPacketsMap& missing_packets,
-                     bool cancel_rtx_if_not_in_list);
+                     bool cancel_rtx_if_not_in_list,
+                     base::TimeDelta dedupe_window);
 
   size_t send_packet_count() const {
     return packetizer_ ? packetizer_->send_packet_count() : 0;
diff --git a/media/cast/video_sender/video_sender.cc b/media/cast/video_sender/video_sender.cc
index d54c623..cf050b7 100644
--- a/media/cast/video_sender/video_sender.cc
+++ b/media/cast/video_sender/video_sender.cc
@@ -321,7 +321,7 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
 
     // A NACK is also used to cancel pending re-transmissions.
     transport_sender_->ResendPackets(
-        false, cast_feedback.missing_frames_and_packets_, true);
+        false, cast_feedback.missing_frames_and_packets_, true, rtt);
   }
 
   base::TimeTicks now = cast_environment_->Clock()->NowTicks();
@@ -348,7 +348,8 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
       latest_acked_frame_id_++;
       missing_frames_and_packets[latest_acked_frame_id_] = missing;
     }
-    transport_sender_->ResendPackets(false, missing_frames_and_packets, true);
+    transport_sender_->ResendPackets(
+        false, missing_frames_and_packets, true, rtt);
     latest_acked_frame_id_ = cast_feedback.ack_frame_id_;
   }
 }
@@ -382,10 +383,16 @@ void VideoSender::ResendForKickstart() {
       std::make_pair(last_sent_frame_id_, missing));
   last_send_time_ = cast_environment_->Clock()->NowTicks();
 
+  base::TimeDelta rtt;
+  base::TimeDelta avg_rtt;
+  base::TimeDelta min_rtt;
+  base::TimeDelta max_rtt;
+  rtcp_.Rtt(&rtt, &avg_rtt, &min_rtt, &max_rtt);
+
   // Sending this extra packet is to kick-start the session. There is
   // no need to optimize re-transmission for this case.
   transport_sender_->ResendPackets(false, missing_frames_and_packets,
-                                   false);
+                                   false, rtt);
 }
 
 }  // namespace cast