Cast: cancel re-sending if a later nack packet says the receiver doesn't want it anymore

media/cast/transport/rtp_sender/rtp_sender.cc

Index: media/cast/transport/rtp_sender/rtp_sender.cc
diff --git a/media/cast/transport/rtp_sender/rtp_sender.cc b/media/cast/transport/rtp_sender/rtp_sender.cc
index 735d086694ca702e01728525b911c83c24596b8a..8ab7f36cfd2a98bc877f97cee247504a23241b79 100644
--- a/media/cast/transport/rtp_sender/rtp_sender.cc
+++ b/media/cast/transport/rtp_sender/rtp_sender.cc
@@ -73,18 +73,20 @@ void RtpSender::ResendPackets(
     const PacketIdSet& packets_set = it->second;
     bool success = false;
 
-    if (packets_set.empty()) {
-      VLOG(3) << "Missing all packets in frame " << static_cast<int>(frame_id);
+    for (uint16 packet_id = 0; ; packet_id++) {
+      // Get packet from storage.
+      success = storage_->GetPacket(frame_id, packet_id, &packets_to_resend);
 
-      uint16 packet_id = 0;
-      do {
-        // Get packet from storage.
-        success = storage_->GetPacket(frame_id, packet_id, &packets_to_resend);
+      // Check that we got at least one packet.
+      DCHECK(packet_id != 0 || success)
+          << "Failed to resend frame " << static_cast<int>(frame_id);
 
-        // Check that we got at least one packet.
-        DCHECK(packet_id != 0 || success)
-            << "Failed to resend frame " << static_cast<int>(frame_id);
+      if (!success) break;
 
+      if (packets_set.find(packet_id) == packets_set.end()) {
+        transport_->CancelSendingPacket(packets_to_resend.back().first);
+        packets_to_resend.pop_back();
+      } else {
         // Resend packet to the network.
         if (success) {

[miu, 2014/05/29 06:20:37]

You can remove this if-statement now.  With your change, success is never false
at this point.

[hubbe, 2014/05/29 19:17:23]

Done.

           VLOG(3) << "Resend " << static_cast<int>(frame_id) << ":"
@@ -93,27 +95,6 @@ void RtpSender::ResendPackets(
           PacketRef packet = packets_to_resend.back().second;
           UpdateSequenceNumber(&packet->data);
           // Set the size as correspond to each frame.

[miu, 2014/05/29 06:20:37]

I don't understand this comment.  Should this LOC be deleted?

[hubbe, 2014/05/29 19:17:23]

I looked up the change that added this comment, I still don't understand it:
Deleted.

-          ++packet_id;
-        }
-      } while (success);
-    } else {
-      // Iterate over all of the packets in the frame.
-      for (PacketIdSet::const_iterator set_it = packets_set.begin();
-           set_it != packets_set.end();
-           ++set_it) {
-        uint16 packet_id = *set_it;
-        success = storage_->GetPacket(frame_id, packet_id, &packets_to_resend);
-
-        // Check that we got at least one packet.
-        DCHECK(set_it != packets_set.begin() || success)
-            << "Failed to resend frame " << frame_id;
-
-        // Resend packet to the network.
-        if (success) {
-          VLOG(3) << "Resend " << static_cast<int>(frame_id) << ":"
-                  << packet_id;
-          PacketRef packet = packets_to_resend.back().second;
-          UpdateSequenceNumber(&packet->data);
         }
       }
     }