Fixed potential crash if rtp packet history is completely full.

Also performance enhanecement in rtp_sender (don't lookup if kDontStore)

BUG=4171
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/39759004

Cr-Commit-Position: refs/heads/master@{#8226}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8226 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/rtp_rtcp/source/rtp_packet_history.cc

@@ -23,7 +23,6 @@
 namespace webrtc {
 
 static const int kMinPacketRequestBytes = 50;
-static const size_t kMaxSize = 9600;  // "Should be enough for anyone."
 
 RTPPacketHistory::RTPPacketHistory(Clock* clock)
   : clock_(clock),
@@ -53,7 +52,7 @@ void RTPPacketHistory::SetStorePacketsStatus(bool enable,
 
 void RTPPacketHistory::Allocate(size_t number_to_store) {
   assert(number_to_store > 0);
-  assert(number_to_store <= kMaxSize);
+  assert(number_to_store <= kMaxHistoryCapacity);
   store_ = true;
   stored_packets_.resize(number_to_store);
   stored_seq_nums_.resize(number_to_store);
@@ -145,13 +144,15 @@ int32_t RTPPacketHistory::PutRTPPacket(const uint8_t* packet,
   if (stored_lengths_[prev_index_] > 0 &&
       stored_send_times_[prev_index_] == 0) {
     size_t current_size = static_cast<uint16_t>(stored_packets_.size());
-    size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);
+    if (current_size < kMaxHistoryCapacity) {
-    expanded_size = std::min(expanded_size, kMaxSize);
+      size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);
-    Allocate(expanded_size);
+      expanded_size = std::min(expanded_size, kMaxHistoryCapacity);
-    VerifyAndAllocatePacketLength(max_packet_length, current_size);
+      Allocate(expanded_size);
-    // Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
+      VerifyAndAllocatePacketLength(max_packet_length, current_size);
-    // but may be slower - at least until buffer has wrapped around once.
+      // Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
-    prev_index_ = current_size;
+      // but may be slower - at least until buffer has wrapped around once.
+      prev_index_ = current_size;
+    }
   }
 
   // Store packet

webrtc/modules/rtp_rtcp/source/rtp_packet_history.h

@@ -25,6 +25,8 @@ namespace webrtc {
 class Clock;
 class CriticalSectionWrapper;
 
+static const size_t kMaxHistoryCapacity = 9600;
+
 class RTPPacketHistory {
  public:
   RTPPacketHistory(Clock* clock);

webrtc/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc

@@ -15,6 +15,7 @@
 #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_packet_history.h"
 #include "webrtc/system_wrappers/interface/clock.h"
+#include "webrtc/video_engine/vie_defines.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
@@ -248,4 +249,26 @@ TEST_F(RtpPacketHistoryTest, DynamicExpansion) {
   }
 }
 
+TEST_F(RtpPacketHistoryTest, FullExpansion) {
+  hist_->SetStorePacketsStatus(true, kSendSidePacketHistorySize);
+  size_t len;
+  int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
+  int64_t time;
+  for (size_t i = 0; i < kMaxHistoryCapacity + 1; ++i) {
+    len = 0;
+    CreateRtpPacket(kSeqNum + i, kSsrc, kPayload, kTimestamp, packet_, &len);
+    EXPECT_EQ(0, hist_->PutRTPPacket(packet_, len, kMaxPacketLength,
+                                     capture_time_ms, kAllowRetransmission));
+  }
+
+  fake_clock_.AdvanceTimeMilliseconds(100);
+
+  // Retransmit all packets currently in buffer.
+  for (size_t i = 1; i < kMaxHistoryCapacity + 1; ++i) {
+    len = kMaxPacketLength;
+    EXPECT_TRUE(hist_->GetPacketAndSetSendTime(kSeqNum + i, 100, false, packet_,
+                                               &len, &time));
+  }
+}
+
 }  // namespace webrtc

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

@@ -1003,13 +1003,21 @@ int32_t RTPSender::SendToNetwork(
   }
 
   size_t length = payload_length + rtp_header_length;
-  if (!SendPacketToNetwork(buffer, length))
+  bool sent = SendPacketToNetwork(buffer, length);
+
+  if (storage != kDontStore) {
+    // Mark the packet as sent in the history even if send failed. Dropping a
+    // packet here should be treated as any other packet drop so we should be
+    // ready for a retransmission.
+    packet_history_.SetSent(rtp_header.sequenceNumber);
+  }
+  if (!sent)
     return -1;
+
   {
     CriticalSectionScoped lock(send_critsect_);
     media_has_been_sent_ = true;
   }
-  packet_history_.SetSent(rtp_header.sequenceNumber);
   UpdateRtpStats(buffer, length, rtp_header, false, false);
   return 0;
 }