Fix sign error in UMA for AbsCapture.Delta

Bug: webrtc:380712819
Change-Id: Icfb42f0455718058a54391e5a586f409cd28728d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370000
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43481}
This commit is contained in:
Harald Alvestrand 2024-12-02 21:11:19 +00:00 committed by WebRTC LUCI CQ
parent 5a4a69f95c
commit 10c7d73688
4 changed files with 95 additions and 4 deletions

View File

@ -18,6 +18,7 @@
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/synchronization/mutex.h"
#include "system_wrappers/include/clock.h"
#include "system_wrappers/include/metrics.h"
@ -78,10 +79,10 @@ AbsoluteCaptureTimeInterpolator::OnReceivePacket(
(receive_time - *first_packet_time_).ms());
first_extension_time_ = receive_time;
}
uint64_t ntp_delta =
int64_t ntp_delta =
uint64_t{clock_->ConvertTimestampToNtpTime(receive_time)} -
received_extension->absolute_capture_timestamp;
TimeDelta capture_delta = TimeDelta::Micros(UQ32x32ToInt64Us(ntp_delta));
TimeDelta capture_delta = TimeDelta::Micros(Q32x32ToInt64Us(ntp_delta));
RTC_HISTOGRAM_COUNTS_1G("WebRTC.Call.AbsCapture.Delta",
abs(capture_delta.us()));
if (previous_capture_delta_) {
@ -96,8 +97,8 @@ AbsoluteCaptureTimeInterpolator::OnReceivePacket(
(receive_time - *first_packet_time_).ms());
first_offset_time_ = receive_time;
}
TimeDelta offset_as_delta = TimeDelta::Micros(UQ32x32ToInt64Us(
*received_extension->estimated_capture_clock_offset));
TimeDelta offset_as_delta = TimeDelta::Micros(
Q32x32ToInt64Us(*received_extension->estimated_capture_clock_offset));
RTC_HISTOGRAM_COUNTS_1G("WebRTC.Call.AbsCapture.Offset",
abs(offset_as_delta.us()));
if (previous_offset_as_delta_) {

View File

@ -18,10 +18,15 @@
#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 {
using testing::AllOf;
using testing::Ge;
using testing::Le;
TEST(AbsoluteCaptureTimeInterpolatorTest, GetSourceWithoutCsrcs) {
constexpr uint32_t kSsrc = 12;
@ -380,4 +385,38 @@ TEST(AbsoluteCaptureTimeInterpolatorTest, MetricsAreUpdated) {
1);
}
TEST(AbsoluteCaptureTimeInterpolatorTest, DeltaRecordedCorrectly) {
constexpr uint32_t kRtpTimestamp0 = 102030000;
constexpr uint32_t kSource = 1234;
constexpr uint32_t kFrequency = 1000;
SimulatedClock clock(0);
AbsoluteCaptureTimeInterpolator interpolator(&clock);
metrics::Reset();
clock.AdvanceTimeMilliseconds(10);
// Packet has extension, with delta 5 ms in the past.
interpolator.OnReceivePacket(
kSource, kRtpTimestamp0 + 10, kFrequency,
AbsoluteCaptureTime{
uint64_t{clock.ConvertTimestampToNtpTime(Timestamp::Millis(5))},
std::nullopt});
EXPECT_METRIC_EQ(metrics::NumSamples("WebRTC.Call.AbsCapture.ExtensionWait"),
1);
int sample = metrics::MinSample("WebRTC.Call.AbsCapture.Delta");
EXPECT_THAT(sample, AllOf(Ge(5000), Le(5000)));
metrics::Reset();
// Packet has extension, with timestamp 6 ms in the future.
interpolator.OnReceivePacket(
kSource, kRtpTimestamp0 + 15, kFrequency,
AbsoluteCaptureTime{
uint64_t{clock.ConvertTimestampToNtpTime(Timestamp::Millis(16))},
std::nullopt});
sample = metrics::MinSample("WebRTC.Call.AbsCapture.Delta");
// Since we capture with abs(), this should also be recorded as 6 ms
EXPECT_THAT(sample, AllOf(Ge(6000), Le(6000)));
}
} // namespace webrtc

View File

@ -126,5 +126,11 @@ inline int64_t UQ32x32ToInt64Us(uint64_t q32x32) {
std::round(q32x32 * (1'000'000.0 / NtpTime::kFractionsPerSecond)));
}
// Converts Q32.32-formatted fixed-point seconds to `int64_t` microseconds.
inline int64_t Q32x32ToInt64Us(int64_t q32x32) {
return rtc::dchecked_cast<int64_t>(
std::round(q32x32 * (1'000'000.0 / NtpTime::kFractionsPerSecond)));
}
} // namespace webrtc
#endif // SYSTEM_WRAPPERS_INCLUDE_NTP_TIME_H_

View File

@ -13,11 +13,14 @@
#include <random>
#include "system_wrappers/include/clock.h"
#include "test/gmock.h"
#include "test/gtest.h"
namespace webrtc {
namespace {
using testing::Lt;
constexpr uint32_t kNtpSec = 0x12345678;
constexpr uint32_t kNtpFrac = 0x23456789;
@ -276,5 +279,47 @@ TEST(NtpTimeTest, VerifyInt64MsToUQ32x32RoundTrip) {
}
}
TEST(NtpTimeTest, VerifyInt64MsToUQ32x32UsRoundTrip) {
constexpr int kIterations = 50000;
std::mt19937 generator(123456789);
std::uniform_int_distribution<uint64_t> distribution(
UQ32x32ToInt64Ms(std::numeric_limits<uint64_t>::min()),
UQ32x32ToInt64Ms(std::numeric_limits<uint64_t>::max()));
for (int iteration = 0; iteration < kIterations; ++iteration) {
uint64_t input_ms = distribution(generator);
uint64_t transit_uq32x32 = Int64MsToUQ32x32(input_ms);
uint64_t output_ms = UQ32x32ToInt64Us(transit_uq32x32);
int64_t difference = input_ms * 1000 - output_ms;
ASSERT_THAT(abs(difference), Lt(2))
<< "iteration = " << iteration << ", input_ms = " << input_ms
<< ", transit_uq32x32 = " << transit_uq32x32
<< ", output_us = " << output_ms;
}
}
TEST(NtpTimeTest, VerifySignedInt64MsToUQ32x32UsRoundTrip) {
constexpr int kIterations = 50000;
std::mt19937 generator(123456789);
std::uniform_int_distribution<int64_t> distribution(
UQ32x32ToInt64Ms(std::numeric_limits<int64_t>::min()),
UQ32x32ToInt64Ms(std::numeric_limits<int64_t>::max()));
for (int iteration = 0; iteration < kIterations; ++iteration) {
int64_t input_ms = distribution(generator);
uint64_t transit_uq32x32 = Int64MsToUQ32x32(input_ms);
int64_t output_ms = UQ32x32ToInt64Us(transit_uq32x32);
int64_t difference = input_ms * 1000 - output_ms;
ASSERT_THAT(abs(difference), Lt(2))
<< "iteration = " << iteration << ", input_ms = " << input_ms
<< ", transit_uq32x32 = " << transit_uq32x32
<< ", output_us = " << output_ms;
}
}
} // namespace
} // namespace webrtc