Change how "all frames missing" is encoded.

(Used to be encoded as an empty set, now uses kRtcpCastAllPacketsLost)

Add kRtcpCastLastPacket to let the sender re-transmit the last packet of a frame. (Sender doesn't actually know how many packets there are in a frame.)

Fix a bug in retransmit cancellation.

Cancel retransmits for a frame when it is ACKed.

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

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

8 files changed, 200 insertions, 24 deletions

diff --git a/media/cast/cast_defines.h b/media/cast/cast_defines.h
index bf6df39..64b20c9 100644
--- a/media/cast/cast_defines.h
+++ b/media/cast/cast_defines.h
@@ -68,14 +68,22 @@ enum PacketType {
   kTooOldPacket,
 };
 
+// kRtcpCastAllPacketsLost is used in PacketIDSet and
+// on the wire to mean that ALL packets for a particular
+// frame are lost.
 const uint16 kRtcpCastAllPacketsLost = 0xffff;
 
+// kRtcpCastLastPacket is used in PacketIDSet to ask for
+// the last packet of a frame to be retransmitted.
+const uint16 kRtcpCastLastPacket = 0xfffe;
+
 const size_t kMinLengthOfRtcp = 8;
 
 // Basic RTP header + cast header.
 const size_t kMinLengthOfRtp = 12 + 6;
 
 // Each uint16 represents one packet id within a cast frame.
+// Can also contain kRtcpCastAllPacketsLost and kRtcpCastLastPacket.
 typedef std::set<uint16> PacketIdSet;
 // Each uint8 represents one cast frame.
 typedef std::map<uint8, PacketIdSet> MissingFramesAndPacketsMap;
diff --git a/media/cast/rtcp/rtcp_receiver.cc b/media/cast/rtcp/rtcp_receiver.cc
index 1639573..3be8e92 100644
--- a/media/cast/rtcp/rtcp_receiver.cc
+++ b/media/cast/rtcp/rtcp_receiver.cc
@@ -523,15 +523,15 @@ void RtcpReceiver::HandlePayloadSpecificCastNackItem(
     frame_it = ret.first;
     DCHECK(frame_it != missing_frames_and_packets->end()) << "Invalid state";
   }
-  if (rtcp_field->cast_nack_item.packet_id == kRtcpCastAllPacketsLost) {
+  uint16 packet_id = rtcp_field->cast_nack_item.packet_id;
+  frame_it->second.insert(packet_id);
+
+  if (packet_id == kRtcpCastAllPacketsLost) {
     // Special case all packets in a frame is missing.
     return;
   }
-  uint16 packet_id = rtcp_field->cast_nack_item.packet_id;
   uint8 bitmask = rtcp_field->cast_nack_item.bitmask;
 
-  frame_it->second.insert(packet_id);
-
   if (bitmask) {
     for (int i = 1; i <= 8; ++i) {
       if (bitmask & 1) {
diff --git a/media/cast/rtcp/rtcp_receiver_unittest.cc b/media/cast/rtcp/rtcp_receiver_unittest.cc
index f898939..51026d1 100644
--- a/media/cast/rtcp/rtcp_receiver_unittest.cc
+++ b/media/cast/rtcp/rtcp_receiver_unittest.cc
@@ -40,7 +40,8 @@ class SenderFeedbackCastVerification : public RtcpSenderFeedback {
 
     EXPECT_TRUE(frame_it != cast_feedback.missing_frames_and_packets_.end());
     EXPECT_EQ(kLostFrameId, frame_it->first);
-    EXPECT_TRUE(frame_it->second.empty());
+    EXPECT_EQ(frame_it->second.size(), 1UL);
+    EXPECT_EQ(*frame_it->second.begin(), kRtcpCastAllPacketsLost);
     ++frame_it;
     EXPECT_TRUE(frame_it != cast_feedback.missing_frames_and_packets_.end());
     EXPECT_EQ(kFrameIdWithLostPackets, frame_it->first);
diff --git a/media/cast/test/utility/video_utility.cc b/media/cast/test/utility/video_utility.cc
index 81475d8..b94c99c 100644
--- a/media/cast/test/utility/video_utility.cc
+++ b/media/cast/test/utility/video_utility.cc
@@ -7,6 +7,7 @@
 #include <math.h>
 #include <cstdio>
 
+#include "base/rand_util.h"
 #include "third_party/libyuv/include/libyuv/compare.h"
 #include "ui/gfx/size.h"
 
@@ -46,25 +47,43 @@ void PopulateVideoFrame(VideoFrame* frame, int start_value) {
   uint8* v_plane = frame->data(VideoFrame::kVPlane);
 
   // Set Y.
-  for (int j = 0; j < height; ++j)
+  for (int j = 0; j < height; ++j) {
     for (int i = 0; i < stride_y; ++i) {
       *y_plane = static_cast<uint8>(start_value + i + j);
       ++y_plane;
     }
+  }
 
   // Set U.
-  for (int j = 0; j < half_height; ++j)
+  for (int j = 0; j < half_height; ++j) {
     for (int i = 0; i < stride_u; ++i) {
       *u_plane = static_cast<uint8>(start_value + i + j);
       ++u_plane;
     }
+  }
 
   // Set V.
-  for (int j = 0; j < half_height; ++j)
+  for (int j = 0; j < half_height; ++j) {
     for (int i = 0; i < stride_v; ++i) {
       *v_plane = static_cast<uint8>(start_value + i + j);
       ++v_plane;
     }
+  }
+}
+
+void PopulateVideoFrameWithNoise(VideoFrame* frame) {
+  int height = frame->coded_size().height();
+  int stride_y = frame->stride(VideoFrame::kYPlane);
+  int stride_u = frame->stride(VideoFrame::kUPlane);
+  int stride_v = frame->stride(VideoFrame::kVPlane);
+  int half_height = (height + 1) / 2;
+  uint8* y_plane = frame->data(VideoFrame::kYPlane);
+  uint8* u_plane = frame->data(VideoFrame::kUPlane);
+  uint8* v_plane = frame->data(VideoFrame::kVPlane);
+
+  base::RandBytes(y_plane, height * stride_y);
+  base::RandBytes(u_plane, half_height * stride_u);
+  base::RandBytes(v_plane, half_height * stride_v);
 }
 
 bool PopulateVideoFrameFromFile(VideoFrame* frame, FILE* video_file) {
diff --git a/media/cast/test/utility/video_utility.h b/media/cast/test/utility/video_utility.h
index c853943..bbb9865 100644
--- a/media/cast/test/utility/video_utility.h
+++ b/media/cast/test/utility/video_utility.h
@@ -18,6 +18,9 @@ double I420PSNR(const scoped_refptr<media::VideoFrame>& frame1,
 // Memory is allocated within the function.
 void PopulateVideoFrame(VideoFrame* frame, int start_value);
 
+// Populate a video frame with noise.
+void PopulateVideoFrameWithNoise(VideoFrame* frame);
+
 // Populate a video frame from a file.
 // Returns true if frame was populated, false if not (EOF).
 bool PopulateVideoFrameFromFile(VideoFrame* frame, FILE* video_file);
diff --git a/media/cast/transport/rtp_sender/rtp_sender.cc b/media/cast/transport/rtp_sender/rtp_sender.cc
index 359ccf3..2604d25 100644
--- a/media/cast/transport/rtp_sender/rtp_sender.cc
+++ b/media/cast/transport/rtp_sender/rtp_sender.cc
@@ -88,6 +88,11 @@ void RtpSender::ResendPackets(
     // If empty, we need to re-send all packets for this frame.
     const PacketIdSet& missing_packet_set = it->second;
 
+    bool resend_all = missing_packet_set.find(kRtcpCastAllPacketsLost) !=
+        missing_packet_set.end();
+    bool resend_last = missing_packet_set.find(kRtcpCastLastPacket) !=
+        missing_packet_set.end();
+
     const SendPacketVector* stored_packets = storage_->GetFrame8(frame_id);
     if (!stored_packets)
       continue;
@@ -97,13 +102,22 @@ void RtpSender::ResendPackets(
       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 (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 {
+      // Should we resend the packet?
+      bool resend = resend_all;
+
+      // Should we resend it because it's in the missing_packet_set?
+      if (!resend &&
+          missing_packet_set.find(packet_id) != missing_packet_set.end()) {
+        resend = true;
+      }
+
+      // If we were asked to resend the last packet, check if it's the
+      // last packet.
+      if (!resend && resend_last && (it + 1) == stored_packets->end()) {
+        resend = true;
+      }
+
+      if (resend) {
         // Resend packet to the network.
         VLOG(3) << "Resend " << static_cast<int>(frame_id) << ":"
                 << packet_id;
@@ -111,6 +125,8 @@ void RtpSender::ResendPackets(
         PacketRef packet_copy = FastCopyPacket(it->second);
         UpdateSequenceNumber(&packet_copy->data);
         packets_to_resend.push_back(std::make_pair(packet_key, packet_copy));
+      } else if (cancel_rtx_if_not_in_list) {
+        transport_->CancelSendingPacket(it->first);
       }
     }
     transport_->ResendPackets(packets_to_resend);
diff --git a/media/cast/video_sender/video_sender.cc b/media/cast/video_sender/video_sender.cc
index f5bc3cd..cc8b158 100644
--- a/media/cast/video_sender/video_sender.cc
+++ b/media/cast/video_sender/video_sender.cc
@@ -340,8 +340,17 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
                              latest_acked_frame_id_) < 0;
   VLOG(2) << "Received ACK" << (is_acked_out_of_order ? " out-of-order" : "")
           << " for frame " << cast_feedback.ack_frame_id_;
-  if (!is_acked_out_of_order)
+  if (!is_acked_out_of_order) {
+    // Cancel resends of acked frames.
+    MissingFramesAndPacketsMap missing_frames_and_packets;
+    PacketIdSet missing;
+    while (latest_acked_frame_id_ != cast_feedback.ack_frame_id_) {
+      latest_acked_frame_id_++;
+      missing_frames_and_packets[latest_acked_frame_id_] = missing;
+    }
+    transport_sender_->ResendPackets(false, missing_frames_and_packets, true);
     latest_acked_frame_id_ = cast_feedback.ack_frame_id_;
+  }
 }
 
 bool VideoSender::AreTooManyFramesInFlight() const {
@@ -361,14 +370,14 @@ bool VideoSender::AreTooManyFramesInFlight() const {
 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_
+  VLOG(1) << "Resending last 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.insert(0);
+  missing.insert(kRtcpCastLastPacket);
   missing_frames_and_packets.insert(
       std::make_pair(last_sent_frame_id_, missing));
   last_send_time_ = cast_environment_->Clock()->NowTicks();
diff --git a/media/cast/video_sender/video_sender_unittest.cc b/media/cast/video_sender/video_sender_unittest.cc
index e3e8901..49fae46 100644
--- a/media/cast/video_sender/video_sender_unittest.cc
+++ b/media/cast/video_sender/video_sender_unittest.cc
@@ -53,11 +53,19 @@ void CreateSharedMemory(
 
 class TestPacketSender : public transport::PacketSender {
  public:
-  TestPacketSender() : number_of_rtp_packets_(0), number_of_rtcp_packets_(0) {}
+  TestPacketSender()
+      : number_of_rtp_packets_(0),
+        number_of_rtcp_packets_(0),
+        paused_(false) {}
 
   // A singular packet implies a RTCP packet.
   virtual bool SendPacket(transport::PacketRef packet,
                           const base::Closure& cb) OVERRIDE {
+    if (paused_) {
+      stored_packet_ = packet;
+      callback_ = cb;
+      return false;
+    }
     if (Rtcp::IsRtcpPacket(&packet->data[0], packet->data.size())) {
       ++number_of_rtcp_packets_;
     } else {
@@ -76,9 +84,20 @@ class TestPacketSender : public transport::PacketSender {
 
   int number_of_rtcp_packets() const { return number_of_rtcp_packets_; }
 
+  void SetPause(bool paused) {
+    paused_ = paused;
+    if (!paused && stored_packet_) {
+      SendPacket(stored_packet_, callback_);
+      callback_.Run();
+    }
+  }
+
  private:
   int number_of_rtp_packets_;
   int number_of_rtcp_packets_;
+  bool paused_;
+  base::Closure callback_;
+  transport::PacketRef stored_packet_;
 
   DISALLOW_COPY_AND_ASSIGN(TestPacketSender);
 };
@@ -184,12 +203,17 @@ class VideoSenderTest : public ::testing::Test {
     return video_frame;
   }
 
+  scoped_refptr<media::VideoFrame> GetLargeNewVideoFrame() {
+    gfx::Size size(kWidth, kHeight);
+    scoped_refptr<media::VideoFrame> video_frame =
+        media::VideoFrame::CreateFrame(
+            VideoFrame::I420, size, gfx::Rect(size), size, base::TimeDelta());
+    PopulateVideoFrameWithNoise(video_frame);
+    return video_frame;
+  }
+
   void RunTasks(int during_ms) {
-    for (int i = 0; i < during_ms; ++i) {
-      // Call process the timers every 1 ms.
-      testing_clock_->Advance(base::TimeDelta::FromMilliseconds(1));
-      task_runner_->RunTasks();
-    }
+    task_runner_->Sleep(base::TimeDelta::FromMilliseconds(during_ms));
   }
 
   base::SimpleTestTickClock* testing_clock_;  // Owned by CastEnvironment.
@@ -401,5 +425,101 @@ TEST_F(VideoSenderTest, DuplicateAckRetransmit) {
   EXPECT_EQ(number_of_packets_sent + 1, transport_.number_of_rtp_packets());
 }
 
+TEST_F(VideoSenderTest, DuplicateAckRetransmitDoesNotCancelRetransmits) {
+  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 2 more frames but don't ACK.
+  for (int i = 0; i < 2; ++i) {
+    scoped_refptr<media::VideoFrame> video_frame = GetNewVideoFrame();
+    video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+    RunTasks(33);
+  }
+  // Pause the transport
+  transport_.SetPause(true);
+
+  // Insert one more video frame.
+  video_frame = GetLargeNewVideoFrame();
+  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);
+  }
+
+  transport_.SetPause(false);
+  RunTasks(100);
+  EXPECT_LT(number_of_packets_sent + 1, transport_.number_of_rtp_packets());
+}
+
+TEST_F(VideoSenderTest, AcksCancelRetransmits) {
+  InitEncoder(false);
+  transport_.SetPause(true);
+  scoped_refptr<media::VideoFrame> video_frame = GetLargeNewVideoFrame();
+  video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+  RunTasks(33);
+
+  // Frame should be in buffer, waiting. Now let's ack it.
+  RtcpCastMessage cast_feedback(1);
+  cast_feedback.media_ssrc_ = 2;
+  cast_feedback.ack_frame_id_ = 0;
+  video_sender_->OnReceivedCastFeedback(cast_feedback);
+
+  transport_.SetPause(false);
+  RunTasks(33);
+  EXPECT_EQ(0, transport_.number_of_rtp_packets());
+}
+
+TEST_F(VideoSenderTest, NAcksCancelRetransmits) {
+  InitEncoder(false);
+  transport_.SetPause(true);
+  // Send two video frames.
+  scoped_refptr<media::VideoFrame> video_frame = GetLargeNewVideoFrame();
+  video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+  RunTasks(33);
+  video_frame = GetLargeNewVideoFrame();
+  video_sender_->InsertRawVideoFrame(video_frame, testing_clock_->NowTicks());
+  RunTasks(33);
+
+  // Frames should be in buffer, waiting. Now let's ack the first one and nack
+  // one packet in the second one.
+  RtcpCastMessage cast_feedback(1);
+  cast_feedback.media_ssrc_ = 2;
+  cast_feedback.ack_frame_id_ = 0;
+  PacketIdSet missing_packets;
+  missing_packets.insert(0);
+  cast_feedback.missing_frames_and_packets_[1] = missing_packets;
+  video_sender_->OnReceivedCastFeedback(cast_feedback);
+
+  transport_.SetPause(false);
+  RunTasks(33);
+  // Only one packet should be retransmitted.
+  EXPECT_EQ(1, transport_.number_of_rtp_packets());
+}
+
 }  // namespace cast
 }  // namespace media