Remove duplicate packet check from webrtc::PacketQueue.
Original CL by eladalon@ (https://codereview.chromium.org/2929213002/).
Bug: webrtc:7786, webrtc:8287, webrtc:8288
Change-Id: I1eaabfbd26b04882b65a3f2a779dd43b953553a6
Reviewed-on: https://webrtc-review.googlesource.com/4721
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20070}diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc
index 7ad3f9f..e4adcd3 100644
--- a/modules/pacing/paced_sender_unittest.cc
+++ b/modules/pacing/paced_sender_unittest.cc
@@ -255,60 +255,18 @@
EXPECT_EQ(1u, send_bucket_->QueueSizePackets());
}
-TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
- uint32_t ssrc = 12345;
- uint16_t sequence_number = 1234;
- uint16_t queued_sequence_number;
-
- // Due to the multiplicative factor we can send 5 packets during a send
- // interval. (network capacity * multiplier / (8 bits per byte *
- // (packet size * #send intervals per second)
- const size_t packets_to_send_per_interval =
- kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200);
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
- SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
- clock_.TimeInMilliseconds(), 250, false);
- }
- queued_sequence_number = sequence_number;
-
- for (size_t j = 0; j < packets_to_send_per_interval * 10; ++j) {
- // Send in duplicate packets.
- send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
- sequence_number, clock_.TimeInMilliseconds(),
- 250, false);
- send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
- sequence_number++, clock_.TimeInMilliseconds(),
- 250, false);
- }
- EXPECT_CALL(callback_, TimeToSendPadding(_, _)).Times(0);
- send_bucket_->Process();
- for (int k = 0; k < 10; ++k) {
- EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
+TEST_F(PacedSenderTest, RepeatedRetransmissionsAllowed) {
+ // Send one packet, then two retransmissions of that packet.
+ for (size_t i = 0; i < 3; i++) {
+ constexpr uint32_t ssrc = 333;
+ constexpr uint16_t sequence_number = 444;
+ constexpr size_t bytes = 250;
+ bool is_retransmission = (i != 0); // Original followed by retransmissions.
+ SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number,
+ clock_.TimeInMilliseconds(), bytes, is_retransmission);
clock_.AdvanceTimeMilliseconds(5);
-
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
- EXPECT_CALL(callback_,
- TimeToSendPacket(ssrc, queued_sequence_number++, _, false, _))
- .Times(1)
- .WillRepeatedly(Return(true));
- }
- EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
- send_bucket_->Process();
}
- EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
- clock_.AdvanceTimeMilliseconds(5);
- EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
send_bucket_->Process();
-
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
- SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
- clock_.TimeInMilliseconds(), 250, false);
- }
- send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
- sequence_number++, clock_.TimeInMilliseconds(),
- 250, false);
- send_bucket_->Process();
- EXPECT_EQ(1u, send_bucket_->QueueSizePackets());
}
TEST_F(PacedSenderTest, CanQueuePacketsWithSameSequenceNumberOnDifferentSsrcs) {
diff --git a/modules/pacing/packet_queue.cc b/modules/pacing/packet_queue.cc
index 765e43f..99f27c8 100644
--- a/modules/pacing/packet_queue.cc
+++ b/modules/pacing/packet_queue.cc
@@ -56,9 +56,6 @@
PacketQueue::~PacketQueue() {}
void PacketQueue::Push(const Packet& packet) {
- if (!AddToDupeSet(packet))
- return;
-
UpdateQueueTime(packet.enqueue_time_ms);
// Store packet in list, use pointers in priority queue for cheaper moves.
@@ -82,7 +79,6 @@
}
void PacketQueue::FinalizePop(const PacketQueue::Packet& packet) {
- RemoveFromDupeSet(packet);
bytes_ -= packet.bytes;
int64_t packet_queue_time_ms = time_last_updated_ - packet.enqueue_time_ms;
RTC_DCHECK_LE(packet.sum_paused_ms, packet_queue_time_ms);
@@ -150,25 +146,4 @@
return queue_time_sum_ / packet_list_.size();
}
-bool PacketQueue::AddToDupeSet(const PacketQueue::Packet& packet) {
- SsrcSeqNoMap::iterator it = dupe_map_.find(packet.ssrc);
- if (it == dupe_map_.end()) {
- // First for this ssrc, just insert.
- dupe_map_[packet.ssrc].insert(packet.sequence_number);
- return true;
- }
-
- // Insert returns a pair, where second is a bool set to true if new element.
- return it->second.insert(packet.sequence_number).second;
-}
-
-void PacketQueue::RemoveFromDupeSet(const PacketQueue::Packet& packet) {
- SsrcSeqNoMap::iterator it = dupe_map_.find(packet.ssrc);
- RTC_DCHECK(it != dupe_map_.end());
- it->second.erase(packet.sequence_number);
- if (it->second.empty()) {
- dupe_map_.erase(it);
- }
-}
-
} // namespace webrtc
diff --git a/modules/pacing/packet_queue.h b/modules/pacing/packet_queue.h
index c6aa843..5e970fb 100644
--- a/modules/pacing/packet_queue.h
+++ b/modules/pacing/packet_queue.h
@@ -12,9 +12,7 @@
#define MODULES_PACING_PACKET_QUEUE_H_
#include <list>
-#include <map>
#include <queue>
-#include <set>
#include <vector>
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -96,9 +94,6 @@
std::priority_queue<Packet*, std::vector<Packet*>, Comparator> prio_queue_;
// Total number of bytes in the queue.
uint64_t bytes_;
- // Map<ssrc, std::set<seq_no> >, for checking duplicates.
- typedef std::map<uint32_t, std::set<uint16_t> > SsrcSeqNoMap;
- SsrcSeqNoMap dupe_map_;
const Clock* const clock_;
int64_t queue_time_sum_;
int64_t time_last_updated_;
