From 048adc7136346105b21dc403acc60bf94e3e931b Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Wed, 10 Mar 2021 15:05:55 +0100 Subject: [PATCH] Add missing remote-outbound stats to RTCPReceiver::NTP In order to add `RTCRemoteOutboundRtpStreamStats` (see [1]), the following stats must be added: - sender's packet count (see [2]) - sender's octet count (see [2]) - total number of RTCP SR blocks sent (see [3]) [1] https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats [2] https://tools.ietf.org/html/rfc3550#section-6.4.1 [3] https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-reportssent Bug: webrtc:12529 Change-Id: I47ac2f79ba53631965d1cd7c1062f3d0f158d66e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/210963 Commit-Queue: Alessio Bazzica Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#33423} --- modules/rtp_rtcp/source/rtcp_receiver.cc | 20 +++++++++++++++++-- modules/rtp_rtcp/source/rtcp_receiver.h | 15 +++++++++++++- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 9 ++++++--- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 10 ++++++++-- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 10 ++++++++-- 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index da59fe0de0..7d2e543a2c 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -174,6 +174,9 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, // TODO(bugs.webrtc.org/10774): Remove fallback. remote_ssrc_(0), remote_sender_rtp_time_(0), + remote_sender_packet_count_(0), + remote_sender_octet_count_(0), + remote_sender_reports_count_(0), xr_rrtr_status_(config.non_sender_rtt_measurement), xr_rr_rtt_ms_(0), oldest_tmmbr_info_ms_(0), @@ -325,7 +328,10 @@ bool RTCPReceiver::NTP(uint32_t* received_ntp_secs, uint32_t* received_ntp_frac, uint32_t* rtcp_arrival_time_secs, uint32_t* rtcp_arrival_time_frac, - uint32_t* rtcp_timestamp) const { + uint32_t* rtcp_timestamp, + uint32_t* remote_sender_packet_count, + uint64_t* remote_sender_octet_count, + uint64_t* remote_sender_reports_count) const { MutexLock lock(&rtcp_receiver_lock_); if (!last_received_sr_ntp_.Valid()) return false; @@ -335,7 +341,6 @@ bool RTCPReceiver::NTP(uint32_t* received_ntp_secs, *received_ntp_secs = remote_sender_ntp_time_.seconds(); if (received_ntp_frac) *received_ntp_frac = remote_sender_ntp_time_.fractions(); - // Rtp time from incoming SenderReport. if (rtcp_timestamp) *rtcp_timestamp = remote_sender_rtp_time_; @@ -346,6 +351,14 @@ bool RTCPReceiver::NTP(uint32_t* received_ntp_secs, if (rtcp_arrival_time_frac) *rtcp_arrival_time_frac = last_received_sr_ntp_.fractions(); + // Counters. + if (remote_sender_packet_count) + *remote_sender_packet_count = remote_sender_packet_count_; + if (remote_sender_octet_count) + *remote_sender_octet_count = remote_sender_octet_count_; + if (remote_sender_reports_count) + *remote_sender_reports_count = remote_sender_reports_count_; + return true; } @@ -519,6 +532,9 @@ 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_ = TimeMicrosToNtp(clock_->TimeInMicroseconds()); + remote_sender_packet_count_ = sender_report.sender_packet_count(); + remote_sender_octet_count_ = sender_report.sender_octet_count(); + remote_sender_reports_count_++; } else { // We will only store the send report from one source, but // we will store all the receive blocks. diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index e5dc915444..350ec28310 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -68,11 +68,21 @@ class RTCPReceiver final { uint32_t RemoteSSRC() const; // Get received NTP. + // The types for the arguments below derive from the specification: + // - `remote_sender_packet_count`: `RTCSentRtpStreamStats.packetsSent` [1] + // - `remote_sender_octet_count`: `RTCSentRtpStreamStats.bytesSent` [1] + // - `remote_sender_reports_count`: + // `RTCRemoteOutboundRtpStreamStats.reportsSent` [2] + // [1] https://www.w3.org/TR/webrtc-stats/#remoteoutboundrtpstats-dict* + // [2] https://www.w3.org/TR/webrtc-stats/#dom-rtcsentrtpstreamstats bool NTP(uint32_t* received_ntp_secs, uint32_t* received_ntp_frac, uint32_t* rtcp_arrival_time_secs, uint32_t* rtcp_arrival_time_frac, - uint32_t* rtcp_timestamp) const; + uint32_t* rtcp_timestamp, + uint32_t* remote_sender_packet_count, + uint64_t* remote_sender_octet_count, + uint64_t* remote_sender_reports_count) const; std::vector ConsumeReceivedXrReferenceTimeInfo(); @@ -239,6 +249,9 @@ class RTCPReceiver final { uint32_t remote_sender_rtp_time_ RTC_GUARDED_BY(rtcp_receiver_lock_); // When did we receive the last send report. NtpTime last_received_sr_ntp_ RTC_GUARDED_BY(rtcp_receiver_lock_); + uint32_t remote_sender_packet_count_ RTC_GUARDED_BY(rtcp_receiver_lock_); + uint64_t remote_sender_octet_count_ RTC_GUARDED_BY(rtcp_receiver_lock_); + uint64_t remote_sender_reports_count_ RTC_GUARDED_BY(rtcp_receiver_lock_); // Received RRTR information in ascending receive time order. std::list received_rrtrs_ diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index a512b0d8e2..13b3fe8b35 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -208,7 +208,8 @@ TEST(RtcpReceiverTest, InjectSrPacket) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr)); + EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr, nullptr)); int64_t now = mocks.clock.TimeInMilliseconds(); rtcp::SenderReport sr; @@ -219,7 +220,8 @@ TEST(RtcpReceiverTest, InjectSrPacket) { OnReceivedRtcpReceiverReport(IsEmpty(), _, now)); receiver.IncomingPacket(sr.Build()); - EXPECT_TRUE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr)); + EXPECT_TRUE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr)); } TEST(RtcpReceiverTest, InjectSrPacketFromUnknownSender) { @@ -239,7 +241,8 @@ TEST(RtcpReceiverTest, InjectSrPacketFromUnknownSender) { receiver.IncomingPacket(sr.Build()); // But will not flag that he's gotten sender information. - EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr)); + EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr, nullptr)); } TEST(RtcpReceiverTest, InjectSrPacketCalculatesRTT) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 62f0fac6ba..c0c67cc64d 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -318,7 +318,10 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl::GetFeedbackState() { if (rtcp_receiver_.NTP(&received_ntp_secs, &received_ntp_frac, /*rtcp_arrival_time_secs=*/&state.last_rr_ntp_secs, /*rtcp_arrival_time_frac=*/&state.last_rr_ntp_frac, - /*rtcp_timestamp=*/nullptr)) { + /*rtcp_timestamp=*/nullptr, + /*remote_sender_packet_count=*/nullptr, + /*remote_sender_octet_count=*/nullptr, + /*remote_sender_reports_count=*/nullptr)) { state.remote_sr = ((received_ntp_secs & 0x0000ffff) << 16) + ((received_ntp_frac & 0xffff0000) >> 16); } @@ -482,7 +485,10 @@ int32_t ModuleRtpRtcpImpl::RemoteNTP(uint32_t* received_ntpsecs, uint32_t* rtcp_timestamp) const { return rtcp_receiver_.NTP(received_ntpsecs, received_ntpfrac, rtcp_arrival_time_secs, rtcp_arrival_time_frac, - rtcp_timestamp) + rtcp_timestamp, + /*remote_sender_packet_count=*/nullptr, + /*remote_sender_octet_count=*/nullptr, + /*remote_sender_reports_count=*/nullptr) ? 0 : -1; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index c211aedadd..ae7d9a0d9b 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -266,7 +266,10 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl2::GetFeedbackState() { if (rtcp_receiver_.NTP(&received_ntp_secs, &received_ntp_frac, /*rtcp_arrival_time_secs=*/&state.last_rr_ntp_secs, /*rtcp_arrival_time_frac=*/&state.last_rr_ntp_frac, - /*rtcp_timestamp=*/nullptr)) { + /*rtcp_timestamp=*/nullptr, + /*remote_sender_packet_count=*/nullptr, + /*remote_sender_octet_count=*/nullptr, + /*remote_sender_reports_count=*/nullptr)) { state.remote_sr = ((received_ntp_secs & 0x0000ffff) << 16) + ((received_ntp_frac & 0xffff0000) >> 16); } @@ -444,7 +447,10 @@ int32_t ModuleRtpRtcpImpl2::RemoteNTP(uint32_t* received_ntpsecs, uint32_t* rtcp_timestamp) const { return rtcp_receiver_.NTP(received_ntpsecs, received_ntpfrac, rtcp_arrival_time_secs, rtcp_arrival_time_frac, - rtcp_timestamp) + rtcp_timestamp, + /*remote_sender_packet_count=*/nullptr, + /*remote_sender_octet_count=*/nullptr, + /*remote_sender_reports_count=*/nullptr) ? 0 : -1; }