diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index 511f69de6d..e238dfa999 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -141,6 +141,8 @@ source_set("rtp_rtcp") { "source/rtp_utility.h", "source/ssrc_database.cc", "source/ssrc_database.h", + "source/time_util.cc", + "source/time_util.h", "source/tmmbr_help.cc", "source/tmmbr_help.h", "source/video_codec_information.h", diff --git a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi index f72e71030a..d14ac2b927 100644 --- a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi +++ b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi @@ -109,6 +109,8 @@ 'source/rtp_utility.h', 'source/ssrc_database.cc', 'source/ssrc_database.h', + 'source/time_util.cc', + 'source/time_util.h', 'source/tmmbr_help.cc', 'source/tmmbr_help.h', # Audio Files diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 0259b73ff4..0873254f63 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -13,8 +13,6 @@ #include #include -#include - #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" @@ -66,6 +64,7 @@ RTCPReceiver::RTCPReceiver( _lastReceivedSRNTPfrac(0), _lastReceivedXRNTPsecs(0), _lastReceivedXRNTPfrac(0), + xr_rrtr_status_(false), xr_rr_rtt_ms_(0), _receivedInfoMap(), _lastReceivedRrMs(0), @@ -191,6 +190,11 @@ int32_t RTCPReceiver::RTT(uint32_t remoteSSRC, return 0; } +void RTCPReceiver::SetRtcpXrRrtrStatus(bool enable) { + CriticalSectionScoped lock(_criticalSectionRTCPReceiver); + xr_rrtr_status_ = enable; +} + bool RTCPReceiver::GetAndResetXrRrRtt(int64_t* rtt_ms) { assert(rtt_ms); CriticalSectionScoped lock(_criticalSectionRTCPReceiver); @@ -520,10 +524,13 @@ void RTCPReceiver::HandleReportBlock( reportBlock->remoteMaxJitter = rtcpPacket.ReportBlockItem.Jitter; } + int64_t rtt = 0; uint32_t send_time = rtcpPacket.ReportBlockItem.LastSR; - uint32_t rtt = 0; - - if (send_time > 0) { + // RFC3550, section 6.4.1, LSR field discription states: + // If no SR has been received yet, the field is set to zero. + // Receiver rtp_rtcp module is not expected to calculate rtt using + // Sender Reports even if it accidentally can. + if (!receiver_only_ && send_time != 0) { uint32_t delay = rtcpPacket.ReportBlockItem.DelayLastSR; // Local NTP time. uint32_t receive_time = CompactNtp(NtpTime(*_clock)); @@ -531,8 +538,7 @@ void RTCPReceiver::HandleReportBlock( // RTT in 1/(2^16) seconds. uint32_t rtt_ntp = receive_time - delay - send_time; // Convert to 1/1000 seconds (milliseconds). - uint32_t rtt_ms = CompactNtpIntervalToMs(rtt_ntp); - rtt = std::max(rtt_ms, 1); + rtt = CompactNtpRttToMs(rtt_ntp); if (rtt > reportBlock->maxRTT) { // Store max RTT. reportBlock->maxRTT = rtt; @@ -916,9 +922,13 @@ void RTCPReceiver::HandleXrDlrrReportBlockItem( rtcpPacketInformation.xr_dlrr_item = true; + // Caller should explicitly enable rtt calculation using extended reports. + if (!xr_rrtr_status_) + return; + // The send_time and delay_rr fields are in units of 1/2^16 sec. uint32_t send_time = packet.XRDLRRReportBlockItem.LastRR; - // RFC3411, section 4.5, LRR field discription states: + // RFC3611, section 4.5, LRR field discription states: // If no such block has been received, the field is set to zero. if (send_time == 0) return; @@ -927,8 +937,7 @@ void RTCPReceiver::HandleXrDlrrReportBlockItem( uint32_t now = CompactNtp(NtpTime(*_clock)); uint32_t rtt_ntp = now - delay_rr - send_time; - uint32_t rtt_ms = CompactNtpIntervalToMs(rtt_ntp); - xr_rr_rtt_ms_ = std::max(rtt_ms, 1); + xr_rr_rtt_ms_ = CompactNtpRttToMs(rtt_ntp); rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpXrDlrrReportBlock; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 30677086fd..1930670064 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -77,6 +77,7 @@ public: int32_t SenderInfoReceived(RTCPSenderInfo* senderInfo) const; + void SetRtcpXrRrtrStatus(bool enable); bool GetAndResetXrRrRtt(int64_t* rtt_ms); // get statistics @@ -292,6 +293,7 @@ protected: uint32_t _lastReceivedXRNTPsecs; uint32_t _lastReceivedXRNTPfrac; // Estimated rtt, zero when there is no valid estimate. + bool xr_rrtr_status_ GUARDED_BY(_criticalSectionRTCPReceiver); int64_t xr_rr_rtt_ms_; // Received report blocks. diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index c6df905688..252b4828ab 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -184,9 +184,9 @@ TEST_F(RtcpReceiverTest, InjectSrPacketCalculatesRTT) { Random r(0x0123456789abcdef); const uint32_t kSenderSsrc = r.Rand(0x00000001u, 0xfffffffeu); const uint32_t kRemoteSsrc = r.Rand(0x00000001u, 0xfffffffeu); - const int64_t kRttMs = r.Rand(1, 18 * 3600 * 1000); - const uint32_t kDelayNtp = r.Rand(); - const uint32_t kDelayMs = CompactNtpIntervalToMs(kDelayNtp); + const int64_t kRttMs = r.Rand(1, 9 * 3600 * 1000); + const uint32_t kDelayNtp = r.Rand(0, 0x7fffffff); + const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); rtcp_receiver_->SetRemoteSSRC(kSenderSsrc); std::set ssrcs; @@ -216,6 +216,42 @@ TEST_F(RtcpReceiverTest, InjectSrPacketCalculatesRTT) { EXPECT_NEAR(kRttMs, rtt_ms, 1); } +TEST_F(RtcpReceiverTest, InjectSrPacketCalculatesNegativeRTTAsOne) { + Random r(0x0123456789abcdef); + const uint32_t kSenderSsrc = r.Rand(0x00000001u, 0xfffffffeu); + const uint32_t kRemoteSsrc = r.Rand(0x00000001u, 0xfffffffeu); + const int64_t kRttMs = r.Rand(-3600 * 1000, -1); + const uint32_t kDelayNtp = r.Rand(0, 0x7fffffff); + const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); + + rtcp_receiver_->SetRemoteSSRC(kSenderSsrc); + std::set ssrcs; + ssrcs.insert(kRemoteSsrc); + rtcp_receiver_->SetSsrcs(kRemoteSsrc, ssrcs); + + int64_t rtt_ms = 0; + EXPECT_EQ( + -1, rtcp_receiver_->RTT(kSenderSsrc, &rtt_ms, nullptr, nullptr, nullptr)); + + uint32_t sent_ntp = CompactNtp(NtpTime(system_clock_)); + system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); + + rtcp::SenderReport sr; + sr.From(kSenderSsrc); + rtcp::ReportBlock block; + block.To(kRemoteSsrc); + block.WithLastSr(sent_ntp); + block.WithDelayLastSr(kDelayNtp); + sr.WithReportBlock(block); + + rtc::Buffer packet = sr.Build(); + EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); + + EXPECT_EQ( + 0, rtcp_receiver_->RTT(kSenderSsrc, &rtt_ms, nullptr, nullptr, nullptr)); + EXPECT_EQ(1, rtt_ms); +} + TEST_F(RtcpReceiverTest, InjectRrPacket) { const uint32_t kSenderSsrc = 0x10203; rtcp::ReceiverReport rr; @@ -790,12 +826,13 @@ TEST_F(RtcpReceiverTest, TestXrRrRttInitiallyFalse) { EXPECT_FALSE(rtcp_receiver_->GetAndResetXrRrRtt(&rtt_ms)); } -TEST_F(RtcpReceiverTest, RttCalculatedAfterXrDlrr) { +TEST_F(RtcpReceiverTest, XrDlrrCalculatesRtt) { Random rand(0x0123456789abcdef); const uint32_t kSourceSsrc = rand.Rand(0x00000001u, 0xfffffffeu); - const uint32_t kRttMs = rand.Rand(1, 18 * 3600 * 1000); - const uint32_t kDelayNtp = rand.Rand(); - const uint32_t kDelayMs = CompactNtpIntervalToMs(kDelayNtp); + const int64_t kRttMs = rand.Rand(1, 9 * 3600 * 1000); + const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); + const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); + rtcp_receiver_->SetRtcpXrRrtrStatus(true); std::set ssrcs; ssrcs.insert(kSourceSsrc); rtcp_receiver_->SetSsrcs(kSourceSsrc, ssrcs); @@ -816,6 +853,33 @@ TEST_F(RtcpReceiverTest, RttCalculatedAfterXrDlrr) { EXPECT_NEAR(kRttMs, rtt_ms, 1); } +TEST_F(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) { + Random rand(0x0123456789abcdef); + const uint32_t kSourceSsrc = rand.Rand(0x00000001u, 0xfffffffeu); + const int64_t kRttMs = rand.Rand(-3600 * 1000, -1); + const uint32_t kDelayNtp = rand.Rand(0, 0x7fffffff); + const int64_t kDelayMs = CompactNtpRttToMs(kDelayNtp); + rtcp_receiver_->SetRtcpXrRrtrStatus(true); + std::set ssrcs; + ssrcs.insert(kSourceSsrc); + rtcp_receiver_->SetSsrcs(kSourceSsrc, ssrcs); + NtpTime now(system_clock_); + uint32_t sent_ntp = CompactNtp(now); + system_clock_.AdvanceTimeMilliseconds(kRttMs + kDelayMs); + + rtcp::Dlrr dlrr; + dlrr.WithDlrrItem(kSourceSsrc, sent_ntp, kDelayNtp); + rtcp::ExtendedReports xr; + xr.From(0x2345); + xr.WithDlrr(dlrr); + rtc::Buffer packet = xr.Build(); + EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); + + int64_t rtt_ms = 0; + EXPECT_TRUE(rtcp_receiver_->GetAndResetXrRrRtt(&rtt_ms)); + EXPECT_EQ(1, rtt_ms); +} + TEST_F(RtcpReceiverTest, LastReceivedXrReferenceTimeInfoInitiallyFalse) { RtcpReceiveTimeInfo info; EXPECT_FALSE(rtcp_receiver_->LastReceivedXrReferenceTimeInfo(&info)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index b966c00fb5..86ed6cec3a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -586,7 +586,8 @@ int32_t ModuleRtpRtcpImpl::SetRTCPVoIPMetrics( } void ModuleRtpRtcpImpl::SetRtcpXrRrtrStatus(bool enable) { - return rtcp_sender_.SendRtcpXrReceiverReferenceTime(enable); + rtcp_receiver_.SetRtcpXrRrtrStatus(enable); + rtcp_sender_.SendRtcpXrReceiverReferenceTime(enable); } bool ModuleRtpRtcpImpl::RtcpXrRrtrStatus() const { diff --git a/webrtc/modules/rtp_rtcp/source/time_util.cc b/webrtc/modules/rtp_rtcp/source/time_util.cc new file mode 100644 index 0000000000..dedba4d7ec --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/time_util.cc @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/modules/rtp_rtcp/source/time_util.h" + +#include + +namespace webrtc { +namespace { +// TODO(danilchap): Make generic, optimize and move to base. +inline int64_t DivideRoundToNearest(int64_t x, uint32_t y) { + // Caller ensure x is positive by converting unsigned value into it. + // So this Divide doesn't need to handle negative argument case. + return (x + y / 2) / y; +} +} // namespace + +int64_t CompactNtpRttToMs(uint32_t compact_ntp_interval) { + // Interval to convert expected to be positive, e.g. rtt or delay. + // Because interval can be derived from non-monotonic ntp clock, + // it might become negative that is indistinguishable from very large values. + // Since very large rtt/delay are less likely than non-monotonic ntp clock, + // those values consider to be negative and convert to minimum value of 1ms. + if (compact_ntp_interval > 0x80000000) + return 1; + // Convert to 64bit value to avoid multiplication overflow. + int64_t value = static_cast(compact_ntp_interval); + // To convert to milliseconds need to divide by 2^16 to get seconds, + // then multiply by 1000 to get milliseconds. To avoid float operations, + // multiplication and division swapped. + int64_t ms = DivideRoundToNearest(value * 1000, 1 << 16); + // Rtt value 0 considered too good to be true and increases to 1. + return std::max(ms, 1); +} +} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/time_util.h b/webrtc/modules/rtp_rtcp/source/time_util.h index a876b14d56..5a9525ae62 100644 --- a/webrtc/modules/rtp_rtcp/source/time_util.h +++ b/webrtc/modules/rtp_rtcp/source/time_util.h @@ -39,11 +39,9 @@ inline uint32_t CompactNtp(NtpTime ntp) { return (ntp.seconds() << 16) | (ntp.fractions() >> 16); } // Converts interval between compact ntp timestamps to milliseconds. -// This interval can be upto ~18.2 hours (2^16 seconds). -inline uint32_t CompactNtpIntervalToMs(uint32_t compact_ntp_interval) { - uint64_t value = static_cast(compact_ntp_interval); - return (value * 1000 + (1 << 15)) >> 16; -} +// This interval can be up to ~9.1 hours (2^15 seconds). +// Values close to 2^16 seconds consider negative and result in minimum rtt = 1. +int64_t CompactNtpRttToMs(uint32_t compact_ntp_interval); } // namespace webrtc #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_TIME_UTIL_H_ diff --git a/webrtc/modules/rtp_rtcp/source/time_util_unittest.cc b/webrtc/modules/rtp_rtcp/source/time_util_unittest.cc index 7efb83ccad..8333a64e70 100644 --- a/webrtc/modules/rtp_rtcp/source/time_util_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/time_util_unittest.cc @@ -21,21 +21,21 @@ TEST(TimeUtilTest, CompactNtp) { EXPECT_EQ(kNtpMid, CompactNtp(kNtp)); } -TEST(TimeUtilTest, CompactNtpToMs) { +TEST(TimeUtilTest, CompactNtpRttToMs) { const NtpTime ntp1(0x12345, 0x23456); const NtpTime ntp2(0x12654, 0x64335); - uint32_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); + int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); - uint32_t ntp_to_ms_diff = CompactNtpIntervalToMs(ntp_diff); + int64_t ntp_to_ms_diff = CompactNtpRttToMs(ntp_diff); EXPECT_NEAR(ms_diff, ntp_to_ms_diff, 1); } -TEST(TimeUtilTest, CompactNtpToMsWithWrap) { +TEST(TimeUtilTest, CompactNtpRttToMsWithWrap) { const NtpTime ntp1(0x1ffff, 0x23456); const NtpTime ntp2(0x20000, 0x64335); - uint32_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); + int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); // While ntp2 > ntp1, there compact ntp presentation happen to be opposite. // That shouldn't be a problem as long as unsigned arithmetic is used. @@ -43,20 +43,31 @@ TEST(TimeUtilTest, CompactNtpToMsWithWrap) { ASSERT_LT(CompactNtp(ntp2), CompactNtp(ntp1)); uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); - uint32_t ntp_to_ms_diff = CompactNtpIntervalToMs(ntp_diff); + int64_t ntp_to_ms_diff = CompactNtpRttToMs(ntp_diff); EXPECT_NEAR(ms_diff, ntp_to_ms_diff, 1); } -TEST(TimeUtilTest, CompactNtpToMsLarge) { - const NtpTime ntp1(0x10000, 0x23456); - const NtpTime ntp2(0x1ffff, 0x64335); - uint32_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); - // Ntp difference close to maximum of ~18 hours should convert correctly too. - ASSERT_GT(ms_diff, 18u * 3600 * 1000); +TEST(TimeUtilTest, CompactNtpRttToMsLarge) { + const NtpTime ntp1(0x10000, 0x00006); + const NtpTime ntp2(0x17fff, 0xffff5); + int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); + // Ntp difference close to 2^15 seconds should convert correctly too. + ASSERT_NEAR(ms_diff, ((1 << 15) - 1) * 1000, 1); uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); - uint32_t ntp_to_ms_diff = CompactNtpIntervalToMs(ntp_diff); + int64_t ntp_to_ms_diff = CompactNtpRttToMs(ntp_diff); EXPECT_NEAR(ms_diff, ntp_to_ms_diff, 1); } + +TEST(TimeUtilTest, CompactNtpRttToMsNegative) { + const NtpTime ntp1(0x20000, 0x23456); + const NtpTime ntp2(0x1ffff, 0x64335); + int64_t ms_diff = ntp2.ToMs() - ntp1.ToMs(); + ASSERT_GT(0, ms_diff); + // Ntp difference close to 2^16 seconds should be treated as negative. + uint32_t ntp_diff = CompactNtp(ntp2) - CompactNtp(ntp1); + int64_t ntp_to_ms_diff = CompactNtpRttToMs(ntp_diff); + EXPECT_EQ(1, ntp_to_ms_diff); +} } // namespace webrtc