From 1b3f4f9b45b16adad7817702f3d5993ca2bbe0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Tue, 18 Jun 2019 10:46:09 +0200 Subject: [PATCH] Allow RtpPacketHistory encapsulator function to abort retransmit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10633 Change-Id: I162b2c2f778e8e4c6f31307028db0c352ded2276 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/142230 Reviewed-by: Danil Chapovalov Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#28312} --- modules/rtp_rtcp/source/rtp_packet_history.cc | 10 +++++++--- modules/rtp_rtcp/source/rtp_packet_history.h | 2 ++ .../source/rtp_packet_history_unittest.cc | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc index 53361811f8..e8488e0fb6 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history.cc @@ -241,10 +241,14 @@ std::unique_ptr RtpPacketHistory::GetPacketAndMarkAsPending( return nullptr; } - packet.pending_transmission_ = true; - // Copy and/or encapsulate packet. - return encapsulate(*packet.packet_); + std::unique_ptr encapsulated_packet = + encapsulate(*packet.packet_); + if (encapsulated_packet) { + packet.pending_transmission_ = true; + } + + return encapsulated_packet; } void RtpPacketHistory::MarkPacketAsSent(uint16_t sequence_number) { diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h index 44ebf4325f..b988b68068 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.h +++ b/modules/rtp_rtcp/source/rtp_packet_history.h @@ -93,6 +93,8 @@ class RtpPacketHistory { // In addition to getting packet and marking as sent, this method takes an // encapsulator function that takes a reference to the packet and outputs a // copy that may be wrapped in a container, eg RTX. + // If the the encapsulator returns nullptr, the retransmit is aborted and the + // packet will not be marked as pending. std::unique_ptr GetPacketAndMarkAsPending( uint16_t sequence_number, rtc::FunctionView( diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc index 6a577fa185..950cbe6a14 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc @@ -734,6 +734,23 @@ TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulation) { EXPECT_EQ(retransmit_packet->Ssrc(), kSsrc + 1); } +TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulationAbortOnNullptr) { + hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); + + hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission, + fake_clock_.TimeInMicroseconds()); + + // Retransmission request, but the encapsulator determines that this packet is + // not suitable for retransmission (bandwidth exhausted?) so the retransmit is + // aborted and the packet is not marked as pending. + EXPECT_FALSE(hist_.GetPacketAndMarkAsPending( + kStartSeqNum, [](const RtpPacketToSend& packet) { return nullptr; })); + + // New try, this time getting the packet should work, and it should not be + // blocked due to any pending status. + EXPECT_TRUE(hist_.GetPacketAndMarkAsPending(kStartSeqNum)); +} + TEST_F(RtpPacketHistoryTest, DontRemovePendingTransmissions) { const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kPacketTimeoutMs =