in rtcp::TransportFeedback delete functions with time represented as raw int

Bug: webrtc:13757
Change-Id: I53c8ed21ac37a3aee13482c6bb68a0c5ee8fcbee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265681
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37259}
This commit is contained in:
Danil Chapovalov
2022-06-17 10:49:16 +02:00
committed by WebRTC LUCI CQ
parent a116da5d03
commit a3cb977679
10 changed files with 56 additions and 102 deletions

View File

@ -205,7 +205,7 @@ TransportFeedbackAdapter::ProcessTransportFeedbackInner(
current_offset_ += delta;
}
}
last_timestamp_ = feedback.GetBaseTime();
last_timestamp_ = feedback.BaseTime();
std::vector<PacketResult> packet_result_vector;
packet_result_vector.reserve(feedback.GetPacketStatusCount());

View File

@ -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_;
};

View File

@ -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<int64_t>(1 << 8) *
static_cast<int64_t>((1 << 23) - 1) / 1000;
TimeDelta kHighArrivalTime =
rtcp::TransportFeedback::kDeltaTick * (1 << 8) * ((1 << 23) - 1);
std::vector<PacketResult> 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<rtcp::TransportFeedback> 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,15 +290,12 @@ TEST_F(TransportFeedbackAdapterTest, HandlesArrivalReordering) {
TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
std::vector<PacketResult> sent_packets;
// TODO(srte): Consider using us resolution in the constants.
const TimeDelta kSmallDelta =
TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF)
const TimeDelta kSmallDelta = (rtcp::TransportFeedback::kDeltaTick * 0xFF)
.RoundDownTo(TimeDelta::Millis(1));
const TimeDelta kLargePositiveDelta =
TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor *
const TimeDelta kLargePositiveDelta = (rtcp::TransportFeedback::kDeltaTick *
std::numeric_limits<int16_t>::max())
.RoundDownTo(TimeDelta::Millis(1));
const TimeDelta kLargeNegativeDelta =
TimeDelta::Micros(rtcp::TransportFeedback::kDeltaScaleFactor *
const TimeDelta kLargeNegativeDelta = (rtcp::TransportFeedback::kDeltaTick *
std::numeric_limits<int16_t>::min())
.RoundDownTo(TimeDelta::Millis(1));
@ -342,15 +338,15 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
std::unique_ptr<rtcp::TransportFeedback> 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());

View File

@ -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);

View File

@ -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) {

View File

@ -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<ReceivedPacket>& GetReceivedPackets() const;
const std::vector<ReceivedPacket>& 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?

View File

@ -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<rtcp::TransportFeedback>();
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<rtcp::Remb>();

View File

@ -829,7 +829,7 @@ TEST_F(RtcpSenderTest, SendsCombinedRtcpPacket) {
std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets;
auto transport_feedback = std::make_unique<rtcp::TransportFeedback>();
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<rtcp::RemoteEstimate>();
packets.push_back(std::move(remote_estimate));

View File

@ -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);
}

View File

@ -13,6 +13,7 @@
#include <memory>
#include <utility>
#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<std::unique_ptr<RtcpPacket>> packets;
auto transport_feedback = std::make_unique<TransportFeedback>();
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<RemoteEstimate>();