From 5e9cac984ff737894a04ba6134969cd3be441348 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 13 Dec 2019 10:18:35 +0100 Subject: [PATCH] Don't try to resend packets that were removed out of order. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:11206 Change-Id: Iae05e1db80afd871d37aca203e17bad40dbc9522 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162041 Reviewed-by: Erik Språng Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#30083} --- modules/rtp_rtcp/source/rtp_packet_history.cc | 3 ++- .../rtp_rtcp/source/rtp_packet_history_unittest.cc | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc index f7bb12e7e9..6a2253cd64 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history.cc @@ -456,7 +456,8 @@ int RtpPacketHistory::GetPacketIndex(uint16_t sequence_number) const { RtpPacketHistory::StoredPacket* RtpPacketHistory::GetStoredPacket( uint16_t sequence_number) { int index = GetPacketIndex(sequence_number); - if (index < 0 || static_cast(index) >= packet_history_.size()) { + if (index < 0 || static_cast(index) >= packet_history_.size() || + packet_history_[index].packet_ == nullptr) { return nullptr; } return &packet_history_[index]; diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc index 9e9d6213c9..fdf64d51bf 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc @@ -735,6 +735,20 @@ TEST_F(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) { EXPECT_EQ(padding_packet->SequenceNumber(), kStartSeqNum + 1); } +TEST_F(RtpPacketHistoryTest, NackAfterAckIsNoop) { + hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 2); + // Add two sent packets. + hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), + fake_clock_.TimeInMilliseconds()); + hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), + fake_clock_.TimeInMilliseconds()); + // Remove newest one. + hist_.CullAcknowledgedPackets(std::vector{kStartSeqNum + 1}); + // Retransmission request for already acked packet, should be noop. + auto packet = hist_.GetPacketAndMarkAsPending(kStartSeqNum + 1); + EXPECT_EQ(packet.get(), nullptr); +} + TEST_F(RtpPacketHistoryTest, OutOfOrderInsertRemoval) { hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);