Reland "Rework rtp packet history"
This is a reland of 6328d7cbbc8a72fdc81a766c0bf4039e1e2e7887
Original change's description:
> Rework rtp packet history
>
> This CL rewrites the history from the ground up, but keeps the logic
> (mostly) intact. It does however lay the groundwork for adding a new
> mode where TransportFeedback messages can be used to remove packets
> from the history as we know the remote end has received them.
>
> This should both reduce memory usage and make the payload based padding
> a little more likely to be useful.
>
> My tests show a reduction of ca 500-800kB reduction in memory usage per
> rtp module. So with simulcast and/or fec this will increase. Lossy
> links and long RTT will use more memory.
>
> I've also slightly update the interface to make usage with/without
> pacer less unintuitive, and avoid making a copy of the entire RTP
> packet just to find the ssrc and sequence number to put into the pacer.
>
> The more aggressive culling is not enabled by default. I will
> wire that up in a follow-up CL, as there's some interface refactoring
> required.
>
> Bug: webrtc:8975
> Change-Id: I0c1bb528f32eeed0fb276b4ae77ae3235656980f
> Reviewed-on: https://webrtc-review.googlesource.com/59441
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#22347}
Bug: webrtc:8975
Change-Id: I162cb9a1eccddf567bdda7285f8296dc2f005503
Reviewed-on: https://webrtc-review.googlesource.com/60900
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22356}
Reviewed-on: https://webrtc-review.googlesource.com/61661
Cr-Commit-Position: refs/heads/master@{#22438}diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc
index b3649f8..2bd06c4 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_history.cc
@@ -17,17 +17,33 @@
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
+#include "rtc_base/ptr_util.h"
#include "system_wrappers/include/clock.h"
namespace webrtc {
namespace {
+// Min packet size for BestFittingPacket() to honor.
constexpr size_t kMinPacketRequestBytes = 50;
-// Don't overwrite a packet within one second, or three RTTs, after transmission
-// whichever is larger. Instead try to dynamically expand history.
-constexpr int64_t kMinPacketDurationMs = 1000;
-constexpr int kMinPacketDurationRtt = 3;
+
+// Utility function to get the absolute difference in size between the provided
+// target size and the size of packet.
+size_t SizeDiff(const std::unique_ptr<RtpPacketToSend>& packet, size_t size) {
+ size_t packet_size = packet->size();
+ if (packet_size > size) {
+ return packet_size - size;
+ }
+ return size - packet_size;
+}
} // namespace
+
constexpr size_t RtpPacketHistory::kMaxCapacity;
+constexpr int64_t RtpPacketHistory::kMinPacketDurationMs;
+constexpr int RtpPacketHistory::kMinPacketDurationRtt;
+constexpr int RtpPacketHistory::kPacketCullingDelayFactor;
+
+RtpPacketHistory::PacketState::PacketState() = default;
+RtpPacketHistory::PacketState::PacketState(const PacketState&) = default;
+RtpPacketHistory::PacketState::~PacketState() = default;
RtpPacketHistory::StoredPacket::StoredPacket() = default;
RtpPacketHistory::StoredPacket::StoredPacket(StoredPacket&&) = default;
@@ -36,209 +52,239 @@
RtpPacketHistory::StoredPacket::~StoredPacket() = default;
RtpPacketHistory::RtpPacketHistory(Clock* clock)
- : clock_(clock), store_(false), prev_index_(0), rtt_ms_(-1) {}
+ : clock_(clock),
+ number_to_store_(0),
+ mode_(StorageMode::kDisabled),
+ rtt_ms_(-1) {}
RtpPacketHistory::~RtpPacketHistory() {}
-void RtpPacketHistory::SetStorePacketsStatus(bool enable,
- uint16_t number_to_store) {
- rtc::CritScope cs(&critsect_);
- if (enable) {
- if (store_) {
- RTC_LOG(LS_WARNING)
- << "Purging packet history in order to re-set status.";
- Free();
- }
- RTC_DCHECK(!store_);
- Allocate(number_to_store);
- } else {
- Free();
- }
-}
-
-void RtpPacketHistory::Allocate(size_t number_to_store) {
- RTC_DCHECK_GT(number_to_store, 0);
+void RtpPacketHistory::SetStorePacketsStatus(StorageMode mode,
+ size_t number_to_store) {
RTC_DCHECK_LE(number_to_store, kMaxCapacity);
- store_ = true;
- stored_packets_.resize(number_to_store);
-}
-
-void RtpPacketHistory::Free() {
- if (!store_) {
- return;
+ rtc::CritScope cs(&lock_);
+ if (mode != StorageMode::kDisabled && mode_ != StorageMode::kDisabled) {
+ RTC_LOG(LS_WARNING) << "Purging packet history in order to re-set status.";
}
-
- stored_packets_.clear();
-
- store_ = false;
- prev_index_ = 0;
+ Reset();
+ mode_ = mode;
+ number_to_store_ = std::min(kMaxCapacity, number_to_store);
}
-bool RtpPacketHistory::StorePackets() const {
- rtc::CritScope cs(&critsect_);
- return store_;
+RtpPacketHistory::StorageMode RtpPacketHistory::GetStorageMode() const {
+ rtc::CritScope cs(&lock_);
+ return mode_;
+}
+
+void RtpPacketHistory::SetRtt(int64_t rtt_ms) {
+ rtc::CritScope cs(&lock_);
+ RTC_DCHECK_GE(rtt_ms, 0);
+ rtt_ms_ = rtt_ms;
}
void RtpPacketHistory::PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
StorageType type,
- bool sent) {
+ rtc::Optional<int64_t> send_time_ms) {
RTC_DCHECK(packet);
- rtc::CritScope cs(&critsect_);
- if (!store_) {
+ rtc::CritScope cs(&lock_);
+ int64_t now_ms = clock_->TimeInMilliseconds();
+ if (mode_ == StorageMode::kDisabled) {
return;
}
- int64_t now_ms = clock_->TimeInMilliseconds();
-
- // If index we're about to overwrite contains a packet that has not
- // yet been sent (probably pending in paced sender), or if the send time is
- // less than 3 round trip times ago, expand the buffer to avoid overwriting
- // valid data.
- StoredPacket* stored_packet = &stored_packets_[prev_index_];
- int64_t packet_duration_ms =
- std::max(kMinPacketDurationRtt * rtt_ms_, kMinPacketDurationMs);
- if (stored_packet->packet &&
- (stored_packet->send_time == 0 ||
- (rtt_ms_ >= 0 &&
- now_ms - stored_packet->send_time <= packet_duration_ms))) {
- size_t current_size = stored_packets_.size();
- if (current_size < kMaxCapacity) {
- size_t expanded_size = std::max(current_size * 3 / 2, current_size + 1);
- expanded_size = std::min(expanded_size, kMaxCapacity);
- Allocate(expanded_size);
- // Causes discontinuity, but that's OK-ish. FindSeqNum() will still work,
- // but may be slower - at least until buffer has wrapped around once.
- prev_index_ = current_size;
- stored_packet = &stored_packets_[prev_index_];
- }
- }
+ CullOldPackets(now_ms);
// Store packet.
- if (packet->capture_time_ms() <= 0)
- packet->set_capture_time_ms(now_ms);
- stored_packet->sequence_number = packet->SequenceNumber();
- stored_packet->send_time = sent ? now_ms : 0;
- stored_packet->storage_type = type;
- stored_packet->has_been_retransmitted = false;
- stored_packet->packet = std::move(packet);
+ const uint16_t rtp_seq_no = packet->SequenceNumber();
+ StoredPacket& stored_packet = packet_history_[rtp_seq_no];
+ RTC_DCHECK(stored_packet.packet == nullptr);
+ stored_packet.packet = std::move(packet);
- prev_index_ = (prev_index_ + 1) % stored_packets_.size();
-}
-
-bool RtpPacketHistory::HasRtpPacket(uint16_t sequence_number) const {
- rtc::CritScope cs(&critsect_);
- if (!store_) {
- return false;
+ if (stored_packet.packet->capture_time_ms() <= 0) {
+ stored_packet.packet->set_capture_time_ms(now_ms);
}
+ stored_packet.send_time_ms = send_time_ms;
+ stored_packet.storage_type = type;
+ stored_packet.times_retransmitted = 0;
- int unused_index = 0;
- return FindSeqNum(sequence_number, &unused_index);
+ if (!start_seqno_) {
+ start_seqno_ = rtp_seq_no;
+ }
}
std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPacketAndSetSendTime(
uint16_t sequence_number,
- int64_t min_elapsed_time_ms,
- bool retransmit) {
- rtc::CritScope cs(&critsect_);
- if (!store_) {
+ bool verify_rtt) {
+ rtc::CritScope cs(&lock_);
+ if (mode_ == StorageMode::kDisabled) {
return nullptr;
}
- int index = 0;
- if (!FindSeqNum(sequence_number, &index)) {
- RTC_LOG(LS_WARNING) << "No match for getting seqNum " << sequence_number;
- return nullptr;
- }
- RTC_DCHECK_EQ(sequence_number,
- stored_packets_[index].packet->SequenceNumber());
-
- // Verify elapsed time since last retrieve, but only for retransmissions and
- // always send packet upon first retransmission request.
- int64_t now = clock_->TimeInMilliseconds();
- if (min_elapsed_time_ms > 0 && retransmit &&
- stored_packets_[index].has_been_retransmitted &&
- ((now - stored_packets_[index].send_time) < min_elapsed_time_ms)) {
+ int64_t now_ms = clock_->TimeInMilliseconds();
+ StoredPacketIterator rtp_it = packet_history_.find(sequence_number);
+ if (rtp_it == packet_history_.end()) {
return nullptr;
}
- if (retransmit) {
- if (stored_packets_[index].storage_type == kDontRetransmit) {
- // No bytes copied since this packet shouldn't be retransmitted.
- return nullptr;
- }
- stored_packets_[index].has_been_retransmitted = true;
+ StoredPacket& packet = rtp_it->second;
+ if (verify_rtt && !VerifyRtt(rtp_it->second, now_ms)) {
+ return nullptr;
}
- stored_packets_[index].send_time = clock_->TimeInMilliseconds();
- return GetPacket(index);
+
+ if (packet.send_time_ms) {
+ ++packet.times_retransmitted;
+ }
+
+ // Update send-time and return copy of packet instance.
+ packet.send_time_ms = now_ms;
+
+ if (packet.storage_type == StorageType::kDontRetransmit) {
+ // Non retransmittable packet, so call must come from paced sender.
+ // Remove from history and return actual packet instance.
+ return RemovePacket(rtp_it);
+ }
+ return rtc::MakeUnique<RtpPacketToSend>(*packet.packet);
}
-std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetPacket(int index) const {
- const RtpPacketToSend& stored = *stored_packets_[index].packet;
- return std::unique_ptr<RtpPacketToSend>(new RtpPacketToSend(stored));
+rtc::Optional<RtpPacketHistory::PacketState> RtpPacketHistory::GetPacketState(
+ uint16_t sequence_number,
+ bool verify_rtt) const {
+ rtc::CritScope cs(&lock_);
+ if (mode_ == StorageMode::kDisabled) {
+ return rtc::nullopt;
+ }
+
+ auto rtp_it = packet_history_.find(sequence_number);
+ if (rtp_it == packet_history_.end()) {
+ return rtc::nullopt;
+ }
+
+ if (verify_rtt && !VerifyRtt(rtp_it->second, clock_->TimeInMilliseconds())) {
+ return rtc::nullopt;
+ }
+
+ return StoredPacketToPacketState(rtp_it->second);
+}
+
+bool RtpPacketHistory::VerifyRtt(const RtpPacketHistory::StoredPacket& packet,
+ int64_t now_ms) const {
+ if (packet.send_time_ms) {
+ // Send-time already set, this check must be for a retransmission.
+ if (packet.times_retransmitted > 0 &&
+ now_ms < *packet.send_time_ms + rtt_ms_) {
+ // This packet has already been retransmitted once, and the time since
+ // that even is lower than on RTT. Ignore request as this packet is
+ // likely already in the network pipe.
+ return false;
+ }
+ }
+
+ return true;
}
std::unique_ptr<RtpPacketToSend> RtpPacketHistory::GetBestFittingPacket(
size_t packet_length) const {
- rtc::CritScope cs(&critsect_);
- if (!store_)
+ // TODO(sprang): Make this smarter, taking retransmit count etc into account.
+ rtc::CritScope cs(&lock_);
+ if (packet_length < kMinPacketRequestBytes || packet_history_.empty()) {
return nullptr;
- int index = FindBestFittingPacket(packet_length);
- if (index < 0)
- return nullptr;
- return GetPacket(index);
-}
-
-bool RtpPacketHistory::FindSeqNum(uint16_t sequence_number, int* index) const {
- if (prev_index_ > 0) {
- *index = prev_index_ - 1;
- } else {
- *index = stored_packets_.size() - 1; // Wrap.
- }
- uint16_t temp_sequence_number = stored_packets_[*index].sequence_number;
-
- int idx = *index - (temp_sequence_number - sequence_number);
- if (idx >= 0 && idx < static_cast<int>(stored_packets_.size())) {
- *index = idx;
- temp_sequence_number = stored_packets_[*index].sequence_number;
}
- if (temp_sequence_number != sequence_number) {
- // We did not found a match, search all.
- for (uint16_t m = 0; m < stored_packets_.size(); m++) {
- if (stored_packets_[m].sequence_number == sequence_number) {
- *index = m;
- temp_sequence_number = stored_packets_[*index].sequence_number;
+ size_t min_diff = std::numeric_limits<size_t>::max();
+ RtpPacketToSend* best_packet = nullptr;
+ for (auto& it : packet_history_) {
+ size_t diff = SizeDiff(it.second.packet, packet_length);
+ if (!min_diff || diff < min_diff) {
+ min_diff = diff;
+ best_packet = it.second.packet.get();
+ if (diff == 0) {
break;
}
}
}
- return temp_sequence_number == sequence_number &&
- stored_packets_[*index].packet;
+
+ return rtc::MakeUnique<RtpPacketToSend>(*best_packet);
}
-int RtpPacketHistory::FindBestFittingPacket(size_t size) const {
- if (size < kMinPacketRequestBytes || stored_packets_.empty())
- return -1;
- size_t min_diff = std::numeric_limits<size_t>::max();
- int best_index = -1; // Returned unchanged if we don't find anything.
- for (size_t i = 0; i < stored_packets_.size(); ++i) {
- if (!stored_packets_[i].packet)
+void RtpPacketHistory::Reset() {
+ packet_history_.clear();
+ start_seqno_.reset();
+}
+
+void RtpPacketHistory::CullOldPackets(int64_t now_ms) {
+ int64_t packet_duration_ms =
+ std::max(kMinPacketDurationRtt * rtt_ms_, kMinPacketDurationMs);
+ while (!packet_history_.empty()) {
+ auto stored_packet_it = packet_history_.find(*start_seqno_);
+ RTC_DCHECK(stored_packet_it != packet_history_.end());
+
+ if (packet_history_.size() >= kMaxCapacity) {
+ // We have reached the absolute max capacity, remove one packet
+ // unconditionally.
+ RemovePacket(stored_packet_it);
continue;
- size_t stored_size = stored_packets_[i].packet->size();
- size_t diff =
- (stored_size > size) ? (stored_size - size) : (size - stored_size);
- if (diff < min_diff) {
- min_diff = diff;
- best_index = static_cast<int>(i);
+ }
+
+ const StoredPacket& stored_packet = stored_packet_it->second;
+ if (!stored_packet.send_time_ms) {
+ // Don't remove packets that have not been sent.
+ return;
+ }
+
+ if (*stored_packet.send_time_ms + packet_duration_ms > now_ms) {
+ // Don't cull packets too early to avoid failed retransmission requests.
+ return;
+ }
+
+ if (packet_history_.size() >= number_to_store_ ||
+ (mode_ == StorageMode::kStoreAndCull &&
+ *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);
+ } else {
+ // No more packets can be removed right now.
+ return;
}
}
- return best_index;
}
-void RtpPacketHistory::SetRtt(int64_t rtt_ms) {
- rtc::CritScope cs(&critsect_);
- RTC_DCHECK_GE(rtt_ms, 0);
- rtt_ms_ = rtt_ms;
+std::unique_ptr<RtpPacketToSend> RtpPacketHistory::RemovePacket(
+ StoredPacketIterator packet_it) {
+ // Move the packet out from the StoredPacket container.
+ std::unique_ptr<RtpPacketToSend> rtp_packet =
+ std::move(packet_it->second.packet);
+ // Erase the packet from the map, and capture iterator to the next one.
+ StoredPacketIterator next_it = packet_history_.erase(packet_it);
+
+ // |next_it| now points to the next element, or to the end. If the end,
+ // check if we can wrap around.
+ if (next_it == packet_history_.end()) {
+ next_it = packet_history_.begin();
+ }
+
+ // Update |start_seq_no| to the new oldest item.
+ if (next_it != packet_history_.end()) {
+ start_seqno_ = next_it->first;
+ } else {
+ start_seqno_.reset();
+ }
+
+ return rtp_packet;
+}
+
+RtpPacketHistory::PacketState RtpPacketHistory::StoredPacketToPacketState(
+ const RtpPacketHistory::StoredPacket& stored_packet) {
+ RtpPacketHistory::PacketState state;
+ state.rtp_sequence_number = stored_packet.packet->SequenceNumber();
+ state.send_time_ms = stored_packet.send_time_ms;
+ state.capture_time_ms = stored_packet.packet->capture_time_ms();
+ state.ssrc = stored_packet.packet->Ssrc();
+ state.payload_size = stored_packet.packet->size();
+ state.times_retransmitted = stored_packet.times_retransmitted;
+ return state;
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_packet_history.h b/modules/rtp_rtcp/source/rtp_packet_history.h
index e9d5808..bfcdcc8 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history.h
+++ b/modules/rtp_rtcp/source/rtp_packet_history.h
@@ -11,6 +11,7 @@
#ifndef MODULES_RTP_RTCP_SOURCE_RTP_PACKET_HISTORY_H_
#define MODULES_RTP_RTCP_SOURCE_RTP_PACKET_HISTORY_H_
+#include <map>
#include <memory>
#include <vector>
@@ -27,66 +28,120 @@
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.
+ };
+
+ // Snapshot indicating the state of a packet in the history.
+ struct PacketState {
+ PacketState();
+ PacketState(const PacketState&);
+ ~PacketState();
+
+ uint16_t rtp_sequence_number = 0;
+ rtc::Optional<int64_t> send_time_ms;
+ int64_t capture_time_ms = 0;
+ uint32_t ssrc = 0;
+ size_t payload_size = 0;
+ // Number of times RE-transmitted, ie not including the first transmission.
+ size_t times_retransmitted = 0;
+ };
+
+ // Maximum number of packets we ever allow in the history.
static constexpr size_t kMaxCapacity = 9600;
+ // Don't remove packets within max(1000ms, 3x RTT).
+ static constexpr int64_t kMinPacketDurationMs = 1000;
+ static constexpr int kMinPacketDurationRtt = 3;
+ // With kStoreAndCull, always remove packets after 3x max(1000ms, 3x rtt).
+ static constexpr int kPacketCullingDelayFactor = 3;
+
explicit RtpPacketHistory(Clock* clock);
~RtpPacketHistory();
- void SetStorePacketsStatus(bool enable, uint16_t number_to_store);
- bool StorePackets() const;
-
- void PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
- StorageType type,
- bool sent);
+ // Set/get storage mode. Note that setting the state will clear the history,
+ // even if setting the same state as is currently used.
+ void SetStorePacketsStatus(StorageMode mode, size_t number_to_store);
+ StorageMode GetStorageMode() const;
// Set RTT, used to avoid premature retransmission and to prevent over-writing
// a packet in the history before we are reasonably sure it has been received.
void SetRtt(int64_t rtt_ms);
+ // If |send_time| is set, packet was sent without using pacer, so state will
+ // be set accordingly.
+ void PutRtpPacket(std::unique_ptr<RtpPacketToSend> packet,
+ StorageType type,
+ rtc::Optional<int64_t> send_time_ms);
+
// Gets stored RTP packet corresponding to the input |sequence number|.
- // Returns nullptr if packet is not found.
- // |min_elapsed_time_ms| is the minimum time that must have elapsed since
- // the last time the packet was resent (parameter is ignored if set to zero).
- // If the packet is found but the minimum time has not elapsed, returns
- // nullptr.
+ // Returns nullptr if packet is not found. If |verify_rtt| is true, doesn't
+ // return packet that was (re)sent too recently.
std::unique_ptr<RtpPacketToSend> GetPacketAndSetSendTime(
uint16_t sequence_number,
- int64_t min_elapsed_time_ms,
- bool retransmit);
+ bool verify_rtt);
+ // Similar to GetPacketAndSetSendTime(), but only returns a snapshot of the
+ // current state for packet, and never updates internal state.
+ rtc::Optional<PacketState> GetPacketState(uint16_t sequence_number,
+ bool verify_rtt) const;
+
+ // Get the packet (if any) from the history, with size closest to
+ // |packet_size|. The exact size of the packet is not guaranteed.
std::unique_ptr<RtpPacketToSend> GetBestFittingPacket(
size_t packet_size) const;
- bool HasRtpPacket(uint16_t sequence_number) const;
-
private:
struct StoredPacket {
StoredPacket();
StoredPacket(StoredPacket&&);
StoredPacket& operator=(StoredPacket&&);
~StoredPacket();
- uint16_t sequence_number = 0;
- int64_t send_time = 0;
- StorageType storage_type = kDontRetransmit;
- bool has_been_retransmitted = false;
+ // The time of last transmission, including retransmissions.
+ rtc::Optional<int64_t> send_time_ms;
+
+ // Number of times RE-transmitted, ie excluding the first transmission.
+ size_t times_retransmitted = 0;
+
+ // Storing a packet with |storage_type| = kDontRetransmit indicates this is
+ // only used as temporary storage until sent by the pacer sender.
+ StorageType storage_type = kDontRetransmit;
+
+ // The actual packet.
std::unique_ptr<RtpPacketToSend> packet;
};
- std::unique_ptr<RtpPacketToSend> GetPacket(int index) const
- RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
- void Allocate(size_t number_to_store) RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
- void Free() RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
- bool FindSeqNum(uint16_t sequence_number, int* index) const
- RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
- int FindBestFittingPacket(size_t size) const
- RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
+ using StoredPacketIterator = std::map<uint16_t, StoredPacket>::iterator;
- Clock* clock_;
- rtc::CriticalSection critsect_;
- bool store_ RTC_GUARDED_BY(critsect_);
- size_t prev_index_ RTC_GUARDED_BY(critsect_);
- std::vector<StoredPacket> stored_packets_ RTC_GUARDED_BY(critsect_);
- int64_t rtt_ms_ RTC_GUARDED_BY(critsect_);
+ // Helper method used by GetPacketAndSetSendTime() and GetPacketState() to
+ // check if packet has too recently been sent.
+ bool VerifyRtt(const StoredPacket& packet, int64_t now_ms) const
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
+ void Reset() RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
+ void CullOldPackets(int64_t now_ms) RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
+ // Removes the packet from the history, and context/mapping that has been
+ // stored. Returns the RTP packet instance contained within the StoredPacket.
+ std::unique_ptr<RtpPacketToSend> RemovePacket(StoredPacketIterator packet)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
+ static PacketState StoredPacketToPacketState(
+ const StoredPacket& stored_packet);
+
+ Clock* const clock_;
+ rtc::CriticalSection lock_;
+ size_t number_to_store_ RTC_GUARDED_BY(lock_);
+ StorageMode mode_ RTC_GUARDED_BY(lock_);
+ int64_t rtt_ms_ RTC_GUARDED_BY(lock_);
+
+ // Map from rtp sequence numbers to stored packet.
+ std::map<uint16_t, StoredPacket> packet_history_ RTC_GUARDED_BY(lock_);
+
+ // The earliest packet in the history. This might not be the lowest sequence
+ // number, in case there is a wraparound.
+ rtc::Optional<uint16_t> start_seqno_ RTC_GUARDED_BY(lock_);
+
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtpPacketHistory);
};
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
index cdfd3a0..c7cd180 100644
--- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
@@ -20,11 +20,20 @@
#include "typedefs.h" // NOLINT(build/include)
namespace webrtc {
+namespace {
+// Set a high sequence number so we'll suffer a wrap-around.
+constexpr uint16_t kStartSeqNum = 65534u;
+
+// Utility method for truncating sequence numbers to uint16.
+uint16_t To16u(size_t sequence_number) {
+ return static_cast<uint16_t>(sequence_number & 0xFFFF);
+}
+} // namespace
+
+using StorageMode = RtpPacketHistory::StorageMode;
class RtpPacketHistoryTest : public ::testing::Test {
protected:
- static constexpr uint16_t kSeqNum = 88;
-
RtpPacketHistoryTest() : fake_clock_(123456), hist_(&fake_clock_) {}
SimulatedClock fake_clock_;
@@ -40,256 +49,444 @@
};
TEST_F(RtpPacketHistoryTest, SetStoreStatus) {
- EXPECT_FALSE(hist_.StorePackets());
- hist_.SetStorePacketsStatus(true, 10);
- EXPECT_TRUE(hist_.StorePackets());
- hist_.SetStorePacketsStatus(false, 0);
- EXPECT_FALSE(hist_.StorePackets());
+ 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::kDisabled, 0);
+ EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
+}
+
+TEST_F(RtpPacketHistoryTest, ClearsHistoryAfterSetStoreStatus) {
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ // Store a packet, but with send-time. It should then not be removed.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ rtc::nullopt);
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Changing store status, even to the current one, will clear the history.
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+}
+
+TEST_F(RtpPacketHistoryTest, StartSeqResetAfterReset) {
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
+ // Store a packet, but with send-time. It should then not be removed.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ rtc::nullopt);
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Changing store status, to clear the history.
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Add a new packet.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), kAllowRetransmission,
+ rtc::nullopt);
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
+
+ // Advance time past where packet expires.
+ fake_clock_.AdvanceTimeMilliseconds(
+ RtpPacketHistory::kPacketCullingDelayFactor *
+ RtpPacketHistory::kMinPacketDurationMs);
+
+ // Add one more packet and verify no state left from packet before reset.
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + 2)),
+ kAllowRetransmission, rtc::nullopt);
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
+ EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2), false));
}
TEST_F(RtpPacketHistoryTest, NoStoreStatus) {
- EXPECT_FALSE(hist_.StorePackets());
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+ EXPECT_EQ(StorageMode::kDisabled, hist_.GetStorageMode());
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
// Packet should not be stored.
- EXPECT_FALSE(hist_.GetPacketAndSetSendTime(kSeqNum, 0, false));
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
}
TEST_F(RtpPacketHistoryTest, GetRtpPacket_NotStored) {
- hist_.SetStorePacketsStatus(true, 10);
- EXPECT_FALSE(hist_.GetPacketAndSetSendTime(0, 0, false));
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ EXPECT_FALSE(hist_.GetPacketState(0, false));
}
TEST_F(RtpPacketHistoryTest, PutRtpPacket) {
- hist_.SetStorePacketsStatus(true, 10);
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
- EXPECT_FALSE(hist_.HasRtpPacket(kSeqNum));
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
}
TEST_F(RtpPacketHistoryTest, GetRtpPacket) {
- hist_.SetStorePacketsStatus(true, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
int64_t capture_time_ms = 1;
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
packet->set_capture_time_ms(capture_time_ms);
rtc::CopyOnWriteBuffer buffer = packet->Buffer();
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
std::unique_ptr<RtpPacketToSend> packet_out =
- hist_.GetPacketAndSetSendTime(kSeqNum, 0, false);
+ hist_.GetPacketAndSetSendTime(kStartSeqNum, false);
EXPECT_TRUE(packet_out);
EXPECT_EQ(buffer, packet_out->Buffer());
EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
}
TEST_F(RtpPacketHistoryTest, NoCaptureTime) {
- hist_.SetStorePacketsStatus(true, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
fake_clock_.AdvanceTimeMilliseconds(1);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
packet->set_capture_time_ms(-1);
rtc::CopyOnWriteBuffer buffer = packet->Buffer();
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
std::unique_ptr<RtpPacketToSend> packet_out =
- hist_.GetPacketAndSetSendTime(kSeqNum, 0, false);
+ hist_.GetPacketAndSetSendTime(kStartSeqNum, false);
EXPECT_TRUE(packet_out);
EXPECT_EQ(buffer, packet_out->Buffer());
EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
}
TEST_F(RtpPacketHistoryTest, DontRetransmit) {
- hist_.SetStorePacketsStatus(true, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
rtc::CopyOnWriteBuffer buffer = packet->Buffer();
- hist_.PutRtpPacket(std::move(packet), kDontRetransmit, false);
+ hist_.PutRtpPacket(std::move(packet), kDontRetransmit, rtc::nullopt);
+ // Get the packet and verify data.
std::unique_ptr<RtpPacketToSend> packet_out;
- packet_out = hist_.GetPacketAndSetSendTime(kSeqNum, 0, true);
- EXPECT_FALSE(packet_out);
-
- packet_out = hist_.GetPacketAndSetSendTime(kSeqNum, 0, false);
- EXPECT_TRUE(packet_out);
-
+ packet_out = hist_.GetPacketAndSetSendTime(kStartSeqNum, false);
+ ASSERT_TRUE(packet_out);
EXPECT_EQ(buffer.size(), packet_out->size());
EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
+
+ // Non-retransmittable packets are immediately removed, so getting in again
+ // should fail.
+ EXPECT_FALSE(hist_.GetPacketAndSetSendTime(kStartSeqNum, false));
}
-TEST_F(RtpPacketHistoryTest, MinResendTime) {
+TEST_F(RtpPacketHistoryTest, PacketStateIsCorrect) {
+ const uint32_t kSsrc = 92384762;
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
+ packet->SetSsrc(kSsrc);
+ packet->SetPayloadSize(1234);
+ const size_t packet_size = packet->size();
+
+ hist_.PutRtpPacket(std::move(packet), StorageType::kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ rtc::Optional<RtpPacketHistory::PacketState> state =
+ hist_.GetPacketState(kStartSeqNum, false);
+ ASSERT_TRUE(state);
+ EXPECT_EQ(state->rtp_sequence_number, kStartSeqNum);
+ EXPECT_EQ(state->send_time_ms, fake_clock_.TimeInMilliseconds());
+ EXPECT_EQ(state->capture_time_ms, fake_clock_.TimeInMilliseconds());
+ EXPECT_EQ(state->ssrc, kSsrc);
+ EXPECT_EQ(state->payload_size, packet_size);
+ EXPECT_EQ(state->times_retransmitted, 0u);
+
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum, false));
+
+ state = hist_.GetPacketState(kStartSeqNum, false);
+ ASSERT_TRUE(state);
+ EXPECT_EQ(state->times_retransmitted, 1u);
+}
+
+TEST_F(RtpPacketHistoryTest, MinResendTimeWithPacer) {
static const int64_t kMinRetransmitIntervalMs = 100;
- hist_.SetStorePacketsStatus(true, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetRtt(kMinRetransmitIntervalMs);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
size_t len = packet->size();
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
// First transmission: TimeToSendPacket() call from pacer.
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum, 0, false));
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum, false));
- fake_clock_.AdvanceTimeMilliseconds(kMinRetransmitIntervalMs);
- // Time has elapsed.
- std::unique_ptr<RtpPacketToSend> packet_out =
- hist_.GetPacketAndSetSendTime(kSeqNum, kMinRetransmitIntervalMs, true);
- EXPECT_TRUE(packet_out);
- EXPECT_EQ(len, packet_out->size());
- EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
+ // First retransmission - allow early retransmission.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ // With pacer there's two calls to history:
+ // 1) When the NACK request arrived, use GetPacketState() to see if the
+ // packet is there and verify RTT constraints. Then we use the ssrc
+ // and sequence number to enqueue the retransmission in the pacer
+ // 2) When the pacer determines that it is time to send the packet, it calls
+ // GetPacketAndSetSendTime(). This time we do not need to verify RTT as
+ // has that has already been done.
+ rtc::Optional<RtpPacketHistory::PacketState> packet_state =
+ hist_.GetPacketState(kStartSeqNum, /*verify_rtt=*/true);
+ EXPECT_TRUE(packet_state);
+ EXPECT_EQ(len, packet_state->payload_size);
+ EXPECT_EQ(capture_time_ms, packet_state->capture_time_ms);
+
+ // Retransmission was allowed, next send it from pacer.
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum,
+ /*verify_rtt=*/false));
+
+ // Second retransmission - advance time to just before retransmission OK.
fake_clock_.AdvanceTimeMilliseconds(kMinRetransmitIntervalMs - 1);
- // Time has not elapsed. Packet should be found, but no bytes copied.
- EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
- EXPECT_FALSE(
- hist_.GetPacketAndSetSendTime(kSeqNum, kMinRetransmitIntervalMs, true));
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, /*verify_rtt=*/true));
+
+ // Advance time to just after retransmission OK.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, /*verify_rtt=*/true));
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum, false));
}
-TEST_F(RtpPacketHistoryTest, EarlyFirstResend) {
+TEST_F(RtpPacketHistoryTest, MinResendTimeWithoutPacer) {
static const int64_t kMinRetransmitIntervalMs = 100;
- hist_.SetStorePacketsStatus(true, 10);
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+ hist_.SetRtt(kMinRetransmitIntervalMs);
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum);
- rtc::CopyOnWriteBuffer buffer = packet->Buffer();
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
+ size_t len = packet->size();
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
- // First transmission: TimeToSendPacket() call from pacer.
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum, 0, false));
+ // First retransmission - allow early retransmission.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ packet = hist_.GetPacketAndSetSendTime(kStartSeqNum, true);
+ EXPECT_TRUE(packet);
+ EXPECT_EQ(len, packet->size());
+ EXPECT_EQ(capture_time_ms, packet->capture_time_ms());
+ // Second retransmission - advance time to just before retransmission OK.
fake_clock_.AdvanceTimeMilliseconds(kMinRetransmitIntervalMs - 1);
- // Time has not elapsed, but this is the first retransmission request so
- // allow anyway.
- std::unique_ptr<RtpPacketToSend> packet_out =
- hist_.GetPacketAndSetSendTime(kSeqNum, kMinRetransmitIntervalMs, true);
- EXPECT_TRUE(packet_out);
- EXPECT_EQ(buffer, packet_out->Buffer());
- EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms());
+ EXPECT_FALSE(hist_.GetPacketAndSetSendTime(kStartSeqNum, true));
- fake_clock_.AdvanceTimeMilliseconds(kMinRetransmitIntervalMs - 1);
- // Time has not elapsed. Packet should be found, but no bytes copied.
- EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
- EXPECT_FALSE(
- hist_.GetPacketAndSetSendTime(kSeqNum, kMinRetransmitIntervalMs, true));
+ // Advance time to just after retransmission OK.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kStartSeqNum, true));
}
-TEST_F(RtpPacketHistoryTest, DynamicExpansion) {
- hist_.SetStorePacketsStatus(true, 10);
+TEST_F(RtpPacketHistoryTest, RemovesOldestSentPacketWhenAtMaxSize) {
+ const size_t kMaxNumPackets = 10;
+ hist_.SetStorePacketsStatus(StorageMode::kStore, kMaxNumPackets);
- // Add 4 packets, and then send them.
- for (int i = 0; i < 4; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- }
- for (int i = 0; i < 4; ++i) {
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, 100, false));
- }
- fake_clock_.AdvanceTimeMilliseconds(33);
+ // History does not allow removing packets within kMinPacketDurationMs,
+ // so in order to test capacity, make sure insertion spans this time.
+ const int64_t kPacketIntervalMs =
+ RtpPacketHistory::kMinPacketDurationMs / kMaxNumPackets;
- // Add 16 packets, and then send them. History should expand to make this
- // work.
- for (int i = 4; i < 20; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- }
- for (int i = 4; i < 20; ++i) {
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, 100, false));
+ // Add packets until the buffer is full.
+ for (size_t i = 0; i < kMaxNumPackets; ++i) {
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum + i);
+ // Immediate mark packet as sent.
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ fake_clock_.AdvanceTimeMilliseconds(kPacketIntervalMs);
}
- fake_clock_.AdvanceTimeMilliseconds(100);
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
- // Retransmit last 16 packets.
- for (int i = 4; i < 20; ++i) {
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, 100, false));
- }
+ // History is full, oldest one should be overwritten.
+ std::unique_ptr<RtpPacketToSend> packet =
+ CreateRtpPacket(To16u(kStartSeqNum + kMaxNumPackets));
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ // Oldest packet should be gone, but packet after than one still present.
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
}
-TEST_F(RtpPacketHistoryTest, FullExpansion) {
- static const int kSendSidePacketHistorySize = 600;
- hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
- for (size_t i = 0; i < RtpPacketHistory::kMaxCapacity + 1; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
+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,
+ RtpPacketHistory::kMaxCapacity);
+
+ // Add packets until the buffer is full.
+ for (size_t i = 0; i < kMaxNumPackets; ++i) {
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum + i);
+ // Don't mark packets as sent, preventing them from being removed.
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, rtc::nullopt);
}
- fake_clock_.AdvanceTimeMilliseconds(100);
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
- // Retransmit all packets currently in buffer.
- for (size_t i = 1; i < RtpPacketHistory::kMaxCapacity + 1; ++i) {
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, 100, false));
- }
+ // History is full, oldest one should be overwritten.
+ std::unique_ptr<RtpPacketToSend> packet =
+ CreateRtpPacket(To16u(kStartSeqNum + kMaxNumPackets));
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ // Oldest packet should be gone, but packet after than one still present.
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
}
-TEST_F(RtpPacketHistoryTest, DontExpandIfPacketIsOldEnough) {
- const size_t kSendSidePacketHistorySize = 600;
- const int64_t kRttMs = 334;
- hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
+TEST_F(RtpPacketHistoryTest, DontRemoveUnsentPackets) {
+ const size_t kMaxNumPackets = 10;
+ hist_.SetStorePacketsStatus(StorageMode::kStore, kMaxNumPackets);
+
+ // Add packets until the buffer is full.
+ for (size_t i = 0; i < kMaxNumPackets; ++i) {
+ // Mark packets as unsent.
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + i)),
+ kAllowRetransmission, rtc::nullopt);
+ }
+ fake_clock_.AdvanceTimeMilliseconds(RtpPacketHistory::kMinPacketDurationMs);
+
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // History is full, but old packets not sent, so allow expansion.
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + kMaxNumPackets)),
+ kAllowRetransmission, fake_clock_.TimeInMilliseconds());
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Set all packet as sent and advance time past min packet duration time,
+ // otherwise packets till still be prevented from being removed.
+ for (size_t i = 0; i <= kMaxNumPackets; ++i) {
+ EXPECT_TRUE(hist_.GetPacketAndSetSendTime(To16u(kStartSeqNum + i), false));
+ }
+ fake_clock_.AdvanceTimeMilliseconds(RtpPacketHistory::kMinPacketDurationMs);
+ // Add a new packet, this means the two oldest ones will be culled.
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + kMaxNumPackets + 1)),
+ kAllowRetransmission, fake_clock_.TimeInMilliseconds());
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum + 1, false));
+ EXPECT_TRUE(hist_.GetPacketState(To16u(kStartSeqNum + 2), false));
+}
+
+TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPackets) {
+ // Set size to remove old packets as soon as possible.
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 1);
+
+ // Add a packet, marked as send, and advance time to just before removal time.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ fake_clock_.AdvanceTimeMilliseconds(RtpPacketHistory::kMinPacketDurationMs -
+ 1);
+
+ // Add a new packet to trigger culling.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Advance time to where packet will be eligible for removal and try again.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + 2)),
+ kAllowRetransmission, fake_clock_.TimeInMilliseconds());
+ // First packet should no be gone, but next one still there.
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
+}
+
+TEST_F(RtpPacketHistoryTest, DontRemoveTooRecentlyTransmittedPacketsHighRtt) {
+ const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
+ const int64_t kPacketTimeoutMs =
+ kRttMs * RtpPacketHistory::kMinPacketDurationRtt;
+
+ // Set size to remove old packets as soon as possible.
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 1);
hist_.SetRtt(kRttMs);
- // Fill up the buffer with packets.
- for (size_t i = 0; i < kSendSidePacketHistorySize; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, 100, false));
- }
+ // Add a packet, marked as send, and advance time to just before removal time.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ fake_clock_.AdvanceTimeMilliseconds(kPacketTimeoutMs - 1);
- // Move clock forward past expiration time.
- fake_clock_.AdvanceTimeMilliseconds(kRttMs * 3 + 1);
+ // Add a new packet to trigger culling.
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
- // Insert a new packet and check that the old one for this index has been
- // overwritten.
- std::unique_ptr<RtpPacketToSend> packet =
- CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
- EXPECT_FALSE(hist_.HasRtpPacket(kSeqNum));
+ // Advance time to where packet will be eligible for removal and try again.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ hist_.PutRtpPacket(CreateRtpPacket(To16u(kStartSeqNum + 2)),
+ kAllowRetransmission, fake_clock_.TimeInMilliseconds());
+ // First packet should no be gone, but next one still there.
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum + 1, false));
}
-TEST_F(RtpPacketHistoryTest, ExpandIfPacketTooRecentlyTransmitted) {
- const size_t kSendSidePacketHistorySize = 600;
- const int64_t kRttMs = 334;
- hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
- hist_.SetRtt(kRttMs);
+TEST_F(RtpPacketHistoryTest, RemovesOldWithCulling) {
+ const size_t kMaxNumPackets = 10;
+ // Enable culling. Even without feedback, this can trigger early removal.
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
- // Fill up the buffer with packets.
- for (size_t i = 0; i < kSendSidePacketHistorySize; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, kRttMs, false));
- }
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
- // Move clock forward to just before expiration time.
- fake_clock_.AdvanceTimeMilliseconds(kRttMs * 3);
+ int64_t kMaxPacketDurationMs = RtpPacketHistory::kMinPacketDurationMs *
+ RtpPacketHistory::kPacketCullingDelayFactor;
+ fake_clock_.AdvanceTimeMilliseconds(kMaxPacketDurationMs - 1);
- // Insert a new packet and verify that the old one for this index still
- // exists - ie the buffer has been expanded.
- std::unique_ptr<RtpPacketToSend> packet =
- CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
- EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Advance to where packet can be culled, even if buffer is not full.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
}
-TEST_F(RtpPacketHistoryTest, ExpandIfPacketTooRecentlyTransmittedOnFastLink) {
- const size_t kSendSidePacketHistorySize = 600;
- const int64_t kRttMs = 5;
- hist_.SetStorePacketsStatus(true, kSendSidePacketHistorySize);
+TEST_F(RtpPacketHistoryTest, RemovesOldWithCullingHighRtt) {
+ const size_t kMaxNumPackets = 10;
+ const int64_t kRttMs = RtpPacketHistory::kMinPacketDurationMs * 2;
+ // Enable culling. Even without feedback, this can trigger early removal.
+ hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, kMaxNumPackets);
hist_.SetRtt(kRttMs);
- // Fill up the buffer with packets.
- for (size_t i = 0; i < kSendSidePacketHistorySize; ++i) {
- std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kSeqNum + i);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, false);
- EXPECT_TRUE(hist_.GetPacketAndSetSendTime(kSeqNum + i, kRttMs, false));
- }
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
- // Move clock forward after expiration time based on RTT, but before
- // expiration time based on absolute time.
- fake_clock_.AdvanceTimeMilliseconds(999);
+ int64_t kMaxPacketDurationMs = kRttMs *
+ RtpPacketHistory::kMinPacketDurationRtt *
+ RtpPacketHistory::kPacketCullingDelayFactor;
+ fake_clock_.AdvanceTimeMilliseconds(kMaxPacketDurationMs - 1);
- // Insert a new packet and verify that the old one for this index still
- // exists - ie the buffer has been expanded.
- std::unique_ptr<RtpPacketToSend> packet =
- CreateRtpPacket(kSeqNum + kSendSidePacketHistorySize);
- hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, true);
- EXPECT_TRUE(hist_.HasRtpPacket(kSeqNum));
+ // First packet should still be there.
+ EXPECT_TRUE(hist_.GetPacketState(kStartSeqNum, false));
+
+ // Advance to where packet can be culled, even if buffer is not full.
+ fake_clock_.AdvanceTimeMilliseconds(1);
+ hist_.PutRtpPacket(CreateRtpPacket(kStartSeqNum + 1), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ EXPECT_FALSE(hist_.GetPacketState(kStartSeqNum, false));
+}
+
+TEST_F(RtpPacketHistoryTest, GetBestFittingPacket) {
+ const size_t kTargetSize = 500;
+ hist_.SetStorePacketsStatus(StorageMode::kStore, 10);
+
+ // Add three packets of various sizes.
+ std::unique_ptr<RtpPacketToSend> packet = CreateRtpPacket(kStartSeqNum);
+ packet->SetPayloadSize(kTargetSize);
+ const size_t target_packet_size = packet->size();
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ packet = CreateRtpPacket(kStartSeqNum + 1);
+ packet->SetPayloadSize(kTargetSize - 1);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+ packet = CreateRtpPacket(To16u(kStartSeqNum + 2));
+ packet->SetPayloadSize(kTargetSize + 1);
+ hist_.PutRtpPacket(std::move(packet), kAllowRetransmission,
+ fake_clock_.TimeInMilliseconds());
+
+ EXPECT_EQ(target_packet_size,
+ hist_.GetBestFittingPacket(target_packet_size)->size());
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index f8ef130..46ffbd1 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -153,7 +153,8 @@
// be found when paced.
if (flexfec_sender) {
flexfec_packet_history_.SetStorePacketsStatus(
- true, kMinFlexfecPacketsToStoreForPacing);
+ RtpPacketHistory::StorageMode::kStore,
+ kMinFlexfecPacketsToStoreForPacing);
}
}
@@ -600,43 +601,60 @@
}
void RTPSender::SetStorePacketsStatus(bool enable, uint16_t number_to_store) {
- packet_history_.SetStorePacketsStatus(enable, number_to_store);
+ RtpPacketHistory::StorageMode mode =
+ enable ? RtpPacketHistory::StorageMode::kStore
+ : RtpPacketHistory::StorageMode::kDisabled;
+ packet_history_.SetStorePacketsStatus(mode, number_to_store);
}
bool RTPSender::StorePackets() const {
- return packet_history_.StorePackets();
+ return packet_history_.GetStorageMode() !=
+ RtpPacketHistory::StorageMode::kDisabled;
}
-int32_t RTPSender::ReSendPacket(uint16_t packet_id, int64_t min_resend_time) {
- std::unique_ptr<RtpPacketToSend> packet =
- packet_history_.GetPacketAndSetSendTime(packet_id, min_resend_time, true);
- if (!packet) {
+int32_t RTPSender::ReSendPacket(uint16_t packet_id) {
+ // Try to find packet in RTP packet history. Also verify RTT here, so that we
+ // don't retransmit too often.
+ rtc::Optional<RtpPacketHistory::PacketState> stored_packet =
+ packet_history_.GetPacketState(packet_id, true);
+ if (!stored_packet) {
// Packet not found.
return 0;
}
+ const int32_t packet_size = static_cast<int32_t>(stored_packet->payload_size);
+
+ RTC_DCHECK(retransmission_rate_limiter_);
// Check if we're overusing retransmission bitrate.
// TODO(sprang): Add histograms for nack success or failure reasons.
- RTC_DCHECK(retransmission_rate_limiter_);
- if (!retransmission_rate_limiter_->TryUseRate(packet->size()))
+ if (!retransmission_rate_limiter_->TryUseRate(packet_size)) {
return -1;
+ }
if (paced_sender_) {
// Convert from TickTime to Clock since capture_time_ms is based on
// TickTime.
int64_t corrected_capture_tims_ms =
- packet->capture_time_ms() + clock_delta_ms_;
- paced_sender_->InsertPacket(RtpPacketSender::kNormalPriority,
- packet->Ssrc(), packet->SequenceNumber(),
- corrected_capture_tims_ms,
- packet->payload_size(), true);
+ stored_packet->capture_time_ms + clock_delta_ms_;
+ paced_sender_->InsertPacket(
+ RtpPacketSender::kNormalPriority, stored_packet->ssrc,
+ stored_packet->rtp_sequence_number, corrected_capture_tims_ms,
+ stored_packet->payload_size, true);
- return packet->size();
+ return packet_size;
}
- bool rtx = (RtxStatus() & kRtxRetransmitted) > 0;
- int32_t packet_size = static_cast<int32_t>(packet->size());
+
+ std::unique_ptr<RtpPacketToSend> packet =
+ packet_history_.GetPacketAndSetSendTime(packet_id, true);
+ if (!packet) {
+ // Packet could theoretically time out between the first check and this one.
+ return 0;
+ }
+
+ const bool rtx = (RtxStatus() & kRtxRetransmitted) > 0;
if (!PrepareAndSendPacket(std::move(packet), rtx, true, PacedPacketInfo()))
return -1;
+
return packet_size;
}
@@ -684,8 +702,9 @@
TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"),
"RTPSender::OnReceivedNACK", "num_seqnum",
nack_sequence_numbers.size(), "avg_rtt", avg_rtt);
+ packet_history_.SetRtt(5 + avg_rtt);
for (uint16_t seq_no : nack_sequence_numbers) {
- const int32_t bytes_sent = ReSendPacket(seq_no, 5 + avg_rtt);
+ const int32_t bytes_sent = ReSendPacket(seq_no);
if (bytes_sent < 0) {
// Failed to send one Sequence number. Give up the rest in this nack.
RTC_LOG(LS_WARNING) << "Failed resending RTP packet " << seq_no
@@ -710,12 +729,13 @@
return true;
std::unique_ptr<RtpPacketToSend> packet;
+ // No need to verify RTT here, it has already been checked before putting the
+ // packet into the pacer. But _do_ update the send time.
if (ssrc == SSRC()) {
- packet = packet_history_.GetPacketAndSetSendTime(sequence_number, 0,
- retransmission);
+ packet = packet_history_.GetPacketAndSetSendTime(sequence_number, false);
} else if (ssrc == FlexfecSsrc()) {
- packet = flexfec_packet_history_.GetPacketAndSetSendTime(sequence_number, 0,
- retransmission);
+ packet =
+ flexfec_packet_history_.GetPacketAndSetSendTime(sequence_number, false);
}
if (!packet) {
@@ -894,9 +914,10 @@
if (ssrc == flexfec_ssrc) {
// Store FlexFEC packets in the history here, so they can be found
// when the pacer calls TimeToSendPacket.
- flexfec_packet_history_.PutRtpPacket(std::move(packet), storage, false);
+ flexfec_packet_history_.PutRtpPacket(std::move(packet), storage,
+ rtc::nullopt);
} else {
- packet_history_.PutRtpPacket(std::move(packet), storage, false);
+ packet_history_.PutRtpPacket(std::move(packet), storage, rtc::nullopt);
}
paced_sender_->InsertPacket(priority, ssrc, seq_no, corrected_time_ms,
@@ -937,7 +958,7 @@
// packet history (even if send failed).
if (storage == kAllowRetransmission) {
RTC_DCHECK_EQ(ssrc, SSRC());
- packet_history_.PutRtpPacket(std::move(packet), storage, true);
+ packet_history_.PutRtpPacket(std::move(packet), storage, now_ms);
}
return sent;
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 65407ba..9977421 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -134,7 +134,7 @@
bool StorePackets() const;
- int32_t ReSendPacket(uint16_t packet_id, int64_t min_resend_time = 0);
+ int32_t ReSendPacket(uint16_t packet_id);
// Feedback to decide when to stop sending playout delay.
void OnReceivedRtcpReportBlocks(const ReportBlockList& report_blocks);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index d64aaa8..c659662 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -1401,7 +1401,7 @@
// Retransmit a frame.
uint16_t seqno = rtp_sender_->SequenceNumber() - 1;
- rtp_sender_->ReSendPacket(seqno, 0);
+ rtp_sender_->ReSendPacket(seqno);
expected.transmitted.payload_bytes = 12;
expected.transmitted.header_bytes = 24;
expected.transmitted.packets = 2;