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

 // 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.
 
 #include "media/cast/transport/rtp_sender/rtp_sender.h"
 
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "media/cast/transport/cast_transport_defines.h"
 #include "media/cast/transport/pacing/paced_sender.h"
@@ skipping 55 matching lines @@
   // Iterate over all frames in the list.
   for (MissingFramesAndPacketsMap::const_iterator it =
            missing_frames_and_packets.begin();
        it != missing_frames_and_packets.end();
        ++it) {
     SendPacketVector packets_to_resend;
     uint8 frame_id = it->first;
     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) << ":"
                   << packet_id;
           // Set a unique incremental sequence number for every packet.
           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);
         }
       }
     }
     transport_->ResendPackets(packets_to_resend);
   }
 }
 
 void RtpSender::UpdateSequenceNumber(Packet* packet) {
   uint16 new_sequence_number = packetizer_->NextSequenceNumber();
   int index = 2;
   (*packet)[index] = (static_cast<uint8>(new_sequence_number));
   (*packet)[index + 1] = (static_cast<uint8>(new_sequence_number >> 8));
 }
 
 }  // namespace transport
 }  //  namespace cast
 }  // namespace media