Cleanup RtcpReceiver::NTP function
Replace it with GetSenderReportStats that returns result instead of
filling lots of output parameters
On the way replace pair of uint32_t with dedicated NtpTime type
Bug: None
Change-Id: I5b821b8d000d63ebd8cdc1b9897a86429d97b19b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295560
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39431}
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index 0a24481..9b4cf18 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -156,10 +156,6 @@
: kDefaultVideoReportInterval)),
// 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),
@@ -189,10 +185,6 @@
: kDefaultVideoReportInterval)),
// 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),
@@ -241,7 +233,7 @@
void RTCPReceiver::SetRemoteSSRC(uint32_t ssrc) {
MutexLock lock(&rtcp_receiver_lock_);
// New SSRC reset old reports.
- last_received_sr_ntp_.Reset();
+ remote_sender_.last_arrival_timestamp.Reset();
remote_ssrc_ = ssrc;
}
@@ -368,42 +360,14 @@
return rtt;
}
-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,
- uint32_t* remote_sender_packet_count,
- uint64_t* remote_sender_octet_count,
- uint64_t* remote_sender_reports_count) const {
+absl::optional<RtpRtcpInterface::SenderReportStats>
+RTCPReceiver::GetSenderReportStats() const {
MutexLock lock(&rtcp_receiver_lock_);
- if (!last_received_sr_ntp_.Valid())
- return false;
+ if (!remote_sender_.last_arrival_timestamp.Valid()) {
+ return absl::nullopt;
+ }
- // NTP from incoming SenderReport.
- if (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_;
-
- // Local NTP time when we received a RTCP packet with a send block.
- if (rtcp_arrival_time_secs)
- *rtcp_arrival_time_secs = last_received_sr_ntp_.seconds();
- 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;
+ return remote_sender_;
}
std::vector<rtcp::ReceiveTimeInfo>
@@ -584,12 +548,12 @@
// Only signal that we have received a SR when we accept one.
packet_information->packet_type_flags |= kRtcpSr;
- remote_sender_ntp_time_ = sender_report.ntp();
- remote_sender_rtp_time_ = sender_report.rtp_timestamp();
- last_received_sr_ntp_ = clock_->CurrentNtpTime();
- remote_sender_packet_count_ = sender_report.sender_packet_count();
- remote_sender_octet_count_ = sender_report.sender_octet_count();
- remote_sender_reports_count_++;
+ remote_sender_.last_remote_timestamp = sender_report.ntp();
+ remote_sender_.last_remote_rtp_timestamp = sender_report.rtp_timestamp();
+ remote_sender_.last_arrival_timestamp = clock_->CurrentNtpTime();
+ remote_sender_.packets_sent = sender_report.sender_packet_count();
+ remote_sender_.bytes_sent = 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 d3c0792..5007b42 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -110,22 +110,9 @@
bool receiver_only() const { return receiver_only_; }
- // 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,
- uint32_t* remote_sender_packet_count,
- uint64_t* remote_sender_octet_count,
- uint64_t* remote_sender_reports_count) const;
+ // Returns stats based on the received RTCP Sender Reports.
+ absl::optional<RtpRtcpInterface::SenderReportStats> GetSenderReportStats()
+ const;
std::vector<rtcp::ReceiveTimeInfo> ConsumeReceivedXrReferenceTimeInfo();
@@ -388,13 +375,8 @@
uint32_t remote_ssrc_ RTC_GUARDED_BY(rtcp_receiver_lock_);
// Received sender report.
- NtpTime remote_sender_ntp_time_ RTC_GUARDED_BY(rtcp_receiver_lock_);
- 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_);
+ RtpRtcpInterface::SenderReportStats remote_sender_
+ RTC_GUARDED_BY(rtcp_receiver_lock_);
// Received RRTR information in ascending receive time order.
std::list<RrtrInformation> received_rrtrs_
diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index ff39ccc..b366731 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -202,8 +202,7 @@
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
- EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr,
- nullptr, nullptr, nullptr));
+ EXPECT_FALSE(receiver.GetSenderReportStats());
int64_t now = mocks.clock.TimeInMilliseconds();
rtcp::SenderReport sr;
@@ -214,8 +213,7 @@
OnReceivedRtcpReceiverReport(IsEmpty(), _, now));
receiver.IncomingPacket(sr.Build());
- EXPECT_TRUE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
- nullptr, nullptr));
+ EXPECT_TRUE(receiver.GetSenderReportStats());
}
TEST(RtcpReceiverTest, InjectSrPacketFromUnknownSender) {
@@ -235,8 +233,7 @@
receiver.IncomingPacket(sr.Build());
// But will not flag that he's gotten sender information.
- EXPECT_FALSE(receiver.NTP(nullptr, nullptr, nullptr, nullptr, nullptr,
- nullptr, nullptr, nullptr));
+ EXPECT_FALSE(receiver.GetSenderReportStats());
}
TEST(RtcpReceiverTest, InjectSrPacketCalculatesRTT) {
diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc
index 7983371..1f58a24c 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -92,8 +92,6 @@
: packets_sent(0),
media_bytes_sent(0),
send_bitrate(0),
- last_rr_ntp_secs(0),
- last_rr_ntp_frac(0),
remote_sr(0),
receiver(nullptr) {}
@@ -823,14 +821,10 @@
// streams.
result = receive_statistics_->RtcpReportBlocks(RTCP_MAX_REPORT_BLOCKS);
- if (!result.empty() && ((feedback_state.last_rr_ntp_secs != 0) ||
- (feedback_state.last_rr_ntp_frac != 0))) {
+ if (!result.empty() && feedback_state.last_rr.Valid()) {
// Get our NTP as late as possible to avoid a race.
uint32_t now = CompactNtp(clock_->CurrentNtpTime());
-
- uint32_t receive_time = feedback_state.last_rr_ntp_secs & 0x0000FFFF;
- receive_time <<= 16;
- receive_time += (feedback_state.last_rr_ntp_frac & 0xffff0000) >> 16;
+ uint32_t receive_time = CompactNtp(feedback_state.last_rr);
uint32_t delay_since_last_sr = now - receive_time;
// TODO(danilchap): Instead of setting same value on all report blocks,
diff --git a/modules/rtp_rtcp/source/rtcp_sender.h b/modules/rtp_rtcp/source/rtcp_sender.h
index 8f51e79..6dae9db 100644
--- a/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/modules/rtp_rtcp/source/rtcp_sender.h
@@ -37,6 +37,7 @@
#include "rtc_base/random.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
+#include "system_wrappers/include/ntp_time.h"
namespace webrtc {
@@ -93,9 +94,8 @@
size_t media_bytes_sent;
uint32_t send_bitrate;
- uint32_t last_rr_ntp_secs;
- uint32_t last_rr_ntp_frac;
uint32_t remote_sr;
+ NtpTime last_rr;
std::vector<rtcp::ReceiveTimeInfo> last_xr_rtis;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 03d0e31..c3e5e93 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -20,11 +20,13 @@
#include <utility>
#include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
#include "api/transport/field_trial_based_config.h"
#include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h"
#include "modules/rtp_rtcp/source/rtcp_sender.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
+#include "modules/rtp_rtcp/source/time_util.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "system_wrappers/include/ntp_time.h"
@@ -282,18 +284,11 @@
}
state.receiver = &rtcp_receiver_;
- uint32_t received_ntp_secs = 0;
- uint32_t received_ntp_frac = 0;
- state.remote_sr = 0;
- 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,
- /*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);
+ if (absl::optional<RtpRtcpInterface::SenderReportStats> last_sr =
+ rtcp_receiver_.GetSenderReportStats();
+ last_sr.has_value()) {
+ state.remote_sr = CompactNtp(last_sr->last_remote_timestamp);
+ state.last_rr = last_sr->last_arrival_timestamp;
}
state.last_xr_rtis = rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo();
@@ -519,22 +514,7 @@
absl::optional<RtpRtcpInterface::SenderReportStats>
ModuleRtpRtcpImpl::GetSenderReportStats() const {
- SenderReportStats stats;
- uint32_t remote_timestamp_secs;
- uint32_t remote_timestamp_frac;
- uint32_t arrival_timestamp_secs;
- uint32_t arrival_timestamp_frac;
- if (rtcp_receiver_.NTP(&remote_timestamp_secs, &remote_timestamp_frac,
- &arrival_timestamp_secs, &arrival_timestamp_frac,
- &stats.last_remote_rtp_timestamp, &stats.packets_sent,
- &stats.bytes_sent, &stats.reports_count)) {
- stats.last_remote_timestamp.Set(remote_timestamp_secs,
- remote_timestamp_frac);
- stats.last_arrival_timestamp.Set(arrival_timestamp_secs,
- arrival_timestamp_frac);
- return stats;
- }
- return absl::nullopt;
+ return rtcp_receiver_.GetSenderReportStats();
}
absl::optional<RtpRtcpInterface::NonSenderRttStats>
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
index 8183e4a..bbcd5d8 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -27,6 +27,7 @@
#include "api/units/timestamp.h"
#include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
+#include "modules/rtp_rtcp/source/time_util.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/time_utils.h"
@@ -253,18 +254,11 @@
}
state.receiver = &rtcp_receiver_;
- uint32_t received_ntp_secs = 0;
- uint32_t received_ntp_frac = 0;
- state.remote_sr = 0;
- 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,
- /*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);
+ if (absl::optional<RtpRtcpInterface::SenderReportStats> last_sr =
+ rtcp_receiver_.GetSenderReportStats();
+ last_sr.has_value()) {
+ state.remote_sr = CompactNtp(last_sr->last_remote_timestamp);
+ state.last_rr = last_sr->last_arrival_timestamp;
}
state.last_xr_rtis = rtcp_receiver_.ConsumeReceivedXrReferenceTimeInfo();
@@ -500,22 +494,7 @@
absl::optional<RtpRtcpInterface::SenderReportStats>
ModuleRtpRtcpImpl2::GetSenderReportStats() const {
- SenderReportStats stats;
- uint32_t remote_timestamp_secs;
- uint32_t remote_timestamp_frac;
- uint32_t arrival_timestamp_secs;
- uint32_t arrival_timestamp_frac;
- if (rtcp_receiver_.NTP(&remote_timestamp_secs, &remote_timestamp_frac,
- &arrival_timestamp_secs, &arrival_timestamp_frac,
- &stats.last_remote_rtp_timestamp, &stats.packets_sent,
- &stats.bytes_sent, &stats.reports_count)) {
- stats.last_remote_timestamp.Set(remote_timestamp_secs,
- remote_timestamp_frac);
- stats.last_arrival_timestamp.Set(arrival_timestamp_secs,
- arrival_timestamp_frac);
- return stats;
- }
- return absl::nullopt;
+ return rtcp_receiver_.GetSenderReportStats();
}
absl::optional<RtpRtcpInterface::NonSenderRttStats>
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
index c1ec671..d49051b 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -159,19 +159,19 @@
// Received (a.k.a., remote) NTP timestamp for the last received RTCP SR.
NtpTime last_remote_timestamp;
// Received (a.k.a., remote) RTP timestamp from the last received RTCP SR.
- uint32_t last_remote_rtp_timestamp;
+ uint32_t last_remote_rtp_timestamp = 0;
// Total number of RTP data packets transmitted by the sender since starting
// transmission up until the time this SR packet was generated. The count
// should be reset if the sender changes its SSRC identifier.
- uint32_t packets_sent;
+ uint32_t packets_sent = 0;
// Total number of payload octets (i.e., not including header or padding)
// transmitted in RTP data packets by the sender since starting transmission
// up until the time this SR packet was generated. The count should be reset
// if the sender changes its SSRC identifier.
- uint64_t bytes_sent;
+ uint64_t bytes_sent = 0;
// Total number of RTCP SR blocks received.
// https://www.w3.org/TR/webrtc-stats/#dom-rtcremoteoutboundrtpstreamstats-reportssent.
- uint64_t reports_count;
+ uint64_t reports_count = 0;
};
// Stats about the non-sender SSRC, based on RTCP extended reports (XR).
// Refer to https://datatracker.ietf.org/doc/html/rfc3611#section-2.
