Cast: Don't cancel re-transmission for some cases

We were aggresively cancelling re-transmission before. This caused an
ill effect with duplicated ACKs that some packets are not transmitted.
This change adds a boolean to control whether or not to cancel rtx.

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

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

chrome/browser/media/cast_transport_host_filter.cc

@@ -178,11 +178,12 @@ void CastTransportHostFilter::OnSendRtcpFromRtpSender(
 void CastTransportHostFilter::OnResendPackets(
     int32 channel_id,
     bool is_audio,
-    const media::cast::MissingFramesAndPacketsMap& missing_packets) {
+    const media::cast::MissingFramesAndPacketsMap& missing_packets,
+    bool cancel_rtx_if_not_in_list) {
   media::cast::transport::CastTransportSender* sender =
       id_map_.Lookup(channel_id);
   if (sender) {
-    sender->ResendPackets(is_audio, missing_packets);
+    sender->ResendPackets(is_audio, missing_packets, cancel_rtx_if_not_in_list);
   } else {
     DVLOG(1)
         << "CastTransportHostFilter::OnResendPackets on non-existing channel";

chrome/browser/media/cast_transport_host_filter.h

@@ -53,7 +53,8 @@ class CastTransportHostFilter : public content::BrowserMessageFilter {
   void OnResendPackets(
       int32 channel_id,
       bool is_audio,
-      const media::cast::MissingFramesAndPacketsMap& missing_packets);
+      const media::cast::MissingFramesAndPacketsMap& missing_packets,
+      bool cancel_rtx_if_not_in_list);
   void OnNew(
       int32 channel_id,
       const net::IPEndPoint& remote_end_point);

chrome/browser/media/cast_transport_host_filter_unittest.cc

@@ -132,7 +132,7 @@ TEST_F(CastTransportHostFilterTest, SimpleMessages) {
   missing_packets[1].insert(4);
   missing_packets[1].insert(7);
   CastHostMsg_ResendPackets resend_msg(
-      kChannelId, false, missing_packets);
+      kChannelId, false, missing_packets, true);
   FakeSend(resend_msg);
 
   CastHostMsg_Delete delete_msg(kChannelId);

chrome/common/cast_messages.h

@@ -133,11 +133,12 @@ IPC_MESSAGE_CONTROL3(
     media::cast::transport::SendRtcpFromRtpSenderData /* data */,
     media::cast::transport::RtcpDlrrReportBlock /* dlrr */)
 
-IPC_MESSAGE_CONTROL3(
+IPC_MESSAGE_CONTROL4(
     CastHostMsg_ResendPackets,
     int32 /* channel_id */,
     bool /* is_audio */,
-    media::cast::MissingFramesAndPacketsMap /* missing_packets */)
+    media::cast::MissingFramesAndPacketsMap /* missing_packets */,
+    bool /* cancel_rtx_if_not_in_list */)
 
 IPC_MESSAGE_CONTROL2(
     CastHostMsg_New,

chrome/renderer/media/cast_transport_sender_ipc.cc

@@ -78,10 +78,12 @@ void CastTransportSenderIPC::SendRtcpFromRtpSender(
 
 void CastTransportSenderIPC::ResendPackets(
     bool is_audio,
-    const media::cast::MissingFramesAndPacketsMap& missing_packets) {
+    const media::cast::MissingFramesAndPacketsMap& missing_packets,
+    bool cancel_rtx_if_not_in_list) {
   Send(new CastHostMsg_ResendPackets(channel_id_,
                                      is_audio,
-                                     missing_packets));
+                                     missing_packets,
+                                     cancel_rtx_if_not_in_list));
 }
 
 void CastTransportSenderIPC::OnReceivedPacket(

chrome/renderer/media/cast_transport_sender_ipc.h

@@ -47,7 +47,8 @@ class CastTransportSenderIPC
       const std::string& c_name) OVERRIDE;
   virtual void ResendPackets(
       bool is_audio,
-      const media::cast::transport::MissingFramesAndPacketsMap& missing_packets)
+      const media::cast::transport::MissingFramesAndPacketsMap& missing_packets,
+      bool cancel_rtx_if_not_in_list)
       OVERRIDE;
 
   void OnReceivedPacket(const media::cast::transport::Packet& packet);

media/cast/audio_sender/audio_sender.cc

@@ -106,7 +106,7 @@ void AudioSender::SendEncodedAudioFrame(
 void AudioSender::ResendPackets(
     const MissingFramesAndPacketsMap& missing_frames_and_packets) {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
-  transport_sender_->ResendPackets(true, missing_frames_and_packets);
+  transport_sender_->ResendPackets(true, missing_frames_and_packets, false);
 }
 
 void AudioSender::IncomingRtcpPacket(scoped_ptr<Packet> packet) {

media/cast/transport/cast_transport_sender.h

@@ -92,9 +92,15 @@ class CastTransportSender : public base::NonThreadSafe {
                                      const std::string& c_name) = 0;
 
   // Retransmission request.
+  // |missing_packets| includes the list of frames and packets in each
+  // 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|.
   virtual void ResendPackets(
       bool is_audio,
-      const MissingFramesAndPacketsMap& missing_packets) = 0;
+      const MissingFramesAndPacketsMap& missing_packets,
+      bool cancel_rtx_if_not_in_list) = 0;
 };
 
 }  // namespace transport

media/cast/transport/cast_transport_sender_impl.cc

@@ -177,13 +177,16 @@ void CastTransportSenderImpl::SendRtcpFromRtpSender(
 
 void CastTransportSenderImpl::ResendPackets(
     bool is_audio,
-    const MissingFramesAndPacketsMap& missing_packets) {
+    const MissingFramesAndPacketsMap& missing_packets,
+    bool cancel_rtx_if_not_in_list) {
   if (is_audio) {
     DCHECK(audio_sender_) << "Audio sender uninitialized";
-    audio_sender_->ResendPackets(missing_packets);
+    audio_sender_->ResendPackets(missing_packets,
+                                 cancel_rtx_if_not_in_list);
   } else {
     DCHECK(video_sender_) << "Video sender uninitialized";
-    video_sender_->ResendPackets(missing_packets);
+    video_sender_->ResendPackets(missing_packets,
+                                 cancel_rtx_if_not_in_list);
   }
 }
 

media/cast/transport/cast_transport_sender_impl.h

@@ -66,7 +66,8 @@ class CastTransportSenderImpl : public CastTransportSender {
                                      const std::string& c_name) OVERRIDE;
 
   virtual void ResendPackets(bool is_audio,
-                             const MissingFramesAndPacketsMap& missing_packets)
+                             const MissingFramesAndPacketsMap& missing_packets,
+                             bool cancel_rtx_if_not_in_list)
       OVERRIDE;
 
  private:

media/cast/transport/rtp_sender/rtp_sender.cc

@@ -74,7 +74,8 @@ void RtpSender::SendFrame(const EncodedFrame& frame) {
 }
 
 void RtpSender::ResendPackets(
-    const MissingFramesAndPacketsMap& missing_frames_and_packets) {
+    const MissingFramesAndPacketsMap& missing_frames_and_packets,
+    bool cancel_rtx_if_not_in_list) {
   DCHECK(storage_);
   // Iterate over all frames in the list.
   for (MissingFramesAndPacketsMap::const_iterator it =
@@ -98,7 +99,8 @@ void RtpSender::ResendPackets(
 
       // If the resend request doesn't include this packet then cancel
       // re-transmission already in queue.
-      if (!missing_packet_set.empty() &&
+      if (cancel_rtx_if_not_in_list &&
+          !missing_packet_set.empty() &&
           missing_packet_set.find(packet_id) == missing_packet_set.end()) {
         transport_->CancelSendingPacket(it->first);
       } else {

media/cast/transport/rtp_sender/rtp_sender.h

@@ -50,7 +50,8 @@ class RtpSender {
 
   void SendFrame(const EncodedFrame& frame);
 
-  void ResendPackets(const MissingFramesAndPacketsMap& missing_packets);
+  void ResendPackets(const MissingFramesAndPacketsMap& missing_packets,
+                     bool cancel_rtx_if_not_in_list);
 
   size_t send_packet_count() const {
     return packetizer_ ? packetizer_->send_packet_count() : 0;

media/cast/video_sender/video_sender.cc

@@ -318,8 +318,10 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
     // Only count duplicated ACKs if there is no NACK request in between.
     // This is to avoid aggresive resend.
     duplicate_ack_counter_ = 0;
+
+    // A NACK is also used to cancel pending re-transmissions.
     transport_sender_->ResendPackets(
-        false, cast_feedback.missing_frames_and_packets_);
+        false, cast_feedback.missing_frames_and_packets_, true);
   }
 
   base::TimeTicks now = cast_environment_->Clock()->NowTicks();
@@ -370,7 +372,11 @@ void VideoSender::ResendForKickstart() {
   missing_frames_and_packets.insert(
       std::make_pair(last_sent_frame_id_, missing));
   last_send_time_ = cast_environment_->Clock()->NowTicks();
-  transport_sender_->ResendPackets(false, missing_frames_and_packets);
+
+  // 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);
 }
 
 }  // namespace cast