Add RtpRtcpInterface::LastRtt function to replace RtpRtcpInterface::RTT

RtpRtcpInterface::RTT follows discouraged style of using return values,
uses raw integers to represent time delta,
and returns values that no code uses (min, max, average RTT)

added LastRtt function addresses all these stylistic issues.

Bug: webrtc:13757
Change-Id: Iaf947dd1b7139026f2beb991e69634c606c6b608
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304520
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40028}
This commit is contained in:
Danil Chapovalov 2023-05-09 09:59:46 +02:00 committed by WebRTC LUCI CQ
parent 20bede7cc7
commit 8095d02884
12 changed files with 72 additions and 48 deletions

View File

@ -342,11 +342,10 @@ void ChannelReceive::OnReceivedPayloadData(
return;
}
int64_t round_trip_time = 0;
rtp_rtcp_->RTT(remote_ssrc_, &round_trip_time, /*avg_rtt=*/nullptr,
/*min_rtt=*/nullptr, /*max_rtt=*/nullptr);
TimeDelta round_trip_time = rtp_rtcp_->LastRtt().value_or(TimeDelta::Zero());
std::vector<uint16_t> nack_list = acm_receiver_.GetNackList(round_trip_time);
std::vector<uint16_t> nack_list =
acm_receiver_.GetNackList(round_trip_time.ms());
if (!nack_list.empty()) {
// Can't use nack_list.data() since it's not supported by all
// compilers.
@ -741,10 +740,8 @@ void ChannelReceive::ReceivedRTCPPacket(const uint8_t* data, size_t length) {
// Deliver RTCP packet to RTP/RTCP module for parsing
rtp_rtcp_->IncomingRtcpPacket(rtc::MakeArrayView(data, length));
int64_t rtt = 0;
rtp_rtcp_->RTT(remote_ssrc_, &rtt, /*avg_rtt=*/nullptr, /*min_rtt=*/nullptr,
/*max_rtt=*/nullptr);
if (rtt == 0) {
absl::optional<TimeDelta> rtt = rtp_rtcp_->LastRtt();
if (!rtt.has_value()) {
// Waiting for valid RTT.
return;
}
@ -758,8 +755,7 @@ void ChannelReceive::ReceivedRTCPPacket(const uint8_t* data, size_t length) {
{
MutexLock lock(&ts_stats_lock_);
ntp_estimator_.UpdateRtcpTimestamp(TimeDelta::Millis(rtt),
last_sr->last_remote_timestamp,
ntp_estimator_.UpdateRtcpTimestamp(*rtt, last_sr->last_remote_timestamp,
last_sr->last_remote_rtp_timestamp);
absl::optional<int64_t> remote_to_local_clock_offset =
ntp_estimator_.EstimateRemoteToLocalClockOffset();

View File

@ -212,8 +212,8 @@ void AudioIngress::ReceivedRTCPPacket(
// Deliver RTCP packet to RTP/RTCP module for parsing and processing.
rtp_rtcp_->IncomingRtcpPacket(rtcp_packet);
int64_t rtt = 0;
if (rtp_rtcp_->RTT(remote_ssrc_, &rtt, nullptr, nullptr, nullptr) != 0) {
absl::optional<TimeDelta> rtt = rtp_rtcp_->LastRtt();
if (!rtt.has_value()) {
// Waiting for valid RTT.
return;
}
@ -227,8 +227,7 @@ void AudioIngress::ReceivedRTCPPacket(
{
MutexLock lock(&lock_);
ntp_estimator_.UpdateRtcpTimestamp(TimeDelta::Millis(rtt),
last_sr->last_remote_timestamp,
ntp_estimator_.UpdateRtcpTimestamp(*rtt, last_sr->last_remote_timestamp,
last_sr->last_remote_rtp_timestamp);
}
}

View File

@ -127,6 +127,7 @@ class MockRtpRtcpInterface : public RtpRtcpInterface {
int64_t* min_rtt,
int64_t* max_rtt),
(const, override));
MOCK_METHOD(absl::optional<TimeDelta>, LastRtt, (), (const, override));
MOCK_METHOD(int64_t, ExpectedRetransmissionTimeMs, (), (const, override));
MOCK_METHOD(int32_t, SendRTCP, (RTCPPacketType packet_type), (override));
MOCK_METHOD(void,

View File

@ -293,6 +293,15 @@ int32_t RTCPReceiver::RTT(uint32_t remote_ssrc,
return 0;
}
absl::optional<TimeDelta> RTCPReceiver::LastRtt() const {
MutexLock lock(&rtcp_receiver_lock_);
auto it = rtts_.find(remote_ssrc_);
if (it == rtts_.end()) {
return absl::nullopt;
}
return it->second.last_rtt();
}
RTCPReceiver::NonSenderRttStats RTCPReceiver::GetNonSenderRTT() const {
MutexLock lock(&rtcp_receiver_lock_);
auto it = non_sender_rtts_.find(remote_ssrc_);

View File

@ -123,6 +123,8 @@ class RTCPReceiver final {
int64_t* min_rtt_ms,
int64_t* max_rtt_ms) const;
absl::optional<TimeDelta> LastRtt() const;
// Returns non-sender RTT metrics for the remote SSRC.
NonSenderRttStats GetNonSenderRTT() const;

View File

@ -478,6 +478,17 @@ int32_t ModuleRtpRtcpImpl::RTT(const uint32_t remote_ssrc,
return ret;
}
absl::optional<TimeDelta> ModuleRtpRtcpImpl::LastRtt() const {
absl::optional<TimeDelta> rtt = rtcp_receiver_.LastRtt();
if (!rtt.has_value()) {
MutexLock lock(&mutex_rtt_);
if (rtt_ms_ > 0) {
rtt = TimeDelta::Millis(rtt_ms_);
}
}
return rtt;
}
int64_t ModuleRtpRtcpImpl::ExpectedRetransmissionTimeMs() const {
int64_t expected_retransmission_time_ms = rtt_ms();
if (expected_retransmission_time_ms > 0) {

View File

@ -170,11 +170,12 @@ class ABSL_DEPRECATED("") ModuleRtpRtcpImpl
int32_t SetCNAME(absl::string_view c_name) override;
// Get RoundTripTime.
int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const override;
[[deprecated]] int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const override;
absl::optional<TimeDelta> LastRtt() const override;
int64_t ExpectedRetransmissionTimeMs() const override;

View File

@ -463,6 +463,17 @@ int32_t ModuleRtpRtcpImpl2::RTT(const uint32_t remote_ssrc,
return ret;
}
absl::optional<TimeDelta> ModuleRtpRtcpImpl2::LastRtt() const {
absl::optional<TimeDelta> rtt = rtcp_receiver_.LastRtt();
if (!rtt.has_value()) {
MutexLock lock(&mutex_rtt_);
if (rtt_ms_ > 0) {
rtt = TimeDelta::Millis(rtt_ms_);
}
}
return rtt;
}
int64_t ModuleRtpRtcpImpl2::ExpectedRetransmissionTimeMs() const {
int64_t expected_retransmission_time_ms = rtt_ms();
if (expected_retransmission_time_ms > 0) {

View File

@ -181,11 +181,12 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface,
int32_t SetCNAME(absl::string_view c_name) override;
// Get RoundTripTime.
int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const override;
[[deprecated]] int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const override;
absl::optional<TimeDelta> LastRtt() const override;
int64_t ExpectedRetransmissionTimeMs() const override;

View File

@ -69,6 +69,10 @@ enum : int {
kTransmissionOffsetExtensionId,
};
MATCHER_P2(Near, value, margin, "") {
return value - margin <= arg && arg <= value + margin;
}
class RtcpRttStatsTestImpl : public RtcpRttStats {
public:
RtcpRttStatsTestImpl() : rtt_ms_(0) {}
@ -424,20 +428,8 @@ TEST_F(RtpRtcpImpl2Test, Rtt) {
AdvanceTime(kOneWayNetworkDelay);
// Verify RTT.
int64_t rtt;
int64_t avg_rtt;
int64_t min_rtt;
int64_t max_rtt;
EXPECT_EQ(
0, sender_.impl_->RTT(kReceiverSsrc, &rtt, &avg_rtt, &min_rtt, &max_rtt));
EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), rtt, 1);
EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), avg_rtt, 1);
EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), min_rtt, 1);
EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), max_rtt, 1);
// No RTT from other ssrc.
EXPECT_EQ(-1, sender_.impl_->RTT(kReceiverSsrc + 1, &rtt, &avg_rtt, &min_rtt,
&max_rtt));
EXPECT_THAT(sender_.impl_->LastRtt(),
Near(2 * kOneWayNetworkDelay, TimeDelta::Millis(1)));
// Verify RTT from rtt_stats config.
EXPECT_EQ(0, sender_.rtt_stats_.LastProcessedRtt());

View File

@ -380,11 +380,14 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
// Returns current RTT (round-trip time) estimate.
// Returns -1 on failure else 0.
virtual int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const = 0;
[[deprecated]] virtual int32_t RTT(uint32_t remote_ssrc,
int64_t* rtt,
int64_t* avg_rtt,
int64_t* min_rtt,
int64_t* max_rtt) const = 0;
// Returns current RTT (round-trip time) estimate.
virtual absl::optional<TimeDelta> LastRtt() const = 0;
// Returns the estimated RTT, with fallback to a default value.
virtual int64_t ExpectedRetransmissionTimeMs() const = 0;

View File

@ -1135,9 +1135,8 @@ bool RtpVideoStreamReceiver2::DeliverRtcp(const uint8_t* rtcp_packet,
rtp_rtcp_->IncomingRtcpPacket(
rtc::MakeArrayView(rtcp_packet, rtcp_packet_length));
int64_t rtt = 0;
rtp_rtcp_->RTT(config_.rtp.remote_ssrc, &rtt, nullptr, nullptr, nullptr);
if (rtt == 0) {
absl::optional<TimeDelta> rtt = rtp_rtcp_->LastRtt();
if (!rtt.has_value()) {
// Waiting for valid rtt.
return true;
}
@ -1152,8 +1151,7 @@ bool RtpVideoStreamReceiver2::DeliverRtcp(const uint8_t* rtcp_packet,
last_sr->last_arrival_timestamp.ToMs();
// Don't use old SRs to estimate time.
if (time_since_received <= 1) {
ntp_estimator_.UpdateRtcpTimestamp(TimeDelta::Millis(rtt),
last_sr->last_remote_timestamp,
ntp_estimator_.UpdateRtcpTimestamp(*rtt, last_sr->last_remote_timestamp,
last_sr->last_remote_rtp_timestamp);
absl::optional<int64_t> remote_to_local_clock_offset =
ntp_estimator_.EstimateRemoteToLocalClockOffset();