Pass and process capture time through SendPacketObserver with Timestamp type

Bug: webrtc:13757
Change-Id: Icc9f650590640f402ca9004171bbddaf918c78d4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308682
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40339}
This commit is contained in:
Danil Chapovalov 2023-06-14 11:29:29 +02:00 committed by WebRTC LUCI CQ
parent 93008bde6c
commit 8beb6314ef
9 changed files with 56 additions and 58 deletions

View File

@ -1285,7 +1285,7 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) {
// on a ProcessThread. This is alright as is since we forward the call to
// implementations that either just do a PostTask or use locking.
video_send_delay_stats_->OnSentPacket(sent_packet.packet_id,
clock_->TimeInMilliseconds());
clock_->CurrentTime());
transport_send_->OnSentPacket(sent_packet);
}

View File

@ -421,9 +421,9 @@ class SendSideDelayObserver {
// Remove SendSideDelayObserver once possible.
class SendPacketObserver {
public:
virtual ~SendPacketObserver() {}
virtual ~SendPacketObserver() = default;
virtual void OnSendPacket(uint16_t packet_id,
int64_t capture_time_ms,
Timestamp capture_time,
uint32_t ssrc) = 0;
};

View File

@ -408,7 +408,8 @@ void DEPRECATED_RtpSenderEgress::UpdateOnSendPacket(int packet_id,
return;
}
send_packet_observer_->OnSendPacket(packet_id, capture_time_ms, ssrc);
send_packet_observer_->OnSendPacket(packet_id,
Timestamp::Millis(capture_time_ms), ssrc);
}
bool DEPRECATED_RtpSenderEgress::SendPacketToNetwork(

View File

@ -162,10 +162,10 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver,
public SendPacketObserver {
public:
struct SentPacket {
SentPacket(uint16_t packet_id, int64_t capture_time_ms, uint32_t ssrc)
: packet_id(packet_id), capture_time_ms(capture_time_ms), ssrc(ssrc) {}
SentPacket(uint16_t packet_id, Timestamp capture_time, uint32_t ssrc)
: packet_id(packet_id), capture_time(capture_time), ssrc(ssrc) {}
uint16_t packet_id;
int64_t capture_time_ms;
Timestamp capture_time;
uint32_t ssrc;
};
@ -198,9 +198,9 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver,
}
void OnSendPacket(uint16_t packet_id,
int64_t capture_time_ms,
Timestamp capture_time,
uint32_t ssrc) override {
last_sent_packet_.emplace(packet_id, capture_time_ms, ssrc);
last_sent_packet_.emplace(packet_id, capture_time, ssrc);
}
absl::optional<SentPacket> last_sent_packet() const {
@ -983,16 +983,15 @@ TEST_F(RtpRtcpImpl2Test, AssignsTransmissionTimeOffset) {
TEST_F(RtpRtcpImpl2Test, PropagatesSentPacketInfo) {
sender_.RegisterHeaderExtension(TransportSequenceNumber::Uri(),
kTransportSequenceNumberExtensionId);
int64_t now_ms = time_controller_.GetClock()->TimeInMilliseconds();
Timestamp now = time_controller_.GetClock()->CurrentTime();
EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid));
EXPECT_THAT(
sender_.last_sent_packet(),
Optional(
AllOf(Field(&RtpRtcpModule::SentPacket::packet_id,
Eq(sender_.last_packet()
.GetExtension<TransportSequenceNumber>())),
Field(&RtpRtcpModule::SentPacket::capture_time_ms, Eq(now_ms)),
Field(&RtpRtcpModule::SentPacket::ssrc, Eq(kSenderSsrc)))));
EXPECT_THAT(sender_.last_sent_packet(),
Optional(AllOf(
Field(&RtpRtcpModule::SentPacket::packet_id,
Eq(sender_.last_packet()
.GetExtension<TransportSequenceNumber>())),
Field(&RtpRtcpModule::SentPacket::capture_time, Eq(now)),
Field(&RtpRtcpModule::SentPacket::ssrc, Eq(kSenderSsrc)))));
}
TEST_F(RtpRtcpImpl2Test, GeneratesFlexfec) {

View File

@ -516,7 +516,7 @@ void RtpSenderEgress::UpdateOnSendPacket(int packet_id,
return;
}
send_packet_observer_->OnSendPacket(packet_id, capture_time.ms(), ssrc);
send_packet_observer_->OnSendPacket(packet_id, capture_time, ssrc);
}
bool RtpSenderEgress::SendPacketToNetwork(const RtpPacketToSend& packet,

View File

@ -57,7 +57,7 @@ enum : int {
class MockSendPacketObserver : public SendPacketObserver {
public:
MOCK_METHOD(void, OnSendPacket, (uint16_t, int64_t, uint32_t), (override));
MOCK_METHOD(void, OnSendPacket, (uint16_t, Timestamp, uint32_t), (override));
};
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
@ -419,9 +419,9 @@ TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) {
TransportSequenceNumber::Uri());
const uint16_t kTransportSequenceNumber = 1;
EXPECT_CALL(send_packet_observer_,
OnSendPacket(kTransportSequenceNumber,
clock_->TimeInMilliseconds(), kSsrc));
EXPECT_CALL(
send_packet_observer_,
OnSendPacket(kTransportSequenceNumber, clock_->CurrentTime(), kSsrc));
std::unique_ptr<RtpPacketToSend> packet = BuildRtpPacket();
packet->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber);
sender->SendPacket(std::move(packet), PacedPacketInfo());
@ -854,7 +854,7 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) {
header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
TransportSequenceNumber::Uri());
const int64_t capture_time_ms = clock_->TimeInMilliseconds();
const Timestamp capture_time = clock_->CurrentTime();
std::unique_ptr<RtpPacketToSend> video_packet = BuildRtpPacket();
video_packet->set_packet_type(RtpPacketMediaType::kVideo);
@ -882,7 +882,7 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) {
EXPECT_CALL(send_side_delay_observer,
SendSideDelayUpdated(kDiffMs, kDiffMs, kFlexFecSsrc));
EXPECT_CALL(send_packet_observer_, OnSendPacket(1, capture_time_ms, kSsrc));
EXPECT_CALL(send_packet_observer_, OnSendPacket(1, capture_time, kSsrc));
sender->SendPacket(std::move(video_packet), PacedPacketInfo());
@ -891,7 +891,7 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) {
sender->SendPacket(std::move(rtx_packet), PacedPacketInfo());
EXPECT_CALL(send_packet_observer_,
OnSendPacket(3, capture_time_ms, kFlexFecSsrc));
OnSendPacket(3, capture_time, kFlexFecSsrc));
sender->SendPacket(std::move(fec_packet), PacedPacketInfo());
time_controller_.AdvanceTime(TimeDelta::Zero());

View File

@ -18,8 +18,8 @@
namespace webrtc {
namespace {
// Packet with a larger delay are removed and excluded from the delay stats.
// Set to larger than max histogram delay which is 10000.
const int64_t kMaxSentPacketDelayMs = 11000;
// Set to larger than max histogram delay which is 10 seconds.
constexpr TimeDelta kMaxSentPacketDelay = TimeDelta::Seconds(11);
const size_t kMaxPacketMapSize = 2000;
// Limit for the maximum number of streams to calculate stats for.
@ -70,25 +70,24 @@ AvgCounter* SendDelayStats::GetSendDelayCounter(uint32_t ssrc) {
}
void SendDelayStats::OnSendPacket(uint16_t packet_id,
int64_t capture_time_ms,
Timestamp capture_time,
uint32_t ssrc) {
// Packet sent to transport.
MutexLock lock(&mutex_);
if (ssrcs_.find(ssrc) == ssrcs_.end())
return;
int64_t now = clock_->TimeInMilliseconds();
Timestamp now = clock_->CurrentTime();
RemoveOld(now, &packets_);
if (packets_.size() > kMaxPacketMapSize) {
++num_skipped_packets_;
return;
}
packets_.insert(
std::make_pair(packet_id, Packet(ssrc, capture_time_ms, now)));
packets_.insert(std::make_pair(packet_id, Packet(ssrc, capture_time, now)));
}
bool SendDelayStats::OnSentPacket(int packet_id, int64_t time_ms) {
bool SendDelayStats::OnSentPacket(int packet_id, Timestamp time) {
// Packet leaving socket.
if (packet_id == -1)
return false;
@ -100,16 +99,16 @@ bool SendDelayStats::OnSentPacket(int packet_id, int64_t time_ms) {
// TODO(asapersson): Remove SendSideDelayUpdated(), use capture -> sent.
// Elapsed time from send (to transport) -> sent (leaving socket).
int diff_ms = time_ms - it->second.send_time_ms;
GetSendDelayCounter(it->second.ssrc)->Add(diff_ms);
TimeDelta diff = time - it->second.send_time;
GetSendDelayCounter(it->second.ssrc)->Add(diff.ms());
packets_.erase(it);
return true;
}
void SendDelayStats::RemoveOld(int64_t now, PacketMap* packets) {
void SendDelayStats::RemoveOld(Timestamp now, PacketMap* packets) {
while (!packets->empty()) {
auto it = packets->begin();
if (now - it->second.capture_time_ms < kMaxSentPacketDelayMs)
if (now - it->second.capture_time < kMaxSentPacketDelay)
break;
packets->erase(it);

View File

@ -18,6 +18,7 @@
#include <memory>
#include <set>
#include "api/units/timestamp.h"
#include "call/video_send_stream.h"
#include "modules/include/module_common_types_public.h"
#include "rtc_base/synchronization/mutex.h"
@ -43,13 +44,13 @@ class SendDelayStats : public SendPacketObserver {
void AddSsrcs(const VideoSendStream::Config& config);
// Called when a packet is sent (leaving socket).
bool OnSentPacket(int packet_id, int64_t time_ms);
bool OnSentPacket(int packet_id, Timestamp time);
protected:
// From SendPacketObserver.
// Called when a packet is sent to the transport.
void OnSendPacket(uint16_t packet_id,
int64_t capture_time_ms,
Timestamp capture_time,
uint32_t ssrc) override;
private:
@ -60,18 +61,16 @@ class SendDelayStats : public SendPacketObserver {
}
};
struct Packet {
Packet(uint32_t ssrc, int64_t capture_time_ms, int64_t send_time_ms)
: ssrc(ssrc),
capture_time_ms(capture_time_ms),
send_time_ms(send_time_ms) {}
Packet(uint32_t ssrc, Timestamp capture_time, Timestamp send_time)
: ssrc(ssrc), capture_time(capture_time), send_time(send_time) {}
uint32_t ssrc;
int64_t capture_time_ms;
int64_t send_time_ms;
Timestamp capture_time;
Timestamp send_time;
};
typedef std::map<uint16_t, Packet, SequenceNumberOlderThan> PacketMap;
void UpdateHistograms();
void RemoveOld(int64_t now, PacketMap* packets)
void RemoveOld(Timestamp now, PacketMap* packets)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
AvgCounter* GetSendDelayCounter(uint32_t ssrc)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);

View File

@ -24,7 +24,7 @@ const uint32_t kSsrc2 = 42;
const uint32_t kRtxSsrc1 = 18;
const uint32_t kRtxSsrc2 = 43;
const uint16_t kPacketId = 2345;
const int64_t kMaxPacketDelayMs = 11000;
const TimeDelta kMaxPacketDelay = TimeDelta::Seconds(11);
const int kMinRequiredPeriodicSamples = 5;
const int kProcessIntervalMs = 2000;
} // namespace
@ -50,16 +50,16 @@ class SendDelayStatsTest : public ::testing::Test {
}
void OnSendPacket(uint16_t id, uint32_t ssrc) {
OnSendPacket(id, ssrc, clock_.TimeInMilliseconds());
OnSendPacket(id, ssrc, clock_.CurrentTime());
}
void OnSendPacket(uint16_t id, uint32_t ssrc, int64_t capture_ms) {
void OnSendPacket(uint16_t id, uint32_t ssrc, Timestamp capture) {
SendPacketObserver* observer = stats_.get();
observer->OnSendPacket(id, capture_ms, ssrc);
observer->OnSendPacket(id, capture, ssrc);
}
bool OnSentPacket(uint16_t id) {
return stats_->OnSentPacket(id, clock_.TimeInMilliseconds());
return stats_->OnSentPacket(id, clock_.CurrentTime());
}
SimulatedClock clock_;
@ -85,19 +85,19 @@ TEST_F(SendDelayStatsTest, SentPacketNotFoundForNonRegisteredSsrc) {
TEST_F(SendDelayStatsTest, SentPacketFoundWithMaxSendDelay) {
OnSendPacket(kPacketId, kSsrc1);
clock_.AdvanceTimeMilliseconds(kMaxPacketDelayMs - 1);
clock_.AdvanceTime(kMaxPacketDelay - TimeDelta::Millis(1));
OnSendPacket(kPacketId + 1, kSsrc1); // kPacketId -> not old/removed.
EXPECT_TRUE(OnSentPacket(kPacketId)); // Packet found.
EXPECT_TRUE(OnSentPacket(kPacketId + 1)); // Packet found.
}
TEST_F(SendDelayStatsTest, OldPacketsRemoved) {
const int64_t kCaptureTimeMs = clock_.TimeInMilliseconds();
OnSendPacket(0xffffu, kSsrc1, kCaptureTimeMs);
OnSendPacket(0u, kSsrc1, kCaptureTimeMs);
OnSendPacket(1u, kSsrc1, kCaptureTimeMs + 1);
clock_.AdvanceTimeMilliseconds(kMaxPacketDelayMs); // 0xffff, 0 -> old.
OnSendPacket(2u, kSsrc1, kCaptureTimeMs + 2);
const Timestamp kCaptureTime = clock_.CurrentTime();
OnSendPacket(0xffffu, kSsrc1, kCaptureTime);
OnSendPacket(0u, kSsrc1, kCaptureTime);
OnSendPacket(1u, kSsrc1, kCaptureTime + TimeDelta::Millis(1));
clock_.AdvanceTime(kMaxPacketDelay); // 0xffff, 0 -> old.
OnSendPacket(2u, kSsrc1, kCaptureTime + TimeDelta::Millis(2));
EXPECT_FALSE(OnSentPacket(0xffffu)); // Old removed.
EXPECT_FALSE(OnSentPacket(0u)); // Old removed.