From 2d162c47020b9b4832e42ab6112b6376ac9df3a8 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 8 Sep 2023 11:51:13 +0200 Subject: [PATCH] In video send statistics proxy merge per ssrc maps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce redundant map lookups, On the way update one the time variable to Timestamp type Bug: None Change-Id: I0224bae866942a8d404e465bd2226befc9ce6763 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319480 Commit-Queue: Danil Chapovalov Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/main@{#40723} --- video/send_statistics_proxy.cc | 38 +++++++++++-------------- video/send_statistics_proxy.h | 22 +++++++------- video/send_statistics_proxy_unittest.cc | 5 ++-- 3 files changed, 31 insertions(+), 34 deletions(-) diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index b857c0535b..61e91ada64 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -129,8 +129,6 @@ absl::optional GetFallbackMaxPixelsIfFieldTrialDisabled( } } // namespace -const int SendStatisticsProxy::kStatsTimeoutMs = 5000; - SendStatisticsProxy::SendStatisticsProxy( Clock* clock, const VideoSendStream::Config& config, @@ -172,6 +170,9 @@ SendStatisticsProxy::~SendStatisticsProxy() { SendStatisticsProxy::FallbackEncoderInfo::FallbackEncoderInfo() = default; +SendStatisticsProxy::Trackers::Trackers() + : encoded_frame_rate(kBucketSizeMs, kBucketCount) {} + SendStatisticsProxy::UmaSamplesContainer::UmaSamplesContainer( const char* prefix, const VideoSendStream::Stats& stats, @@ -753,25 +754,20 @@ VideoSendStream::Stats SendStatisticsProxy::GetStats() { stats_.quality_limitation_durations_ms = quality_limitation_reason_tracker_.DurationsMs(); - for (auto& substream : stats_.substreams) { - uint32_t ssrc = substream.first; - if (encoded_frame_rate_trackers_.count(ssrc) > 0) { - substream.second.encode_frame_rate = - encoded_frame_rate_trackers_[ssrc]->ComputeRate(); + for (auto& [ssrc, substream] : stats_.substreams) { + if (auto it = trackers_.find(ssrc); it != trackers_.end()) { + substream.encode_frame_rate = it->second.encoded_frame_rate.ComputeRate(); } } return stats_; } void SendStatisticsProxy::PurgeOldStats() { - int64_t old_stats_ms = clock_->TimeInMilliseconds() - kStatsTimeoutMs; - for (std::map::iterator it = - stats_.substreams.begin(); - it != stats_.substreams.end(); ++it) { - uint32_t ssrc = it->first; - if (update_times_[ssrc].resolution_update_ms <= old_stats_ms) { - it->second.width = 0; - it->second.height = 0; + Timestamp now = clock_->CurrentTime(); + for (auto& [ssrc, substream] : stats_.substreams) { + if (now - trackers_[ssrc].resolution_update >= kStatsTimeout) { + substream.width = 0; + substream.height = 0; } } } @@ -969,10 +965,7 @@ void SendStatisticsProxy::OnSendEncodedImage( if (!stats) return; - if (encoded_frame_rate_trackers_.count(ssrc) == 0) { - encoded_frame_rate_trackers_[ssrc] = - std::make_unique(kBucketSizeMs, kBucketCount); - } + Trackers& track = trackers_[ssrc]; stats->frames_encoded++; stats->total_encode_time_ms += encoded_image.timing_.encode_finish_ms - @@ -986,7 +979,7 @@ void SendStatisticsProxy::OnSendEncodedImage( if (!stats->width || !stats->height || is_top_spatial_layer) { stats->width = encoded_image._encodedWidth; stats->height = encoded_image._encodedHeight; - update_times_[ssrc].resolution_update_ms = clock_->TimeInMilliseconds(); + track.resolution_update = clock_->CurrentTime(); } uma_container_->key_frame_counter_.Add(encoded_image._frameType == @@ -1036,8 +1029,9 @@ void SendStatisticsProxy::OnSendEncodedImage( } // is_top_spatial_layer pertains only to SVC, will always be true for // simulcast. - if (is_top_spatial_layer) - encoded_frame_rate_trackers_[ssrc]->AddSamples(1); + if (is_top_spatial_layer) { + track.encoded_frame_rate.AddSamples(1); + } absl::optional downscales = adaptation_limitations_.MaskedQualityCounts().resolution_adaptations; diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h index 4203b1c873..a0dcea17cc 100644 --- a/video/send_statistics_proxy.h +++ b/video/send_statistics_proxy.h @@ -45,7 +45,7 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, public FrameCountObserver, public SendSideDelayObserver { public: - static const int kStatsTimeoutMs; + static constexpr TimeDelta kStatsTimeout = TimeDelta::Seconds(5); // Number of required samples to be collected before a metric is added // to a rtc histogram. static const int kMinRequiredMetricsSamples = 200; @@ -157,11 +157,6 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, int64_t sum; int64_t num_samples; }; - struct StatsUpdateTimes { - StatsUpdateTimes() : resolution_update_ms(0), bitrate_update_ms(0) {} - int64_t resolution_update_ms; - int64_t bitrate_update_ms; - }; struct TargetRateUpdates { TargetRateUpdates() : pause_resume_events(0), last_paused_or_resumed(false), last_ms(-1) {} @@ -253,6 +248,15 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, MaskedAdaptationCounts Mask(const VideoAdaptationCounters& counters, const AdaptationSettings& settings) const; }; + // Collection of various stats that are tracked per ssrc. + struct Trackers { + Trackers(); + Trackers(const Trackers&) = delete; + Trackers& operator=(const Trackers&) = delete; + + Timestamp resolution_update = Timestamp::MinusInfinity(); + rtc::RateTracker encoded_frame_rate; + }; void SetAdaptTimer(const MaskedAdaptationCounts& counts, StatsTimer* timer) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); @@ -280,15 +284,13 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, VideoEncoderConfig::ContentType content_type_ RTC_GUARDED_BY(mutex_); const int64_t start_ms_; VideoSendStream::Stats stats_ RTC_GUARDED_BY(mutex_); - std::map update_times_ RTC_GUARDED_BY(mutex_); rtc::ExpFilter encode_time_ RTC_GUARDED_BY(mutex_); QualityLimitationReasonTracker quality_limitation_reason_tracker_ RTC_GUARDED_BY(mutex_); rtc::RateTracker media_byte_rate_tracker_ RTC_GUARDED_BY(mutex_); rtc::RateTracker encoded_frame_rate_tracker_ RTC_GUARDED_BY(mutex_); - // Rate trackers mapped by ssrc. - std::map> - encoded_frame_rate_trackers_ RTC_GUARDED_BY(mutex_); + // Trackers mapped by ssrc. + std::map trackers_ RTC_GUARDED_BY(mutex_); absl::optional last_outlier_timestamp_ RTC_GUARDED_BY(mutex_); diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index 9db774145e..625ffb19bc 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -69,7 +69,7 @@ class SendStatisticsProxyTest : public ::testing::Test { SendStatisticsProxyTest() : SendStatisticsProxyTest("") {} explicit SendStatisticsProxyTest(const std::string& field_trials) : override_field_trials_(field_trials), - fake_clock_(1234), + fake_clock_(Timestamp::Seconds(1234)), config_(GetTestConfig()) {} virtual ~SendStatisticsProxyTest() {} @@ -2353,7 +2353,8 @@ TEST_F(SendStatisticsProxyTest, EncodedResolutionTimesOut) { EXPECT_EQ(kEncodedHeight, stats.substreams[config_.rtp.ssrcs[1]].height); // Forward almost to timeout, this should not have removed stats. - fake_clock_.AdvanceTimeMilliseconds(SendStatisticsProxy::kStatsTimeoutMs - 1); + fake_clock_.AdvanceTime(SendStatisticsProxy::kStatsTimeout - + TimeDelta::Millis(1)); stats = statistics_proxy_->GetStats(); EXPECT_EQ(kEncodedWidth, stats.substreams[config_.rtp.ssrcs[0]].width); EXPECT_EQ(kEncodedHeight, stats.substreams[config_.rtp.ssrcs[0]].height);