From 88c2c50dbd4aaf912af11f1e0a0cb2cecccab996 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Fri, 26 Oct 2018 16:00:08 +0200 Subject: [PATCH] Use monotonic clock to derive NTP timestamps in RTCP module Use helper TimeMicrosToNtp() on clock TimeInMicroseconds() instead of CurrentNtpTime() and CurrentNtpTimeMillis() Also update TimeMicrosToNtp() to not introduce fractional in milliseconds offset. Expose that offset in time_utils.h Add test showing indended behavior. Bug: webrtc:9919 Change-Id: I8b019e11ae5b79d0b8ba113a84066b0369cd2575 Reviewed-on: https://webrtc-review.googlesource.com/c/107889 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#25391} --- modules/rtp_rtcp/source/rtcp_receiver.cc | 13 ++++--- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 19 +++++---- modules/rtp_rtcp/source/rtcp_sender.cc | 18 +++++---- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 39 ++++++++++++++++++- modules/rtp_rtcp/source/time_util.cc | 21 ++++++---- modules/rtp_rtcp/source/time_util.h | 7 ++++ 6 files changed, 88 insertions(+), 29 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 496675497a..202d42d7e2 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -280,7 +280,8 @@ RTCPReceiver::ConsumeReceivedXrReferenceTimeInfo() { std::vector last_xr_rtis; last_xr_rtis.reserve(last_xr_rtis_size); - const uint32_t now_ntp = CompactNtp(clock_->CurrentNtpTime()); + const uint32_t now_ntp = + CompactNtp(TimeMicrosToNtp(clock_->TimeInMicroseconds())); for (size_t i = 0; i < last_xr_rtis_size; ++i) { RrtrInformation& rrtr = received_rrtrs_.front(); @@ -427,7 +428,7 @@ void RTCPReceiver::HandleSenderReport(const CommonHeader& rtcp_block, remote_sender_ntp_time_ = sender_report.ntp(); remote_sender_rtp_time_ = sender_report.rtp_timestamp(); - last_received_sr_ntp_ = clock_->CurrentNtpTime(); + last_received_sr_ntp_ = TimeMicrosToNtp(clock_->TimeInMicroseconds()); } else { // We will only store the send report from one source, but // we will store all the receive blocks. @@ -506,7 +507,8 @@ void RTCPReceiver::HandleReportBlock(const ReportBlock& report_block, if (!receiver_only_ && send_time_ntp != 0) { uint32_t delay_ntp = report_block.delay_since_last_sr(); // Local NTP time. - uint32_t receive_time_ntp = CompactNtp(clock_->CurrentNtpTime()); + uint32_t receive_time_ntp = + CompactNtp(TimeMicrosToNtp(clock_->TimeInMicroseconds())); // RTT in 1/(2^16) seconds. uint32_t rtt_ntp = receive_time_ntp - delay_ntp - send_time_ntp; @@ -720,7 +722,8 @@ void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, void RTCPReceiver::HandleXrReceiveReferenceTime(uint32_t sender_ssrc, const rtcp::Rrtr& rrtr) { uint32_t received_remote_mid_ntp_time = CompactNtp(rrtr.ntp()); - uint32_t local_receive_mid_ntp_time = CompactNtp(clock_->CurrentNtpTime()); + uint32_t local_receive_mid_ntp_time = + CompactNtp(TimeMicrosToNtp(clock_->TimeInMicroseconds())); auto it = received_rrtrs_ssrc_it_.find(sender_ssrc); if (it != received_rrtrs_ssrc_it_.end()) { @@ -754,7 +757,7 @@ void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { return; uint32_t delay_ntp = rti.delay_since_last_rr; - uint32_t now_ntp = CompactNtp(clock_->CurrentNtpTime()); + uint32_t now_ntp = CompactNtp(TimeMicrosToNtp(clock_->TimeInMicroseconds())); uint32_t rtt_ntp = now_ntp - delay_ntp - send_time_ntp; xr_rr_rtt_ms_ = CompactNtpRttToMs(rtt_ntp); diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index c7708f0bdd..4be8d73750 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -208,7 +208,8 @@ TEST_F(RtcpReceiverTest, InjectSrPacketCalculatesRTT) { EXPECT_EQ( -1, rtcp_receiver_.RTT(kSenderSsrc, &rtt_ms, nullptr, nullptr, nullptr)); - uint32_t sent_ntp = CompactNtp(system_clock_.CurrentNtpTime()); + uint32_t sent_ntp = + CompactNtp(TimeMicrosToNtp(system_clock_.TimeInMicroseconds())); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); rtcp::SenderReport sr; @@ -238,7 +239,8 @@ TEST_F(RtcpReceiverTest, InjectSrPacketCalculatesNegativeRTTAsOne) { EXPECT_EQ( -1, rtcp_receiver_.RTT(kSenderSsrc, &rtt_ms, nullptr, nullptr, nullptr)); - uint32_t sent_ntp = CompactNtp(system_clock_.CurrentNtpTime()); + uint32_t sent_ntp = + CompactNtp(TimeMicrosToNtp(system_clock_.TimeInMicroseconds())); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); rtcp::SenderReport sr; @@ -266,7 +268,8 @@ TEST_F( const uint32_t kDelayNtp = 123000; const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - uint32_t sent_ntp = CompactNtp(system_clock_.CurrentNtpTime()); + uint32_t sent_ntp = + CompactNtp(TimeMicrosToNtp(system_clock_.TimeInMicroseconds())); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); rtcp::SenderReport sr; @@ -737,7 +740,8 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) { InjectRtcpPacket(xr); - uint32_t compact_ntp_now = CompactNtp(system_clock_.CurrentNtpTime()); + uint32_t compact_ntp_now = + CompactNtp(TimeMicrosToNtp(system_clock_.TimeInMicroseconds())); EXPECT_TRUE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR; EXPECT_NEAR(CompactNtpRttToMs(rtt_ntp), rtt_ms, 1); @@ -756,7 +760,8 @@ TEST_F(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithMultipleSubBlocks) { InjectRtcpPacket(xr); - uint32_t compact_ntp_now = CompactNtp(system_clock_.CurrentNtpTime()); + uint32_t compact_ntp_now = + CompactNtp(TimeMicrosToNtp(system_clock_.TimeInMicroseconds())); int64_t rtt_ms = 0; EXPECT_TRUE(rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)); uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR; @@ -818,7 +823,7 @@ TEST_F(RtcpReceiverTest, RttCalculatedAfterExtendedReportsDlrr) { const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); rtcp_receiver_.SetRtcpXrRrtrStatus(true); - NtpTime now = system_clock_.CurrentNtpTime(); + NtpTime now = TimeMicrosToNtp(system_clock_.TimeInMicroseconds()); uint32_t sent_ntp = CompactNtp(now); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); @@ -838,7 +843,7 @@ TEST_F(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { const int64_t kRttMs = rand.Rand(-3600 * 1000, -1); const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); - NtpTime now = system_clock_.CurrentNtpTime(); + NtpTime now = TimeMicrosToNtp(system_clock_.TimeInMicroseconds()); uint32_t sent_ntp = CompactNtp(now); system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); rtcp_receiver_.SetRtcpXrRrtrStatus(true); diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index 80a22edaf8..67fdf34d6f 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -99,16 +99,16 @@ class RTCPSender::RtcpContext { RtcpContext(const FeedbackState& feedback_state, int32_t nack_size, const uint16_t* nack_list, - NtpTime now) + int64_t now_us) : feedback_state_(feedback_state), nack_size_(nack_size), nack_list_(nack_list), - now_(now) {} + now_us_(now_us) {} const FeedbackState& feedback_state_; const int32_t nack_size_; const uint16_t* nack_list_; - const NtpTime now_; + const int64_t now_us_; }; RTCPSender::RTCPSender( @@ -429,13 +429,15 @@ std::unique_ptr RTCPSender::BuildSR(const RtcpContext& ctx) { (audio_ ? kBogusRtpRateForAudioRtcp : kVideoPayloadTypeFrequency) / 1000; } + // Round now_us_ to the closest millisecond, because Ntp time is rounded + // when converted to milliseconds, uint32_t rtp_timestamp = timestamp_offset_ + last_rtp_timestamp_ + - (clock_->TimeInMilliseconds() - last_frame_capture_time_ms_) * rtp_rate; + ((ctx.now_us_ + 500) / 1000 - last_frame_capture_time_ms_) * rtp_rate; rtcp::SenderReport* report = new rtcp::SenderReport(); report->SetSenderSsrc(ssrc_); - report->SetNtp(ctx.now_); + report->SetNtp(TimeMicrosToNtp(ctx.now_us_)); report->SetRtpTimestamp(rtp_timestamp); report->SetPacketCount(ctx.feedback_state_.packets_sent); report->SetOctetCount(ctx.feedback_state_.media_bytes_sent); @@ -616,7 +618,7 @@ std::unique_ptr RTCPSender::BuildExtendedReports( if (!sending_ && xr_send_receiver_reference_time_enabled_) { rtcp::Rrtr rrtr; - rrtr.SetNtp(ctx.now_); + rrtr.SetNtp(TimeMicrosToNtp(ctx.now_us_)); xr->SetRrtr(rrtr); } @@ -691,7 +693,7 @@ int32_t RTCPSender::SendCompoundRTCP( // We need to send our NTP even if we haven't received any reports. RtcpContext context(feedback_state, nack_size, nack_list, - clock_->CurrentNtpTime()); + clock_->TimeInMicroseconds()); PrepareReport(feedback_state); @@ -807,7 +809,7 @@ std::vector RTCPSender::CreateReportBlocks( if (!result.empty() && ((feedback_state.last_rr_ntp_secs != 0) || (feedback_state.last_rr_ntp_frac != 0))) { // Get our NTP as late as possible to avoid a race. - uint32_t now = CompactNtp(clock_->CurrentNtpTime()); + uint32_t now = CompactNtp(TimeMicrosToNtp(clock_->TimeInMicroseconds())); uint32_t receive_time = feedback_state.last_rr_ntp_secs & 0x0000FFFF; receive_time <<= 16; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index a2f1d2140b..3e37cc6001 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -11,11 +11,13 @@ #include #include "common_types.h" // NOLINT(build/include) +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "modules/rtp_rtcp/source/rtcp_sender.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_rtcp_impl.h" +#include "modules/rtp_rtcp/source/time_util.h" #include "rtc_base/rate_limiter.h" #include "test/gmock.h" #include "test/gtest.h" @@ -142,7 +144,7 @@ TEST_F(RtcpSenderTest, SendSr) { rtcp_sender_->SetSendingStatus(feedback_state, true); feedback_state.packets_sent = kPacketCount; feedback_state.media_bytes_sent = kOctetCount; - NtpTime ntp = clock_.CurrentNtpTime(); + NtpTime ntp = TimeMicrosToNtp(clock_.TimeInMicroseconds()); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpSr)); EXPECT_EQ(1, parser()->sender_report()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->sender_report()->sender_ssrc()); @@ -154,6 +156,39 @@ TEST_F(RtcpSenderTest, SendSr) { EXPECT_EQ(0U, parser()->sender_report()->report_blocks().size()); } +TEST_F(RtcpSenderTest, SendConsecutiveSrWithExactSlope) { + const uint32_t kPacketCount = 0x12345; + const uint32_t kOctetCount = 0x23456; + const int kTimeBetweenSRsUs = 10043; // Not exact value in milliseconds. + const int kExtraPackets = 30; + // Make sure clock is not exactly at some milliseconds point. + clock_.AdvanceTimeMicroseconds(kTimeBetweenSRsUs); + rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); + RTCPSender::FeedbackState feedback_state = rtp_rtcp_impl_->GetFeedbackState(); + rtcp_sender_->SetSendingStatus(feedback_state, true); + feedback_state.packets_sent = kPacketCount; + feedback_state.media_bytes_sent = kOctetCount; + + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpSr)); + EXPECT_EQ(1, parser()->sender_report()->num_packets()); + NtpTime ntp1 = parser()->sender_report()->ntp(); + uint32_t rtp1 = parser()->sender_report()->rtp_timestamp(); + + // Send more SRs to ensure slope is always exact for different offsets + for (int packets = 1; packets <= kExtraPackets; ++packets) { + clock_.AdvanceTimeMicroseconds(kTimeBetweenSRsUs); + EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpSr)); + EXPECT_EQ(packets + 1, parser()->sender_report()->num_packets()); + + NtpTime ntp2 = parser()->sender_report()->ntp(); + uint32_t rtp2 = parser()->sender_report()->rtp_timestamp(); + + uint32_t ntp_diff_in_rtp_units = + (ntp2.ToMs() - ntp1.ToMs()) * (kVideoPayloadTypeFrequency / 1000); + EXPECT_EQ(rtp2 - rtp1, ntp_diff_in_rtp_units); + } +} + TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) { rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(), nullptr, nullptr, &test_transport_, @@ -448,7 +483,7 @@ TEST_F(RtcpSenderTest, SendXrWithRrtr) { rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound); EXPECT_EQ(0, rtcp_sender_->SetSendingStatus(feedback_state(), false)); rtcp_sender_->SendRtcpXrReceiverReferenceTime(true); - NtpTime ntp = clock_.CurrentNtpTime(); + NtpTime ntp = TimeMicrosToNtp(clock_.TimeInMicroseconds()); EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpReport)); EXPECT_EQ(1, parser()->xr()->num_packets()); EXPECT_EQ(kSenderSsrc, parser()->xr()->sender_ssrc()); diff --git a/modules/rtp_rtcp/source/time_util.cc b/modules/rtp_rtcp/source/time_util.cc index d967f3a98b..e65329d06b 100644 --- a/modules/rtp_rtcp/source/time_util.cc +++ b/modules/rtp_rtcp/source/time_util.cc @@ -23,20 +23,27 @@ inline int64_t DivideRoundToNearest(int64_t x, uint32_t y) { return (x + y / 2) / y; } -int64_t NtpOffsetUs() { +int64_t NtpOffsetMsCalledOnce() { constexpr int64_t kNtpJan1970Sec = 2208988800; - int64_t clock_time = rtc::TimeMicros(); - int64_t utc_time = rtc::TimeUTCMicros(); - return utc_time - clock_time + kNtpJan1970Sec * rtc::kNumMicrosecsPerSec; + int64_t clock_time = rtc::TimeMillis(); + int64_t utc_time = rtc::TimeUTCMillis(); + return utc_time - clock_time + kNtpJan1970Sec * rtc::kNumMillisecsPerSec; } } // namespace -NtpTime TimeMicrosToNtp(int64_t time_us) { +int64_t NtpOffsetMs() { // Calculate the offset once. - static int64_t ntp_offset_us = NtpOffsetUs(); + static int64_t ntp_offset_ms = NtpOffsetMsCalledOnce(); + return ntp_offset_ms; +} - int64_t time_ntp_us = time_us + ntp_offset_us; +NtpTime TimeMicrosToNtp(int64_t time_us) { + // Since this doesn't return a wallclock time, but only NTP representation + // of rtc::TimeMillis() clock, the exact offset doesn't matter. + // To simplify conversions between NTP and RTP time, this offset is + // limited to milliseconds in resolution. + int64_t time_ntp_us = time_us + NtpOffsetMs() * 1000; RTC_DCHECK_GE(time_ntp_us, 0); // Time before year 1900 is unsupported. // TODO(danilchap): Convert both seconds and fraction together using int128 diff --git a/modules/rtp_rtcp/source/time_util.h b/modules/rtp_rtcp/source/time_util.h index 672722c07f..1e01c94c2d 100644 --- a/modules/rtp_rtcp/source/time_util.h +++ b/modules/rtp_rtcp/source/time_util.h @@ -22,8 +22,15 @@ namespace webrtc { // difference of the passed values. // As a result TimeMicrosToNtp(rtc::TimeMicros()) doesn't guarantee to match // system time. +// However, TimeMicrosToNtp Guarantees that returned NtpTime will be offsetted +// from rtc::TimeMicros() by integral number of milliseconds. +// Use NtpOffsetMs() to get that offset value. NtpTime TimeMicrosToNtp(int64_t time_us); +// Difference between Ntp time and local relative time returned by +// rtc::TimeMicros() +int64_t NtpOffsetMs(); + // Converts NTP timestamp to RTP timestamp. inline uint32_t NtpToRtp(NtpTime ntp, uint32_t freq) { uint32_t tmp = (static_cast(ntp.fractions()) * freq) >> 32;