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
git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@8226 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc
index eeb1fc4..8fb1835 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.cc
+++ b/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::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 @@
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);
- expanded_size = std::min(expanded_size, kMaxSize);
- Allocate(expanded_size);
- VerifyAndAllocatePacketLength(max_packet_length, current_size);
- // Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
- // but may be slower - at least until buffer has wrapped around once.
- prev_index_ = current_size;
+ if (current_size < kMaxHistoryCapacity) {
+ size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);
+ expanded_size = std::min(expanded_size, kMaxHistoryCapacity);
+ Allocate(expanded_size);
+ VerifyAndAllocatePacketLength(max_packet_length, current_size);
+ // Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
+ // but may be slower - at least until buffer has wrapped around once.
+ prev_index_ = current_size;
+ }
}
// Store packet
diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h
index b88d9d3..b6da99a 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.h
+++ b/modules/rtp_rtcp/source/rtp_packet_history.h
@@ -25,6 +25,8 @@
class Clock;
class CriticalSectionWrapper;
+static const size_t kMaxHistoryCapacity = 9600;
+
class RTPPacketHistory {
public:
RTPPacketHistory(Clock* clock);
diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
index 2d9d306..fe33b01 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
+++ b/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, 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
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 6ea11c2..71077c5 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -1003,13 +1003,21 @@
}
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;
}
