admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add ability to disable padding prioritization.

Browse Source 
This allows trading off some potential media quality for CPU usage.

Bug: webrtc:8975
Change-Id: I447a03f596e9e711ba5d7038fe71f27bd80bf795
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172085
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30936}
This commit is contained in:
Erik Språng 2020-03-30 10:01:29 +02:00 committed by Commit Bot
parent e9286d7273
commit 641d59b337
6 changed files with 141 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
modules/rtp_rtcp/include/rtp_rtcp.h
View File

@ -149,6 +149,13 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
bool need_rtp_packet_infos = false; bool need_rtp_packet_infos = false;
// If true, the RTP packet history will select RTX packets based on
// heuristics such as send time, retransmission count etc, in order to
// make padding potentially more useful.
// If false, the last packet will always be picked. This may reduce CPU
// overhead.
bool enable_rtx_padding_prioritization = false;
private: private:
RTC_DISALLOW_COPY_AND_ASSIGN(Configuration); RTC_DISALLOW_COPY_AND_ASSIGN(Configuration);
}; };

38
modules/rtp_rtcp/source/rtp_packet_history.cc
View File

@ -56,7 +56,7 @@ void RtpPacketHistory::StoredPacket::IncrementTimesRetransmitted(
// Check if this StoredPacket is in the priority set. If so, we need to remove // Check if this StoredPacket is in the priority set. If so, we need to remove
// it before updating |times_retransmitted_| since that is used in sorting, // it before updating |times_retransmitted_| since that is used in sorting,
// and then add it back. // and then add it back.
const bool in_priority_set = priority_set->erase(this) > 0; const bool in_priority_set = priority_set && priority_set->erase(this) > 0;
++times_retransmitted_; ++times_retransmitted_;
if (in_priority_set) { if (in_priority_set) {
auto it = priority_set->insert(this); auto it = priority_set->insert(this);
@ -80,8 +80,9 @@ bool RtpPacketHistory::MoreUseful::operator()(StoredPacket* lhs,
return lhs->insert_order() > rhs->insert_order(); return lhs->insert_order() > rhs->insert_order();
} }
RtpPacketHistory::RtpPacketHistory(Clock* clock) RtpPacketHistory::RtpPacketHistory(Clock* clock, bool enable_padding_prio)
: clock_(clock), : clock_(clock),
enable_padding_prio_(enable_padding_prio),
number_to_store_(0), number_to_store_(0),
mode_(StorageMode::kDisabled), mode_(StorageMode::kDisabled),
rtt_ms_(-1), rtt_ms_(-1),
@ -158,11 +159,13 @@ void RtpPacketHistory::PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
packet_history_[packet_index] = packet_history_[packet_index] =
StoredPacket(std::move(packet), send_time_ms, packets_inserted_++); StoredPacket(std::move(packet), send_time_ms, packets_inserted_++);
if (enable_padding_prio_) {
if (padding_priority_.size() >= kMaxPaddingtHistory - 1) { if (padding_priority_.size() >= kMaxPaddingtHistory - 1) {
padding_priority_.erase(std::prev(padding_priority_.end())); padding_priority_.erase(std::prev(padding_priority_.end()));
} }
auto prio_it = padding_priority_.insert(&packet_history_[packet_index]); auto prio_it = padding_priority_.insert(&packet_history_[packet_index]);
RTC_DCHECK(prio_it.second) << "Failed to insert packet into prio set."; RTC_DCHECK(prio_it.second) << "Failed to insert packet into prio set.";
}
} }
std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPacketAndSetSendTime( std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPacketAndSetSendTime(
@ -183,7 +186,8 @@ std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPacketAndSetSendTime(
} }
if (packet->send_time_ms_) { if (packet->send_time_ms_) {
packet->IncrementTimesRetransmitted(&padding_priority_); packet->IncrementTimesRetransmitted(
enable_padding_prio_ ? &padding_priority_ : nullptr);
} }
// Update send-time and mark as no long in pacer queue. // Update send-time and mark as no long in pacer queue.
@ -253,7 +257,8 @@ void RtpPacketHistory::MarkPacketAsSent(uint16_t sequence_number) {
// transmission count. // transmission count.
packet->send_time_ms_ = clock_->TimeInMilliseconds(); packet->send_time_ms_ = clock_->TimeInMilliseconds();
packet->pending_transmission_ = false; packet->pending_transmission_ = false;
packet->IncrementTimesRetransmitted(&padding_priority_); packet->IncrementTimesRetransmitted(enable_padding_prio_ ? &padding_priority_
: nullptr);
} }
absl::optional<RtpPacketHistory::PacketState> RtpPacketHistory::GetPacketState( absl::optional<RtpPacketHistory::PacketState> RtpPacketHistory::GetPacketState(
@ -307,12 +312,28 @@ std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPayloadPaddingPacket(
rtc::FunctionView<std::unique_ptr<RtpPacketToSend>(const RtpPacketToSend&)> rtc::FunctionView<std::unique_ptr<RtpPacketToSend>(const RtpPacketToSend&)>
encapsulate) { encapsulate) {
rtc::CritScope cs(&lock_); rtc::CritScope cs(&lock_);
if (mode_ == StorageMode::kDisabled || padding_priority_.empty()) { if (mode_ == StorageMode::kDisabled) {
return nullptr; return nullptr;
} }
StoredPacket* best_packet = nullptr;
if (enable_padding_prio_ && !padding_priority_.empty()) {
auto best_packet_it = padding_priority_.begin(); auto best_packet_it = padding_priority_.begin();
StoredPacket* best_packet = *best_packet_it; best_packet = *best_packet_it;
} else if (!enable_padding_prio_ && !packet_history_.empty()) {
// Prioritization not available, pick the last packet.
for (auto it = packet_history_.rbegin(); it != packet_history_.rend();
++it) {
if (it->packet_ != nullptr) {
best_packet = &(*it);
break;
}
}
}
if (best_packet == nullptr) {
return nullptr;
}
if (best_packet->pending_transmission_) { if (best_packet->pending_transmission_) {
// Because PacedSender releases it's lock when it calls // Because PacedSender releases it's lock when it calls
// GeneratePadding() there is the potential for a race where a new // GeneratePadding() there is the potential for a race where a new
@ -328,7 +349,8 @@ std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPayloadPaddingPacket(
} }
best_packet->send_time_ms_ = clock_->TimeInMilliseconds(); best_packet->send_time_ms_ = clock_->TimeInMilliseconds();
best_packet->IncrementTimesRetransmitted(&padding_priority_); best_packet->IncrementTimesRetransmitted(
enable_padding_prio_ ? &padding_priority_ : nullptr);
return padding_packet; return padding_packet;
} }
@ -414,7 +436,9 @@ std::unique_ptr<RtpPacketToSend> RtpPacketHistory::RemovePacket(
std::move(packet_history_[packet_index].packet_); std::move(packet_history_[packet_index].packet_);
// Erase from padding priority set, if eligible. // Erase from padding priority set, if eligible.
if (enable_padding_prio_) {
padding_priority_.erase(&packet_history_[packet_index]); padding_priority_.erase(&packet_history_[packet_index]);
}
if (packet_index == 0) { if (packet_index == 0) {
while (!packet_history_.empty() && while (!packet_history_.empty() &&

3
modules/rtp_rtcp/source/rtp_packet_history.h
View File

@ -62,7 +62,7 @@ class RtpPacketHistory {
// With kStoreAndCull, always remove packets after 3x max(1000ms, 3x rtt). // With kStoreAndCull, always remove packets after 3x max(1000ms, 3x rtt).
static constexpr int kPacketCullingDelayFactor = 3; static constexpr int kPacketCullingDelayFactor = 3;
explicit RtpPacketHistory(Clock* clock); RtpPacketHistory(Clock* clock, bool enable_padding_prio);
~RtpPacketHistory(); ~RtpPacketHistory();
// Set/get storage mode. Note that setting the state will clear the history, // Set/get storage mode. Note that setting the state will clear the history,
@ -192,6 +192,7 @@ class RtpPacketHistory {
const StoredPacket& stored_packet); const StoredPacket& stored_packet);
Clock* const clock_; Clock* const clock_;
const bool enable_padding_prio_;
rtc::CriticalSection lock_; rtc::CriticalSection lock_;
size_t number_to_store_ RTC_GUARDED_BY(lock_); size_t number_to_store_ RTC_GUARDED_BY(lock_);
StorageMode mode_ RTC_GUARDED_BY(lock_); StorageMode mode_ RTC_GUARDED_BY(lock_);

128
modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
View File

@ -32,9 +32,11 @@ uint16_t To16u(size_t sequence_number) {
using StorageMode = RtpPacketHistory::StorageMode; using StorageMode = RtpPacketHistory::StorageMode;
class RtpPacketHistoryTest : public ::testing::Test { class RtpPacketHistoryTest : public ::testing::TestWithParam<bool> {
protected: protected:
RtpPacketHistoryTest() : fake_clock_(123456), hist_(&fake_clock_) {} RtpPacketHistoryTest()
: fake_clock_(123456),
hist_(&fake_clock_, /*enable_padding_prio=*/GetParam()) {}
SimulatedClock fake_clock_; SimulatedClock fake_clock_;
RtpPacketHistory hist_; RtpPacketHistory hist_;
@ -49,7 +51,7 @@ class RtpPacketHistoryTest : public ::testing::Test {
} }
}; };
TEST_F(RtpPacketHistoryTest, SetStoreStatus) { TEST_P(RtpPacketHistoryTest, SetStoreStatus) {
EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode()); EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
EXPECT_EQ(StorageMode::kStoreAndCull, hist_.GetStorageMode()); EXPECT_EQ(StorageMode::kStoreAndCull, hist_.GetStorageMode());
@ -59,7 +61,7 @@ TEST_F(RtpPacketHistoryTest, SetStoreStatus) {
EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode()); EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
} }
TEST_F(RtpPacketHistoryTest, ClearsHistoryAfterSetStoreStatus) { TEST_P(RtpPacketHistoryTest, ClearsHistoryAfterSetStoreStatus) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
// Store a packet, but with send-time. It should then not be removed. // Store a packet, but with send-time. It should then not be removed.
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), absl::nullopt); hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), absl::nullopt);
@ -70,7 +72,7 @@ TEST_F(RtpPacketHistoryTest, ClearsHistoryAfterSetStoreStatus) {
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum)); EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, StartSeqResetAfterReset) { TEST_P(RtpPacketHistoryTest, StartSeqResetAfterReset) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
// Store a packet, but with send-time. It should then not be removed. // Store a packet, but with send-time. It should then not be removed.
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), absl::nullopt); hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), absl::nullopt);
@ -96,7 +98,7 @@ TEST_F(RtpPacketHistoryTest, StartSeqResetAfterReset) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2)));
} }
TEST_F(RtpPacketHistoryTest, NoStoreStatus) { TEST_P(RtpPacketHistoryTest, NoStoreStatus) {
EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode()); EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum); std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
hist_.PutRtpPacket(std::move(packet), absl::nullopt); hist_.PutRtpPacket(std::move(packet), absl::nullopt);
@ -104,12 +106,12 @@ TEST_F(RtpPacketHistoryTest, NoStoreStatus) {
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum)); EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, GetRtpPacket_NotStored) { TEST_P(RtpPacketHistoryTest, GetRtpPacket_NotStored) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
EXPECT_FALSE(hist_.GetPacketState(0)); EXPECT_FALSE(hist_.GetPacketState(0));
} }
TEST_F(RtpPacketHistoryTest, PutRtpPacket) { TEST_P(RtpPacketHistoryTest, PutRtpPacket) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum); std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
@ -118,7 +120,7 @@ TEST_F(RtpPacketHistoryTest, PutRtpPacket) {
EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum)); EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, GetRtpPacket) { TEST_P(RtpPacketHistoryTest, GetRtpPacket) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
int64_t capture_time_ms = 1; int64_t capture_time_ms = 1;
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum); std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
@ -133,7 +135,7 @@ TEST_F(RtpPacketHistoryTest, GetRtpPacket) {
EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms()); EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
} }
TEST_F(RtpPacketHistoryTest, PacketStateIsCorrect) { TEST_P(RtpPacketHistoryTest, PacketStateIsCorrect) {
const uint32_t kSsrc = 92384762; const uint32_t kSsrc = 92384762;
const int64_t kRttMs = 100; const int64_t kRttMs = 100;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
@ -164,7 +166,7 @@ TEST_F(RtpPacketHistoryTest, PacketStateIsCorrect) {
EXPECT_EQ(state->times_retransmitted, 1u); EXPECT_EQ(state->times_retransmitted, 1u);
} }
TEST_F(RtpPacketHistoryTest, MinResendTimeWithPacer) { TEST_P(RtpPacketHistoryTest, MinResendTimeWithPacer) {
static const int64_t kMinRetransmitIntervalMs = 100; static const int64_t kMinRetransmitIntervalMs = 100;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
@ -205,7 +207,7 @@ TEST_F(RtpPacketHistoryTest, MinResendTimeWithPacer) {
EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum)); EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, MinResendTimeWithoutPacer) { TEST_P(RtpPacketHistoryTest, MinResendTimeWithoutPacer) {
static const int64_t kMinRetransmitIntervalMs = 100; static const int64_t kMinRetransmitIntervalMs = 100;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
@ -231,7 +233,7 @@ TEST_F(RtpPacketHistoryTest, MinResendTimeWithoutPacer) {
EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum)); EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, RemovesOldestSentPacketWhenAtMaxSize) { TEST_P(RtpPacketHistoryTest, RemovesOldestSentPacketWhenAtMaxSize) {
const size_t kMaxNumPackets = 10; const size_t kMaxNumPackets = 10;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
@ -262,7 +264,7 @@ TEST_F(RtpPacketHistoryTest, RemovesOldestSentPacketWhenAtMaxSize) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1)));
} }
TEST_F(RtpPacketHistoryTest, RemovesOldestPacketWhenAtMaxCapacity) { TEST_P(RtpPacketHistoryTest, RemovesOldestPacketWhenAtMaxCapacity) {
// Tests the absolute upper bound on number of stored packets. Don't allow // Tests the absolute upper bound on number of stored packets. Don't allow
// storing more than this, even if packets have not yet been sent. // storing more than this, even if packets have not yet been sent.
const size_t kMaxNumPackets = RtpPacketHistory::kMaxCapacity; const size_t kMaxNumPackets = RtpPacketHistory::kMaxCapacity;
@ -290,7 +292,12 @@ TEST_F(RtpPacketHistoryTest, RemovesOldestPacketWhenAtMaxCapacity) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1)));
} }
TEST_F(RtpPacketHistoryTest, RemovesLowestPrioPaddingWhenAtMaxCapacity) { TEST_P(RtpPacketHistoryTest, RemovesLowestPrioPaddingWhenAtMaxCapacity) {
if (!GetParam()) {
// Padding prioritization is off, ignore this test.
return;
}
// Tests the absolute upper bound on number of packets in the prioritized // Tests the absolute upper bound on number of packets in the prioritized
// set of potential padding packets. // set of potential padding packets.
const size_t kMaxNumPackets = RtpPacketHistory::kMaxPaddingtHistory; const size_t kMaxNumPackets = RtpPacketHistory::kMaxPaddingtHistory;
@ -322,7 +329,7 @@ TEST_F(RtpPacketHistoryTest, RemovesLowestPrioPaddingWhenAtMaxCapacity) {
EXPECT_EQ(packet->SequenceNumber(), To16u(kStartSeqNum + kMaxNumPackets)); EXPECT_EQ(packet->SequenceNumber(), To16u(kStartSeqNum + kMaxNumPackets));
} }
TEST_F(RtpPacketHistoryTest, DontRemoveUnsentPackets) { TEST_P(RtpPacketHistoryTest, DontRemoveUnsentPackets) {
const size_t kMaxNumPackets = 10; const size_t kMaxNumPackets = 10;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
@ -355,7 +362,7 @@ TEST_F(RtpPacketHistoryTest, DontRemoveUnsentPackets) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2)));
} }
TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPackets) { TEST_P(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPackets) {
// Set size to remove old packets as soon as possible. // Set size to remove old packets as soon as possible.
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
@ -380,7 +387,7 @@ TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPackets) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1)));
} }
TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPacketsHighRtt) { TEST_P(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPacketsHighRtt) {
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
const int64_t kPacketTimeoutMs = const int64_t kPacketTimeoutMs =
kRttMs * RtpPacketHistory::kMinPacketDurationRtt; kRttMs * RtpPacketHistory::kMinPacketDurationRtt;
@ -409,7 +416,7 @@ TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPacketsHighRtt) {
EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1))); EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 1)));
} }
TEST_F(RtpPacketHistoryTest, RemovesOldWithCulling) { TEST_P(RtpPacketHistoryTest, RemovesOldWithCulling) {
const size_t kMaxNumPackets = 10; const size_t kMaxNumPackets = 10;
// Enable culling. Even without feedback, this can trigger early removal. // Enable culling. Even without feedback, this can trigger early removal.
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
@ -432,7 +439,7 @@ TEST_F(RtpPacketHistoryTest, RemovesOldWithCulling) {
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum)); EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, RemovesOldWithCullingHighRtt) { TEST_P(RtpPacketHistoryTest, RemovesOldWithCullingHighRtt) {
const size_t kMaxNumPackets = 10; const size_t kMaxNumPackets = 10;
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
// Enable culling. Even without feedback, this can trigger early removal. // Enable culling. Even without feedback, this can trigger early removal.
@ -458,7 +465,7 @@ TEST_F(RtpPacketHistoryTest, RemovesOldWithCullingHighRtt) {
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum)); EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, CullWithAcks) { TEST_P(RtpPacketHistoryTest, CullWithAcks) {
const int64_t kPacketLifetime = RtpPacketHistory::kMinPacketDurationMs * const int64_t kPacketLifetime = RtpPacketHistory::kMinPacketDurationMs *
RtpPacketHistory::kPacketCullingDelayFactor; RtpPacketHistory::kPacketCullingDelayFactor;
@ -511,7 +518,7 @@ TEST_F(RtpPacketHistoryTest, CullWithAcks) {
EXPECT_FALSE(hist_.GetPacketState(To16u(kStartSeqNum + 2)).has_value()); EXPECT_FALSE(hist_.GetPacketState(To16u(kStartSeqNum + 2)).has_value());
} }
TEST_F(RtpPacketHistoryTest, SetsPendingTransmissionState) { TEST_P(RtpPacketHistoryTest, SetsPendingTransmissionState) {
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
hist_.SetRtt(kRttMs); hist_.SetRtt(kRttMs);
@ -553,7 +560,7 @@ TEST_F(RtpPacketHistoryTest, SetsPendingTransmissionState) {
EXPECT_FALSE(packet_state->pending_transmission); EXPECT_FALSE(packet_state->pending_transmission);
} }
TEST_F(RtpPacketHistoryTest, GetPacketAndSetSent) { TEST_P(RtpPacketHistoryTest, GetPacketAndSetSent) {
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
hist_.SetRtt(kRttMs); hist_.SetRtt(kRttMs);
@ -580,7 +587,7 @@ TEST_F(RtpPacketHistoryTest, GetPacketAndSetSent) {
EXPECT_TRUE(hist_.GetPacketAndMarkAsPending(kStartSeqNum)); EXPECT_TRUE(hist_.GetPacketAndMarkAsPending(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulation) { TEST_P(RtpPacketHistoryTest, GetPacketWithEncapsulation) {
const uint32_t kSsrc = 92384762; const uint32_t kSsrc = 92384762;
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
hist_.SetRtt(kRttMs); hist_.SetRtt(kRttMs);
@ -607,7 +614,7 @@ TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulation) {
EXPECT_EQ(retransmit_packet->Ssrc(), kSsrc + 1); EXPECT_EQ(retransmit_packet->Ssrc(), kSsrc + 1);
} }
TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulationAbortOnNullptr) { TEST_P(RtpPacketHistoryTest, GetPacketWithEncapsulationAbortOnNullptr) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum),
@ -617,14 +624,14 @@ TEST_F(RtpPacketHistoryTest, GetPacketWithEncapsulationAbortOnNullptr) {
// not suitable for retransmission (bandwidth exhausted?) so the retransmit is // not suitable for retransmission (bandwidth exhausted?) so the retransmit is
// aborted and the packet is not marked as pending. // aborted and the packet is not marked as pending.
EXPECT_FALSE(hist_.GetPacketAndMarkAsPending( EXPECT_FALSE(hist_.GetPacketAndMarkAsPending(
kStartSeqNum, [](const RtpPacketToSend& packet) { return nullptr; })); kStartSeqNum, [](const RtpPacketToSend&) { return nullptr; }));
// New try, this time getting the packet should work, and it should not be // New try, this time getting the packet should work, and it should not be
// blocked due to any pending status. // blocked due to any pending status.
EXPECT_TRUE(hist_.GetPacketAndMarkAsPending(kStartSeqNum)); EXPECT_TRUE(hist_.GetPacketAndMarkAsPending(kStartSeqNum));
} }
TEST_F(RtpPacketHistoryTest, DontRemovePendingTransmissions) { TEST_P(RtpPacketHistoryTest, DontRemovePendingTransmissions) {
const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2; const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
const int64_t kPacketTimeoutMs = const int64_t kPacketTimeoutMs =
kRttMs * RtpPacketHistory::kMinPacketDurationRtt; kRttMs * RtpPacketHistory::kMinPacketDurationRtt;
@ -657,7 +664,12 @@ TEST_F(RtpPacketHistoryTest, DontRemovePendingTransmissions) {
ASSERT_FALSE(packet_state.has_value()); ASSERT_FALSE(packet_state.has_value());
} }
TEST_F(RtpPacketHistoryTest, PrioritizedPayloadPadding) { TEST_P(RtpPacketHistoryTest, PrioritizedPayloadPadding) {
if (!GetParam()) {
// Padding prioritization is off, ignore this test.
return;
}
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
// Add two sent packets, one millisecond apart. // Add two sent packets, one millisecond apart.
@ -694,7 +706,7 @@ TEST_F(RtpPacketHistoryTest, PrioritizedPayloadPadding) {
EXPECT_EQ(hist_.GetPayloadPaddingPacket(), nullptr); EXPECT_EQ(hist_.GetPayloadPaddingPacket(), nullptr);
} }
TEST_F(RtpPacketHistoryTest, NoPendingPacketAsPadding) { TEST_P(RtpPacketHistoryTest, NoPendingPacketAsPadding) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum),
@ -712,7 +724,7 @@ TEST_F(RtpPacketHistoryTest, NoPendingPacketAsPadding) {
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(), kStartSeqNum); EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(), kStartSeqNum);
} }
TEST_F(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) { TEST_P(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum),
@ -720,9 +732,8 @@ TEST_F(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) {
fake_clock_.AdvanceTimeMilliseconds(1); fake_clock_.AdvanceTimeMilliseconds(1);
// Aborted padding. // Aborted padding.
EXPECT_EQ(nullptr, EXPECT_EQ(nullptr, hist_.GetPayloadPaddingPacket(
hist_.GetPayloadPaddingPacket( [](const RtpPacketToSend&) { return nullptr; }));
[](const RtpPacketToSend& packet) { return nullptr; }));
// Get copy of packet, but with sequence number modified. // Get copy of packet, but with sequence number modified.
auto padding_packet = auto padding_packet =
@ -735,7 +746,7 @@ TEST_F(RtpPacketHistoryTest, PayloadPaddingWithEncapsulation) {
EXPECT_EQ(padding_packet->SequenceNumber(), kStartSeqNum + 1); EXPECT_EQ(padding_packet->SequenceNumber(), kStartSeqNum + 1);
} }
TEST_F(RtpPacketHistoryTest, NackAfterAckIsNoop) { TEST_P(RtpPacketHistoryTest, NackAfterAckIsNoop) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 2); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 2);
// Add two sent packets. // Add two sent packets.
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum),
@ -749,7 +760,7 @@ TEST_F(RtpPacketHistoryTest, NackAfterAckIsNoop) {
EXPECT_EQ(packet.get(), nullptr); EXPECT_EQ(packet.get(), nullptr);
} }
TEST_F(RtpPacketHistoryTest, OutOfOrderInsertRemoval) { TEST_P(RtpPacketHistoryTest, OutOfOrderInsertRemoval) {
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
// Insert packets, out of order, including both forwards and backwards // Insert packets, out of order, including both forwards and backwards
@ -780,4 +791,51 @@ TEST_F(RtpPacketHistoryTest, OutOfOrderInsertRemoval) {
expected_time_offset_ms += 33; expected_time_offset_ms += 33;
} }
} }
TEST_P(RtpPacketHistoryTest, UsesLastPacketAsPaddingWithPrioOff) {
if (GetParam()) {
// Padding prioritization is enabled, ignore this test.
return;
}
const size_t kHistorySize = 10;
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kHistorySize);
EXPECT_EQ(hist_.GetPayloadPaddingPacket(), nullptr);
for (size_t i = 0; i < kHistorySize; ++i) {
hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + i)),
fake_clock_.TimeInMilliseconds());
hist_.MarkPacketAsSent(To16u(kStartSeqNum + i));
fake_clock_.AdvanceTimeMilliseconds(1);
// Last packet always returned.
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i));
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i));
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i));
}
// Remove packets from the end, last in the list should be returned.
for (size_t i = kHistorySize - 1; i > 0; --i) {
hist_.CullAcknowledgedPackets(
std::vector<uint16_t>{To16u(kStartSeqNum + i)});
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i - 1));
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i - 1));
EXPECT_EQ(hist_.GetPayloadPaddingPacket()->SequenceNumber(),
To16u(kStartSeqNum + i - 1));
}
hist_.CullAcknowledgedPackets(std::vector<uint16_t>{kStartSeqNum});
EXPECT_EQ(hist_.GetPayloadPaddingPacket(), nullptr);
}
INSTANTIATE_TEST_SUITE_P(WithAndWithoutPaddingPrio,
RtpPacketHistoryTest,
::testing::Bool());
} // namespace webrtc } // namespace webrtc

2
modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -40,7 +40,7 @@ const int64_t kDefaultExpectedRetransmissionTimeMs = 125;
ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext( ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext(
const RtpRtcp::Configuration& config) const RtpRtcp::Configuration& config)
: packet_history(config.clock), : packet_history(config.clock, config.enable_rtx_padding_prioritization),
packet_sender(config, &packet_history), packet_sender(config, &packet_history),
non_paced_sender(&packet_sender), non_paced_sender(&packet_sender),
packet_generator( packet_generator(

2
modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -223,7 +223,7 @@ class StreamDataTestCallback : public StreamDataCountersCallback {
// wherever possible. // wherever possible.
struct RtpSenderContext { struct RtpSenderContext {
explicit RtpSenderContext(const RtpRtcp::Configuration& config) explicit RtpSenderContext(const RtpRtcp::Configuration& config)
: packet_history_(config.clock), : packet_history_(config.clock, config.enable_rtx_padding_prioritization),
packet_sender_(config, &packet_history_), packet_sender_(config, &packet_history_),
non_paced_sender_(&packet_sender_), non_paced_sender_(&packet_sender_),
packet_generator_( packet_generator_(