diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc index 87e917e978..d4cc915fd1 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc @@ -205,7 +205,7 @@ TransportFeedbackAdapter::ProcessTransportFeedbackInner( current_offset_ += delta; } } - last_timestamp_ = feedback.GetBaseTime(); + last_timestamp_ = feedback.BaseTime(); std::vector packet_result_vector; packet_result_vector.reserve(feedback.GetPacketStatusCount()); diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.h b/modules/congestion_controller/rtp/transport_feedback_adapter.h index deb7925d77..f9f939db9c 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter.h +++ b/modules/congestion_controller/rtp/transport_feedback_adapter.h @@ -18,6 +18,7 @@ #include "api/sequence_checker.h" #include "api/transport/network_types.h" +#include "api/units/timestamp.h" #include "modules/include/module_common_types_public.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/network/sent_packet.h" @@ -91,7 +92,7 @@ class TransportFeedbackAdapter { InFlightBytesTracker in_flight_; Timestamp current_offset_ = Timestamp::MinusInfinity(); - TimeDelta last_timestamp_ = TimeDelta::MinusInfinity(); + Timestamp last_timestamp_ = Timestamp::MinusInfinity(); rtc::NetworkRoute network_route_; }; diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc index 933abd9bf0..14a2b13831 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc @@ -137,11 +137,11 @@ TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) { rtcp::TransportFeedback feedback; feedback.SetBase(packets[0].sent_packet.sequence_number, - packets[0].receive_time.us()); + packets[0].receive_time); for (const auto& packet : packets) { EXPECT_TRUE(feedback.AddReceivedPacket(packet.sent_packet.sequence_number, - packet.receive_time.us())); + packet.receive_time)); } feedback.Build(); @@ -171,11 +171,11 @@ TEST_F(TransportFeedbackAdapterTest, FeedbackVectorReportsUnreceived) { rtcp::TransportFeedback feedback; feedback.SetBase(received_packets[0].sent_packet.sequence_number, - received_packets[0].receive_time.us()); + received_packets[0].receive_time); for (const auto& packet : received_packets) { EXPECT_TRUE(feedback.AddReceivedPacket(packet.sent_packet.sequence_number, - packet.receive_time.us())); + packet.receive_time)); } feedback.Build(); @@ -202,12 +202,12 @@ TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { rtcp::TransportFeedback feedback; feedback.SetBase(packets[0].sent_packet.sequence_number, - packets[0].receive_time.us()); + packets[0].receive_time); for (const auto& packet : packets) { if (packet.sent_packet.sequence_number <= kReceiveSideDropAfter) { EXPECT_TRUE(feedback.AddReceivedPacket(packet.sent_packet.sequence_number, - packet.receive_time.us())); + packet.receive_time)); } } @@ -225,16 +225,15 @@ TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) { } TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { - int64_t kHighArrivalTimeMs = rtcp::TransportFeedback::kDeltaScaleFactor * - static_cast(1 << 8) * - static_cast((1 << 23) - 1) / 1000; + TimeDelta kHighArrivalTime = + rtcp::TransportFeedback::kDeltaTick * (1 << 8) * ((1 << 23) - 1); std::vector packets; + packets.push_back(CreatePacket(kHighArrivalTime.ms() + 64, 210, 0, 1500, + PacedPacketInfo())); + packets.push_back(CreatePacket(kHighArrivalTime.ms() - 64, 210, 1, 1500, + PacedPacketInfo())); packets.push_back( - CreatePacket(kHighArrivalTimeMs + 64, 210, 0, 1500, PacedPacketInfo())); - packets.push_back( - CreatePacket(kHighArrivalTimeMs - 64, 210, 1, 1500, PacedPacketInfo())); - packets.push_back( - CreatePacket(kHighArrivalTimeMs, 220, 2, 1500, PacedPacketInfo())); + CreatePacket(kHighArrivalTime.ms(), 220, 2, 1500, PacedPacketInfo())); for (const auto& packet : packets) OnSentPacket(packet); @@ -243,10 +242,10 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { std::unique_ptr feedback( new rtcp::TransportFeedback()); feedback->SetBase(packets[i].sent_packet.sequence_number, - packets[i].receive_time.us()); + packets[i].receive_time); EXPECT_TRUE(feedback->AddReceivedPacket( - packets[i].sent_packet.sequence_number, packets[i].receive_time.us())); + packets[i].sent_packet.sequence_number, packets[i].receive_time)); rtc::Buffer raw_packet = feedback->Build(); feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(), @@ -272,11 +271,11 @@ TEST_F(TransportFeedbackAdapterTest, HandlesArrivalReordering) { rtcp::TransportFeedback feedback; feedback.SetBase(packets[0].sent_packet.sequence_number, - packets[0].receive_time.us()); + packets[0].receive_time); for (const auto& packet : packets) { EXPECT_TRUE(feedback.AddReceivedPacket(packet.sent_packet.sequence_number, - packet.receive_time.us())); + packet.receive_time)); } feedback.Build(); @@ -291,17 +290,14 @@ TEST_F(TransportFeedbackAdapterTest, HandlesArrivalReordering) { TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { std::vector sent_packets; // TODO(srte): Consider using us resolution in the constants. - const TimeDelta kSmallDelta = - TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) - .RoundDownTo(TimeDelta::Millis(1)); - const TimeDelta kLargePositiveDelta = - TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor * - std::numeric_limits::max()) - .RoundDownTo(TimeDelta::Millis(1)); - const TimeDelta kLargeNegativeDelta = - TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor * - std::numeric_limits::min()) - .RoundDownTo(TimeDelta::Millis(1)); + const TimeDelta kSmallDelta = (rtcp::TransportFeedback::kDeltaTick * 0xFF) + .RoundDownTo(TimeDelta::Millis(1)); + const TimeDelta kLargePositiveDelta = (rtcp::TransportFeedback::kDeltaTick * + std::numeric_limits::max()) + .RoundDownTo(TimeDelta::Millis(1)); + const TimeDelta kLargeNegativeDelta = (rtcp::TransportFeedback::kDeltaTick * + std::numeric_limits::min()) + .RoundDownTo(TimeDelta::Millis(1)); PacketResult packet_feedback; packet_feedback.sent_packet.sequence_number = 1; @@ -342,15 +338,15 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { std::unique_ptr feedback( new rtcp::TransportFeedback()); feedback->SetBase(sent_packets[0].sent_packet.sequence_number, - sent_packets[0].receive_time.us()); + sent_packets[0].receive_time); for (const auto& packet : sent_packets) { EXPECT_TRUE(feedback->AddReceivedPacket(packet.sent_packet.sequence_number, - packet.receive_time.us())); + packet.receive_time)); } EXPECT_FALSE( feedback->AddReceivedPacket(packet_feedback.sent_packet.sequence_number, - packet_feedback.receive_time.us())); + packet_feedback.receive_time)); rtc::Buffer raw_packet = feedback->Build(); feedback = @@ -366,10 +362,10 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { // Create a new feedback message and add the trailing item. feedback.reset(new rtcp::TransportFeedback()); feedback->SetBase(packet_feedback.sent_packet.sequence_number, - packet_feedback.receive_time.us()); + packet_feedback.receive_time); EXPECT_TRUE( feedback->AddReceivedPacket(packet_feedback.sent_packet.sequence_number, - packet_feedback.receive_time.us())); + packet_feedback.receive_time)); raw_packet = feedback->Build(); feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(), raw_packet.size()); diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc b/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc index 482f58d1bb..52d8018bff 100644 --- a/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc +++ b/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc @@ -61,10 +61,12 @@ TEST(TransportFeedbackDemuxerTest, ObserverSanity) { kSsrc, kRtpStartSeq + 2, kTransportStartSeq + 2, /*is_retransmit=*/true)); rtcp::TransportFeedback feedback; - feedback.SetBase(kTransportStartSeq, 1000); - ASSERT_TRUE(feedback.AddReceivedPacket(kTransportStartSeq, 1000)); + feedback.SetBase(kTransportStartSeq, Timestamp::Millis(1)); + ASSERT_TRUE( + feedback.AddReceivedPacket(kTransportStartSeq, Timestamp::Millis(1))); // Drop middle packet. - ASSERT_TRUE(feedback.AddReceivedPacket(kTransportStartSeq + 2, 3000)); + ASSERT_TRUE( + feedback.AddReceivedPacket(kTransportStartSeq + 2, Timestamp::Millis(3))); EXPECT_CALL( mock, OnPacketFeedbackVector(ElementsAre( @@ -87,8 +89,9 @@ TEST(TransportFeedbackDemuxerTest, ObserverSanity) { demuxer.AddPacket( CreatePacket(kSsrc, kRtpStartSeq + 3, kTransportStartSeq + 3, false)); rtcp::TransportFeedback second_feedback; - second_feedback.SetBase(kTransportStartSeq + 3, 4000); - ASSERT_TRUE(second_feedback.AddReceivedPacket(kTransportStartSeq + 3, 4000)); + second_feedback.SetBase(kTransportStartSeq + 3, Timestamp::Millis(4)); + ASSERT_TRUE(second_feedback.AddReceivedPacket(kTransportStartSeq + 3, + Timestamp::Millis(4))); EXPECT_CALL(mock, OnPacketFeedbackVector).Times(0); demuxer.OnTransportFeedback(second_feedback); diff --git a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc index 719fbf636e..d570a92c8c 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc @@ -387,20 +387,9 @@ Timestamp TransportFeedback::BaseTime() const { 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() % kTimeWrapPeriod.us(); - if (base_time_us >= kTimeWrapPeriod.us() / 2) { - return base_time_us - kTimeWrapPeriod.us(); - } else { - return base_time_us; - } -} - -namespace { -TimeDelta CompensateForWrapAround(TimeDelta delta) { +TimeDelta TransportFeedback::GetBaseDelta(Timestamp prev_timestamp) const { + TimeDelta delta = BaseTime() - prev_timestamp; + // Compensate for wrap around. if ((delta - kTimeWrapPeriod).Abs() < delta.Abs()) { delta -= kTimeWrapPeriod; // Wrap backwards. } else if ((delta + kTimeWrapPeriod).Abs() < delta.Abs()) { @@ -408,20 +397,6 @@ TimeDelta CompensateForWrapAround(TimeDelta delta) { } return delta; } -} // namespace - -int64_t TransportFeedback::GetBaseDeltaUs(int64_t prev_timestamp_us) const { - int64_t delta_us = GetBaseTimeUs() - prev_timestamp_us; - return CompensateForWrapAround(TimeDelta::Micros(delta_us)).us(); -} - -TimeDelta TransportFeedback::GetBaseDelta(TimeDelta prev_timestamp) const { - return CompensateForWrapAround(GetBaseTime() - prev_timestamp); -} - -TimeDelta TransportFeedback::GetBaseDelta(Timestamp prev_timestamp) const { - return CompensateForWrapAround(BaseTime() - prev_timestamp); -} // De-serialize packet. bool TransportFeedback::Parse(const CommonHeader& packet) { diff --git a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h index 8c92708bae..22f384fe05 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h +++ b/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h @@ -23,8 +23,6 @@ namespace webrtc { namespace rtcp { class CommonHeader; -// TODO(bugs.webrtc.org/13757): Uncomment ABSL_DEPRECATED attributes or delete -// functions they are attached to when all usage within webrtc is updated. class TransportFeedback : public Rtpfb { public: class ReceivedPacket { @@ -40,8 +38,6 @@ 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().us(); } TimeDelta delta() const { return delta_ticks_ * kDeltaTick; } bool received() const { return received_; } @@ -53,8 +49,6 @@ class TransportFeedback : public Rtpfb { // TODO(sprang): IANA reg? static constexpr uint8_t kFeedbackMessageType = 15; // Convert to multiples of 0.25ms. - // ABSL_DEPRECATED("Use kDeltaTick") - static constexpr int kDeltaScaleFactor = 250; static constexpr TimeDelta kDeltaTick = TimeDelta::Micros(250); // Maximum number of packets (including missing) TransportFeedback can report. static constexpr size_t kMaxReportedPackets = 0xffff; @@ -70,20 +64,11 @@ 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. - 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) { - 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; @@ -94,17 +79,9 @@ class TransportFeedback : public Rtpfb { size_t GetPacketStatusCount() const { return num_seq_no_; } // Get the reference time including any precision loss. - // ABSL_DEPRECATED("Use BaseTime that returns Timestamp") - int64_t GetBaseTimeUs() const; - // ABSL_DEPRECATED("Use BaseTime that returns Timestamp") - TimeDelta GetBaseTime() const { return BaseTime() - Timestamp::Zero(); } Timestamp BaseTime() const; - // Get the unwrapped delta between current base time and `prev_timestamp_us`. - // ABSL_DEPRECATED("Use GetBaseDelta that takes Timestamp") - int64_t GetBaseDeltaUs(int64_t prev_timestamp_us) const; - // ABSL_DEPRECATED("Use GetBaseDelta that takes Timestamp") - TimeDelta GetBaseDelta(TimeDelta prev_timestamp) const; + // Get the unwrapped delta between current base time and `prev_timestamp`. TimeDelta GetBaseDelta(Timestamp prev_timestamp) const; // Does the feedback packet contain timestamp information? diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 45b7902ec6..a3446c43f0 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -1780,8 +1780,8 @@ TEST(RtcpReceiverTest, ReceivesTransportFeedback) { rtcp::TransportFeedback packet; packet.SetMediaSsrc(kReceiverMainSsrc); packet.SetSenderSsrc(kSenderSsrc); - packet.SetBase(1, 1000); - packet.AddReceivedPacket(1, 1000); + packet.SetBase(1, Timestamp::Millis(1)); + packet.AddReceivedPacket(1, Timestamp::Millis(1)); EXPECT_CALL( mocks.transport_feedback_observer, @@ -1815,8 +1815,8 @@ TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) { auto packet = std::make_unique(); packet->SetMediaSsrc(kReceiverMainSsrc); packet->SetSenderSsrc(kSenderSsrc); - packet->SetBase(1, 1000); - packet->AddReceivedPacket(1, 1000); + packet->SetBase(1, Timestamp::Millis(1)); + packet->AddReceivedPacket(1, Timestamp::Millis(1)); static uint32_t kBitrateBps = 50000; auto remb = std::make_unique(); diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 4007cf14d8..ae59dc5b0c 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -829,7 +829,7 @@ TEST_F(RtcpSenderTest, SendsCombinedRtcpPacket) { std::vector> packets; auto transport_feedback = std::make_unique(); - transport_feedback->AddReceivedPacket(321, 10000); + transport_feedback->AddReceivedPacket(321, Timestamp::Millis(10)); packets.push_back(std::move(transport_feedback)); auto remote_estimate = std::make_unique(); packets.push_back(std::move(remote_estimate)); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index da7a875ae1..8eaa3915ea 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -1473,9 +1473,9 @@ TEST_F(RtcpTransceiverImplTest, ParsesTransportFeedback) { })); rtcp::TransportFeedback tb; - tb.SetBase(/*base_sequence=*/321, /*ref_timestamp_us=*/15); - tb.AddReceivedPacket(/*base_sequence=*/321, /*timestamp_us=*/15); - tb.AddReceivedPacket(/*base_sequence=*/322, /*timestamp_us=*/17); + tb.SetBase(/*base_sequence=*/321, Timestamp::Micros(15)); + tb.AddReceivedPacket(/*base_sequence=*/321, Timestamp::Micros(15)); + tb.AddReceivedPacket(/*base_sequence=*/322, Timestamp::Micros(17)); rtcp_transceiver.ReceivePacket(tb.Build(), receive_time); } diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc index 290aa48ff4..57652f2305 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc @@ -13,6 +13,7 @@ #include #include +#include "api/units/timestamp.h" #include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h" #include "modules/rtp_rtcp/source/rtcp_packet/sender_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" @@ -37,6 +38,7 @@ using ::webrtc::RtcpTransceiver; using ::webrtc::RtcpTransceiverConfig; using ::webrtc::SimulatedClock; using ::webrtc::TaskQueueForTest; +using ::webrtc::Timestamp; using ::webrtc::rtcp::RemoteEstimate; using ::webrtc::rtcp::RtcpPacket; using ::webrtc::rtcp::TransportFeedback; @@ -309,7 +311,7 @@ TEST(RtcpTransceiverTest, SendsCombinedRtcpPacketOnTaskQueue) { // Create minimalistic transport feedback packet. std::vector> packets; auto transport_feedback = std::make_unique(); - transport_feedback->AddReceivedPacket(321, 10000); + transport_feedback->AddReceivedPacket(321, Timestamp::Millis(10)); packets.push_back(std::move(transport_feedback)); auto remote_estimate = std::make_unique();