diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index c3d60fe35d..f8da97ae75 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -112,33 +112,24 @@ std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( } last_timestamp_us_ = timestamp_us; - uint16_t sequence_number = feedback.GetBaseSequence(); - std::vector delta_vec = feedback.GetReceiveDeltasUs(); - auto delta_it = delta_vec.begin(); std::vector packet_feedback_vector; - packet_feedback_vector.reserve(delta_vec.size()); - + packet_feedback_vector.reserve(feedback.GetReceivedPackets().size()); { rtc::CritScope cs(&lock_); size_t failed_lookups = 0; int64_t offset_us = 0; - for (auto symbol : feedback.GetStatusVector()) { - if (symbol != rtcp::TransportFeedback::StatusSymbol::kNotReceived) { - RTC_DCHECK(delta_it != delta_vec.end()); - offset_us += *(delta_it++); - int64_t timestamp_ms = current_offset_ms_ + (offset_us / 1000); - PacketInfo info(timestamp_ms, sequence_number); - if (send_time_history_.GetInfo(&info, true) && info.send_time_ms >= 0) { - packet_feedback_vector.push_back(info); - } else { - ++failed_lookups; - } + for (const auto& packet : feedback.GetReceivedPackets()) { + offset_us += packet.delta_us(); + int64_t timestamp_ms = current_offset_ms_ + (offset_us / 1000); + PacketInfo info(timestamp_ms, packet.sequence_number()); + if (send_time_history_.GetInfo(&info, true) && info.send_time_ms >= 0) { + packet_feedback_vector.push_back(info); + } else { + ++failed_lookups; } - ++sequence_number; } std::sort(packet_feedback_vector.begin(), packet_feedback_vector.end(), PacketInfoComparator()); - RTC_DCHECK(delta_it == delta_vec.end()); if (failed_lookups > 0) { LOG(LS_WARNING) << "Failed to lookup send time for " << failed_lookups << " packet" << (failed_lookups > 1 ? "s" : "") diff --git a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index dbb3eaa4d7..1aeb55bcea 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -16,15 +16,43 @@ #include "webrtc/test/gtest.h" using ::testing::_; -using ::testing::InSequence; +using ::testing::ElementsAre; using ::testing::Invoke; using ::testing::Return; namespace webrtc { +namespace { + +constexpr size_t kDefaultPacketSize = 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; + +std::vector SequenceNumbers( + const rtcp::TransportFeedback& feedback_packet) { + std::vector sequence_numbers; + for (const auto& rtp_packet_received : feedback_packet.GetReceivedPackets()) { + sequence_numbers.push_back(rtp_packet_received.sequence_number()); + } + return sequence_numbers; +} + +std::vector TimestampsMs( + const rtcp::TransportFeedback& feedback_packet) { + std::vector timestamps; + int64_t timestamp_us = feedback_packet.GetBaseTimeUs(); + for (const auto& rtp_packet_received : feedback_packet.GetReceivedPackets()) { + timestamp_us += rtp_packet_received.delta_us(); + timestamps.push_back(timestamp_us / 1000); + } + return timestamps; +} class MockPacketRouter : public PacketRouter { public: - MOCK_METHOD1(SendFeedback, bool(rtcp::TransportFeedback* packet)); + MOCK_METHOD1(SendFeedback, bool(rtcp::TransportFeedback* feedback_packet)); }; class RemoteEstimatorProxyTest : public ::testing::Test { @@ -49,33 +77,18 @@ class RemoteEstimatorProxyTest : public ::testing::Test { SimulatedClock clock_; testing::StrictMock router_; RemoteEstimatorProxy proxy_; - - const size_t kDefaultPacketSize = 100; - const uint32_t kMediaSsrc = 456; - const uint16_t kBaseSeq = 10; - const int64_t kBaseTimeMs = 123; - const int64_t kMaxSmallDeltaMs = - (rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) / 1000; }; TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { IncomingPacket(kBaseSeq, kBaseTimeMs); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(1u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[0]); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); + EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); return true; })); @@ -87,20 +100,12 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { IncomingPacket(kBaseSeq, kBaseTimeMs + 1000); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(1u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[0]); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); + EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); return true; })); @@ -111,30 +116,21 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { // First feedback. IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000); - EXPECT_CALL(router_, SendFeedback(_)).Times(1).WillOnce(Return(true)); + EXPECT_CALL(router_, SendFeedback(_)).WillOnce(Return(true)); Process(); // Second feedback starts with a missing packet (DROP kBaseSeq + 2). IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq + 2, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(2u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kNotReceived, - status_vec[0]); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[1]); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs + 3000, - (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq + 3)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs + 3000)); return true; })); @@ -147,27 +143,15 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(3u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[0]); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[1]); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedLargeDelta, - status_vec[2]); - - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(3u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); - EXPECT_EQ(kMaxSmallDeltaMs, delta_vec[1] / 1000); - EXPECT_EQ(kMaxSmallDeltaMs + 1, delta_vec[2] / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 2)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs, + kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1)); return true; })); @@ -175,51 +159,31 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { } TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { - const int64_t kTooLargeDelta = + static constexpr int64_t kTooLargeDelta = rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 16); IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta); - InSequence s; EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([kTooLargeDelta, this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(1u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[0]); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), ElementsAre(kBaseSeq)); + EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); return true; })) - .RetiresOnSaturation(); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([kTooLargeDelta, this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq + 1, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); - - std::vector status_vec = - packet->GetStatusVector(); - EXPECT_EQ(1u, status_vec.size()); - EXPECT_EQ(rtcp::TransportFeedback::StatusSymbol::kReceivedSmallDelta, - status_vec[0]); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs + kTooLargeDelta, - (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq + 1)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs + kTooLargeDelta)); return true; - })) - .RetiresOnSaturation(); + })); Process(); } @@ -231,15 +195,11 @@ TEST_F(RemoteEstimatorProxyTest, GracefullyHandlesReorderingAndWrap) { IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); return true; })); @@ -251,16 +211,14 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(2u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); - EXPECT_EQ(2, delta_vec[1] / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq, kBaseSeq + 2)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs, kBaseTimeMs + 2)); return true; })); @@ -269,17 +227,14 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq + 1, packet->GetBaseSequence()); - EXPECT_EQ(kMediaSsrc, packet->media_ssrc()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq + 1, feedback_packet->GetBaseSequence()); + EXPECT_EQ(kMediaSsrc, feedback_packet->media_ssrc()); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(2u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs + 1, - (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); - EXPECT_EQ(1, delta_vec[1] / 1000); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq + 1, kBaseSeq + 2)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2)); return true; })); @@ -293,14 +248,10 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 2, kBaseTimeMs); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq + 2, packet->GetBaseSequence()); + .WillOnce(Invoke([](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq + 2, feedback_packet->GetBaseSequence()); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs, (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); + EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); return true; })); @@ -309,17 +260,14 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs); // kBaseSeq + 2 times out here. EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq + 3, packet->GetBaseSequence()); + .WillOnce( + Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq + 3, feedback_packet->GetBaseSequence()); - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(1u, delta_vec.size()); - EXPECT_EQ(kTimeoutTimeMs, - (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); - return true; - })); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kTimeoutTimeMs)); + return true; + })); Process(); @@ -329,23 +277,17 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1); EXPECT_CALL(router_, SendFeedback(_)) - .Times(1) - .WillOnce(Invoke([kTimeoutTimeMs, this](rtcp::TransportFeedback* packet) { - packet->Build(); - EXPECT_EQ(kBaseSeq, packet->GetBaseSequence()); + .WillOnce( + Invoke([kTimeoutTimeMs](rtcp::TransportFeedback* feedback_packet) { + EXPECT_EQ(kBaseSeq, feedback_packet->GetBaseSequence()); - // Four status entries (kBaseSeq + 3 missing). - EXPECT_EQ(4u, packet->GetStatusVector().size()); - - // Only three actual timestamps. - std::vector delta_vec = packet->GetReceiveDeltasUs(); - EXPECT_EQ(3u, delta_vec.size()); - EXPECT_EQ(kBaseTimeMs - 1, - (packet->GetBaseTimeUs() + delta_vec[0]) / 1000); - EXPECT_EQ(kTimeoutTimeMs - kBaseTimeMs, delta_vec[1] / 1000); - EXPECT_EQ(1, delta_vec[2] / 1000); - return true; - })); + EXPECT_THAT(SequenceNumbers(*feedback_packet), + ElementsAre(kBaseSeq, kBaseSeq + 1, kBaseSeq + 3)); + EXPECT_THAT(TimestampsMs(*feedback_packet), + ElementsAre(kBaseTimeMs - 1, kTimeoutTimeMs - 1, + kTimeoutTimeMs)); + return true; + })); Process(); } @@ -391,4 +333,5 @@ TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { EXPECT_EQ(136, proxy_.TimeUntilNextProcess()); } +} // namespace } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc index 42c0cc63ec..78407e5a74 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc @@ -366,6 +366,11 @@ bool TransportFeedback::AddReceivedPacket(uint16_t sequence_number, return true; } +const std::vector& +TransportFeedback::GetReceivedPackets() const { + return packets_; +} + uint16_t TransportFeedback::GetBaseSequence() const { return base_seq_no_; } @@ -375,9 +380,9 @@ TransportFeedback::GetStatusVector() const { std::vector symbols; uint16_t seq_no = GetBaseSequence(); for (const auto& packet : packets_) { - for (; seq_no != packet.sequence_number; ++seq_no) + for (; seq_no != packet.sequence_number(); ++seq_no) symbols.push_back(StatusSymbol::kNotReceived); - if (packet.delta_ticks >= 0x00 && packet.delta_ticks <= 0xff) { + if (packet.delta_ticks() >= 0x00 && packet.delta_ticks() <= 0xff) { symbols.push_back(StatusSymbol::kReceivedSmallDelta); } else { symbols.push_back(StatusSymbol::kReceivedLargeDelta); @@ -390,7 +395,7 @@ TransportFeedback::GetStatusVector() const { std::vector TransportFeedback::GetReceiveDeltas() const { std::vector deltas; for (const auto& packet : packets_) - deltas.push_back(packet.delta_ticks); + deltas.push_back(packet.delta_ticks()); return deltas; } @@ -401,7 +406,7 @@ int64_t TransportFeedback::GetBaseTimeUs() const { std::vector TransportFeedback::GetReceiveDeltasUs() const { std::vector us_deltas; for (const auto& packet : packets_) - us_deltas.push_back(packet.delta_ticks * kDeltaScaleFactor); + us_deltas.push_back(packet.delta_us()); return us_deltas; } @@ -534,18 +539,18 @@ bool TransportFeedback::IsConsistent() const { LOG(LS_ERROR) << "Failed to find delta for seq_no " << seq_no; return false; } - if (packet_it->sequence_number != seq_no) { + if (packet_it->sequence_number() != seq_no) { LOG(LS_ERROR) << "Expected to find delta for seq_no " << seq_no - << ". Next delta is for " << packet_it->sequence_number; + << ". Next delta is for " << packet_it->sequence_number(); return false; } if (delta_size == 1 && - (packet_it->delta_ticks < 0 || packet_it->delta_ticks > 0xff)) { - LOG(LS_ERROR) << "Delta " << packet_it->delta_ticks << " for seq_no " + (packet_it->delta_ticks() < 0 || packet_it->delta_ticks() > 0xff)) { + LOG(LS_ERROR) << "Delta " << packet_it->delta_ticks() << " for seq_no " << seq_no << " doesn't fit into one byte"; return false; } - timestamp_us += packet_it->delta_ticks * kDeltaScaleFactor; + timestamp_us += packet_it->delta_us(); ++packet_it; } packet_size += delta_size; @@ -553,7 +558,7 @@ bool TransportFeedback::IsConsistent() const { } if (packet_it != packets_.end()) { LOG(LS_ERROR) << "Unencoded delta for seq_no " - << packet_it->sequence_number; + << packet_it->sequence_number(); return false; } if (timestamp_us != last_timestamp_us_) { @@ -610,7 +615,7 @@ bool TransportFeedback::Create(uint8_t* packet, } for (const auto& received_packet : packets_) { - int16_t delta = received_packet.delta_ticks; + int16_t delta = received_packet.delta_ticks(); if (delta >= 0 && delta <= 0xFF) { packet[(*position)++] = delta; } else { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h index a1208293bc..9f2071174d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h @@ -23,6 +23,21 @@ class CommonHeader; class TransportFeedback : public Rtpfb { public: + class ReceivedPacket { + public: + ReceivedPacket(uint16_t sequence_number, int16_t delta_ticks) + : sequence_number_(sequence_number), delta_ticks_(delta_ticks) {} + ReceivedPacket(const ReceivedPacket&) = default; + ReceivedPacket& operator=(const ReceivedPacket&) = default; + + uint16_t sequence_number() const { return sequence_number_; } + int16_t delta_ticks() const { return delta_ticks_; } + int32_t delta_us() const { return delta_ticks_ * kDeltaScaleFactor; } + + private: + uint16_t sequence_number_; + int16_t delta_ticks_; + }; // TODO(sprang): IANA reg? static constexpr uint8_t kFeedbackMessageType = 15; // Convert to multiples of 0.25ms. @@ -38,6 +53,7 @@ class TransportFeedback : public Rtpfb { void SetFeedbackSequenceNumber(uint8_t feedback_sequence); // NOTE: This method requires increasing sequence numbers (excepting wraps). bool AddReceivedPacket(uint16_t sequence_number, int64_t timestamp_us); + const std::vector& GetReceivedPackets() const; enum class StatusSymbol { kNotReceived, @@ -76,12 +92,6 @@ class TransportFeedback : public Rtpfb { using DeltaSize = uint8_t; // Keeps DeltaSizes that can be encoded into single chunk if it is last chunk. class LastChunk; - struct ReceivedPacket { - ReceivedPacket(uint16_t sequence_number, int16_t delta_ticks) - : sequence_number(sequence_number), delta_ticks(delta_ticks) {} - uint16_t sequence_number; - int16_t delta_ticks; - }; // Reset packet to consistent empty state. void Clear(); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc index fac17a2524..b2923da8bf 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc @@ -14,13 +14,15 @@ #include #include "webrtc/modules/rtp_rtcp/source/byte_io.h" +#include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" -using webrtc::rtcp::TransportFeedback; - namespace webrtc { namespace { +using ::testing::ElementsAreArray; +using rtcp::TransportFeedback; + static const int kHeaderSize = 20; static const int kStatusChunkSize = 2; static const int kSmallDeltaSize = 1; @@ -94,31 +96,14 @@ class FeedbackTester { EXPECT_EQ(expected_size_bytes, serialized_.size()); } - std::vector symbols = - feedback_->GetStatusVector(); - uint16_t seq = feedback_->GetBaseSequence(); - auto seq_it = expected_seq_.begin(); - for (TransportFeedback::StatusSymbol symbol : symbols) { - bool received = - (symbol == TransportFeedback::StatusSymbol::kReceivedSmallDelta || - symbol == TransportFeedback::StatusSymbol::kReceivedLargeDelta); - if (seq_it != expected_seq_.end()) { - if (seq == *seq_it) { - ASSERT_NE(expected_seq_.end(), seq_it); - ASSERT_TRUE(received) << "Expected received packet @ " << seq; - ++seq_it; - } else { - ASSERT_FALSE(received) << "Did not expect received packet @ " << seq; - } - } - ++seq; + std::vector actual_seq_nos; + std::vector actual_deltas_us; + for (const auto& packet : feedback_->GetReceivedPackets()) { + actual_seq_nos.push_back(packet.sequence_number()); + actual_deltas_us.push_back(packet.delta_us()); } - ASSERT_EQ(expected_seq_.end(), seq_it); - - std::vector deltas = feedback_->GetReceiveDeltasUs(); - ASSERT_EQ(expected_deltas_.size(), deltas.size()); - for (size_t i = 0; i < expected_deltas_.size(); ++i) - EXPECT_EQ(expected_deltas_[i], deltas[i]) << "Delta mismatch @ " << i; + EXPECT_THAT(actual_seq_nos, ElementsAreArray(expected_seq_)); + EXPECT_THAT(actual_deltas_us, ElementsAreArray(expected_deltas_)); } void GenerateDeltas(const uint16_t seq[], @@ -344,12 +329,11 @@ TEST(RtcpPacketTest, TransportFeedback_Aliasing) { feedback.AddReceivedPacket(i, i * kTooSmallDelta); feedback.Build(); - std::vector deltas = feedback.GetReceiveDeltasUs(); int64_t accumulated_delta = 0; int num_samples = 0; - for (int64_t delta : deltas) { - accumulated_delta += delta; + for (const auto& packet : feedback.GetReceivedPackets()) { + accumulated_delta += packet.delta_us(); int64_t expected_time = num_samples * kTooSmallDelta; ++num_samples;