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}
This commit is contained in:
Danil Chapovalov 2023-06-16 16:12:06 +02:00 committed by WebRTC LUCI CQ
parent 18aba66271
commit 4d2a219436
4 changed files with 29 additions and 50 deletions

View File

@ -154,7 +154,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
// 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 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
// 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 @@ void RTCPReceiver::SetNonSenderRttMeasurement(bool enabled) {
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 @@ absl::optional<TimeDelta> RTCPReceiver::OnPeriodicRttUpdate(
}
} 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 @@ bool RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) {
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 @@ void RTCPReceiver::HandleXrDlrrReportBlock(uint32_t sender_ssrc,
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);
}

View File

@ -123,7 +123,7 @@ class RTCPReceiver final {
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 @@ class RTCPReceiver final {
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.

View File

@ -135,6 +135,7 @@ constexpr uint32_t kNotToUsSsrc = 0x654321;
constexpr uint32_t kUnknownSenderSsrc = 0x54321;
constexpr int64_t kRtcpIntervalMs = 1000;
constexpr TimeDelta kEpsilon = TimeDelta::Millis(1);
} // namespace
@ -780,8 +781,7 @@ TEST(RtcpReceiverTest, ExtendedReportsDlrrPacketNotToUsIgnored) {
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 @@ TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) {
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 @@ TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithSubBlock) {
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 @@ TEST(RtcpReceiverTest, InjectExtendedReportsDlrrPacketWithMultipleSubBlocks) {
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 @@ TEST(RtcpReceiverTest, InjectExtendedReportsPacketWithMultipleReportBlocks) {
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 @@ TEST(RtcpReceiverTest, InjectExtendedReportsPacketWithUnknownReportBlock) {
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 @@ TEST(RtcpReceiverTest, TestExtendedReportsRrRttInitiallyFalse) {
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 @@ TEST(RtcpReceiverTest, RttCalculatedAfterExtendedReportsDlrr) {
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 @@ TEST(RtcpReceiverTest, SetterEnablesReceiverRtt) {
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 @@ TEST(RtcpReceiverTest, DoesntCalculateRttOnReceivedDlrr) {
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 @@ TEST(RtcpReceiverTest, DoesntCalculateRttOnReceivedDlrr) {
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 @@ TEST(RtcpReceiverTest, XrDlrrCalculatesNegativeRttAsOne) {
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());

View File

@ -141,10 +141,10 @@ void ModuleRtpRtcpImpl::Process() {
}
} 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());
}
}
}