From 86c452ac5ae9cf84e92b8d28c35432b188728edf Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 7 Jun 2022 19:27:06 +0200 Subject: [PATCH] Refactor rtcp::TransportFeedback to use Timestamp and TimeDelta internally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:13757 Change-Id: I9815e54288a064c6c8ff40f130b52786b4e398b7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264559 Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#37149} --- .../source/rtcp_packet/transport_feedback.cc | 70 +-- .../source/rtcp_packet/transport_feedback.h | 22 +- .../transport_feedback_unittest.cc | 435 ++++++++++-------- 3 files changed, 292 insertions(+), 235 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc index 3059778137..719fbf636e 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc @@ -42,10 +42,8 @@ constexpr size_t kMaxSizeBytes = (1 << 16) * 4; // * 8 bytes FeedbackPacket header. // * 2 bytes for one chunk. constexpr size_t kMinPayloadSizeBytes = 8 + 8 + 2; -constexpr int kBaseScaleFactor = - TransportFeedback::kDeltaScaleFactor * (1 << 8); -constexpr int64_t kTimeWrapPeriodUs = (1ll << 24) * kBaseScaleFactor; -constexpr TimeDelta kTimeWrapPeriod = TimeDelta::Micros(kTimeWrapPeriodUs); +constexpr TimeDelta kBaseTimeTick = TransportFeedback::kDeltaTick * (1 << 8); +constexpr TimeDelta kTimeWrapPeriod = kBaseTimeTick * (1 << 24); // Message format // @@ -274,7 +272,7 @@ TransportFeedback::TransportFeedback(bool include_timestamps, bool include_lost) base_time_ticks_(0), feedback_seq_(0), include_timestamps_(include_timestamps), - last_timestamp_us_(0), + last_timestamp_(Timestamp::Zero()), size_bytes_(kTransportFeedbackHeaderSizeBytes) {} TransportFeedback::TransportFeedback(const TransportFeedback&) = default; @@ -286,7 +284,7 @@ TransportFeedback::TransportFeedback(TransportFeedback&& other) base_time_ticks_(other.base_time_ticks_), feedback_seq_(other.feedback_seq_), include_timestamps_(other.include_timestamps_), - last_timestamp_us_(other.last_timestamp_us_), + last_timestamp_(other.last_timestamp_), received_packets_(std::move(other.received_packets_)), all_packets_(std::move(other.all_packets_)), encoded_chunks_(std::move(other.encoded_chunks_)), @@ -298,12 +296,12 @@ TransportFeedback::TransportFeedback(TransportFeedback&& other) TransportFeedback::~TransportFeedback() {} void TransportFeedback::SetBase(uint16_t base_sequence, - int64_t ref_timestamp_us) { + Timestamp ref_timestamp) { RTC_DCHECK_EQ(num_seq_no_, 0); - RTC_DCHECK_GE(ref_timestamp_us, 0); base_seq_no_ = base_sequence; - base_time_ticks_ = (ref_timestamp_us % kTimeWrapPeriodUs) / kBaseScaleFactor; - last_timestamp_us_ = GetBaseTimeUs(); + base_time_ticks_ = + (ref_timestamp.us() % kTimeWrapPeriod.us()) / kBaseTimeTick.us(); + last_timestamp_ = BaseTime(); } void TransportFeedback::SetFeedbackSequenceNumber(uint8_t feedback_sequence) { @@ -311,19 +309,25 @@ void TransportFeedback::SetFeedbackSequenceNumber(uint8_t feedback_sequence) { } bool TransportFeedback::AddReceivedPacket(uint16_t sequence_number, - int64_t timestamp_us) { + Timestamp timestamp) { // Set delta to zero if timestamps are not included, this will simplify the // encoding process. int16_t delta = 0; if (include_timestamps_) { // Convert to ticks and round. + if (last_timestamp_ > timestamp) { + timestamp += (last_timestamp_ - timestamp).RoundUpTo(kTimeWrapPeriod); + } + RTC_DCHECK_GE(timestamp, last_timestamp_); int64_t delta_full = - (timestamp_us - last_timestamp_us_) % kTimeWrapPeriodUs; - if (delta_full > kTimeWrapPeriodUs / 2) - delta_full -= kTimeWrapPeriodUs; - delta_full += - delta_full < 0 ? -(kDeltaScaleFactor / 2) : kDeltaScaleFactor / 2; - delta_full /= kDeltaScaleFactor; + (timestamp - last_timestamp_).us() % kTimeWrapPeriod.us(); + if (delta_full > kTimeWrapPeriod.us() / 2) { + delta_full -= kTimeWrapPeriod.us(); + delta_full -= kDeltaTick.us() / 2; + } else { + delta_full += kDeltaTick.us() / 2; + } + delta_full /= kDeltaTick.us(); delta = static_cast(delta_full); // If larger than 16bit signed, we can't represent it - need new fb packet. @@ -353,7 +357,7 @@ bool TransportFeedback::AddReceivedPacket(uint16_t sequence_number, received_packets_.emplace_back(sequence_number, delta); if (include_lost_) all_packets_.emplace_back(sequence_number, delta); - last_timestamp_us_ += delta * kDeltaScaleFactor; + last_timestamp_ += delta * kDeltaTick; if (include_timestamps_) { size_bytes_ += delta_size; } @@ -376,17 +380,20 @@ uint16_t TransportFeedback::GetBaseSequence() const { } Timestamp TransportFeedback::BaseTime() const { - return Timestamp::Zero() + - TimeDelta::Micros(int64_t{base_time_ticks_} * kBaseScaleFactor); + // Add an extra kTimeWrapPeriod to allow add received packets arrived earlier + // than the first added packet (and thus allow to record negative deltas) + // even when base_time_ticks_ == 0. + return Timestamp::Zero() + kTimeWrapPeriod + + int64_t{base_time_ticks_} * kBaseTimeTick; } int64_t TransportFeedback::GetBaseTimeUs() const { // Historically BaseTime was stored as signed integer and could be negative. // However with new api it is not possible, but for compatibility with legacy // tests return base time as negative when it used to be negative. - int64_t base_time_us = BaseTime().us() % kTimeWrapPeriodUs; - if (base_time_us >= kTimeWrapPeriodUs / 2) { - return base_time_us - kTimeWrapPeriodUs; + int64_t base_time_us = BaseTime().us() % kTimeWrapPeriod.us(); + if (base_time_us >= kTimeWrapPeriod.us() / 2) { + return base_time_us - kTimeWrapPeriod.us(); } else { return base_time_us; } @@ -483,7 +490,7 @@ bool TransportFeedback::Parse(const CommonHeader& packet) { received_packets_.emplace_back(seq_no, delta); if (include_lost_) all_packets_.emplace_back(seq_no, delta); - last_timestamp_us_ += delta * kDeltaScaleFactor; + last_timestamp_ += delta * kDeltaTick; index += delta_size; break; } @@ -492,7 +499,7 @@ bool TransportFeedback::Parse(const CommonHeader& packet) { received_packets_.emplace_back(seq_no, delta); if (include_lost_) all_packets_.emplace_back(seq_no, delta); - last_timestamp_us_ += delta * kDeltaScaleFactor; + last_timestamp_ += delta * kDeltaTick; index += delta_size; break; } @@ -562,7 +569,7 @@ bool TransportFeedback::IsConsistent() const { << num_seq_no_; return false; } - int64_t timestamp_us = GetBaseTimeUs(); + Timestamp timestamp = BaseTime(); auto packet_it = received_packets_.begin(); uint16_t seq_no = base_seq_no_; for (DeltaSize delta_size : delta_sizes) { @@ -584,7 +591,7 @@ bool TransportFeedback::IsConsistent() const { << " doesn't fit into one byte"; return false; } - timestamp_us += packet_it->delta_us(); + timestamp += packet_it->delta(); ++packet_it; } if (include_timestamps_) { @@ -597,9 +604,10 @@ bool TransportFeedback::IsConsistent() const { << packet_it->sequence_number(); return false; } - if (timestamp_us != last_timestamp_us_) { - RTC_LOG(LS_ERROR) << "Last timestamp mismatch. Calculated: " << timestamp_us - << ". Saved: " << last_timestamp_us_; + if (timestamp != last_timestamp_) { + RTC_LOG(LS_ERROR) << "Last timestamp mismatch. Calculated: " + << ToLogString(timestamp) + << ". Saved: " << ToLogString(last_timestamp_); return false; } if (size_bytes_ != packet_size) { @@ -684,7 +692,7 @@ bool TransportFeedback::Create(uint8_t* packet, void TransportFeedback::Clear() { num_seq_no_ = 0; - last_timestamp_us_ = GetBaseTimeUs(); + last_timestamp_ = BaseTime(); received_packets_.clear(); all_packets_.clear(); encoded_chunks_.clear(); diff --git a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h index 21633743d3..8c92708bae 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h +++ b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h @@ -41,8 +41,8 @@ class TransportFeedback : public Rtpfb { uint16_t sequence_number() const { return sequence_number_; } int16_t delta_ticks() const { return delta_ticks_; } // ABSL_DEPRECATED("Use delta() that returns TimeDelta") - int32_t delta_us() const { return delta_ticks_ * kDeltaScaleFactor; } - TimeDelta delta() const { return TimeDelta::Micros(delta_us()); } + int32_t delta_us() const { return delta().us(); } + TimeDelta delta() const { return delta_ticks_ * kDeltaTick; } bool received() const { return received_; } private: @@ -71,20 +71,20 @@ class TransportFeedback : public Rtpfb { ~TransportFeedback() override; // ABSL_DEPRECATED("Use version that takes Timestamp") - void SetBase(uint16_t base_sequence, // Seq# of first packet in this msg. - int64_t ref_timestamp_us); // Reference timestamp for this msg. - void SetBase(uint16_t base_sequence, // Seq# of first packet in this msg. - Timestamp ref_timestamp) { // Reference timestamp for this msg. - SetBase(base_sequence, ref_timestamp.us()); + void SetBase(uint16_t base_sequence, // Seq# of first packet in this msg. + int64_t ref_timestamp_us) { // Reference timestamp for this msg. + SetBase(base_sequence, Timestamp::Micros(ref_timestamp_us)); } + void SetBase(uint16_t base_sequence, // Seq# of first packet in this msg. + Timestamp ref_timestamp); // Reference timestamp for this msg. void SetFeedbackSequenceNumber(uint8_t feedback_sequence); // NOTE: This method requires increasing sequence numbers (excepting wraps). // ABSL_DEPRECATED("Use version that takes Timestamp") - bool AddReceivedPacket(uint16_t sequence_number, int64_t timestamp_us); - bool AddReceivedPacket(uint16_t sequence_number, Timestamp timestamp) { - return AddReceivedPacket(sequence_number, timestamp.us()); + bool AddReceivedPacket(uint16_t sequence_number, int64_t timestamp_us) { + return AddReceivedPacket(sequence_number, Timestamp::Micros(timestamp_us)); } + bool AddReceivedPacket(uint16_t sequence_number, Timestamp timestamp); const std::vector& GetReceivedPackets() const; const std::vector& GetAllPackets() const; @@ -189,7 +189,7 @@ class TransportFeedback : public Rtpfb { uint8_t feedback_seq_; bool include_timestamps_; - int64_t last_timestamp_us_; + Timestamp last_timestamp_; std::vector received_packets_; std::vector all_packets_; // All but last encoded packet chunks. diff --git a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc index 6003df45d0..b3b1175305 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc @@ -14,6 +14,9 @@ #include #include +#include "api/array_view.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "test/gmock.h" @@ -23,74 +26,103 @@ namespace webrtc { namespace { using rtcp::TransportFeedback; +using ::testing::AllOf; +using ::testing::Each; using ::testing::ElementsAreArray; +using ::testing::Eq; +using ::testing::Property; +using ::testing::SizeIs; -static const int kHeaderSize = 20; -static const int kStatusChunkSize = 2; -static const int kSmallDeltaSize = 1; -static const int kLargeDeltaSize = 2; +constexpr int kHeaderSize = 20; +constexpr int kStatusChunkSize = 2; +constexpr int kSmallDeltaSize = 1; +constexpr int kLargeDeltaSize = 2; -static const int64_t kDeltaLimit = 0xFF * TransportFeedback::kDeltaScaleFactor; +constexpr TimeDelta kDeltaLimit = 0xFF * TransportFeedback::kDeltaTick; +constexpr TimeDelta kBaseTimeTick = TransportFeedback::kDeltaTick * (1 << 8); +constexpr TimeDelta kBaseTimeWrapPeriod = kBaseTimeTick * (1 << 24); + +MATCHER_P2(Near, value, max_abs_error, "") { + return value - max_abs_error <= arg && arg <= value + max_abs_error; +} + +MATCHER(IsValidFeedback, "") { + rtcp::CommonHeader rtcp_header; + TransportFeedback feedback; + return rtcp_header.Parse(std::data(arg), std::size(arg)) && + rtcp_header.type() == TransportFeedback::kPacketType && + rtcp_header.fmt() == TransportFeedback::kFeedbackMessageType && + feedback.Parse(rtcp_header); +} + +TransportFeedback Parse(rtc::ArrayView buffer) { + rtcp::CommonHeader header; + RTC_DCHECK(header.Parse(buffer.data(), buffer.size())); + RTC_DCHECK_EQ(header.type(), TransportFeedback::kPacketType); + RTC_DCHECK_EQ(header.fmt(), TransportFeedback::kFeedbackMessageType); + TransportFeedback feedback; + RTC_DCHECK(feedback.Parse(header)); + return feedback; +} class FeedbackTester { public: FeedbackTester() : FeedbackTester(true) {} explicit FeedbackTester(bool include_timestamps) : expected_size_(kAnySize), - default_delta_(TransportFeedback::kDeltaScaleFactor * 4), + default_delta_(TransportFeedback::kDeltaTick * 4), include_timestamps_(include_timestamps) {} void WithExpectedSize(size_t expected_size) { expected_size_ = expected_size; } - void WithDefaultDelta(int64_t delta) { default_delta_ = delta; } + void WithDefaultDelta(TimeDelta delta) { default_delta_ = delta; } - void WithInput(const uint16_t received_seq[], - const int64_t received_ts[], - uint16_t length) { - std::unique_ptr temp_timestamps; - if (received_ts == nullptr) { - temp_timestamps.reset(new int64_t[length]); - GenerateReceiveTimestamps(received_seq, length, temp_timestamps.get()); - received_ts = temp_timestamps.get(); + void WithInput(rtc::ArrayView received_seq, + rtc::ArrayView received_ts = {}) { + std::vector temp_timestamps; + if (received_ts.empty()) { + temp_timestamps = GenerateReceiveTimestamps(received_seq); + received_ts = temp_timestamps; } + RTC_DCHECK_EQ(received_seq.size(), received_ts.size()); - expected_seq_.clear(); expected_deltas_.clear(); - feedback_.reset(new TransportFeedback(include_timestamps_)); + feedback_.emplace(include_timestamps_); feedback_->SetBase(received_seq[0], received_ts[0]); ASSERT_TRUE(feedback_->IsConsistent()); + // First delta is special: it doesn't represent the delta between two times, + // but a compensation for the reduced precision of the base time. + EXPECT_TRUE(feedback_->AddReceivedPacket(received_seq[0], received_ts[0])); + // GetBaseDelta suppose to return balanced diff between base time of the new + // feedback message (stored internally) and base time of the old feedback + // message (passed as parameter), but first delta is the difference between + // 1st timestamp (passed as parameter) and base time (stored internally), + // thus to get the first delta need to negate whatever GetBaseDelta returns. + expected_deltas_.push_back(-feedback_->GetBaseDelta(received_ts[0])); - int64_t last_time = feedback_->GetBaseTimeUs(); - for (int i = 0; i < length; ++i) { - int64_t time = received_ts[i]; - EXPECT_TRUE(feedback_->AddReceivedPacket(received_seq[i], time)); - - if (last_time != -1) { - int64_t delta = time - last_time; - expected_deltas_.push_back(delta); - } - last_time = time; + for (size_t i = 1; i < received_ts.size(); ++i) { + EXPECT_TRUE( + feedback_->AddReceivedPacket(received_seq[i], received_ts[i])); + expected_deltas_.push_back(received_ts[i] - received_ts[i - 1]); } ASSERT_TRUE(feedback_->IsConsistent()); - expected_seq_.insert(expected_seq_.begin(), &received_seq[0], - &received_seq[length]); + expected_seq_.assign(received_seq.begin(), received_seq.end()); } void VerifyPacket() { ASSERT_TRUE(feedback_->IsConsistent()); serialized_ = feedback_->Build(); VerifyInternal(); - feedback_ = - TransportFeedback::ParseFrom(serialized_.data(), serialized_.size()); - ASSERT_NE(nullptr, feedback_); + + feedback_.emplace(Parse(serialized_)); ASSERT_TRUE(feedback_->IsConsistent()); EXPECT_EQ(include_timestamps_, feedback_->IncludeTimestamps()); VerifyInternal(); } - static const size_t kAnySize = static_cast(0) - 1; + static constexpr size_t kAnySize = static_cast(0) - 1; private: void VerifyInternal() { @@ -102,37 +134,39 @@ class FeedbackTester { } std::vector actual_seq_nos; - std::vector actual_deltas_us; + std::vector actual_deltas; for (const auto& packet : feedback_->GetReceivedPackets()) { actual_seq_nos.push_back(packet.sequence_number()); - actual_deltas_us.push_back(packet.delta_us()); + actual_deltas.push_back(packet.delta()); } EXPECT_THAT(actual_seq_nos, ElementsAreArray(expected_seq_)); if (include_timestamps_) { - EXPECT_THAT(actual_deltas_us, ElementsAreArray(expected_deltas_)); + EXPECT_THAT(actual_deltas, ElementsAreArray(expected_deltas_)); } } - void GenerateReceiveTimestamps(const uint16_t seq[], - const size_t length, - int64_t* timestamps) { - uint16_t last_seq = seq[0]; - int64_t offset = 0; + std::vector GenerateReceiveTimestamps( + rtc::ArrayView seq_nums) { + RTC_DCHECK(!seq_nums.empty()); + uint16_t last_seq = seq_nums[0]; + Timestamp time = Timestamp::Zero(); + std::vector result; - for (size_t i = 0; i < length; ++i) { - if (seq[i] < last_seq) - offset += 0x10000 * default_delta_; - last_seq = seq[i]; + for (uint16_t seq : seq_nums) { + if (seq < last_seq) + time += 0x10000 * default_delta_; + last_seq = seq; - timestamps[i] = offset + (last_seq * default_delta_); + result.push_back(time + last_seq * default_delta_); } + return result; } std::vector expected_seq_; - std::vector expected_deltas_; + std::vector expected_deltas_; size_t expected_size_; - int64_t default_delta_; - std::unique_ptr feedback_; + TimeDelta default_delta_; + absl::optional feedback_; rtc::Buffer serialized_; bool include_timestamps_; }; @@ -153,18 +187,17 @@ TEST(RtcpPacketTest, TransportFeedbackOneBitVector) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, nullptr, kLength); + test.WithInput(kReceived); test.VerifyPacket(); } TEST(RtcpPacketTest, TransportFeedbackOneBitVectorNoRecvDelta) { const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13}; - const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); const size_t kExpectedSizeBytes = kHeaderSize + kStatusChunkSize; FeedbackTester test(/*include_timestamps=*/false); test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, nullptr, kLength); + test.WithInput(kReceived); test.VerifyPacket(); } @@ -176,7 +209,7 @@ TEST(RtcpPacketTest, TransportFeedbackFullOneBitVector) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, nullptr, kLength); + test.WithInput(kReceived); test.VerifyPacket(); } @@ -189,7 +222,7 @@ TEST(RtcpPacketTest, TransportFeedbackOneBitVectorWrapReceived) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, nullptr, kLength); + test.WithInput(kReceived); test.VerifyPacket(); } @@ -202,7 +235,7 @@ TEST(RtcpPacketTest, TransportFeedbackOneBitVectorWrapMissing) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, nullptr, kLength); + test.WithInput(kReceived); test.VerifyPacket(); } @@ -214,8 +247,8 @@ TEST(RtcpPacketTest, TransportFeedbackTwoBitVector) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithDefaultDelta(kDeltaLimit + TransportFeedback::kDeltaScaleFactor); - test.WithInput(kReceived, nullptr, kLength); + test.WithDefaultDelta(kDeltaLimit + TransportFeedback::kDeltaTick); + test.WithInput(kReceived); test.VerifyPacket(); } @@ -227,32 +260,32 @@ TEST(RtcpPacketTest, TransportFeedbackTwoBitVectorFull) { FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithDefaultDelta(kDeltaLimit + TransportFeedback::kDeltaScaleFactor); - test.WithInput(kReceived, nullptr, kLength); + test.WithDefaultDelta(kDeltaLimit + TransportFeedback::kDeltaTick); + test.WithInput(kReceived); test.VerifyPacket(); } TEST(RtcpPacketTest, TransportFeedbackWithLargeBaseTimeIsConsistent) { TransportFeedback tb; - constexpr int64_t kTimestampUs = - int64_t{0x7fff'ffff} * TransportFeedback::kDeltaScaleFactor; - tb.SetBase(/*base_sequence=*/0, /*ref_timestamp_us=*/kTimestampUs); - tb.AddReceivedPacket(/*base_sequence=*/0, /*ref_timestamp_us=*/kTimestampUs); + constexpr Timestamp kTimestamp = + Timestamp::Zero() + int64_t{0x7fff'ffff} * TransportFeedback::kDeltaTick; + tb.SetBase(/*base_sequence=*/0, /*ref_timestamp=*/kTimestamp); + tb.AddReceivedPacket(/*base_sequence=*/0, /*ref_timestamp=*/kTimestamp); EXPECT_TRUE(tb.IsConsistent()); } TEST(RtcpPacketTest, TransportFeedbackLargeAndNegativeDeltas) { const uint16_t kReceived[] = {1, 2, 6, 7, 8}; - const int64_t kReceiveTimes[] = { - 2000, 1000, 4000, 3000, - 3000 + TransportFeedback::kDeltaScaleFactor * (1 << 8)}; - const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); + const Timestamp kReceiveTimes[] = { + Timestamp::Millis(2), Timestamp::Millis(1), Timestamp::Millis(4), + Timestamp::Millis(3), + Timestamp::Millis(3) + TransportFeedback::kDeltaTick * (1 << 8)}; const size_t kExpectedSizeBytes = kHeaderSize + kStatusChunkSize + (3 * kLargeDeltaSize) + kSmallDeltaSize; FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kLength); + test.WithInput(kReceived, kReceiveTimes); test.VerifyPacket(); } @@ -264,14 +297,15 @@ TEST(RtcpPacketTest, TransportFeedbackMaxRle) { const size_t kPacketCount = (1 << 13) - 1 + 14; const uint16_t kReceived[] = {0, kPacketCount}; - const int64_t kReceiveTimes[] = {1000, 2000}; + const Timestamp kReceiveTimes[] = {Timestamp::Millis(1), + Timestamp::Millis(2)}; const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); const size_t kExpectedSizeBytes = kHeaderSize + (3 * kStatusChunkSize) + (kLength * kSmallDeltaSize); FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kLength); + test.WithInput(kReceived, kReceiveTimes); test.VerifyPacket(); } @@ -282,44 +316,45 @@ TEST(RtcpPacketTest, TransportFeedbackMinRle) { // * 1-bit vector chunk (1xreceived + 13xdropped) const uint16_t kReceived[] = {0, (14 * 2) + 1}; - const int64_t kReceiveTimes[] = {1000, 2000}; + const Timestamp kReceiveTimes[] = {Timestamp::Millis(1), + Timestamp::Millis(2)}; const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); const size_t kExpectedSizeBytes = kHeaderSize + (3 * kStatusChunkSize) + (kLength * kSmallDeltaSize); FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kLength); + test.WithInput(kReceived, kReceiveTimes); test.VerifyPacket(); } TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVector) { const size_t kTwoBitVectorCapacity = 7; const uint16_t kReceived[] = {0, kTwoBitVectorCapacity - 1}; - const int64_t kReceiveTimes[] = { - 0, kDeltaLimit + TransportFeedback::kDeltaScaleFactor}; - const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); + const Timestamp kReceiveTimes[] = { + Timestamp::Zero(), + Timestamp::Zero() + kDeltaLimit + TransportFeedback::kDeltaTick}; const size_t kExpectedSizeBytes = kHeaderSize + kStatusChunkSize + kSmallDeltaSize + kLargeDeltaSize; FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kLength); + test.WithInput(kReceived, kReceiveTimes); test.VerifyPacket(); } TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVectorSimpleSplit) { const size_t kTwoBitVectorCapacity = 7; const uint16_t kReceived[] = {0, kTwoBitVectorCapacity}; - const int64_t kReceiveTimes[] = { - 0, kDeltaLimit + TransportFeedback::kDeltaScaleFactor}; - const size_t kLength = sizeof(kReceived) / sizeof(uint16_t); + const Timestamp kReceiveTimes[] = { + Timestamp::Zero(), + Timestamp::Zero() + kDeltaLimit + TransportFeedback::kDeltaTick}; const size_t kExpectedSizeBytes = kHeaderSize + (kStatusChunkSize * 2) + kSmallDeltaSize + kLargeDeltaSize; FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kLength); + test.WithInput(kReceived, kReceiveTimes); test.VerifyPacket(); } @@ -328,7 +363,7 @@ TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVectorSplit) { // SSSSSSSSLSSSSSSSSSSSS. This will cause a 1:2 split at the L. // After split there will be two symbols in symbol_vec: SL. - const int64_t kLargeDelta = TransportFeedback::kDeltaScaleFactor * (1 << 8); + const TimeDelta kLargeDelta = TransportFeedback::kDeltaTick * (1 << 8); const size_t kNumPackets = (3 * 7) + 1; const size_t kExpectedSizeBytes = kHeaderSize + (kStatusChunkSize * 3) + (kSmallDeltaSize * (kNumPackets - 1)) + @@ -338,107 +373,138 @@ TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVectorSplit) { for (size_t i = 0; i < kNumPackets; ++i) kReceived[i] = i; - int64_t kReceiveTimes[kNumPackets]; - kReceiveTimes[0] = 1000; + std::vector receive_times; + receive_times.reserve(kNumPackets); + receive_times.push_back(Timestamp::Millis(1)); for (size_t i = 1; i < kNumPackets; ++i) { - int delta = (i == 8) ? kLargeDelta : 1000; - kReceiveTimes[i] = kReceiveTimes[i - 1] + delta; + TimeDelta delta = (i == 8) ? kLargeDelta : TimeDelta::Millis(1); + receive_times.push_back(receive_times.back() + delta); } FeedbackTester test; test.WithExpectedSize(kExpectedSizeBytes); - test.WithInput(kReceived, kReceiveTimes, kNumPackets); + test.WithInput(kReceived, receive_times); test.VerifyPacket(); } TEST(RtcpPacketTest, TransportFeedbackAliasing) { TransportFeedback feedback; - feedback.SetBase(0, 0); + feedback.SetBase(0, Timestamp::Zero()); const int kSamples = 100; - const int64_t kTooSmallDelta = TransportFeedback::kDeltaScaleFactor / 3; + const TimeDelta kTooSmallDelta = TransportFeedback::kDeltaTick / 3; for (int i = 0; i < kSamples; ++i) - feedback.AddReceivedPacket(i, i * kTooSmallDelta); + feedback.AddReceivedPacket(i, Timestamp::Zero() + i * kTooSmallDelta); feedback.Build(); - int64_t accumulated_delta = 0; + TimeDelta accumulated_delta = TimeDelta::Zero(); int num_samples = 0; for (const auto& packet : feedback.GetReceivedPackets()) { - accumulated_delta += packet.delta_us(); - int64_t expected_time = num_samples * kTooSmallDelta; + accumulated_delta += packet.delta(); + TimeDelta expected_time = num_samples * kTooSmallDelta; ++num_samples; - EXPECT_NEAR(expected_time, accumulated_delta, - TransportFeedback::kDeltaScaleFactor / 2); + EXPECT_THAT(accumulated_delta, + Near(expected_time, TransportFeedback::kDeltaTick / 2)); } } TEST(RtcpPacketTest, TransportFeedbackLimits) { // Sequence number wrap above 0x8000. std::unique_ptr packet(new TransportFeedback()); - packet->SetBase(0, 0); - EXPECT_TRUE(packet->AddReceivedPacket(0x0, 0)); - EXPECT_TRUE(packet->AddReceivedPacket(0x8000, 1000)); + packet->SetBase(0, Timestamp::Zero()); + EXPECT_TRUE(packet->AddReceivedPacket(0x0, Timestamp::Zero())); + EXPECT_TRUE(packet->AddReceivedPacket(0x8000, Timestamp::Millis(1))); packet.reset(new TransportFeedback()); - packet->SetBase(0, 0); - EXPECT_TRUE(packet->AddReceivedPacket(0x0, 0)); - EXPECT_FALSE(packet->AddReceivedPacket(0x8000 + 1, 1000)); + packet->SetBase(0, Timestamp::Zero()); + EXPECT_TRUE(packet->AddReceivedPacket(0x0, Timestamp::Zero())); + EXPECT_FALSE(packet->AddReceivedPacket(0x8000 + 1, Timestamp::Millis(1))); // Packet status count max 0xFFFF. packet.reset(new TransportFeedback()); - packet->SetBase(0, 0); - EXPECT_TRUE(packet->AddReceivedPacket(0x0, 0)); - EXPECT_TRUE(packet->AddReceivedPacket(0x8000, 1000)); - EXPECT_TRUE(packet->AddReceivedPacket(0xFFFE, 2000)); - EXPECT_FALSE(packet->AddReceivedPacket(0xFFFF, 3000)); + packet->SetBase(0, Timestamp::Zero()); + EXPECT_TRUE(packet->AddReceivedPacket(0x0, Timestamp::Zero())); + EXPECT_TRUE(packet->AddReceivedPacket(0x8000, Timestamp::Millis(1))); + EXPECT_TRUE(packet->AddReceivedPacket(0xFFFE, Timestamp::Millis(2))); + EXPECT_FALSE(packet->AddReceivedPacket(0xFFFF, Timestamp::Millis(3))); // Too large delta. packet.reset(new TransportFeedback()); - packet->SetBase(0, 0); - int64_t kMaxPositiveTimeDelta = std::numeric_limits::max() * - TransportFeedback::kDeltaScaleFactor; - EXPECT_FALSE(packet->AddReceivedPacket( - 1, kMaxPositiveTimeDelta + TransportFeedback::kDeltaScaleFactor)); - EXPECT_TRUE(packet->AddReceivedPacket(1, kMaxPositiveTimeDelta)); + packet->SetBase(0, Timestamp::Zero()); + TimeDelta kMaxPositiveTimeDelta = + std::numeric_limits::max() * TransportFeedback::kDeltaTick; + EXPECT_FALSE(packet->AddReceivedPacket(1, Timestamp::Zero() + + kMaxPositiveTimeDelta + + TransportFeedback::kDeltaTick)); + EXPECT_TRUE( + packet->AddReceivedPacket(1, Timestamp::Zero() + kMaxPositiveTimeDelta)); // Too large negative delta. packet.reset(new TransportFeedback()); - packet->SetBase(0, 0); - int64_t kMaxNegativeTimeDelta = std::numeric_limits::min() * - TransportFeedback::kDeltaScaleFactor; + TimeDelta kMaxNegativeTimeDelta = + std::numeric_limits::min() * TransportFeedback::kDeltaTick; + // Use larger base time to avoid kBaseTime + kNegativeDelta to be negative. + Timestamp kBaseTime = Timestamp::Seconds(1'000'000); + packet->SetBase(0, kBaseTime); EXPECT_FALSE(packet->AddReceivedPacket( - 1, kMaxNegativeTimeDelta - TransportFeedback::kDeltaScaleFactor)); - EXPECT_TRUE(packet->AddReceivedPacket(1, kMaxNegativeTimeDelta)); - - // Base time at maximum value. - int64_t kMaxBaseTime = - static_cast(TransportFeedback::kDeltaScaleFactor) * (1L << 8) * - ((1L << 23) - 1); - packet.reset(new TransportFeedback()); - packet->SetBase(0, kMaxBaseTime); - EXPECT_TRUE(packet->AddReceivedPacket(0, kMaxBaseTime)); - // Serialize and de-serialize (verify 24bit parsing). - rtc::Buffer raw_packet = packet->Build(); - packet = TransportFeedback::ParseFrom(raw_packet.data(), raw_packet.size()); - EXPECT_EQ(kMaxBaseTime, packet->GetBaseTimeUs()); - - // Base time above maximum value. - int64_t kTooLargeBaseTime = - kMaxBaseTime + (TransportFeedback::kDeltaScaleFactor * (1L << 8)); - packet.reset(new TransportFeedback()); - packet->SetBase(0, kTooLargeBaseTime); - packet->AddReceivedPacket(0, kTooLargeBaseTime); - raw_packet = packet->Build(); - packet = TransportFeedback::ParseFrom(raw_packet.data(), raw_packet.size()); - EXPECT_NE(kTooLargeBaseTime, packet->GetBaseTimeUs()); + 1, kBaseTime + kMaxNegativeTimeDelta - TransportFeedback::kDeltaTick)); + EXPECT_TRUE(packet->AddReceivedPacket(1, kBaseTime + kMaxNegativeTimeDelta)); // TODO(sprang): Once we support max length lower than RTCP length limit, // add back test for max size in bytes. } +TEST(RtcpPacketTest, BaseTimeIsConsistentAcrossMultiplePackets) { + constexpr Timestamp kMaxBaseTime = + Timestamp::Zero() + kBaseTimeWrapPeriod - kBaseTimeTick; + + TransportFeedback packet1; + packet1.SetBase(0, kMaxBaseTime); + packet1.AddReceivedPacket(0, kMaxBaseTime); + // Build and parse packet to simulate sending it over the wire. + TransportFeedback parsed_packet1 = Parse(packet1.Build()); + + TransportFeedback packet2; + packet2.SetBase(1, kMaxBaseTime + kBaseTimeTick); + packet2.AddReceivedPacket(1, kMaxBaseTime + kBaseTimeTick); + TransportFeedback parsed_packet2 = Parse(packet2.Build()); + + EXPECT_EQ(parsed_packet2.GetBaseDelta(parsed_packet1.BaseTime()), + kBaseTimeTick); +} + +TEST(RtcpPacketTest, SupportsMaximumNumberOfNegativeDeltas) { + TransportFeedback feedback; + // Use large base time to avoid hitting zero limit while filling the feedback, + // but use multiple of kBaseTimeWrapPeriod to hit edge case where base time + // is recorded as zero in the raw rtcp packet. + Timestamp time = Timestamp::Zero() + 1'000 * kBaseTimeWrapPeriod; + feedback.SetBase(0, time); + static constexpr TimeDelta kMinDelta = + TransportFeedback::kDeltaTick * std::numeric_limits::min(); + uint16_t num_received_rtp_packets = 0; + time += kMinDelta; + while (feedback.AddReceivedPacket(++num_received_rtp_packets, time)) { + ASSERT_GE(time, Timestamp::Zero() - kMinDelta); + time += kMinDelta; + } + // Subtract one last packet that failed to add. + --num_received_rtp_packets; + EXPECT_TRUE(feedback.IsConsistent()); + + TransportFeedback parsed = Parse(feedback.Build()); + EXPECT_EQ(parsed.GetReceivedPackets().size(), num_received_rtp_packets); + EXPECT_THAT(parsed.GetReceivedPackets(), + AllOf(SizeIs(num_received_rtp_packets), + Each(Property(&TransportFeedback::ReceivedPacket::delta, + Eq(kMinDelta))))); + EXPECT_GE(parsed.BaseTime(), + Timestamp::Zero() - kMinDelta * num_received_rtp_packets); +} + TEST(RtcpPacketTest, TransportFeedbackPadding) { const size_t kExpectedSizeBytes = kHeaderSize + kStatusChunkSize + kSmallDeltaSize; @@ -447,8 +513,8 @@ TEST(RtcpPacketTest, TransportFeedbackPadding) { 4 * kExpectedSizeWords - kExpectedSizeBytes; TransportFeedback feedback; - feedback.SetBase(0, 0); - EXPECT_TRUE(feedback.AddReceivedPacket(0, 0)); + feedback.SetBase(0, Timestamp::Zero()); + EXPECT_TRUE(feedback.AddReceivedPacket(0, Timestamp::Zero())); rtc::Buffer packet = feedback.Build(); EXPECT_EQ(kExpectedSizeWords * 4, packet.size()); @@ -475,10 +541,7 @@ TEST(RtcpPacketTest, TransportFeedbackPadding) { &mod_buffer[2], ByteReader::ReadBigEndian(&mod_buffer[2]) + ((kPaddingBytes + 3) / 4)); - std::unique_ptr parsed_packet( - TransportFeedback::ParseFrom(mod_buffer, kExpectedSizeWithPadding)); - ASSERT_TRUE(parsed_packet != nullptr); - EXPECT_EQ(kExpectedSizeWords * 4, packet.size()); // Padding not included. + EXPECT_THAT(mod_buffer, IsValidFeedback()); } TEST(RtcpPacketTest, TransportFeedbackPaddingBackwardsCompatibility) { @@ -489,8 +552,8 @@ TEST(RtcpPacketTest, TransportFeedbackPaddingBackwardsCompatibility) { 4 * kExpectedSizeWords - kExpectedSizeBytes; TransportFeedback feedback; - feedback.SetBase(0, 0); - EXPECT_TRUE(feedback.AddReceivedPacket(0, 0)); + feedback.SetBase(0, Timestamp::Zero()); + EXPECT_TRUE(feedback.AddReceivedPacket(0, Timestamp::Zero())); rtc::Buffer packet = feedback.Build(); EXPECT_EQ(kExpectedSizeWords * 4, packet.size()); @@ -510,47 +573,40 @@ TEST(RtcpPacketTest, TransportFeedbackPaddingBackwardsCompatibility) { const uint8_t padding_flag = 1 << 5; mod_buffer[0] &= ~padding_flag; // Unset padding flag. - std::unique_ptr parsed_packet( - TransportFeedback::ParseFrom(mod_buffer, kExpectedSizeWords * 4)); - ASSERT_TRUE(parsed_packet != nullptr); - EXPECT_EQ(kExpectedSizeWords * 4, packet.size()); + EXPECT_THAT(mod_buffer, IsValidFeedback()); } TEST(RtcpPacketTest, TransportFeedbackCorrectlySplitsVectorChunks) { const int kOneBitVectorCapacity = 14; - const int64_t kLargeTimeDelta = - TransportFeedback::kDeltaScaleFactor * (1 << 8); + const TimeDelta kLargeTimeDelta = TransportFeedback::kDeltaTick * (1 << 8); // Test that a number of small deltas followed by a large delta results in a // correct split into multiple chunks, as needed. for (int deltas = 0; deltas <= kOneBitVectorCapacity + 1; ++deltas) { TransportFeedback feedback; - feedback.SetBase(0, 0); + feedback.SetBase(0, Timestamp::Zero()); for (int i = 0; i < deltas; ++i) - feedback.AddReceivedPacket(i, i * 1000); - feedback.AddReceivedPacket(deltas, deltas * 1000 + kLargeTimeDelta); + feedback.AddReceivedPacket(i, Timestamp::Millis(i)); + feedback.AddReceivedPacket(deltas, + Timestamp::Millis(deltas) + kLargeTimeDelta); - rtc::Buffer serialized_packet = feedback.Build(); - std::unique_ptr deserialized_packet = - TransportFeedback::ParseFrom(serialized_packet.data(), - serialized_packet.size()); - EXPECT_TRUE(deserialized_packet != nullptr); + EXPECT_THAT(feedback.Build(), IsValidFeedback()); } } TEST(RtcpPacketTest, TransportFeedbackMoveConstructor) { const int kSamples = 100; - const int64_t kDelta = TransportFeedback::kDeltaScaleFactor; const uint16_t kBaseSeqNo = 7531; - const int64_t kBaseTimestampUs = 123456789; + const Timestamp kBaseTimestamp = Timestamp::Micros(123'456'789); const uint8_t kFeedbackSeqNo = 90; TransportFeedback feedback; - feedback.SetBase(kBaseSeqNo, kBaseTimestampUs); + feedback.SetBase(kBaseSeqNo, kBaseTimestamp); feedback.SetFeedbackSequenceNumber(kFeedbackSeqNo); for (int i = 0; i < kSamples; ++i) { - feedback.AddReceivedPacket(kBaseSeqNo + i, kBaseTimestampUs + i * kDelta); + feedback.AddReceivedPacket( + kBaseSeqNo + i, kBaseTimestamp + i * TransportFeedback::kDeltaTick); } EXPECT_TRUE(feedback.IsConsistent()); @@ -567,49 +623,42 @@ TEST(RtcpPacketTest, TransportFeedbackMoveConstructor) { TEST(TransportFeedbackTest, ReportsMissingPackets) { const uint16_t kBaseSeqNo = 1000; - const int64_t kBaseTimestampUs = 10000; + const Timestamp kBaseTimestamp = Timestamp::Millis(10); const uint8_t kFeedbackSeqNo = 90; TransportFeedback feedback_builder(/*include_timestamps*/ true); - feedback_builder.SetBase(kBaseSeqNo, kBaseTimestampUs); + feedback_builder.SetBase(kBaseSeqNo, kBaseTimestamp); feedback_builder.SetFeedbackSequenceNumber(kFeedbackSeqNo); - feedback_builder.AddReceivedPacket(kBaseSeqNo + 0, kBaseTimestampUs); + feedback_builder.AddReceivedPacket(kBaseSeqNo + 0, kBaseTimestamp); // Packet losses indicated by jump in sequence number. - feedback_builder.AddReceivedPacket(kBaseSeqNo + 3, kBaseTimestampUs + 2000); - rtc::Buffer coded = feedback_builder.Build(); + feedback_builder.AddReceivedPacket(kBaseSeqNo + 3, + kBaseTimestamp + TimeDelta::Millis(2)); - rtcp::CommonHeader header; - header.Parse(coded.data(), coded.size()); - TransportFeedback feedback(/*include_timestamps*/ true, - /*include_lost*/ true); - feedback.Parse(header); - auto packets = feedback.GetAllPackets(); - EXPECT_TRUE(packets[0].received()); - EXPECT_FALSE(packets[1].received()); - EXPECT_FALSE(packets[2].received()); - EXPECT_TRUE(packets[3].received()); + EXPECT_THAT( + Parse(feedback_builder.Build()).GetAllPackets(), + ElementsAre( + Property(&TransportFeedback::ReceivedPacket::received, true), + Property(&TransportFeedback::ReceivedPacket::received, false), + Property(&TransportFeedback::ReceivedPacket::received, false), + Property(&TransportFeedback::ReceivedPacket::received, true))); } TEST(TransportFeedbackTest, ReportsMissingPacketsWithoutTimestamps) { const uint16_t kBaseSeqNo = 1000; const uint8_t kFeedbackSeqNo = 90; TransportFeedback feedback_builder(/*include_timestamps*/ false); - feedback_builder.SetBase(kBaseSeqNo, 10000); + feedback_builder.SetBase(kBaseSeqNo, Timestamp::Millis(10)); feedback_builder.SetFeedbackSequenceNumber(kFeedbackSeqNo); - feedback_builder.AddReceivedPacket(kBaseSeqNo + 0, /*timestamp_us*/ 0); + feedback_builder.AddReceivedPacket(kBaseSeqNo + 0, Timestamp::Zero()); // Packet losses indicated by jump in sequence number. - feedback_builder.AddReceivedPacket(kBaseSeqNo + 3, /*timestamp_us*/ 0); - rtc::Buffer coded = feedback_builder.Build(); + feedback_builder.AddReceivedPacket(kBaseSeqNo + 3, Timestamp::Zero()); - rtcp::CommonHeader header; - header.Parse(coded.data(), coded.size()); - TransportFeedback feedback(/*include_timestamps*/ true, - /*include_lost*/ true); - feedback.Parse(header); - auto packets = feedback.GetAllPackets(); - EXPECT_TRUE(packets[0].received()); - EXPECT_FALSE(packets[1].received()); - EXPECT_FALSE(packets[2].received()); - EXPECT_TRUE(packets[3].received()); + EXPECT_THAT( + Parse(feedback_builder.Build()).GetAllPackets(), + ElementsAre( + Property(&TransportFeedback::ReceivedPacket::received, true), + Property(&TransportFeedback::ReceivedPacket::received, false), + Property(&TransportFeedback::ReceivedPacket::received, false), + Property(&TransportFeedback::ReceivedPacket::received, true))); } } // namespace } // namespace webrtc