From 652eccf5523ef387c0d288491b2ca14ef4176678 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 13 Sep 2023 10:37:27 +0200 Subject: [PATCH] Move send delay calculation to SendStatisticsProxy from RtpSenderEgress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: None Change-Id: I5d14c8898d16b12062cf0b172fcc138c23d28b3b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319562 Reviewed-by: Åsa Persson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#40741} --- video/send_statistics_proxy.cc | 48 ++++++++++++++++++++----- video/send_statistics_proxy.h | 19 +++++++++- video/send_statistics_proxy_unittest.cc | 31 +++++++++------- video/video_send_stream.cc | 2 +- 4 files changed, 77 insertions(+), 23 deletions(-) diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 4abf10d3a4..9a5f69de89 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -1382,20 +1382,52 @@ void SendStatisticsProxy::FrameCountUpdated(const FrameCounts& frame_counts, stats->frame_counts = frame_counts; } -void SendStatisticsProxy::OnSendPacket(uint32_t ssrc, Timestamp capture_time) { - [[maybe_unused]] TimeDelta send_delay = clock_->CurrentTime() - capture_time; - // TODO(danilchap): Calculate average and max send_delay per ssrc over last - // second, Use that measurement instead of whatever is received in - // SendSideDelayUpdated below. +void SendStatisticsProxy::Trackers::AddSendDelay(Timestamp now, + TimeDelta send_delay) { + // Add the new measurement. + send_delays.push_back({.when = now, .send_delay = send_delay}); + send_delay_sum += send_delay; + if (send_delay_max == nullptr || *send_delay_max <= send_delay) { + send_delay_max = &send_delays.back().send_delay; + } + + // Remove old. No need to check for emptiness because newly added entry would + // never be too old. + while (now - send_delays.front().when > TimeDelta::Seconds(1)) { + send_delay_sum -= send_delays.front().send_delay; + if (send_delay_max == &send_delays.front().send_delay) { + send_delay_max = nullptr; + } + send_delays.pop_front(); + } + + // Check if max value was pushed out from the queue as too old. + if (send_delay_max == nullptr) { + send_delay_max = &send_delays.front().send_delay; + for (const SendDelayEntry& entry : send_delays) { + // Use '>=' rather than '>' to prefer latest maximum as it would be pushed + // out later and thus trigger less recalculations. + if (entry.send_delay >= *send_delay_max) { + send_delay_max = &entry.send_delay; + } + } + } } -void SendStatisticsProxy::SendSideDelayUpdated(int avg_delay_ms, - int max_delay_ms, - uint32_t ssrc) { +void SendStatisticsProxy::OnSendPacket(uint32_t ssrc, Timestamp capture_time) { + Timestamp now = clock_->CurrentTime(); + MutexLock lock(&mutex_); VideoSendStream::StreamStats* stats = GetStatsEntry(ssrc); if (!stats) return; + + Trackers& track = trackers_[ssrc]; + track.AddSendDelay(now, now - capture_time); + + int64_t avg_delay_ms = (track.send_delay_sum / track.send_delays.size()).ms(); + int64_t max_delay_ms = track.send_delay_max->ms(); + stats->avg_delay_ms = avg_delay_ms; stats->max_delay_ms = max_delay_ms; diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h index df8dd252a1..b7c8d7c5a4 100644 --- a/video/send_statistics_proxy.h +++ b/video/send_statistics_proxy.h @@ -12,6 +12,7 @@ #define VIDEO_SEND_STATISTICS_PROXY_H_ #include +#include #include #include #include @@ -131,7 +132,7 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, // From SendSideDelayObserver. void SendSideDelayUpdated(int avg_delay_ms, int max_delay_ms, - uint32_t ssrc) override; + uint32_t ssrc) override {} private: class SampleCounter { @@ -252,12 +253,28 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, }; // Collection of various stats that are tracked per ssrc. struct Trackers { + struct SendDelayEntry { + Timestamp when; + TimeDelta send_delay; + }; + Trackers(); Trackers(const Trackers&) = delete; Trackers& operator=(const Trackers&) = delete; + void AddSendDelay(Timestamp now, TimeDelta send_delay); + Timestamp resolution_update = Timestamp::MinusInfinity(); rtc::RateTracker encoded_frame_rate; + + std::deque send_delays; + + // The sum of `send_delay` in `send_delays`. + TimeDelta send_delay_sum = TimeDelta::Zero(); + + // Pointer to the maximum `send_delay` in `send_delays` or nullptr if + // `send_delays.empty()` + const TimeDelta* send_delay_max = nullptr; }; void SetAdaptTimer(const MaskedAdaptationCounts& counts, StatsTimer* timer) diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index 625ffb19bc..c83739d3ed 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -320,24 +320,29 @@ TEST_F(SendStatisticsProxyTest, Bitrate) { } TEST_F(SendStatisticsProxyTest, SendSideDelay) { - SendSideDelayObserver* observer = statistics_proxy_.get(); - for (const auto& ssrc : config_.rtp.ssrcs) { + for (uint32_t ssrc : config_.rtp.ssrcs) { // Use ssrc as avg_delay_ms and max_delay_ms to get a unique value for each // stream. - int avg_delay_ms = ssrc; - int max_delay_ms = ssrc + 1; - observer->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc); - expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms; - expected_.substreams[ssrc].max_delay_ms = max_delay_ms; + expected_.substreams[ssrc].avg_delay_ms = ssrc; + expected_.substreams[ssrc].max_delay_ms = ssrc + 1; + statistics_proxy_->OnSendPacket(ssrc, + /*capture_time=*/fake_clock_.CurrentTime() - + TimeDelta::Millis(ssrc + 1)); + statistics_proxy_->OnSendPacket(ssrc, + /*capture_time=*/fake_clock_.CurrentTime() - + TimeDelta::Millis(ssrc - 1)); } - for (const auto& ssrc : config_.rtp.rtx.ssrcs) { + for (uint32_t ssrc : config_.rtp.rtx.ssrcs) { // Use ssrc as avg_delay_ms and max_delay_ms to get a unique value for each // stream. - int avg_delay_ms = ssrc; - int max_delay_ms = ssrc + 1; - observer->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc); - expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms; - expected_.substreams[ssrc].max_delay_ms = max_delay_ms; + expected_.substreams[ssrc].avg_delay_ms = ssrc; + expected_.substreams[ssrc].max_delay_ms = ssrc + 1; + statistics_proxy_->OnSendPacket(ssrc, + /*capture_time=*/fake_clock_.CurrentTime() - + TimeDelta::Millis(ssrc + 1)); + statistics_proxy_->OnSendPacket(ssrc, + /*capture_time=*/fake_clock_.CurrentTime() - + TimeDelta::Millis(ssrc - 1)); } VideoSendStream::Stats stats = statistics_proxy_->GetStats(); ExpectEqual(expected_, stats); diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index b2ba098599..a01c1004a3 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -101,7 +101,7 @@ RtpSenderObservers CreateObservers(RtcpRttStats* call_stats, observers.bitrate_observer = stats_proxy; observers.frame_count_observer = stats_proxy; observers.rtcp_type_observer = stats_proxy; - observers.send_delay_observer = stats_proxy; + observers.send_delay_observer = nullptr; observers.send_packet_observer = send_packet_observer; return observers; }