Remove legacy/unused RtpPacketHistory::StorageMode::kStore
The kStoreAndCull mode has been the default since May 3rd 2019:
https://webrtc.googlesource.com/src/+/d2a634447f42d6856656a9fcdb65d5845b736941
Let's clean away the old code.
Bug: webrtc:8975
Change-Id: I5f41b48b68aecce281cbb713e50db60c8a89da9a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146213
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28650}
diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc
index 8bcdfb9..0dd3229 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_history.cc
@@ -113,10 +113,12 @@
rtc::CritScope cs(&lock_);
RTC_DCHECK_GE(rtt_ms, 0);
rtt_ms_ = rtt_ms;
- // If kStoreAndCull mode is used, packets will be removed after a timeout
+ // If storage is not disabled, packets will be removed after a timeout
// that depends on the RTT. Changing the RTT may thus cause some packets
// become "old" and subject to removal.
- CullOldPackets(clock_->TimeInMilliseconds());
+ if (mode_ != StorageMode::kDisabled) {
+ CullOldPackets(clock_->TimeInMilliseconds());
+ }
}
void RtpPacketHistory::PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
@@ -336,12 +338,14 @@
void RtpPacketHistory::CullAcknowledgedPackets(
rtc::ArrayView<const uint16_t> sequence_numbers) {
rtc::CritScope cs(&lock_);
- if (mode_ == StorageMode::kStoreAndCull) {
- for (uint16_t sequence_number : sequence_numbers) {
- auto stored_packet_it = packet_history_.find(sequence_number);
- if (stored_packet_it != packet_history_.end()) {
- RemovePacket(stored_packet_it);
- }
+ if (mode_ == StorageMode::kDisabled) {
+ return;
+ }
+
+ for (uint16_t sequence_number : sequence_numbers) {
+ auto stored_packet_it = packet_history_.find(sequence_number);
+ if (stored_packet_it != packet_history_.end()) {
+ RemovePacket(stored_packet_it);
}
}
}
@@ -393,10 +397,9 @@
}
if (packet_history_.size() >= number_to_store_ ||
- (mode_ == StorageMode::kStoreAndCull &&
- *stored_packet.send_time_ms_ +
- (packet_duration_ms * kPacketCullingDelayFactor) <=
- now_ms)) {
+ *stored_packet.send_time_ms_ +
+ (packet_duration_ms * kPacketCullingDelayFactor) <=
+ now_ms) {
// Too many packets in history, or this packet has timed out. Remove it
// and continue.
RemovePacket(stored_packet_it);
diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h
index ca4ab3d..e477b45 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.h
+++ b/modules/rtp_rtcp/source/rtp_packet_history.h
@@ -31,7 +31,6 @@
public:
enum class StorageMode {
kDisabled, // Don't store any packets.
- kStore, // Store and keep at least |number_to_store| packets.
kStoreAndCull // Store up to |number_to_store| packets, but try to remove
// packets as they time out or as signaled as received.
};
diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
index b801ae8..25c1f86 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
@@ -51,8 +51,8 @@
TEST_F(RtpPacketHistoryTest, SetStoreStatus) {
EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
- EXPECT_EQ(StorageMode::kStore, hist_.GetStorageMode());
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
+ EXPECT_EQ(StorageMode::kStoreAndCull, hist_.GetStorageMode());
hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
EXPECT_EQ(StorageMode::kStoreAndCull, hist_.GetStorageMode());
hist_.SetStorePacketsStatus(StorageMode::kDisabled, 0);
@@ -60,14 +60,14 @@
}
TEST_F(RtpPacketHistoryTest, ClearsHistoryAfterSetStoreStatus) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
// Store a packet, but with send-time. It should then not be removed.
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
absl::nullopt);
EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum));
// Changing store status, even to the current one, will clear the history.
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
}
@@ -109,12 +109,12 @@
}
TEST_F(RtpPacketHistoryTest, GetRtpPacket_NotStored) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
EXPECT_FALSE(hist_.GetPacketState(0));
}
TEST_F(RtpPacketHistoryTest, PutRtpPacket) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum));
@@ -123,7 +123,7 @@
}
TEST_F(RtpPacketHistoryTest, GetRtpPacket) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
int64_t capture_time_ms = 1;
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
packet->set_capture_time_ms(capture_time_ms);
@@ -138,7 +138,7 @@
}
TEST_F(RtpPacketHistoryTest, NoCaptureTime) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
fake_clock_.AdvanceTimeMilliseconds(1);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
@@ -154,7 +154,7 @@
}
TEST_F(RtpPacketHistoryTest, DontRetransmit) {
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
rtc::CopyOnWriteBuffer buffer = packet->Buffer();
@@ -207,7 +207,7 @@
TEST_F(RtpPacketHistoryTest, MinResendTimeWithPacer) {
static const int64_t kMinRetransmitIntervalMs = 100;
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
hist_.SetRtt(kMinRetransmitIntervalMs);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
@@ -248,7 +248,7 @@
TEST_F(RtpPacketHistoryTest, MinResendTimeWithoutPacer) {
static const int64_t kMinRetransmitIntervalMs = 100;
- hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
hist_.SetRtt(kMinRetransmitIntervalMs);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
@@ -274,7 +274,7 @@
TEST_F(RtpPacketHistoryTest, RemovesOldestSentPacketWhenAtMaxSize) {
const size_t kMaxNumPackets = 10;
- hist_.SetStorePacketsStatus(StorageMode::kStore, kMaxNumPackets);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
// History does not allow removing packets within kMinPacketDurationMs,
// so in order to test capacity, make sure insertion spans this time.
@@ -309,7 +309,7 @@
// 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.
const size_t kMaxNumPackets = RtpPacketHistory::kMaxCapacity;
- hist_.SetStorePacketsStatus(StorageMode::kStore,
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull,
RtpPacketHistory::kMaxCapacity);
// Add packets until the buffer is full.
@@ -336,7 +336,7 @@
TEST_F(RtpPacketHistoryTest, DontRemoveUnsentPackets) {
const size_t kMaxNumPackets = 10;
- hist_.SetStorePacketsStatus(StorageMode::kStore, kMaxNumPackets);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
// Add packets until the buffer is full.
for (size_t i = 0; i < kMaxNumPackets; ++i) {
@@ -370,7 +370,7 @@
TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPackets) {
// Set size to remove old packets as soon as possible.
- hist_.SetStorePacketsStatus(StorageMode::kStore, 1);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
// Add a packet, marked as send, and advance time to just before removal time.
hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
@@ -399,7 +399,7 @@
kRttMs * RtpPacketHistory::kMinPacketDurationRtt;
// Set size to remove old packets as soon as possible.
- hist_.SetStorePacketsStatus(StorageMode::kStore, 1);
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 1);
hist_.SetRtt(kRttMs);
// Add a packet, marked as send, and advance time to just before removal time.
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index b762e60..66d6b71 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -198,9 +198,6 @@
populate_network2_timestamp_(config.populate_network2_timestamp),
send_side_bwe_with_overhead_(
IsEnabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)),
- legacy_packet_history_storage_mode_(
- IsEnabled("WebRTC-UseRtpPacketHistoryLegacyStorageMode",
- config.field_trials)),
pacer_legacy_packet_referencing_(
!IsDisabled("WebRTC-Pacer-LegacyPacketReferencing",
config.field_trials)) {
@@ -213,13 +210,9 @@
// Store FlexFEC packets in the packet history data structure, so they can
// be found when paced.
if (flexfec_ssrc_) {
- RtpPacketHistory::StorageMode storage_mode =
- legacy_packet_history_storage_mode_
- ? RtpPacketHistory::StorageMode::kStore
- : RtpPacketHistory::StorageMode::kStoreAndCull;
-
flexfec_packet_history_.SetStorePacketsStatus(
- storage_mode, kMinFlexfecPacketsToStoreForPacing);
+ RtpPacketHistory::StorageMode::kStoreAndCull,
+ kMinFlexfecPacketsToStoreForPacing);
}
}
@@ -287,9 +280,6 @@
send_side_bwe_with_overhead_(
field_trials.Lookup("WebRTC-SendSideBwe-WithOverhead")
.find("Enabled") == 0),
- legacy_packet_history_storage_mode_(
- field_trials.Lookup("WebRTC-UseRtpPacketHistoryLegacyStorageMode")
- .find("Enabled") == 0),
pacer_legacy_packet_referencing_(
field_trials.Lookup("WebRTC-Pacer-LegacyPacketReferencing")
.find("Disabled") != 0) {
@@ -302,13 +292,9 @@
// Store FlexFEC packets in the packet history data structure, so they can
// be found when paced.
if (flexfec_ssrc_) {
- RtpPacketHistory::StorageMode storage_mode =
- legacy_packet_history_storage_mode_
- ? RtpPacketHistory::StorageMode::kStore
- : RtpPacketHistory::StorageMode::kStoreAndCull;
-
flexfec_packet_history_.SetStorePacketsStatus(
- storage_mode, kMinFlexfecPacketsToStoreForPacing);
+ RtpPacketHistory::StorageMode::kStoreAndCull,
+ kMinFlexfecPacketsToStoreForPacing);
}
}
@@ -576,15 +562,10 @@
}
void RTPSender::SetStorePacketsStatus(bool enable, uint16_t number_to_store) {
- RtpPacketHistory::StorageMode mode;
- if (enable) {
- mode = legacy_packet_history_storage_mode_
- ? RtpPacketHistory::StorageMode::kStore
- : RtpPacketHistory::StorageMode::kStoreAndCull;
- } else {
- mode = RtpPacketHistory::StorageMode::kDisabled;
- }
- packet_history_.SetStorePacketsStatus(mode, number_to_store);
+ packet_history_.SetStorePacketsStatus(
+ enable ? RtpPacketHistory::StorageMode::kStoreAndCull
+ : RtpPacketHistory::StorageMode::kDisabled,
+ number_to_store);
}
bool RTPSender::StorePackets() const {
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 823710e..f79b71d 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -318,7 +318,6 @@
const bool populate_network2_timestamp_;
const bool send_side_bwe_with_overhead_;
- const bool legacy_packet_history_storage_mode_;
// If true, PacedSender should only reference packets as in legacy mode.
// If false, PacedSender may have direct ownership of RtpPacketToSend objects.
