From 639494be578238f6cd46e17c5e69054d06060512 Mon Sep 17 00:00:00 2001 From: Per K Date: Tue, 22 Oct 2024 14:35:44 +0000 Subject: [PATCH] Add method CompactNtpIntervalToTimeDelta Similar to CompactNtpRttToTimeDelta but can return negative values. Bug: webrtc:42225697 Change-Id: Iea97502ea73eb6240f42c2040cdc576e51298704 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366422 Reviewed-by: Danil Chapovalov Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#43284} --- modules/rtp_rtcp/source/ntp_time_util.cc | 26 +++++++++------ modules/rtp_rtcp/source/ntp_time_util.h | 5 +++ .../rtp_rtcp/source/ntp_time_util_unittest.cc | 33 +++++++++++++++---- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/modules/rtp_rtcp/source/ntp_time_util.cc b/modules/rtp_rtcp/source/ntp_time_util.cc index 683fa74774..0e74de81fb 100644 --- a/modules/rtp_rtcp/source/ntp_time_util.cc +++ b/modules/rtp_rtcp/source/ntp_time_util.cc @@ -11,8 +11,9 @@ #include "modules/rtp_rtcp/source/ntp_time_util.h" #include +#include -#include "rtc_base/checks.h" +#include "api/units/time_delta.h" #include "rtc_base/numerics/divide_round.h" #include "rtc_base/time_utils.h" @@ -33,6 +34,19 @@ uint32_t SaturatedToCompactNtp(TimeDelta delta) { rtc::kNumMicrosecsPerSec); } +TimeDelta CompactNtpIntervalToTimeDelta(uint32_t compact_ntp_interval) { + // Convert to 64bit value to avoid multiplication overflow. + int64_t value = int64_t{compact_ntp_interval}; + if (compact_ntp_interval > 0x8000'0000) { + value -= (int64_t{1} << 32); + } + // To convert to TimeDelta need to divide by 2^16 to get seconds, + // then multiply by 1'000'000 to get microseconds. To avoid float operations, + // multiplication and division are swapped. + int64_t us = DivideRoundToNearest(value * rtc::kNumMicrosecsPerSec, 1 << 16); + return TimeDelta::Micros(us); +} + TimeDelta CompactNtpRttToTimeDelta(uint32_t compact_ntp_interval) { static constexpr TimeDelta kMinRtt = TimeDelta::Millis(1); // Interval to convert expected to be positive, e.g. RTT or delay. @@ -40,15 +54,7 @@ TimeDelta CompactNtpRttToTimeDelta(uint32_t compact_ntp_interval) { // it might become negative that is indistinguishable from very large values. // Since very large RTT/delay is less likely than non-monotonic ntp clock, // such value is considered negative and converted to minimum value of 1ms. - if (compact_ntp_interval > 0x80000000) - return kMinRtt; - // Convert to 64bit value to avoid multiplication overflow. - int64_t value = static_cast(compact_ntp_interval); - // To convert to TimeDelta need to divide by 2^16 to get seconds, - // then multiply by 1'000'000 to get microseconds. To avoid float operations, - // multiplication and division are swapped. - int64_t us = DivideRoundToNearest(value * rtc::kNumMicrosecsPerSec, 1 << 16); // Small RTT value is considered too good to be true and increased to 1ms. - return std::max(TimeDelta::Micros(us), kMinRtt); + return std::max(CompactNtpIntervalToTimeDelta(compact_ntp_interval), kMinRtt); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/ntp_time_util.h b/modules/rtp_rtcp/source/ntp_time_util.h index 600ceb5082..fdcdf8fd19 100644 --- a/modules/rtp_rtcp/source/ntp_time_util.h +++ b/modules/rtp_rtcp/source/ntp_time_util.h @@ -46,6 +46,11 @@ inline constexpr int64_t ToNtpUnits(TimeDelta delta) { 1'000'000; } +// Converts interval from compact ntp (1/2^16 seconds) resolution to TimeDelta. +// This interval can be up to ~9.1 hours (2^15 seconds). +// Values close to 2^16 seconds are considered negative. +TimeDelta CompactNtpIntervalToTimeDelta(uint32_t compact_ntp_interval); + // Converts interval from compact ntp (1/2^16 seconds) resolution to TimeDelta. // This interval can be up to ~9.1 hours (2^15 seconds). // Values close to 2^16 seconds are considered negative and are converted to diff --git a/modules/rtp_rtcp/source/ntp_time_util_unittest.cc b/modules/rtp_rtcp/source/ntp_time_util_unittest.cc index 00003a0d13..f629481dcf 100644 --- a/modules/rtp_rtcp/source/ntp_time_util_unittest.cc +++ b/modules/rtp_rtcp/source/ntp_time_util_unittest.cc @@ -7,11 +7,13 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ +#include "modules/rtp_rtcp/source/ntp_time_util.h" + #include #include #include "api/units/time_delta.h" -#include "modules/rtp_rtcp/source/ntp_time_util.h" +#include "system_wrappers/include/ntp_time.h" #include "test/gtest.h" namespace webrtc { @@ -24,16 +26,16 @@ TEST(NtpTimeUtilTest, CompactNtp) { EXPECT_EQ(kNtpMid, CompactNtp(kNtp)); } -TEST(NtpTimeUtilTest, CompactNtpRttToTimeDelta) { +TEST(NtpTimeUtilTest, CompactNtpIntervalToTimeDelta) { const NtpTime ntp1(0x12345, 0x23456); const NtpTime ntp2(0x12654, 0x64335); int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); - EXPECT_NEAR(CompactNtpRttToTimeDelta(ntp_diff).ms(), ms_diff, 1); + EXPECT_NEAR(CompactNtpIntervalToTimeDelta(ntp_diff).ms(), ms_diff, 1); } -TEST(NtpTimeUtilTest, CompactNtpRttToTimeDeltaWithWrap) { +TEST(NtpTimeUtilTest, CompactNtpIntervalToTimeDeltaWithWrap) { const NtpTime ntp1(0x1ffff, 0x23456); const NtpTime ntp2(0x20000, 0x64335); int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); @@ -44,10 +46,10 @@ TEST(NtpTimeUtilTest, CompactNtpRttToTimeDeltaWithWrap) { ASSERT_LT(CompactNtp(ntp2), CompactNtp(ntp1)); uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); - EXPECT_NEAR(CompactNtpRttToTimeDelta(ntp_diff).ms(), ms_diff, 1); + EXPECT_NEAR(CompactNtpIntervalToTimeDelta(ntp_diff).ms(), ms_diff, 1); } -TEST(NtpTimeUtilTest, CompactNtpRttToTimeDeltaLarge) { +TEST(NtpTimeUtilTest, CompactNtpIntervalToTimeDeltaLarge) { const NtpTime ntp1(0x10000, 0x00006); const NtpTime ntp2(0x17fff, 0xffff5); int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); @@ -57,11 +59,28 @@ TEST(NtpTimeUtilTest, CompactNtpRttToTimeDeltaLarge) { EXPECT_NEAR(CompactNtpRttToTimeDelta(ntp_diff).ms(), ms_diff, 1); } +TEST(NtpTimeUtilTest, CompactNtpIntervalToTimeDeltaNegative) { + const NtpTime ntp1(0x20000, 0x23456); + const NtpTime ntp2(0x1ffff, 0x64335); + int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); + ASSERT_LT(ms_diff, 0); + // Ntp difference close to 2^16 seconds should be treated as negative. + uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); + EXPECT_NEAR(CompactNtpIntervalToTimeDelta(ntp_diff).ms(), ms_diff, 1); +} + +TEST(NtpTimeUtilTest, CompactNtpIntervalToTimeDeltaBorderToNegative) { + // Both +0x8000 and -x0x8000 seconds can be valid result when converting value + // exactly in the middle. + EXPECT_EQ(CompactNtpIntervalToTimeDelta(0x8000'0000).Abs(), + TimeDelta::Seconds(0x8000)); +} + TEST(NtpTimeUtilTest, CompactNtpRttToTimeDeltaNegative) { const NtpTime ntp1(0x20000, 0x23456); const NtpTime ntp2(0x1ffff, 0x64335); int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); - ASSERT_GT(0, ms_diff); + ASSERT_LT(ms_diff, 0); // Ntp difference close to 2^16 seconds should be treated as negative. uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); EXPECT_EQ(CompactNtpRttToTimeDelta(ntp_diff), TimeDelta::Millis(1));