From e9193d7031cc1735bd833ffe85a068dd706ca330 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 26 Nov 2024 10:58:24 +0000 Subject: [PATCH] Add histograms for Abs-Capture-Timestamp Bug: webrtc:380712819 Change-Id: I5f56caffe33a257432551321f7c097c852b134dd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368903 Reviewed-by: Johannes Kron Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43458} --- modules/rtp_rtcp/BUILD.gn | 1 + .../absolute_capture_time_interpolator.cc | 49 ++++++++++++++++++- .../absolute_capture_time_interpolator.h | 9 ++++ ...lute_capture_time_interpolator_unittest.cc | 40 ++++++++++++++- system_wrappers/include/metrics.h | 12 +++++ system_wrappers/include/ntp_time.h | 6 +++ 6 files changed, 114 insertions(+), 3 deletions(-) diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 1342dbae9e..3bbddc78d1 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -731,6 +731,7 @@ if (rtc_include_tests) { "../../rtc_base:timeutils", "../../rtc_base/network:ecn_marking", "../../system_wrappers", + "../../system_wrappers:metrics", "../../test:explicit_key_value_config", "../../test:mock_transport", "../../test:rtp_test_utils", diff --git a/modules/rtp_rtcp/source/absolute_capture_time_interpolator.cc b/modules/rtp_rtcp/source/absolute_capture_time_interpolator.cc index fdbd31bbb2..481dc35dd2 100644 --- a/modules/rtp_rtcp/source/absolute_capture_time_interpolator.cc +++ b/modules/rtp_rtcp/source/absolute_capture_time_interpolator.cc @@ -10,9 +10,18 @@ #include "modules/rtp_rtcp/source/absolute_capture_time_interpolator.h" -#include +#include +#include +#include "api/array_view.h" +#include "api/rtp_headers.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" #include "rtc_base/checks.h" +#include "rtc_base/synchronization/mutex.h" +#include "system_wrappers/include/clock.h" +#include "system_wrappers/include/metrics.h" +#include "system_wrappers/include/ntp_time.h" namespace webrtc { @@ -36,6 +45,9 @@ AbsoluteCaptureTimeInterpolator::OnReceivePacket( int rtp_clock_frequency_hz, const std::optional& received_extension) { const Timestamp receive_time = clock_->CurrentTime(); + if (!first_packet_time_) { + first_packet_time_ = receive_time; + } MutexLock lock(&mutex_); @@ -60,7 +72,40 @@ AbsoluteCaptureTimeInterpolator::OnReceivePacket( last_received_extension_ = *received_extension; last_receive_time_ = receive_time; - + // Record statistics on the abs-capture-time extension + if (!first_extension_time_) { + RTC_HISTOGRAM_COUNTS_1M("WebRTC.Call.AbsCapture.ExtensionWait", + (receive_time - *first_packet_time_).ms()); + first_extension_time_ = receive_time; + } + Timestamp capture_as_timestamp = Timestamp::Micros( + UQ32x32ToInt64Us(received_extension->absolute_capture_timestamp)); + TimeDelta capture_delta = receive_time - capture_as_timestamp; + RTC_HISTOGRAM_COUNTS_1G("WebRTC.Call.AbsCapture.Delta", + abs(capture_delta.us())); + if (previous_capture_delta_) { + RTC_HISTOGRAM_COUNTS_1G( + "WebRTC.Call.AbsCapture.DeltaDeviation", + abs((capture_delta - *previous_capture_delta_).us())); + } + previous_capture_delta_ = capture_delta; + if (received_extension->estimated_capture_clock_offset) { + if (!first_offset_time_) { + RTC_HISTOGRAM_COUNTS_1M("WebRTC.Call.AbsCapture.OffsetWait", + (receive_time - *first_packet_time_).ms()); + first_offset_time_ = receive_time; + } + TimeDelta offset_as_delta = TimeDelta::Micros(UQ32x32ToInt64Us( + *received_extension->estimated_capture_clock_offset)); + RTC_HISTOGRAM_COUNTS_1G("WebRTC.Call.AbsCapture.Offset", + abs(offset_as_delta.us())); + if (previous_offset_as_delta_) { + RTC_HISTOGRAM_COUNTS_1G( + "WebRTC.Call.AbsCapture.OffsetDeviation", + abs((offset_as_delta - *previous_offset_as_delta_).us())); + } + previous_offset_as_delta_ = offset_as_delta; + } return received_extension; } } diff --git a/modules/rtp_rtcp/source/absolute_capture_time_interpolator.h b/modules/rtp_rtcp/source/absolute_capture_time_interpolator.h index 59ddeadb20..5b9f817ee1 100644 --- a/modules/rtp_rtcp/source/absolute_capture_time_interpolator.h +++ b/modules/rtp_rtcp/source/absolute_capture_time_interpolator.h @@ -11,6 +11,9 @@ #ifndef MODULES_RTP_RTCP_SOURCE_ABSOLUTE_CAPTURE_TIME_INTERPOLATOR_H_ #define MODULES_RTP_RTCP_SOURCE_ABSOLUTE_CAPTURE_TIME_INTERPOLATOR_H_ +#include +#include + #include "api/array_view.h" #include "api/rtp_headers.h" #include "api/units/time_delta.h" @@ -80,6 +83,12 @@ class AbsoluteCaptureTimeInterpolator { uint32_t last_rtp_timestamp_ RTC_GUARDED_BY(mutex_); int last_rtp_clock_frequency_hz_ RTC_GUARDED_BY(mutex_); AbsoluteCaptureTime last_received_extension_ RTC_GUARDED_BY(mutex_); + // Variables used for statistics generation + std::optional first_packet_time_; + std::optional first_offset_time_; + std::optional first_extension_time_; + std::optional previous_capture_delta_; + std::optional previous_offset_as_delta_; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/absolute_capture_time_interpolator_unittest.cc b/modules/rtp_rtcp/source/absolute_capture_time_interpolator_unittest.cc index 7471496574..140db51f9e 100644 --- a/modules/rtp_rtcp/source/absolute_capture_time_interpolator_unittest.cc +++ b/modules/rtp_rtcp/source/absolute_capture_time_interpolator_unittest.cc @@ -10,8 +10,14 @@ #include "modules/rtp_rtcp/source/absolute_capture_time_interpolator.h" +#include +#include + +#include "api/rtp_headers.h" +#include "api/units/time_delta.h" +#include "system_wrappers/include/clock.h" +#include "system_wrappers/include/metrics.h" #include "system_wrappers/include/ntp_time.h" -#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { @@ -342,4 +348,36 @@ TEST(AbsoluteCaptureTimeInterpolatorTest, SkipInterpolateIsSticky) { std::nullopt); } +TEST(AbsoluteCaptureTimeInterpolatorTest, MetricsAreUpdated) { + constexpr uint32_t kRtpTimestamp0 = 102030000; + constexpr uint32_t kSource = 1234; + constexpr uint32_t kFrequency = 1000; + SimulatedClock clock(0); + AbsoluteCaptureTimeInterpolator interpolator(&clock); + + metrics::Reset(); + // First packet has no extension. + interpolator.OnReceivePacket(kSource, kRtpTimestamp0, kFrequency, + std::nullopt); + EXPECT_METRIC_EQ(metrics::NumSamples("WebRTC.Call.AbsCapture.ExtensionWait"), + 0); + + // Second packet has extension, but no offset. + clock.AdvanceTimeMilliseconds(10); + interpolator.OnReceivePacket( + kSource, kRtpTimestamp0 + 10, kFrequency, + AbsoluteCaptureTime{Int64MsToUQ32x32(5000), std::nullopt}); + EXPECT_METRIC_EQ(metrics::NumSamples("WebRTC.Call.AbsCapture.ExtensionWait"), + 1); + + // Third packet has extension with offset, value zero. + clock.AdvanceTimeMilliseconds(10); + interpolator.OnReceivePacket( + kSource, kRtpTimestamp0 + 20, kFrequency, + AbsoluteCaptureTime{Int64MsToUQ32x32(20), Int64MsToUQ32x32(0)}); + EXPECT_METRIC_EQ(metrics::NumSamples("WebRTC.Call.AbsCapture.Delta"), 2); + EXPECT_METRIC_EQ(metrics::NumSamples("WebRTC.Call.AbsCapture.DeltaDeviation"), + 1); +} + } // namespace webrtc diff --git a/system_wrappers/include/metrics.h b/system_wrappers/include/metrics.h index 6e2da1b9ac..61365dd3c8 100644 --- a/system_wrappers/include/metrics.h +++ b/system_wrappers/include/metrics.h @@ -125,6 +125,12 @@ void NoOp(const Ts&...) {} #define RTC_HISTOGRAM_COUNTS_100000(name, sample) \ RTC_HISTOGRAM_COUNTS(name, sample, 1, 100000, 50) +#define RTC_HISTOGRAM_COUNTS_1M(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 1'000'000, 50) + +#define RTC_HISTOGRAM_COUNTS_1G(name, sample) \ + RTC_HISTOGRAM_COUNTS(name, sample, 1, 1'000'000'000, 50) + #define RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count) \ RTC_HISTOGRAM_COMMON_BLOCK(name, sample, \ webrtc::metrics::HistogramFactoryGetCounts( \ @@ -300,6 +306,12 @@ void NoOp(const Ts&...) {} #define RTC_HISTOGRAM_COUNTS_100000(name, sample) \ webrtc::metrics_impl::NoOp(name, sample) +#define RTC_HISTOGRAM_COUNTS_1M(name, sample) \ + webrtc::metrics_impl::NoOp(name, sample) + +#define RTC_HISTOGRAM_COUNTS_1G(name, sample) \ + webrtc::metrics_impl::NoOp(name, sample) + #define RTC_HISTOGRAM_COUNTS(name, sample, min, max, bucket_count) \ webrtc::metrics_impl::NoOp(name, sample, min, max, bucket_count) diff --git a/system_wrappers/include/ntp_time.h b/system_wrappers/include/ntp_time.h index b912bc8a0c..2edfd4a0d8 100644 --- a/system_wrappers/include/ntp_time.h +++ b/system_wrappers/include/ntp_time.h @@ -120,5 +120,11 @@ inline int64_t UQ32x32ToInt64Ms(uint64_t q32x32) { std::round(q32x32 * (1000.0 / NtpTime::kFractionsPerSecond))); } +// Converts UQ32.32-formatted fixed-point seconds to `int64_t` microseconds. +inline int64_t UQ32x32ToInt64Us(uint64_t q32x32) { + return rtc::dchecked_cast( + std::round(q32x32 * (1'000'000.0 / NtpTime::kFractionsPerSecond))); +} + } // namespace webrtc #endif // SYSTEM_WRAPPERS_INCLUDE_NTP_TIME_H_