From 122085543050977affa1a38041a33efab6f6609b Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 13 Jun 2022 10:04:43 +0200 Subject: [PATCH] In RemoteEstimatorProxy use Timestamp type to assemble rtcp::TransportFeedback Bug: webrtc:13757 Change-Id: I668d9e61d82b454a6884eff223804afc882d86a3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264900 Reviewed-by: Per Kjellander Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#37192} --- api/rtp_headers.h | 9 - modules/remote_bitrate_estimator/BUILD.gn | 2 + .../packet_arrival_map.cc | 55 ++-- .../packet_arrival_map.h | 17 +- .../packet_arrival_map_test.cc | 80 ++--- .../remote_estimator_proxy.cc | 88 +++-- .../remote_estimator_proxy.h | 15 +- .../remote_estimator_proxy_unittest.cc | 307 +++++++++--------- 8 files changed, 306 insertions(+), 267 deletions(-) diff --git a/api/rtp_headers.h b/api/rtp_headers.h index cf3d909499..a640eb7d38 100644 --- a/api/rtp_headers.h +++ b/api/rtp_headers.h @@ -103,15 +103,6 @@ struct RTPHeaderExtension { (1 << kAbsSendTimeFraction)); } - TimeDelta GetAbsoluteSendTimeDelta(uint32_t previous_sendtime) const { - RTC_DCHECK(hasAbsoluteSendTime); - RTC_DCHECK(absoluteSendTime < (1ul << 24)); - RTC_DCHECK(previous_sendtime < (1ul << 24)); - int32_t delta = - static_cast((absoluteSendTime - previous_sendtime) << 8) >> 8; - return TimeDelta::Micros((delta * 1000000ll) / (1 << kAbsSendTimeFraction)); - } - bool hasTransmissionTimeOffset; int32_t transmissionTimeOffset; bool hasAbsoluteSendTime; diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn index 1cc69ae05b..9e2352fd61 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn @@ -128,6 +128,8 @@ if (rtc_include_tests) { "../../api/transport:network_control", "../../api/units:data_rate", "../../api/units:data_size", + "../../api/units:time_delta", + "../../api/units:timestamp", "../../rtc_base", "../../rtc_base:checks", "../../rtc_base:random", diff --git a/modules/remote_bitrate_estimator/packet_arrival_map.cc b/modules/remote_bitrate_estimator/packet_arrival_map.cc index 72696f6c80..deeb208c15 100644 --- a/modules/remote_bitrate_estimator/packet_arrival_map.cc +++ b/modules/remote_bitrate_estimator/packet_arrival_map.cc @@ -18,19 +18,19 @@ namespace webrtc { constexpr size_t PacketArrivalTimeMap::kMaxNumberOfPackets; void PacketArrivalTimeMap::AddPacket(int64_t sequence_number, - int64_t arrival_time_ms) { + Timestamp arrival_time) { if (!has_seen_packet_) { // First packet. has_seen_packet_ = true; begin_sequence_number_ = sequence_number; - arrival_times.push_back(arrival_time_ms); + arrival_times_.push_back(arrival_time); return; } int64_t pos = sequence_number - begin_sequence_number_; - if (pos >= 0 && pos < static_cast(arrival_times.size())) { + if (pos >= 0 && pos < static_cast(arrival_times_.size())) { // The packet is within the buffer - no need to expand it. - arrival_times[pos] = arrival_time_ms; + arrival_times_[pos] = arrival_time; return; } @@ -38,14 +38,15 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number, // The packet goes before the current buffer. Expand to add packet, but only // if it fits within kMaxNumberOfPackets. size_t missing_packets = -pos; - if (missing_packets + arrival_times.size() > kMaxNumberOfPackets) { + if (missing_packets + arrival_times_.size() > kMaxNumberOfPackets) { // Don't expand the buffer further, as that would remove newly received // packets. return; } - arrival_times.insert(arrival_times.begin(), missing_packets, 0); - arrival_times[0] = arrival_time_ms; + arrival_times_.insert(arrival_times_.begin(), missing_packets, + Timestamp::Zero()); + arrival_times_[0] = arrival_time; begin_sequence_number_ = sequence_number; return; } @@ -55,20 +56,20 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number, if (static_cast(pos) >= kMaxNumberOfPackets) { // The buffer grows too large - old packets have to be removed. size_t packets_to_remove = pos - kMaxNumberOfPackets + 1; - if (packets_to_remove >= arrival_times.size()) { - arrival_times.clear(); + if (packets_to_remove >= arrival_times_.size()) { + arrival_times_.clear(); begin_sequence_number_ = sequence_number; pos = 0; } else { // Also trim the buffer to remove leading non-received packets, to // ensure that the buffer only spans received packets. - while (packets_to_remove < arrival_times.size() && - arrival_times[packets_to_remove] == 0) { + while (packets_to_remove < arrival_times_.size() && + arrival_times_[packets_to_remove] == Timestamp::Zero()) { ++packets_to_remove; } - arrival_times.erase(arrival_times.begin(), - arrival_times.begin() + packets_to_remove); + arrival_times_.erase(arrival_times_.begin(), + arrival_times_.begin() + packets_to_remove); begin_sequence_number_ += packets_to_remove; pos -= packets_to_remove; RTC_DCHECK_GE(pos, 0); @@ -77,28 +78,29 @@ void PacketArrivalTimeMap::AddPacket(int64_t sequence_number, // Packets can be received out-of-order. If this isn't the next expected // packet, add enough placeholders to fill the gap. - size_t missing_gap_packets = pos - arrival_times.size(); + size_t missing_gap_packets = pos - arrival_times_.size(); if (missing_gap_packets > 0) { - arrival_times.insert(arrival_times.end(), missing_gap_packets, 0); + arrival_times_.insert(arrival_times_.end(), missing_gap_packets, + Timestamp::Zero()); } - RTC_DCHECK_EQ(arrival_times.size(), pos); - arrival_times.push_back(arrival_time_ms); - RTC_DCHECK_LE(arrival_times.size(), kMaxNumberOfPackets); + RTC_DCHECK_EQ(arrival_times_.size(), pos); + arrival_times_.push_back(arrival_time); + RTC_DCHECK_LE(arrival_times_.size(), kMaxNumberOfPackets); } void PacketArrivalTimeMap::RemoveOldPackets(int64_t sequence_number, - int64_t arrival_time_limit) { - while (!arrival_times.empty() && begin_sequence_number_ < sequence_number && - arrival_times.front() <= arrival_time_limit) { - arrival_times.pop_front(); + Timestamp arrival_time_limit) { + while (!arrival_times_.empty() && begin_sequence_number_ < sequence_number && + arrival_times_.front() <= arrival_time_limit) { + arrival_times_.pop_front(); ++begin_sequence_number_; } } bool PacketArrivalTimeMap::has_received(int64_t sequence_number) const { int64_t pos = sequence_number - begin_sequence_number_; - if (pos >= 0 && pos < static_cast(arrival_times.size()) && - arrival_times[pos] != 0) { + if (pos >= 0 && pos < static_cast(arrival_times_.size()) && + arrival_times_[pos] != Timestamp::Zero()) { return true; } return false; @@ -108,9 +110,10 @@ void PacketArrivalTimeMap::EraseTo(int64_t sequence_number) { if (sequence_number > begin_sequence_number_) { size_t count = std::min(static_cast(sequence_number - begin_sequence_number_), - arrival_times.size()); + arrival_times_.size()); - arrival_times.erase(arrival_times.begin(), arrival_times.begin() + count); + arrival_times_.erase(arrival_times_.begin(), + arrival_times_.begin() + count); begin_sequence_number_ += count; } } diff --git a/modules/remote_bitrate_estimator/packet_arrival_map.h b/modules/remote_bitrate_estimator/packet_arrival_map.h index 10659e0f65..8bda4a86e1 100644 --- a/modules/remote_bitrate_estimator/packet_arrival_map.h +++ b/modules/remote_bitrate_estimator/packet_arrival_map.h @@ -14,6 +14,7 @@ #include #include +#include "api/units/timestamp.h" #include "rtc_base/checks.h" namespace webrtc { @@ -43,15 +44,15 @@ class PacketArrivalTimeMap { // Returns the sequence number of the element just after the map, i.e. the // sequence number that an `end()` iterator would represent. int64_t end_sequence_number() const { - return begin_sequence_number_ + arrival_times.size(); + return begin_sequence_number_ + arrival_times_.size(); } // Returns an element by `sequence_number`, which must be valid, i.e. // between [begin_sequence_number, end_sequence_number). - int64_t get(int64_t sequence_number) { + Timestamp get(int64_t sequence_number) { int64_t pos = sequence_number - begin_sequence_number_; - RTC_DCHECK(pos >= 0 && pos < static_cast(arrival_times.size())); - return arrival_times[pos]; + RTC_DCHECK(pos >= 0 && pos < static_cast(arrival_times_.size())); + return arrival_times_[pos]; } // Clamps `sequence_number` between [begin_sequence_number, @@ -63,19 +64,19 @@ class PacketArrivalTimeMap { // Records the fact that a packet with `sequence_number` arrived at // `arrival_time_ms`. - void AddPacket(int64_t sequence_number, int64_t arrival_time_ms); + void AddPacket(int64_t sequence_number, Timestamp arrival_time); // Removes packets from the beginning of the map as long as they are received // before `sequence_number` and with an age older than `arrival_time_limit` - void RemoveOldPackets(int64_t sequence_number, int64_t arrival_time_limit); + void RemoveOldPackets(int64_t sequence_number, Timestamp arrival_time_limit); private: // Deque representing unwrapped sequence number -> time, where the index + // `begin_sequence_number_` represents the packet's sequence number. - std::deque arrival_times; + std::deque arrival_times_; // The unwrapped sequence number for the first element in - // `arrival_times`. + // `arrival_times_`. int64_t begin_sequence_number_ = 0; // Indicates if this map has had any packet added to it. The first packet diff --git a/modules/remote_bitrate_estimator/packet_arrival_map_test.cc b/modules/remote_bitrate_estimator/packet_arrival_map_test.cc index afc7038832..de506386ba 100644 --- a/modules/remote_bitrate_estimator/packet_arrival_map_test.cc +++ b/modules/remote_bitrate_estimator/packet_arrival_map_test.cc @@ -27,7 +27,7 @@ TEST(PacketArrivalMapTest, IsConsistentWhenEmpty) { TEST(PacketArrivalMapTest, InsertsFirstItemIntoMap) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); + map.AddPacket(42, Timestamp::Millis(10)); EXPECT_EQ(map.begin_sequence_number(), 42); EXPECT_EQ(map.end_sequence_number(), 43); @@ -43,8 +43,8 @@ TEST(PacketArrivalMapTest, InsertsFirstItemIntoMap) { TEST(PacketArrivalMapTest, InsertsWithGaps) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(45, 11); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(45, Timestamp::Millis(11)); EXPECT_EQ(map.begin_sequence_number(), 42); EXPECT_EQ(map.end_sequence_number(), 46); @@ -55,10 +55,10 @@ TEST(PacketArrivalMapTest, InsertsWithGaps) { EXPECT_TRUE(map.has_received(45)); EXPECT_FALSE(map.has_received(46)); - EXPECT_EQ(map.get(42), 10); - EXPECT_EQ(map.get(43), 0); - EXPECT_EQ(map.get(44), 0); - EXPECT_EQ(map.get(45), 11); + EXPECT_EQ(map.get(42), Timestamp::Millis(10)); + EXPECT_EQ(map.get(43), Timestamp::Zero()); + EXPECT_EQ(map.get(44), Timestamp::Zero()); + EXPECT_EQ(map.get(45), Timestamp::Millis(11)); EXPECT_EQ(map.clamp(-100), 42); EXPECT_EQ(map.clamp(44), 44); @@ -68,11 +68,11 @@ TEST(PacketArrivalMapTest, InsertsWithGaps) { TEST(PacketArrivalMapTest, InsertsWithinBuffer) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(45, 11); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(45, Timestamp::Millis(11)); - map.AddPacket(43, 12); - map.AddPacket(44, 13); + map.AddPacket(43, Timestamp::Millis(12)); + map.AddPacket(44, Timestamp::Millis(13)); EXPECT_EQ(map.begin_sequence_number(), 42); EXPECT_EQ(map.end_sequence_number(), 46); @@ -84,21 +84,21 @@ TEST(PacketArrivalMapTest, InsertsWithinBuffer) { EXPECT_TRUE(map.has_received(45)); EXPECT_FALSE(map.has_received(46)); - EXPECT_EQ(map.get(42), 10); - EXPECT_EQ(map.get(43), 12); - EXPECT_EQ(map.get(44), 13); - EXPECT_EQ(map.get(45), 11); + EXPECT_EQ(map.get(42), Timestamp::Millis(10)); + EXPECT_EQ(map.get(43), Timestamp::Millis(12)); + EXPECT_EQ(map.get(44), Timestamp::Millis(13)); + EXPECT_EQ(map.get(45), Timestamp::Millis(11)); } TEST(PacketArrivalMapTest, GrowsBufferAndRemoveOld) { PacketArrivalTimeMap map; constexpr int64_t kLargeSeq = 42 + PacketArrivalTimeMap::kMaxNumberOfPackets; - map.AddPacket(42, 10); - map.AddPacket(43, 11); - map.AddPacket(44, 12); - map.AddPacket(45, 13); - map.AddPacket(kLargeSeq, 12); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(43, Timestamp::Millis(11)); + map.AddPacket(44, Timestamp::Millis(12)); + map.AddPacket(45, Timestamp::Millis(13)); + map.AddPacket(kLargeSeq, Timestamp::Millis(12)); EXPECT_EQ(map.begin_sequence_number(), 43); EXPECT_EQ(map.end_sequence_number(), kLargeSeq + 1); @@ -120,10 +120,10 @@ TEST(PacketArrivalMapTest, GrowsBufferAndRemoveOldTrimsBeginning) { PacketArrivalTimeMap map; constexpr int64_t kLargeSeq = 42 + PacketArrivalTimeMap::kMaxNumberOfPackets; - map.AddPacket(42, 10); + map.AddPacket(42, Timestamp::Millis(10)); // Missing: 43, 44 - map.AddPacket(45, 13); - map.AddPacket(kLargeSeq, 12); + map.AddPacket(45, Timestamp::Millis(13)); + map.AddPacket(kLargeSeq, Timestamp::Millis(12)); EXPECT_EQ(map.begin_sequence_number(), 45); EXPECT_EQ(map.end_sequence_number(), kLargeSeq + 1); @@ -140,8 +140,8 @@ TEST(PacketArrivalMapTest, SequenceNumberJumpsDeletesAll) { constexpr int64_t kLargeSeq = 42 + 2 * PacketArrivalTimeMap::kMaxNumberOfPackets; - map.AddPacket(42, 10); - map.AddPacket(kLargeSeq, 12); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(kLargeSeq, Timestamp::Millis(12)); EXPECT_EQ(map.begin_sequence_number(), kLargeSeq); EXPECT_EQ(map.end_sequence_number(), kLargeSeq + 1); @@ -154,8 +154,8 @@ TEST(PacketArrivalMapTest, SequenceNumberJumpsDeletesAll) { TEST(PacketArrivalMapTest, ExpandsBeforeBeginning) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(-1000, 13); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(-1000, Timestamp::Millis(13)); EXPECT_EQ(map.begin_sequence_number(), -1000); EXPECT_EQ(map.end_sequence_number(), 43); @@ -170,10 +170,10 @@ TEST(PacketArrivalMapTest, ExpandsBeforeBeginning) { TEST(PacketArrivalMapTest, ExpandingBeforeBeginningKeepsReceived) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); + map.AddPacket(42, Timestamp::Millis(10)); constexpr int64_t kSmallSeq = static_cast(42) - 2 * PacketArrivalTimeMap::kMaxNumberOfPackets; - map.AddPacket(kSmallSeq, 13); + map.AddPacket(kSmallSeq, Timestamp::Millis(13)); EXPECT_EQ(map.begin_sequence_number(), 42); EXPECT_EQ(map.end_sequence_number(), 43); @@ -182,10 +182,10 @@ TEST(PacketArrivalMapTest, ExpandingBeforeBeginningKeepsReceived) { TEST(PacketArrivalMapTest, ErasesToRemoveElements) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(43, 11); - map.AddPacket(44, 12); - map.AddPacket(45, 13); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(43, Timestamp::Millis(11)); + map.AddPacket(44, Timestamp::Millis(12)); + map.AddPacket(45, Timestamp::Millis(13)); map.EraseTo(44); @@ -210,8 +210,8 @@ TEST(PacketArrivalMapTest, ErasesInEmptyMap) { TEST(PacketArrivalMapTest, IsTolerantToWrongArgumentsForErase) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(43, 11); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(43, Timestamp::Millis(11)); map.EraseTo(1); @@ -227,14 +227,14 @@ TEST(PacketArrivalMapTest, IsTolerantToWrongArgumentsForErase) { TEST(PacketArrivalMapTest, EraseAllRemembersBeginningSeqNbr) { PacketArrivalTimeMap map; - map.AddPacket(42, 10); - map.AddPacket(43, 11); - map.AddPacket(44, 12); - map.AddPacket(45, 13); + map.AddPacket(42, Timestamp::Millis(10)); + map.AddPacket(43, Timestamp::Millis(11)); + map.AddPacket(44, Timestamp::Millis(12)); + map.AddPacket(45, Timestamp::Millis(13)); map.EraseTo(46); - map.AddPacket(50, 10); + map.AddPacket(50, Timestamp::Millis(10)); EXPECT_EQ(map.begin_sequence_number(), 46); EXPECT_EQ(map.end_sequence_number(), 51); diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index a570d5ebca..4691fa6283 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -23,12 +23,28 @@ #include "system_wrappers/include/clock.h" namespace webrtc { - +namespace { // The maximum allowed value for a timestamp in milliseconds. This is lower // than the numerical limit since we often convert to microseconds. static constexpr int64_t kMaxTimeMs = std::numeric_limits::max() / 1000; +TimeDelta GetAbsoluteSendTimeDelta(uint32_t new_sendtime, + uint32_t previous_sendtime) { + static constexpr uint32_t kWrapAroundPeriod = 0x0100'0000; + RTC_DCHECK_LT(new_sendtime, kWrapAroundPeriod); + RTC_DCHECK_LT(previous_sendtime, kWrapAroundPeriod); + uint32_t delta = (new_sendtime - previous_sendtime) % kWrapAroundPeriod; + if (delta >= kWrapAroundPeriod / 2) { + // absolute send time wraps around, thus treat deltas larger than half of + // the wrap around period as negative. Ignore reordering of packets and + // treat them as they have approximately the same send time. + return TimeDelta::Zero(); + } + return TimeDelta::Micros(int64_t{delta} * 1'000'000 / (1 << 18)); +} +} // namespace + RemoteEstimatorProxy::RemoteEstimatorProxy( Clock* clock, TransportFeedbackSender feedback_sender, @@ -54,14 +70,13 @@ RemoteEstimatorProxy::RemoteEstimatorProxy( RemoteEstimatorProxy::~RemoteEstimatorProxy() {} void RemoteEstimatorProxy::MaybeCullOldPackets(int64_t sequence_number, - int64_t arrival_time_ms) { - if (periodic_window_start_seq_.has_value()) { - if (*periodic_window_start_seq_ >= - packet_arrival_times_.end_sequence_number()) { - // Start new feedback packet, cull old packets. - packet_arrival_times_.RemoveOldPackets( - sequence_number, arrival_time_ms - send_config_.back_window->ms()); - } + Timestamp arrival_time) { + if (periodic_window_start_seq_ >= + packet_arrival_times_.end_sequence_number() && + arrival_time - Timestamp::Zero() >= send_config_.back_window.Get()) { + // Start new feedback packet, cull old packets. + packet_arrival_times_.RemoveOldPackets( + sequence_number, arrival_time - send_config_.back_window.Get()); } } @@ -72,15 +87,30 @@ void RemoteEstimatorProxy::IncomingPacket(int64_t arrival_time_ms, RTC_LOG(LS_WARNING) << "Arrival time out of bounds: " << arrival_time_ms; return; } + Packet packet = {.arrival_time = Timestamp::Millis(arrival_time_ms), + .size = DataSize::Bytes(header.headerLength + payload_size), + .ssrc = header.ssrc}; + if (header.extension.hasTransportSequenceNumber) { + packet.transport_sequence_number = header.extension.transportSequenceNumber; + } + if (header.extension.hasAbsoluteSendTime) { + packet.absolute_send_time_24bits = header.extension.absoluteSendTime; + } + packet.feedback_request = header.extension.feedback_request; + + IncomingPacket(packet); +} + +void RemoteEstimatorProxy::IncomingPacket(Packet packet) { MutexLock lock(&lock_); - media_ssrc_ = header.ssrc; + media_ssrc_ = packet.ssrc; int64_t seq = 0; - if (header.extension.hasTransportSequenceNumber) { - seq = unwrapper_.Unwrap(header.extension.transportSequenceNumber); + if (packet.transport_sequence_number.has_value()) { + seq = unwrapper_.Unwrap(*packet.transport_sequence_number); if (send_periodic_feedback_) { - MaybeCullOldPackets(seq, arrival_time_ms); + MaybeCullOldPackets(seq, packet.arrival_time); if (!periodic_window_start_seq_ || seq < *periodic_window_start_seq_) { periodic_window_start_seq_ = seq; @@ -92,7 +122,7 @@ void RemoteEstimatorProxy::IncomingPacket(int64_t arrival_time_ms, return; } - packet_arrival_times_.AddPacket(seq, arrival_time_ms); + packet_arrival_times_.AddPacket(seq, packet.arrival_time); // Limit the range of sequence numbers to send feedback for. if (!periodic_window_start_seq_.has_value() || @@ -102,24 +132,20 @@ void RemoteEstimatorProxy::IncomingPacket(int64_t arrival_time_ms, packet_arrival_times_.begin_sequence_number(); } - if (header.extension.feedback_request) { + if (packet.feedback_request) { // Send feedback packet immediately. - SendFeedbackOnRequest(seq, header.extension.feedback_request.value()); + SendFeedbackOnRequest(seq, *packet.feedback_request); } } - if (network_state_estimator_ && header.extension.hasAbsoluteSendTime) { + if (network_state_estimator_ && packet.absolute_send_time_24bits) { PacketResult packet_result; - packet_result.receive_time = Timestamp::Millis(arrival_time_ms); - // Ignore reordering of packets and assume they have approximately the - // same send time. - abs_send_timestamp_ += std::max( - header.extension.GetAbsoluteSendTimeDelta(previous_abs_send_time_), - TimeDelta::Millis(0)); - previous_abs_send_time_ = header.extension.absoluteSendTime; + packet_result.receive_time = packet.arrival_time; + abs_send_timestamp_ += GetAbsoluteSendTimeDelta( + *packet.absolute_send_time_24bits, previous_abs_send_time_); + previous_abs_send_time_ = *packet.absolute_send_time_24bits; packet_result.sent_packet.send_time = abs_send_timestamp_; - packet_result.sent_packet.size = - DataSize::Bytes(header.headerLength + payload_size) + packet_overhead_; + packet_result.sent_packet.size = packet.size + packet_overhead_; packet_result.sent_packet.sequence_number = seq; network_state_estimator_->OnReceivedPacket(packet_result); } @@ -274,8 +300,8 @@ RemoteEstimatorProxy::MaybeBuildFeedbackPacket( int64_t next_sequence_number = begin_sequence_number_inclusive; for (int64_t seq = start_seq; seq < end_seq; ++seq) { - int64_t arrival_time_ms = packet_arrival_times_.get(seq); - if (arrival_time_ms == 0) { + Timestamp arrival_time = packet_arrival_times_.get(seq); + if (arrival_time == Timestamp::Zero()) { // Packet not received. continue; } @@ -283,20 +309,18 @@ RemoteEstimatorProxy::MaybeBuildFeedbackPacket( if (feedback_packet == nullptr) { feedback_packet = std::make_unique(include_timestamps); - // TODO(sprang): Measure receive times in microseconds and remove the - // conversions below. feedback_packet->SetMediaSsrc(media_ssrc_); // Base sequence number is the expected first sequence number. This is // known, but we might not have actually received it, so the base time // shall be the time of the first received packet in the feedback. feedback_packet->SetBase( static_cast(begin_sequence_number_inclusive & 0xFFFF), - arrival_time_ms * 1000); + arrival_time); feedback_packet->SetFeedbackSequenceNumber(feedback_packet_count_++); } if (!feedback_packet->AddReceivedPacket(static_cast(seq & 0xFFFF), - arrival_time_ms * 1000)) { + arrival_time)) { // Could not add timestamp, feedback packet might be full. Return and // try again with a fresh packet. break; diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index b4ec39687d..6aba92fec0 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -16,9 +16,12 @@ #include #include +#include "absl/types/optional.h" #include "api/field_trials_view.h" #include "api/transport/network_control.h" #include "api/units/data_size.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/remote_bitrate_estimator/packet_arrival_map.h" #include "rtc_base/experiments/field_trial_parser.h" @@ -47,6 +50,16 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { NetworkStateEstimator* network_state_estimator); ~RemoteEstimatorProxy() override; + struct Packet { + Timestamp arrival_time; + DataSize size; + uint32_t ssrc; + absl::optional absolute_send_time_24bits; + absl::optional transport_sequence_number; + absl::optional feedback_request; + }; + void IncomingPacket(Packet packet); + void IncomingPacket(int64_t arrival_time_ms, size_t payload_size, const RTPHeader& header) override; @@ -78,7 +91,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { } }; - void MaybeCullOldPackets(int64_t sequence_number, int64_t arrival_time_ms) + void MaybeCullOldPackets(int64_t sequence_number, Timestamp arrival_time) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); void SendPeriodicFeedbacks() RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); void SendFeedbackOnRequest(int64_t sequence_number, diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 7ebfc8d3e4..02d428f580 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -17,12 +17,17 @@ #include "api/transport/network_types.h" #include "api/transport/test/mock_network_control.h" #include "api/units/data_size.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "system_wrappers/include/clock.h" #include "test/gmock.h" #include "test/gtest.h" +namespace webrtc { +namespace { + using ::testing::_; using ::testing::ElementsAre; using ::testing::Invoke; @@ -30,20 +35,18 @@ using ::testing::MockFunction; using ::testing::Return; using ::testing::SizeIs; -namespace webrtc { -namespace { - -constexpr size_t kDefaultPacketSize = 100; +constexpr DataSize kDefaultPacketSize = DataSize::Bytes(100); constexpr uint32_t kMediaSsrc = 456; constexpr uint16_t kBaseSeq = 10; -constexpr int64_t kBaseTimeMs = 123; -constexpr int64_t kMaxSmallDeltaMs = - (rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) / 1000; +constexpr Timestamp kBaseTime = Timestamp::Millis(123); +constexpr TimeDelta kBaseTimeWrapAround = + rtcp::TransportFeedback::kDeltaTick * (int64_t{1} << 32); +constexpr TimeDelta kMaxSmallDelta = rtcp::TransportFeedback::kDeltaTick * 0xFF; -constexpr int kBackWindowMs = 500; -constexpr int kMinSendIntervalMs = 50; -constexpr int kMaxSendIntervalMs = 250; -constexpr int kDefaultSendIntervalMs = 100; +constexpr TimeDelta kBackWindow = TimeDelta::Millis(500); +constexpr TimeDelta kMinSendInterval = TimeDelta::Millis(50); +constexpr TimeDelta kMaxSendInterval = TimeDelta::Millis(250); +constexpr TimeDelta kDefaultSendInterval = TimeDelta::Millis(100); std::vector SequenceNumbers( const rtcp::TransportFeedback& feedback_packet) { @@ -54,13 +57,20 @@ std::vector SequenceNumbers( return sequence_numbers; } -std::vector TimestampsMs( +std::vector Timestamps( const rtcp::TransportFeedback& feedback_packet) { - std::vector timestamps; - int64_t timestamp_us = feedback_packet.GetBaseTimeUs(); + std::vector timestamps; + Timestamp timestamp = feedback_packet.BaseTime(); + // rtcp::TransportFeedback makes no promises about epoch of the base time, + // It may add several kBaseTimeWrapAround periods to make it large enough and + // thus to support negative deltas. Align it close to the kBaseTime to make + // tests expectations simpler. + if (timestamp > kBaseTime) { + timestamp -= (timestamp - kBaseTime).RoundTo(kBaseTimeWrapAround); + } for (const auto& rtp_packet_received : feedback_packet.GetReceivedPackets()) { - timestamp_us += rtp_packet_received.delta_us(); - timestamps.push_back(timestamp_us / 1000); + timestamp += rtp_packet_received.delta(); + timestamps.push_back(timestamp); } return timestamps; } @@ -77,31 +87,17 @@ class RemoteEstimatorProxyTest : public ::testing::Test { protected: void IncomingPacket( uint16_t seq, - int64_t time_ms, + Timestamp arrival_time, absl::optional feedback_request = absl::nullopt) { - proxy_.IncomingPacket(time_ms, kDefaultPacketSize, - CreateHeader(seq, feedback_request, absl::nullopt)); - } - - RTPHeader CreateHeader(absl::optional transport_sequence, - absl::optional feedback_request, - absl::optional absolute_send_time) { - RTPHeader header; - if (transport_sequence) { - header.extension.hasTransportSequenceNumber = true; - header.extension.transportSequenceNumber = transport_sequence.value(); - } - header.extension.feedback_request = feedback_request; - if (absolute_send_time) { - header.extension.hasAbsoluteSendTime = true; - header.extension.absoluteSendTime = absolute_send_time.value(); - } - header.ssrc = kMediaSsrc; - return header; + proxy_.IncomingPacket({.arrival_time = arrival_time, + .size = DataSize::Bytes(100), + .ssrc = kMediaSsrc, + .transport_sequence_number = seq, + .feedback_request = feedback_request}); } void Process() { - clock_.AdvanceTimeMilliseconds(kDefaultSendIntervalMs); + clock_.AdvanceTime(kDefaultSendInterval); proxy_.Process(); } @@ -114,7 +110,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { }; TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { - IncomingPacket(kBaseSeq, kBaseTimeMs); + IncomingPacket(kBaseSeq, kBaseTime); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -127,16 +123,15 @@ TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), ElementsAre(kBaseTime)); })); Process(); } TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq, kBaseTimeMs + 1000); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq, kBaseTime + TimeDelta::Seconds(1)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -149,8 +144,7 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), ElementsAre(kBaseTime)); return true; })); @@ -159,13 +153,13 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { // First feedback. - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq + 1, kBaseTime + TimeDelta::Seconds(1)); EXPECT_CALL(feedback_sender_, Call); Process(); // Second feedback starts with a missing packet (DROP kBaseSeq + 2). - IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000); + IncomingPacket(kBaseSeq + 3, kBaseTime + TimeDelta::Seconds(3)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -178,17 +172,18 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 3)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 3000)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + TimeDelta::Seconds(3))); })); Process(); } TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs); - IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq + 1, kBaseTime + kMaxSmallDelta); + IncomingPacket(kBaseSeq + 2, + kBaseTime + (2 * kMaxSmallDelta) + TimeDelta::Millis(1)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -201,20 +196,21 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 2)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs, - kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime, kBaseTime + kMaxSmallDelta, + kBaseTime + (2 * kMaxSmallDelta) + + TimeDelta::Millis(1))); })); Process(); } TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { - static constexpr int64_t kTooLargeDelta = - rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 16); + static constexpr TimeDelta kTooLargeDelta = + rtcp::TransportFeedback::kDeltaTick * (1 << 16); - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq + 1, kBaseTime + kTooLargeDelta); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -227,8 +223,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), ElementsAre(kBaseTime)); })) .WillOnce(Invoke( [](std::vector> feedback_packets) { @@ -240,18 +235,18 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 1)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + kTooLargeDelta)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + kTooLargeDelta)); })); Process(); } TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { - const int64_t kDeltaMs = 1000; + const TimeDelta kDelta = TimeDelta::Seconds(1); const uint16_t kLargeSeq = 62762; - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kLargeSeq, kBaseTime + kDelta); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -262,8 +257,8 @@ TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { EXPECT_EQ(kLargeSeq, feedback_packet->GetBaseSequence()); EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + kDelta, kBaseTime)); })); Process(); @@ -274,11 +269,11 @@ TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { // When unwrapped, the sequeunce numbers of these 30 incoming packets, will // span a range of roughly 650k packets. Test that we only send feedback for // the last packets. Test for regression found in chromium:949020. - const int64_t kDeltaMs = 1000; + const TimeDelta kDelta = TimeDelta::Seconds(1); for (int i = 0; i < 10; ++i) { - IncomingPacket(kBaseSeq + i, kBaseTimeMs + 3 * i * kDeltaMs); - IncomingPacket(kBaseSeq + 20000 + i, kBaseTimeMs + (3 * i + 1) * kDeltaMs); - IncomingPacket(kBaseSeq + 40000 + i, kBaseTimeMs + (3 * i + 2) * kDeltaMs); + IncomingPacket(kBaseSeq + i, kBaseTime + 3 * i * kDelta); + IncomingPacket(kBaseSeq + 20000 + i, kBaseTime + (3 * i + 1) * kDelta); + IncomingPacket(kBaseSeq + 40000 + i, kBaseTime + (3 * i + 2) * kDelta); } // Only expect feedback for the last two packets. @@ -292,9 +287,9 @@ TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 20009, kBaseSeq + 40009)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 28 * kDeltaMs, - kBaseTimeMs + 29 * kDeltaMs)); + EXPECT_THAT( + Timestamps(*feedback_packet), + ElementsAre(kBaseTime + 28 * kDelta, kBaseTime + 29 * kDelta)); })); Process(); @@ -304,11 +299,11 @@ TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { // This test is like HandlesMalformedSequenceNumbers but for negative wrap // arounds. Test that we only send feedback for the packets with highest // sequence numbers. Test for regression found in chromium:949020. - const int64_t kDeltaMs = 1000; + const TimeDelta kDelta = TimeDelta::Seconds(1); for (int i = 0; i < 10; ++i) { - IncomingPacket(kBaseSeq + i, kBaseTimeMs + 3 * i * kDeltaMs); - IncomingPacket(kBaseSeq + 40000 + i, kBaseTimeMs + (3 * i + 1) * kDeltaMs); - IncomingPacket(kBaseSeq + 20000 + i, kBaseTimeMs + (3 * i + 2) * kDeltaMs); + IncomingPacket(kBaseSeq + i, kBaseTime + 3 * i * kDelta); + IncomingPacket(kBaseSeq + 40000 + i, kBaseTime + (3 * i + 1) * kDelta); + IncomingPacket(kBaseSeq + 20000 + i, kBaseTime + (3 * i + 2) * kDelta); } // Only expect feedback for the first two packets. @@ -322,16 +317,16 @@ TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 40000, kBaseSeq)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + kDelta, kBaseTime)); })); Process(); } TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq + 2, kBaseTime + TimeDelta::Millis(2)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -344,13 +339,14 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq, kBaseSeq + 2)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs, kBaseTimeMs + 2)); + EXPECT_THAT( + Timestamps(*feedback_packet), + ElementsAre(kBaseTime, kBaseTime + TimeDelta::Millis(2))); })); Process(); - IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1); + IncomingPacket(kBaseSeq + 1, kBaseTime + TimeDelta::Millis(1)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -363,17 +359,18 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 1, kBaseSeq + 2)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + TimeDelta::Millis(1), + kBaseTime + TimeDelta::Millis(2))); })); Process(); } TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { - const int64_t kTimeoutTimeMs = kBaseTimeMs + kBackWindowMs; + const Timestamp kTimeoutTime = kBaseTime + kBackWindow; - IncomingPacket(kBaseSeq + 2, kBaseTimeMs); + IncomingPacket(kBaseSeq + 2, kBaseTime); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -383,13 +380,12 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { feedback_packets[0].get()); EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence()); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), ElementsAre(kBaseTime)); })); Process(); - IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs); // kBaseSeq + 2 times out here. + IncomingPacket(kBaseSeq + 3, kTimeoutTime); // kBaseSeq + 2 times out here. EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -399,16 +395,16 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { feedback_packets[0].get()); EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence()); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kTimeoutTimeMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kTimeoutTime)); })); Process(); // New group, with sequence starting below the first so that they may be // retransmitted. - IncomingPacket(kBaseSeq, kBaseTimeMs - 1); - IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1); + IncomingPacket(kBaseSeq, kBaseTime - TimeDelta::Millis(1)); + IncomingPacket(kBaseSeq + 1, kTimeoutTime - TimeDelta::Millis(1)); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -420,9 +416,10 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 3)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs - 1, kTimeoutTimeMs - 1, - kTimeoutTimeMs)); + EXPECT_THAT( + Timestamps(*feedback_packet), + ElementsAre(kBaseTime - TimeDelta::Millis(1), + kTimeoutTime - TimeDelta::Millis(1), kTimeoutTime)); })); Process(); @@ -434,13 +431,13 @@ TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsZeroBeforeFirstProcess) { TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsDefaultOnUnkownBitrate) { Process(); - EXPECT_EQ(kDefaultSendIntervalMs, proxy_.TimeUntilNextProcess()); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kDefaultSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMinIntervalOn300kbps) { Process(); - proxy_.OnBitrateChanged(300000); - EXPECT_EQ(kMinSendIntervalMs, proxy_.TimeUntilNextProcess()); + proxy_.OnBitrateChanged(300'000); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMinSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { @@ -449,13 +446,13 @@ TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { // bitrate is small. We choose 0 bps as a special case, which also tests // erroneous behaviors like division-by-zero. proxy_.OnBitrateChanged(0); - EXPECT_EQ(kMaxSendIntervalMs, proxy_.TimeUntilNextProcess()); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMaxSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn20kbps) { Process(); - proxy_.OnBitrateChanged(20000); - EXPECT_EQ(kMaxSendIntervalMs, proxy_.TimeUntilNextProcess()); + proxy_.OnBitrateChanged(20'000); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMaxSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { @@ -477,16 +474,16 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, TimeUntilNextProcessIsHigh) { TEST_F(RemoteEstimatorProxyOnRequestTest, ProcessDoesNotSendFeedback) { proxy_.SetSendPeriodicFeedback(false); - IncomingPacket(kBaseSeq, kBaseTimeMs); + IncomingPacket(kBaseSeq, kBaseTime); EXPECT_CALL(feedback_sender_, Call).Times(0); Process(); } TEST_F(RemoteEstimatorProxyOnRequestTest, RequestSinglePacketFeedback) { proxy_.SetSendPeriodicFeedback(false); - IncomingPacket(kBaseSeq, kBaseTimeMs); - IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs); - IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2 * kMaxSmallDeltaMs); + IncomingPacket(kBaseSeq, kBaseTime); + IncomingPacket(kBaseSeq + 1, kBaseTime + kMaxSmallDelta); + IncomingPacket(kBaseSeq + 2, kBaseTime + 2 * kMaxSmallDelta); EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( @@ -499,13 +496,13 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestSinglePacketFeedback) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 3)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 3 * kMaxSmallDeltaMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + 3 * kMaxSmallDelta)); })); constexpr FeedbackRequest kSinglePacketFeedbackRequest = { /*include_timestamps=*/true, /*sequence_count=*/1}; - IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3 * kMaxSmallDeltaMs, + IncomingPacket(kBaseSeq + 3, kBaseTime + 3 * kMaxSmallDelta, kSinglePacketFeedbackRequest); } @@ -513,7 +510,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestLastFivePacketFeedback) { proxy_.SetSendPeriodicFeedback(false); int i = 0; for (; i < 10; ++i) { - IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs); + IncomingPacket(kBaseSeq + i, kBaseTime + i * kMaxSmallDelta); } EXPECT_CALL(feedback_sender_, Call) @@ -528,17 +525,17 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestLastFivePacketFeedback) { EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 6, kBaseSeq + 7, kBaseSeq + 8, kBaseSeq + 9, kBaseSeq + 10)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs, - kBaseTimeMs + 7 * kMaxSmallDeltaMs, - kBaseTimeMs + 8 * kMaxSmallDeltaMs, - kBaseTimeMs + 9 * kMaxSmallDeltaMs, - kBaseTimeMs + 10 * kMaxSmallDeltaMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + 6 * kMaxSmallDelta, + kBaseTime + 7 * kMaxSmallDelta, + kBaseTime + 8 * kMaxSmallDelta, + kBaseTime + 9 * kMaxSmallDelta, + kBaseTime + 10 * kMaxSmallDelta)); })); constexpr FeedbackRequest kFivePacketsFeedbackRequest = { /*include_timestamps=*/true, /*sequence_count=*/5}; - IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs, + IncomingPacket(kBaseSeq + i, kBaseTime + i * kMaxSmallDelta, kFivePacketsFeedbackRequest); } @@ -548,7 +545,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, int i = 0; for (; i < 10; ++i) { if (i != 7 && i != 9) - IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs); + IncomingPacket(kBaseSeq + i, kBaseTime + i * kMaxSmallDelta); } EXPECT_CALL(feedback_sender_, Call) @@ -562,49 +559,51 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq + 6, kBaseSeq + 8, kBaseSeq + 10)); - EXPECT_THAT(TimestampsMs(*feedback_packet), - ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs, - kBaseTimeMs + 8 * kMaxSmallDeltaMs, - kBaseTimeMs + 10 * kMaxSmallDeltaMs)); + EXPECT_THAT(Timestamps(*feedback_packet), + ElementsAre(kBaseTime + 6 * kMaxSmallDelta, + kBaseTime + 8 * kMaxSmallDelta, + kBaseTime + 10 * kMaxSmallDelta)); })); constexpr FeedbackRequest kFivePacketsFeedbackRequest = { /*include_timestamps=*/true, /*sequence_count=*/5}; - IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs, + IncomingPacket(kBaseSeq + i, kBaseTime + i * kMaxSmallDelta, kFivePacketsFeedbackRequest); } TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) { Timestamp first_send_timestamp = Timestamp::Millis(0); const DataSize kPacketOverhead = DataSize::Bytes(38); - webrtc::RTPHeader first_header = CreateHeader( - absl::nullopt, absl::nullopt, AbsoluteSendTime::MsTo24Bits(kBaseTimeMs)); proxy_.SetTransportOverhead(kPacketOverhead); EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) .WillOnce(Invoke([&](const PacketResult& packet) { - EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs)); - EXPECT_EQ( - packet.sent_packet.size, - DataSize::Bytes(kDefaultPacketSize + first_header.headerLength) + - kPacketOverhead); + EXPECT_EQ(packet.receive_time, kBaseTime); + EXPECT_EQ(packet.sent_packet.size, + kDefaultPacketSize + kPacketOverhead); first_send_timestamp = packet.sent_packet.send_time; })); // Incoming packet with abs sendtime but without transport sequence number. - proxy_.IncomingPacket(kBaseTimeMs, kDefaultPacketSize, first_header); + proxy_.IncomingPacket({.arrival_time = kBaseTime, + .size = kDefaultPacketSize, + .ssrc = kMediaSsrc, + .absolute_send_time_24bits = + AbsoluteSendTime::MsTo24Bits(kBaseTime.ms())}); // Expect packet with older abs send time to be treated as sent at the same // time as the previous packet due to reordering. EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) .WillOnce(Invoke([&first_send_timestamp](const PacketResult& packet) { - EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs)); + EXPECT_EQ(packet.receive_time, kBaseTime); EXPECT_EQ(packet.sent_packet.send_time, first_send_timestamp); })); proxy_.IncomingPacket( - kBaseTimeMs, kDefaultPacketSize, - CreateHeader(absl::nullopt, absl::nullopt, - AbsoluteSendTime::MsTo24Bits(kBaseTimeMs - 12))); + {.arrival_time = kBaseTime, + .size = kDefaultPacketSize, + .ssrc = kMediaSsrc, + .absolute_send_time_24bits = + AbsoluteSendTime::MsTo24Bits(kBaseTime.ms() - 12)}); } TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { @@ -618,30 +617,36 @@ TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { Timestamp first_send_timestamp = Timestamp::Millis(0); EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) .WillOnce(Invoke([&first_send_timestamp](const PacketResult& packet) { - EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs)); + EXPECT_EQ(packet.receive_time, kBaseTime); first_send_timestamp = packet.sent_packet.send_time; })); - proxy_.IncomingPacket( - kBaseTimeMs, kDefaultPacketSize, - CreateHeader(kBaseSeq, absl::nullopt, kFirstAbsSendTime)); + proxy_.IncomingPacket({.arrival_time = kBaseTime, + .size = kDefaultPacketSize, + .ssrc = kMediaSsrc, + .absolute_send_time_24bits = kFirstAbsSendTime, + .transport_sequence_number = kBaseSeq}); EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) .WillOnce(Invoke([first_send_timestamp, kExpectedAbsSendTimeDelta](const PacketResult& packet) { - EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs + 123)); + EXPECT_EQ(packet.receive_time, kBaseTime + TimeDelta::Millis(123)); EXPECT_EQ(packet.sent_packet.send_time.ms(), (first_send_timestamp + kExpectedAbsSendTimeDelta).ms()); })); - proxy_.IncomingPacket( - kBaseTimeMs + 123, kDefaultPacketSize, - CreateHeader(kBaseSeq + 1, absl::nullopt, kSecondAbsSendTime)); + proxy_.IncomingPacket({.arrival_time = kBaseTime + TimeDelta::Millis(123), + .size = kDefaultPacketSize, + .ssrc = kMediaSsrc, + .absolute_send_time_24bits = kSecondAbsSendTime, + .transport_sequence_number = kBaseSeq + 1}); } TEST_F(RemoteEstimatorProxyTest, SendTransportFeedbackAndNetworkStateUpdate) { - proxy_.IncomingPacket( - kBaseTimeMs, kDefaultPacketSize, - CreateHeader(kBaseSeq, absl::nullopt, - AbsoluteSendTime::MsTo24Bits(kBaseTimeMs - 1))); + proxy_.IncomingPacket({.arrival_time = kBaseTime, + .size = kDefaultPacketSize, + .ssrc = kMediaSsrc, + .absolute_send_time_24bits = + AbsoluteSendTime::MsTo24Bits(kBaseTime.ms() - 1), + .transport_sequence_number = kBaseSeq}); EXPECT_CALL(network_state_estimator_, GetCurrentEstimate()) .WillOnce(Return(NetworkStateEstimate())); EXPECT_CALL(feedback_sender_, Call(SizeIs(2)));