zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove legacy/unused RtpPacketHistory::StorageMode::kStore

Browse Source 
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}
This commit is contained in:
Erik Språng
2019-07-19 13:52:13 +02:00
committed by Commit Bot
parent d48fbfc513
commit b9f5989e05
5 changed files with 39 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -113,10 +113,12 @@ void RtpPacketHistory::SetRtt(int64_t rtt_ms) {
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 @@ std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPayloadPaddingPacket(
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 @@ void RtpPacketHistory::CullOldPackets(int64_t now_ms) {
}
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);

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

@ -31,7 +31,6 @@ class RtpPacketHistory {
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.
};

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

@ -51,8 +51,8 @@ class RtpPacketHistoryTest : public ::testing::Test {
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, SetStoreStatus) {
}
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, NoStoreStatus) {
}
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, PutRtpPacket) {
}
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, GetRtpPacket) {
}
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, NoCaptureTime) {
}
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, PacketStateIsCorrect) {
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, MinResendTimeWithPacer) {
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, MinResendTimeWithoutPacer) {
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 @@ TEST_F(RtpPacketHistoryTest, RemovesOldestPacketWhenAtMaxCapacity) {
// 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, RemovesOldestPacketWhenAtMaxCapacity) {
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, DontRemoveUnsentPackets) {
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 @@ TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPacketsHighRtt) {
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.

35
modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -198,9 +198,6 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config)
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 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config)
// 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 @@ RTPSender::RTPSender(
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 @@ RTPSender::RTPSender(
// 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 @@ size_t RTPSender::SendPadData(size_t bytes,
}
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 {

1
modules/rtp_rtcp/source/rtp_sender.h
View File

@ -318,7 +318,6 @@ class RTPSender {
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.