Change RTCPReceiver::GetAndResetXrRrRtt to return TimeDelta
Bug: webrtc:13757
Change-Id: Iaf3a540fbab51990fb6b983912e3b8c1bb1aaa81
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308940
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40302}
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc
index c437138..dd9d266 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -154,7 +154,6 @@
// TODO(bugs.webrtc.org/10774): Remove fallback.
remote_ssrc_(0),
xr_rrtr_status_(config.non_sender_rtt_measurement),
- xr_rr_rtt_ms_(0),
oldest_tmmbr_info_ms_(0),
cname_callback_(config.rtcp_cname_callback),
report_block_data_observer_(config.report_block_data_observer),
@@ -182,7 +181,6 @@
// TODO(bugs.webrtc.org/10774): Remove fallback.
remote_ssrc_(0),
xr_rrtr_status_(config.non_sender_rtt_measurement),
- xr_rr_rtt_ms_(0),
oldest_tmmbr_info_ms_(0),
cname_callback_(config.rtcp_cname_callback),
report_block_data_observer_(config.report_block_data_observer),
@@ -284,15 +282,11 @@
xr_rrtr_status_ = enabled;
}
-bool RTCPReceiver::GetAndResetXrRrRtt(int64_t* rtt_ms) {
- RTC_DCHECK(rtt_ms);
+absl::optional<TimeDelta> RTCPReceiver::GetAndResetXrRrRtt() {
MutexLock lock(&rtcp_receiver_lock_);
- if (xr_rr_rtt_ms_ == 0) {
- return false;
- }
- *rtt_ms = xr_rr_rtt_ms_;
- xr_rr_rtt_ms_ = 0;
- return true;
+ absl::optional<TimeDelta> rtt = xr_rr_rtt_;
+ xr_rr_rtt_ = absl::nullopt;
+ return rtt;
}
// Called regularly (1/sec) on the worker thread to do rtt calculations.
@@ -328,10 +322,7 @@
}
} else {
// Report rtt from receiver.
- int64_t rtt_ms;
- if (GetAndResetXrRrRtt(&rtt_ms)) {
- rtt.emplace(TimeDelta::Millis(rtt_ms));
- }
+ rtt = GetAndResetXrRrRtt();
}
return rtt;
@@ -794,7 +785,7 @@
received_rrtrs_.erase(it->second);
received_rrtrs_ssrc_it_.erase(it);
}
- xr_rr_rtt_ms_ = 0;
+ xr_rr_rtt_ = absl::nullopt;
return true;
}
@@ -869,7 +860,7 @@
uint32_t rtt_ntp = now_ntp - delay_ntp - send_time_ntp;
TimeDelta rtt = CompactNtpRttToTimeDelta(rtt_ntp);
- xr_rr_rtt_ms_ = rtt.ms();
+ xr_rr_rtt_ = rtt;
non_sender_rtts_[sender_ssrc].Update(rtt);
}
diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h
index e5e0ab1..b097869 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -123,7 +123,7 @@
NonSenderRttStats GetNonSenderRTT() const;
void SetNonSenderRttMeasurement(bool enabled);
- bool GetAndResetXrRrRtt(int64_t* rtt_ms);
+ absl::optional<TimeDelta> GetAndResetXrRrRtt();
// Called once per second on the worker thread to do rtt calculations.
// Returns an optional rtt value if one is available.
@@ -376,9 +376,9 @@
flat_map<uint32_t, std::list<RrtrInformation>::iterator>
received_rrtrs_ssrc_it_ RTC_GUARDED_BY(rtcp_receiver_lock_);
- // Estimated rtt, zero when there is no valid estimate.
+ // Estimated rtt, nullopt when there is no valid estimate.
bool xr_rrtr_status_ RTC_GUARDED_BY(rtcp_receiver_lock_);
- int64_t xr_rr_rtt_ms_;
+ absl::optional<TimeDelta> xr_rr_rtt_;
int64_t oldest_tmmbr_info_ms_ RTC_GUARDED_BY(rtcp_receiver_lock_);
// Mapped by remote ssrc.
diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index 159086b..1f5f138 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -135,6 +135,7 @@
constexpr uint32_t kUnknownSenderSsrc = 0x54321;
constexpr int64_t kRtcpIntervalMs = 1000;
+constexpr TimeDelta kEpsilon = TimeDelta::Millis(1);
} // namespace
@@ -780,8 +781,7 @@
receiver.IncomingPacket(xr.Build());
- int64_t rtt_ms = 0;
- EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_FALSE(receiver.GetAndResetXrRrRtt());
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -798,8 +798,7 @@
const uint32_t kLastRR = 0x12345;
const uint32_t kDelay = 0x23456;
- int64_t rtt_ms = 0;
- EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_FALSE(receiver.GetAndResetXrRrRtt());
rtcp::ExtendedReports xr;
xr.SetSenderSsrc(kSenderSsrc);
@@ -808,9 +807,9 @@
receiver.IncomingPacket(xr.Build());
uint32_t compact_ntp_now = CompactNtp(mocks.clock.CurrentNtpTime());
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR;
- EXPECT_NEAR(CompactNtpRttToTimeDelta(rtt_ntp).ms(), rtt_ms, 1);
+ EXPECT_THAT(receiver.GetAndResetXrRrRtt(),
+ Near(CompactNtpRttToTimeDelta(rtt_ntp), kEpsilon));
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_GT(non_sender_rtt_stats.round_trip_time(), TimeDelta::Zero());
@@ -837,10 +836,9 @@
receiver.IncomingPacket(xr.Build());
uint32_t compact_ntp_now = CompactNtp(mocks.clock.CurrentNtpTime());
- int64_t rtt_ms = 0;
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
uint32_t rtt_ntp = compact_ntp_now - kDelay - kLastRR;
- EXPECT_NEAR(CompactNtpRttToTimeDelta(rtt_ntp).ms(), rtt_ms, 1);
+ EXPECT_THAT(receiver.GetAndResetXrRrRtt(),
+ Near(CompactNtpRttToTimeDelta(rtt_ntp), kEpsilon));
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_GT(non_sender_rtt_stats.round_trip_time(), TimeDelta::Zero());
@@ -866,8 +864,7 @@
std::vector<rtcp::ReceiveTimeInfo> last_xr_rtis =
receiver.ConsumeReceivedXrReferenceTimeInfo();
EXPECT_THAT(last_xr_rtis, SizeIs(1));
- int64_t rtt_ms = 0;
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_TRUE(receiver.GetAndResetXrRrRtt());
}
TEST(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) {
@@ -894,8 +891,7 @@
receiver.ConsumeReceivedXrReferenceTimeInfo();
EXPECT_THAT(last_xr_rtis, SizeIs(1));
// Validate Dlrr report wasn't processed.
- int64_t rtt_ms = 0;
- EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_FALSE(receiver.GetAndResetXrRrRtt());
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -910,8 +906,7 @@
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
- int64_t rtt_ms;
- EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_FALSE(receiver.GetAndResetXrRrRtt());
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -940,9 +935,7 @@
receiver.IncomingPacket(xr.Build());
- int64_t rtt_ms = 0;
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
- EXPECT_NEAR(kRtt.ms(), rtt_ms, 1);
+ EXPECT_THAT(receiver.GetAndResetXrRrRtt(), Near(kRtt, kEpsilon));
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -975,9 +968,7 @@
receiver.IncomingPacket(xr.Build());
- int64_t rtt_ms = 0;
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
- EXPECT_NEAR(rtt_ms, kRtt.ms(), 1);
+ EXPECT_THAT(receiver.GetAndResetXrRrRtt(), Near(kRtt, kEpsilon));
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -1011,8 +1002,7 @@
receiver.IncomingPacket(xr.Build());
// We expect that no RTT is available (because receive-side RTT was disabled).
- int64_t rtt_ms = 0;
- EXPECT_FALSE(receiver.GetAndResetXrRrRtt(&rtt_ms));
+ EXPECT_FALSE(receiver.GetAndResetXrRrRtt());
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_FALSE(non_sender_rtt_stats.round_trip_time().has_value());
@@ -1020,7 +1010,7 @@
EXPECT_EQ(non_sender_rtt_stats.round_trip_time_measurements(), 0);
}
-TEST(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) {
+TEST(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOneMillisecond) {
ReceiverMocks mocks;
auto config = DefaultConfiguration(&mocks);
config.non_sender_rtt_measurement = true;
@@ -1041,9 +1031,7 @@
receiver.IncomingPacket(xr.Build());
- int64_t rtt_ms = 0;
- EXPECT_TRUE(receiver.GetAndResetXrRrRtt(&rtt_ms));
- EXPECT_EQ(1, rtt_ms);
+ EXPECT_EQ(receiver.GetAndResetXrRrRtt(), TimeDelta::Millis(1));
RTCPReceiver::NonSenderRttStats non_sender_rtt_stats =
receiver.GetNonSenderRTT();
EXPECT_TRUE(non_sender_rtt_stats.round_trip_time().has_value());
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index f2cb9db..ab5d419 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -141,10 +141,10 @@
}
} else {
// Report rtt from receiver.
- if (process_rtt) {
- int64_t rtt_ms;
- if (rtt_stats_ && rtcp_receiver_.GetAndResetXrRrRtt(&rtt_ms)) {
- rtt_stats_->OnRttUpdate(rtt_ms);
+ if (process_rtt && rtt_stats_ != nullptr) {
+ absl::optional<TimeDelta> rtt = rtcp_receiver_.GetAndResetXrRrRtt();
+ if (rtt.has_value()) {
+ rtt_stats_->OnRttUpdate(rtt->ms());
}
}
}
