Don't overwrite RTP packets in history within one second or 3x RTT.

This prevents us from prematurely overwriting the packets in the history
if the RTT is underestimated.

Bug: webrtc:8766
Change-Id: I042e8ce74cdce2a0451596f4217779fc856b51f4
Reviewed-on: https://webrtc-review.googlesource.com/42960
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21735}
This commit is contained in:
Erik Språng 2018-01-23 08:10:22 -08:00 committed by Commit Bot
parent b8e1201020
commit 788ac70c1f
2 changed files with 37 additions and 5 deletions

View File

@ -22,6 +22,10 @@
namespace webrtc {
namespace {
constexpr size_t kMinPacketRequestBytes = 50;
// Don't overwrite a packet within one second, or three RTTs, after transmission
// whichever is larger. Instead try to dynamically expand history.
constexpr int64_t kMinPacketDurationMs = 1000;
constexpr int kMinPacketDurationRtt = 3;
} // namespace
constexpr size_t RtpPacketHistory::kMaxCapacity;
@ -85,9 +89,12 @@ void RtpPacketHistory::PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
// less than 3 round trip times ago, expand the buffer to avoid overwriting
// valid data.
StoredPacket* stored_packet = &stored_packets_[prev_index_];
int64_t packet_duration_ms =
std::max(kMinPacketDurationRtt * rtt_ms_, kMinPacketDurationMs);
if (stored_packet->packet &&
(stored_packet->send_time == 0 ||
(rtt_ms_ >= 0 && now_ms - stored_packet->send_time <= 3 * rtt_ms_))) {
(rtt_ms_ >= 0 &&
now_ms - stored_packet->send_time <= packet_duration_ms))) {
size_t current_size = stored_packets_.size();
if (current_size < kMaxCapacity) {
size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);

View File

@ -222,7 +222,7 @@ TEST_F(RtpPacketHistoryTest, FullExpansion) {
TEST_F(RtpPacketHistoryTest, DontExpandIfPacketIsOldEnough) {
const size_t kSendSidePacketHistorySize = 600;
const int64_t kRttMs = 100;
const int64_t kRttMs = 334;
hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
hist_.SetRtt(kRttMs);
@ -241,12 +241,12 @@ TEST_F(RtpPacketHistoryTest, DontExpandIfPacketIsOldEnough) {
std::unique_ptr<RtpPacketToSend> packet =
CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
EXPECT_FALSE(hist_.GetPacketAndSetSendTime(kSeqNum, kRttMs, true));
EXPECT_FALSE(hist_.HasRtpPacket(kSeqNum));
}
TEST_F(RtpPacketHistoryTest, ExpandIfPacketTooRecentlyTransmitted) {
const size_t kSendSidePacketHistorySize = 600;
const int64_t kRttMs = 100;
const int64_t kRttMs = 334;
hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
hist_.SetRtt(kRttMs);
@ -265,6 +265,31 @@ TEST_F(RtpPacketHistoryTest, ExpandIfPacketTooRecentlyTransmitted) {
std::unique_ptr<RtpPacketToSend> packet =
CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum, kRttMs, true));
EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
}
TEST_F(RtpPacketHistoryTest, ExpandIfPacketTooRecentlyTransmittedOnFastLink) {
const size_t kSendSidePacketHistorySize = 600;
const int64_t kRttMs = 5;
hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
hist_.SetRtt(kRttMs);
// Fill up the buffer with packets.
for (size_t i = 0; i < kSendSidePacketHistorySize; ++i) {
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, kRttMs, false));
}
// Move clock forward after expiration time based on RTT, but before
// expiration time based on absolute time.
fake_clock_.AdvanceTimeMilliseconds(999);
// Insert a new packet and verify that the old one for this index still
// exists - ie the buffer has been expanded.
std::unique_ptr<RtpPacketToSend> packet =
CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
}
} // namespace webrtc