Cast: reduce the amount of retransmission packets

Upon duplicated ACKs for a frame the previous behavior was to resend
all packets of the first unacked frame. It was shown in reports that it
is excessive. Instead this change sends the first packet of the last
encoded frame. This significantly reduces the amount of packet
re-transmitted.

PacketStorage is also redesigned in this code. It is now a simple deque
instead of a map.

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

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

media/cast/transport/rtp_sender/packet_storage/packet_storage.cc

@@ -12,123 +12,52 @@ namespace media {
 namespace cast {
 namespace transport {
 
-typedef PacketMap::iterator PacketMapIterator;
-
-PacketStorage::PacketStorage(int stored_frames)
-    : stored_frames_(stored_frames) {
+PacketStorage::PacketStorage(size_t stored_frames)
+    : max_stored_frames_(stored_frames),
+      first_frame_id_in_list_(0),
+      last_frame_id_in_list_(0) {
 }
 
 PacketStorage::~PacketStorage() {
 }
 
 bool PacketStorage::IsValid() const {
-  return stored_frames_ > 0 && stored_frames_ <= kMaxStoredFrames;
+  return max_stored_frames_ > 0 &&
+      static_cast<int>(max_stored_frames_) <= kMaxUnackedFrames;
 }
 
-void PacketStorage::CleanupOldPackets(uint32 current_frame_id) {
-  uint32 frame_to_remove = current_frame_id - stored_frames_;
-  while (!stored_packets_.empty()) {
-    if (IsOlderFrameId(stored_packets_.begin()->first.first,
-                       frame_to_remove)) {
-      stored_packets_.erase(stored_packets_.begin());
-    } else {
-      break;
-    }
-  }
+size_t PacketStorage::GetNumberOfStoredFrames() const {
+  return frames_.size();
 }
 
-void PacketStorage::StorePacket(uint32 frame_id,
-                                uint16 packet_id,
-                                const PacketKey& key,
-                                PacketRef packet) {
-  CleanupOldPackets(frame_id);
-  StorageIndex index(frame_id, packet_id);
-  PacketMapIterator it = stored_packets_.find(index);
-  if (it != stored_packets_.end()) {
-    // We have already saved this.
-    DCHECK(false) << "Invalid state";
-    return;
+void PacketStorage::StoreFrame(uint32 frame_id,
+                               const SendPacketVector& packets) {
+  if (frames_.empty()) {
+    first_frame_id_in_list_ = frame_id;
+  } else {
+    // Make sure frame IDs are consecutive.
+    DCHECK_EQ(last_frame_id_in_list_ + 1, frame_id);
   }
-  stored_packets_[index] = std::make_pair(key, packet);
-}
-
-void PacketStorage::GetPackets(
-    const MissingFramesAndPacketsMap& missing_frames_and_packets,
-    SendPacketVector* packets_to_resend) {
-
-  // Iterate over all frames in the list.
-  for (MissingFramesAndPacketsMap::const_iterator it =
-           missing_frames_and_packets.begin();
-       it != missing_frames_and_packets.end();
-       ++it) {
-    uint8 frame_id = it->first;
-    const PacketIdSet& packets_set = it->second;
-    bool success = false;
 
-    if (packets_set.empty()) {
-      VLOG(1) << "Missing all packets in frame " << static_cast<int>(frame_id);
-      uint16 packet_id = 0;
-      do {
-        // Get packet from storage.
-        success = GetPacket(frame_id, packet_id, packets_to_resend);
-        ++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) {
-        GetPacket(frame_id, *set_it, packets_to_resend);
-      }
-    }
-  }
-}
+  // Save new frame to the end of the list.
+  last_frame_id_in_list_ = frame_id;
+  frames_.push_back(packets);
 
-bool PacketStorage::GetPacket32(uint32 frame_id,
-                                uint16 packet_id,
-                                SendPacketVector* packets) {
-  StorageIndex index(frame_id, packet_id);
-  PacketMapIterator it = stored_packets_.find(index);
-  if (it == stored_packets_.end()) {
-    return false;
-  }
-  // Minor trickery, the caller (rtp_sender.cc) really wants a copy of the
-  // packet so that it can update the sequence number before it sends it to
-  // the transport. If the packet only has one ref, we can safely let
-  // rtp_sender.cc have our packet and modify it. If it has more references
-  // then we must return a copy of it instead. This should really only happen
-  // when rtp_sender.cc is trying to re-send a packet that is already in the
-  // queue to sent.
-  if (it->second.second->HasOneRef()) {
-    packets->push_back(it->second);
-  } else {
-    packets->push_back(
-        std::make_pair(it->second.first,
-                       make_scoped_refptr(
-                           new base::RefCountedData<Packet>(
-                               it->second.second->data))));
+  // Evict the oldest frame if the list is too long.
+  if (frames_.size() > max_stored_frames_) {
+    frames_.pop_front();
+    ++first_frame_id_in_list_;
   }
-  return true;
 }
 
-bool PacketStorage::GetPacket(uint8 frame_id_8bit,
-                              uint16 packet_id,
-                              SendPacketVector* packets) {
-  if (stored_packets_.empty()) {
-    return false;
-  }
-  uint32 last_stored = stored_packets_.rbegin()->first.first;
-  uint32 frame_id_32bit = (last_stored & ~0xFF) | frame_id_8bit;
-  if (IsNewerFrameId(frame_id_32bit, last_stored)) {
-    frame_id_32bit -= 0x100;
-  }
-  DCHECK_EQ(frame_id_8bit, frame_id_32bit & 0xff);
-  DCHECK(IsOlderFrameId(frame_id_32bit, last_stored) &&
-         IsNewerFrameId(frame_id_32bit + stored_frames_ + 1, last_stored))
-      << " 32bit: " << frame_id_32bit
-      << " 8bit: " << static_cast<int>(frame_id_8bit)
-      << " last_stored: " << last_stored;
-  return GetPacket32(frame_id_32bit, packet_id, packets);
+const SendPacketVector* PacketStorage::GetFrame8(uint8 frame_id_8bits) const {
+  // The requested frame ID has only 8-bits so convert the first frame ID
+  // in list to match.
+  uint8 index_8bits = first_frame_id_in_list_ & 0xFF;
+  index_8bits = frame_id_8bits - index_8bits;
+  if (index_8bits >= frames_.size())
+    return NULL;
+  return &(frames_[index_8bits]);
 }
 
 }  // namespace transport

media/cast/transport/rtp_sender/packet_storage/packet_storage.h

@@ -5,6 +5,7 @@
 #ifndef MEDIA_CAST_TRANSPORT_RTP_SENDER_PACKET_STORAGE_PACKET_STORAGE_H_
 #define MEDIA_CAST_TRANSPORT_RTP_SENDER_PACKET_STORAGE_PACKET_STORAGE_H_
 
+#include <deque>
 #include <list>
 #include <map>
 #include <vector>
@@ -22,51 +23,34 @@ namespace media {
 namespace cast {
 namespace transport {
 
-class StoredPacket;
-
-// StorageIndex contains {frame_id, packet_id}.
-typedef std::pair<uint32, uint16> StorageIndex;
-typedef std::map<StorageIndex, std::pair<PacketKey, PacketRef> > PacketMap;
-
-// Frame IDs are generally stored as 8-bit values when sent over the
-// wire. This means that having a history longer than 255 frames makes
-// no sense.
-const int kMaxStoredFrames = 255;
+// Stores a list of frames. Each frame consists a list of packets.
+typedef std::deque<SendPacketVector> FrameQueue;
 
 class PacketStorage {
  public:
-  PacketStorage(int stored_frames);
+  explicit PacketStorage(size_t stored_frames);
   virtual ~PacketStorage();
 
   // Returns true if this class is configured correctly.
   // (stored frames > 0 && stored_frames < kMaxStoredFrames)
   bool IsValid() const;
 
-  void StorePacket(uint32 frame_id,
-                   uint16 packet_id,
-                   const PacketKey& key,
-                   PacketRef packet);
+  // Store all of the packets for a frame.
+  void StoreFrame(uint32 frame_id, const SendPacketVector& packets);
 
-  // Copies all missing packets into the packet list.
-  void GetPackets(
-      const MissingFramesAndPacketsMap& missing_frames_and_packets,
-      SendPacketVector* packets_to_resend);
+  // Returns a list of packets for a frame indexed by a 8-bits ID.
+  // It is the lowest 8 bits of a frame ID.
+  // Returns NULL if the frame cannot be found.
+  const SendPacketVector* GetFrame8(uint8 frame_id_8bits) const;
 
-  // Copies packet into the packet list.
-  bool GetPacket(uint8 frame_id_8bit,
-                 uint16 packet_id,
-                 SendPacketVector* packets);
- private:
-  FRIEND_TEST_ALL_PREFIXES(PacketStorageTest, PacketContent);
+  // Get the number of stored frames.
+  size_t GetNumberOfStoredFrames() const;
 
-  // Same as GetPacket, but takes a 32-bit frame id.
-  bool GetPacket32(uint32 frame_id,
-                   uint16 packet_id,
-                   SendPacketVector* packets);
-  void CleanupOldPackets(uint32 current_frame_id);
-
-  PacketMap stored_packets_;
-  int stored_frames_;
+ private:
+  const size_t max_stored_frames_;
+  FrameQueue frames_;
+  uint32 first_frame_id_in_list_;
+  uint32 last_frame_id_in_list_;
 
   DISALLOW_COPY_AND_ASSIGN(PacketStorage);
 };

media/cast/transport/rtp_sender/packet_storage/packet_storage_unittest.cc

@@ -16,47 +16,97 @@ namespace media {
 namespace cast {
 namespace transport {
 
-static int kStoredFrames = 10;
+static size_t kStoredFrames = 10;
 
-class PacketStorageTest : public ::testing::Test {
- protected:
-  PacketStorageTest() : packet_storage_(kStoredFrames) {
+// Generate |number_of_frames| and store into |*storage|.
+// First frame has 1 packet, second frame has 2 packets, etc.
+static void StoreFrames(size_t number_of_frames,
+                        uint32 first_frame_id,
+                        PacketStorage* storage) {
+  const base::TimeTicks kTicks;
+  const int kSsrc = 1;
+  for (size_t i = 0; i < number_of_frames; ++i) {
+    SendPacketVector packets;
+    // First frame has 1 packet, second frame has 2 packets, etc.
+    const size_t kNumberOfPackets = i + 1;
+    for (size_t j = 0; j < kNumberOfPackets; ++j) {
+      Packet test_packet(1, 0);
+      packets.push_back(
+          std::make_pair(
+              PacedPacketSender::MakePacketKey(kTicks, kSsrc, j),
+              new base::RefCountedData<Packet>(test_packet)));
+    }
+    storage->StoreFrame(first_frame_id, packets);
+    ++first_frame_id;
   }
+}
 
-  PacketStorage packet_storage_;
-
-  DISALLOW_COPY_AND_ASSIGN(PacketStorageTest);
-};
-
-TEST_F(PacketStorageTest, PacketContent) {
-  base::TimeTicks frame_tick;
-  for (uint32 frame_id = 0; frame_id < 200; ++frame_id) {
-    for (uint16 packet_id = 0; packet_id < 5; ++packet_id) {
-      Packet test_packet(frame_id + 1, packet_id);
-      packet_storage_.StorePacket(
-          frame_id,
-          packet_id,
-          PacedPacketSender::MakePacketKey(frame_tick,
-                                           1, // ssrc
-                                           packet_id),
-          new base::RefCountedData<Packet>(test_packet));
-    }
+TEST(PacketStorageTest, NumberOfStoredFrames) {
+  PacketStorage storage(kStoredFrames);
 
-    for (uint32 f = 0; f <= frame_id; f++) {
-      for (uint16 packet_id = 0; packet_id < 5; ++packet_id) {
-        SendPacketVector packets;
-        if (packet_storage_.GetPacket32(f, packet_id, &packets)) {
-          EXPECT_GT(f + kStoredFrames, frame_id);
-          EXPECT_EQ(f + 1, packets.back().second->data.size());
-          EXPECT_EQ(packet_id, packets.back().second->data[0]);
-          EXPECT_TRUE(packet_storage_.GetPacket(f & 0xff, packet_id, &packets));
-          EXPECT_TRUE(packets.back().second->data ==
-                      packets.front().second->data);
-        } else {
-          EXPECT_LE(f + kStoredFrames, frame_id);
-        }
-      }
-    }
+  uint32 frame_id = 0;
+  frame_id = ~frame_id; // The maximum value of uint32.
+  StoreFrames(200, frame_id, &storage);
+  EXPECT_EQ(kStoredFrames, storage.GetNumberOfStoredFrames());
+}
+
+TEST(PacketStorageTest, GetFrameWrapAround8bits) {
+  PacketStorage storage(kStoredFrames);
+
+  const uint32 kFirstFrameId = 250;
+  StoreFrames(kStoredFrames, kFirstFrameId, &storage);
+  EXPECT_EQ(kStoredFrames, storage.GetNumberOfStoredFrames());
+
+  // Expect we get the correct frames by looking at the number of
+  // packets.
+  uint32 frame_id = kFirstFrameId;
+  for (size_t i = 0; i < kStoredFrames; ++i) {
+    ASSERT_TRUE(storage.GetFrame8(frame_id));
+    EXPECT_EQ(i + 1, storage.GetFrame8(frame_id)->size());
+    ++frame_id;
+  }
+}
+
+TEST(PacketStorageTest, GetFrameWrapAround32bits) {
+  PacketStorage storage(kStoredFrames);
+
+  // First frame ID is close to the maximum value of uint32.
+  uint32 first_frame_id = 0xffffffff - 5;
+  StoreFrames(kStoredFrames, first_frame_id, &storage);
+  EXPECT_EQ(kStoredFrames, storage.GetNumberOfStoredFrames());
+
+  // Expect we get the correct frames by looking at the number of
+  // packets.
+  uint32 frame_id = first_frame_id;
+  for (size_t i = 0; i < kStoredFrames; ++i) {
+    ASSERT_TRUE(storage.GetFrame8(frame_id));
+    EXPECT_EQ(i + 1, storage.GetFrame8(frame_id)->size());
+    ++frame_id;
+  }
+}
+
+TEST(PacketStorageTest, GetFrameTooOld) {
+  PacketStorage storage(kStoredFrames);
+
+  // First frame ID is close to the maximum value of uint32.
+  uint32 first_frame_id = 0xffffffff - 5;
+
+  // Store two times the capacity.
+  StoreFrames(2 * kStoredFrames, first_frame_id, &storage);
+  EXPECT_EQ(kStoredFrames, storage.GetNumberOfStoredFrames());
+
+  uint32 frame_id = first_frame_id;
+  // Old frames are evicted.
+  for (size_t i = 0; i < kStoredFrames; ++i) {
+    EXPECT_FALSE(storage.GetFrame8(frame_id));
+    ++frame_id;
+  }
+  // Check recent frames are there.
+  for (size_t i = 0; i < kStoredFrames; ++i) {
+    ASSERT_TRUE(storage.GetFrame8(frame_id));
+    EXPECT_EQ(kStoredFrames + i + 1,
+              storage.GetFrame8(frame_id)->size());
+    ++frame_id;
   }
 }
 

media/cast/transport/rtp_sender/rtp_packetizer/rtp_packetizer.cc

@@ -93,23 +93,22 @@ void RtpPacketizer::SendFrameAsPackets(const EncodedFrame& frame) {
     packet->data.insert(packet->data.end(),
                         data_iter,
                         data_iter + payload_length);
-
-    PacketKey key = PacedPacketSender::MakePacketKey(frame.reference_time,
-                                                     config_.ssrc,
-                                                     packet_id_);
-
-    // Store packet.
-    packet_storage_->StorePacket(frame.frame_id, packet_id_, key, packet);
-    ++packet_id_;
     data_iter += payload_length;
 
+    const PacketKey key =
+        PacedPacketSender::MakePacketKey(frame.reference_time,
+                                         config_.ssrc,
+                                         packet_id_++);
+    packets.push_back(make_pair(key, packet));
+
     // Update stats.
     ++send_packet_count_;
     send_octet_count_ += payload_length;
-    packets.push_back(make_pair(key, packet));
   }
   DCHECK(packet_id_ == num_packets) << "Invalid state";
 
+  packet_storage_->StoreFrame(frame.frame_id, packets);
+
   // Send to network.
   transport_->SendPackets(packets);
 

media/cast/transport/rtp_sender/rtp_sender.cc

@@ -13,6 +13,19 @@ namespace media {
 namespace cast {
 namespace transport {
 
+namespace {
+
+// If there is only one referecne to the packet then copy the
+// reference and return.
+// Otherwise return a deep copy of the packet.
+PacketRef FastCopyPacket(const PacketRef& packet) {
+  if (packet->HasOneRef())
+    return packet;
+  return make_scoped_refptr(new base::RefCountedData<Packet>(packet->data));
+}
+
+}  // namespace
+
 RtpSender::RtpSender(
     base::TickClock* clock,
     const scoped_refptr<base::SingleThreadTaskRunner>& transport_task_runner,
@@ -73,29 +86,29 @@ void RtpSender::ResendPackets(
     // Set of packets that the receiver wants us to re-send.
     // If empty, we need to re-send all packets for this frame.
     const PacketIdSet& missing_packet_set = it->second;
-    bool success = false;
-
-    for (uint16 packet_id = 0; ; packet_id++) {
-      // 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);
+    const SendPacketVector* stored_packets = storage_->GetFrame8(frame_id);
+    if (!stored_packets)
+      continue;
 
-      if (!success) break;
+    for (SendPacketVector::const_iterator it = stored_packets->begin();
+         it != stored_packets->end(); ++it) {
+      const PacketKey& packet_key = it->first;
+      const uint16 packet_id = packet_key.second.second;
 
+      // If the resend request doesn't include this packet then cancel
+      // re-transmission already in queue.
       if (!missing_packet_set.empty() &&
           missing_packet_set.find(packet_id) == missing_packet_set.end()) {
-        transport_->CancelSendingPacket(packets_to_resend.back().first);
-        packets_to_resend.pop_back();
+        transport_->CancelSendingPacket(it->first);
       } else {
         // Resend packet to the network.
         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);
+        PacketRef packet_copy = FastCopyPacket(it->second);
+        UpdateSequenceNumber(&packet_copy->data);
+        packets_to_resend.push_back(std::make_pair(packet_key, packet_copy));
       }
     }
     transport_->ResendPackets(packets_to_resend);

media/cast/video_sender/video_sender.cc

@@ -256,9 +256,8 @@ void VideoSender::ResendCheck() {
     if (latest_acked_frame_id_ == last_sent_frame_id_) {
       // Last frame acked, no point in doing anything
     } else {
-      const uint32 kickstart_frame_id = latest_acked_frame_id_ + 1;
-      VLOG(1) << "ACK timeout, re-sending frame " << kickstart_frame_id;
-      ResendFrame(kickstart_frame_id);
+      VLOG(1) << "ACK timeout; last acked frame: " << latest_acked_frame_id_;
+      ResendForKickstart();
     }
   }
   ScheduleNextResendCheck();
@@ -310,13 +309,13 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
     }
     // TODO(miu): The values "2" and "3" should be derived from configuration.
     if (duplicate_ack_counter_ >= 2 && duplicate_ack_counter_ % 3 == 2) {
-      // Resend last ACK + 1 frame.
-      const uint32 frame_to_resend = latest_acked_frame_id_ + 1;
-      VLOG(1) << "Received duplicate ACK for frame " << latest_acked_frame_id_
-              << ", will re-send frame " << frame_to_resend;
-      ResendFrame(frame_to_resend);
+      VLOG(1) << "Received duplicate ACK for frame " << latest_acked_frame_id_;
+      ResendForKickstart();
     }
   } else {
+    // Only count duplicated ACKs if there is no NACK request in between.
+    // This is to avoid aggresive resend.
+    duplicate_ack_counter_ = 0;
     transport_sender_->ResendPackets(
         false, cast_feedback.missing_frames_and_packets_);
     uint32 new_bitrate = 0;
@@ -354,12 +353,19 @@ bool VideoSender::AreTooManyFramesInFlight() const {
   return frames_in_flight >= max_unacked_frames_;
 }
 
-void VideoSender::ResendFrame(uint32 resend_frame_id) {
+void VideoSender::ResendForKickstart() {
   DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
   DCHECK(!last_send_time_.is_null());
+  VLOG(1) << "Resending first packet of frame " << last_sent_frame_id_
+          << " to kick-start.";
+  // Send the first packet of the last encoded frame to kick start
+  // retransmission. This gives enough information to the receiver what
+  // packets and frames are missing.
   MissingFramesAndPacketsMap missing_frames_and_packets;
   PacketIdSet missing;
-  missing_frames_and_packets.insert(std::make_pair(resend_frame_id, missing));
+  missing.insert(0);
+  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);
 }

media/cast/video_sender/video_sender.h

@@ -84,8 +84,8 @@ class VideoSender : public RtcpSenderFeedback,
   void ScheduleNextResendCheck();
   void ResendCheck();
 
-  // Asks |transport_sender_| to resend all the packets for a particular frame.
-  void ResendFrame(uint32 resend_frame_id);
+  // Resend certain packets of an unacked frame to kick start re-transmission.
+  void ResendForKickstart();
 
   // Returns true if there are too many frames in flight, as defined by the
   // configured target playout delay plus simple logic.  When this is true,

media/cast/video_sender/video_sender_unittest.cc

@@ -111,6 +111,7 @@ class VideoSenderTest : public ::testing::Test {
                             task_runner_,
                             task_runner_,
                             task_runner_);
+    last_pixel_value_ = kPixelValue;
     net::IPEndPoint dummy_endpoint;
     transport_sender_.reset(new transport::CastTransportSenderImpl(
         NULL,
@@ -179,7 +180,7 @@ class VideoSenderTest : public ::testing::Test {
     scoped_refptr<media::VideoFrame> video_frame =
         media::VideoFrame::CreateFrame(
             VideoFrame::I420, size, gfx::Rect(size), size, base::TimeDelta());
-    PopulateVideoFrame(video_frame, kPixelValue);
+    PopulateVideoFrame(video_frame, last_pixel_value_++);
     return video_frame;
   }
 
@@ -197,6 +198,7 @@ class VideoSenderTest : public ::testing::Test {
   scoped_refptr<test::FakeSingleThreadTaskRunner> task_runner_;
   scoped_ptr<PeerVideoSender> video_sender_;
   scoped_refptr<CastEnvironment> cast_environment_;
+  int last_pixel_value_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoSenderTest);
 };
@@ -317,28 +319,29 @@ TEST_F(VideoSenderTest, StopSendingInTheAbsenceOfAck) {
   InitEncoder(false);
   // Send a stream of frames and don't ACK; by default we shouldn't have more
   // than 4 frames in flight.
-  // Store size in packets of frame 0, as it should be resent sue to timeout.
   scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
   video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
   RunTasks(33);
-  const int size_of_frame0 = transport_.number_of_rtp_packets();
 
-  for (int i = 1; i < 4; ++i) {
+  // Send 3 more frames and record the number of packets sent.
+  for (int i = 0; i < 3; ++i) {
     scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
     video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
     RunTasks(33);
   }
-
   const int number_of_packets_sent = transport_.number_of_rtp_packets();
-  // Send 4 more frames - they should not be sent to the transport, as we have
-  // received any acks.
+
+  // Send 3 more frames - they should not be encoded, as we have not received
+  // any acks.
   for (int i = 0; i < 3; ++i) {
     scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
     video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
     RunTasks(33);
   }
 
-  EXPECT_EQ(number_of_packets_sent + size_of_frame0,
+  // We expect a frame to be retransmitted because of duplicated ACKs.
+  // Only one packet of the frame is re-transmitted.
+  EXPECT_EQ(number_of_packets_sent + 1,
             transport_.number_of_rtp_packets());
 
   // Start acking and make sure we're back to steady-state.
@@ -358,5 +361,45 @@ TEST_F(VideoSenderTest, StopSendingInTheAbsenceOfAck) {
       transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets());
 }
 
+TEST_F(VideoSenderTest, DuplicateAckRetransmit) {
+  InitEncoder(false);
+  scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
+  video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+  RunTasks(33);
+  RtcpCastMessage cast_feedback(1);
+  cast_feedback.media_ssrc_ = 2;
+  cast_feedback.ack_frame_id_ = 0;
+
+  // Send 3 more frames but don't ACK.
+  for (int i = 0; i < 3; ++i) {
+    scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
+    video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+    RunTasks(33);
+  }
+  const int number_of_packets_sent = transport_.number_of_rtp_packets();
+
+  // Send duplicated ACKs and mix some invalid NACKs.
+  for (int i = 0; i < 10; ++i) {
+    RtcpCastMessage ack_feedback(1);
+    ack_feedback.media_ssrc_ = 2;
+    ack_feedback.ack_frame_id_ = 0;
+    RtcpCastMessage nack_feedback(1);
+    nack_feedback.media_ssrc_ = 2;
+    nack_feedback.missing_frames_and_packets_[255] = PacketIdSet();
+    video_sender_->OnReceivedCastFeedback(ack_feedback);
+    video_sender_->OnReceivedCastFeedback(nack_feedback);
+  }
+  EXPECT_EQ(number_of_packets_sent, transport_.number_of_rtp_packets());
+
+  // Re-transmit one packet because of duplicated ACKs.
+  for (int i = 0; i < 3; ++i) {
+    RtcpCastMessage ack_feedback(1);
+    ack_feedback.media_ssrc_ = 2;
+    ack_feedback.ack_frame_id_ = 0;
+    video_sender_->OnReceivedCastFeedback(ack_feedback);
+  }
+  EXPECT_EQ(number_of_packets_sent + 1, transport_.number_of_rtp_packets());
+}
+
 }  // namespace cast
 }  // namespace media