RTCIceCandidatePairStats.[total/current]RoundTripTime collected.
Collected in accordance with the spec:
https://w3c.github.io/webrtc-stats/#candidatepair-dict*
totalRoundTripTime is collected as the sum of rtt measurements, it was
previously not collected.
currentRoundTripTime is collected as the latest rtt measurement, it
was previously collected as a smoothed value, which was incorrect.
Connection is updated to collect these values which are surfaced
through ConnectionInfo.
BUG=webrtc:7062, webrtc:7204
Review-Url: https://codereview.webrtc.org/2719523002
Cr-Commit-Position: refs/heads/master@{#16905}
diff --git a/webrtc/api/stats/rtcstats_objects.h b/webrtc/api/stats/rtcstats_objects.h
index 29e2d76..baf0d28 100644
--- a/webrtc/api/stats/rtcstats_objects.h
+++ b/webrtc/api/stats/rtcstats_objects.h
@@ -144,14 +144,11 @@
RTCStatsMember<bool> readable;
RTCStatsMember<uint64_t> bytes_sent;
RTCStatsMember<uint64_t> bytes_received;
- // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7062
RTCStatsMember<double> total_round_trip_time;
- // TODO(hbos): Collect this the way the spec describes it. We have a value for
- // it but it is not spec-compliant. https://bugs.webrtc.org/7062
RTCStatsMember<double> current_round_trip_time;
RTCStatsMember<double> available_outgoing_bitrate;
// TODO(hbos): Populate this value. It is wired up and collected the same way
- // |VideoBwe.googAvailableReceiveBandwidth| is, but that value is always
+ // "VideoBwe.googAvailableReceiveBandwidth" is, but that value is always
// undefined. https://bugs.webrtc.org/7062
RTCStatsMember<double> available_incoming_bitrate;
RTCStatsMember<uint64_t> requests_received;
diff --git a/webrtc/p2p/base/jseptransport.cc b/webrtc/p2p/base/jseptransport.cc
index 86dc6a3..4bc24fd 100644
--- a/webrtc/p2p/base/jseptransport.cc
+++ b/webrtc/p2p/base/jseptransport.cc
@@ -61,7 +61,8 @@
key(nullptr),
state(IceCandidatePairState::WAITING),
priority(0),
- nominated(false) {}
+ nominated(false),
+ total_round_trip_time_ms(0) {}
bool BadTransportDescription(const std::string& desc, std::string* err_desc) {
if (err_desc) {
diff --git a/webrtc/p2p/base/jseptransport.h b/webrtc/p2p/base/jseptransport.h
index 2d68543..e29dd0d 100644
--- a/webrtc/p2p/base/jseptransport.h
+++ b/webrtc/p2p/base/jseptransport.h
@@ -115,6 +115,10 @@
uint64_t priority;
// https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-nominated
bool nominated;
+ // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-totalroundtriptime
+ uint64_t total_round_trip_time_ms;
+ // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentroundtriptime
+ rtc::Optional<uint32_t> current_round_trip_time_ms;
};
// Information about all the connections of a channel.
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index 80b09c5..7410b20 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -1251,6 +1251,7 @@
}
void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) {
+ RTC_DCHECK_GE(rtt, 0);
// We've already validated that this is a STUN binding response with
// the correct local and remote username for this connection.
// So if we're not already, become writable. We may be bringing a pruned
@@ -1264,6 +1265,10 @@
acked_nomination_ = iter->nomination;
}
+ total_round_trip_time_ms_ += rtt;
+ current_round_trip_time_ms_ = rtc::Optional<uint32_t>(
+ static_cast<uint32_t>(rtt));
+
pings_since_last_response_.clear();
last_ping_response_received_ = rtc::TimeMillis();
UpdateReceiving(last_ping_response_received_);
@@ -1504,6 +1509,8 @@
stats_.state = state_;
stats_.priority = priority();
stats_.nominated = nominated();
+ stats_.total_round_trip_time_ms = total_round_trip_time_ms_;
+ stats_.current_round_trip_time_ms = current_round_trip_time_ms_;
return stats_;
}
diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index a80fde7..4d15656 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -27,6 +27,7 @@
#include "webrtc/base/asyncpacketsocket.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/network.h"
+#include "webrtc/base/optional.h"
#include "webrtc/base/proxyinfo.h"
#include "webrtc/base/ratetracker.h"
#include "webrtc/base/sigslot.h"
@@ -706,6 +707,10 @@
StunRequestManager requests_;
int rtt_;
int rtt_samples_ = 0;
+ // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-totalroundtriptime
+ uint64_t total_round_trip_time_ms_ = 0;
+ // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentroundtriptime
+ rtc::Optional<uint32_t> current_round_trip_time_ms_;
int64_t last_ping_sent_; // last time we sent a ping to the other side
int64_t last_ping_received_; // last time we received a ping from the other
// side
diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc
index ecd5c41..4029118 100644
--- a/webrtc/p2p/base/port_unittest.cc
+++ b/webrtc/p2p/base/port_unittest.cc
@@ -222,6 +222,23 @@
int type_preference_ = 0;
};
+static void SendPingAndReceiveResponse(
+ Connection* lconn, TestPort* lport, Connection* rconn, TestPort* rport,
+ rtc::ScopedFakeClock* clock, int64_t ms) {
+ lconn->Ping(rtc::TimeMillis());
+ ASSERT_TRUE_WAIT(lport->last_stun_msg(), kDefaultTimeout);
+ ASSERT_TRUE(lport->last_stun_buf());
+ rconn->OnReadPacket(lport->last_stun_buf()->data<char>(),
+ lport->last_stun_buf()->size(),
+ rtc::PacketTime());
+ clock->AdvanceTime(rtc::TimeDelta::FromMilliseconds(ms));
+ ASSERT_TRUE_WAIT(rport->last_stun_msg(), kDefaultTimeout);
+ ASSERT_TRUE(rport->last_stun_buf());
+ lconn->OnReadPacket(rport->last_stun_buf()->data<char>(),
+ rport->last_stun_buf()->size(),
+ rtc::PacketTime());
+}
+
class TestChannel : public sigslot::has_slots<> {
public:
// Takes ownership of |p1| (but not |p2|).
@@ -1844,6 +1861,49 @@
EXPECT_EQ(rconn->nominated(), rconn->stats().nominated);
}
+TEST_F(PortTest, TestRoundTripTime) {
+ rtc::ScopedFakeClock clock;
+
+ std::unique_ptr<TestPort> lport(
+ CreateTestPort(kLocalAddr1, "lfrag", "lpass"));
+ std::unique_ptr<TestPort> rport(
+ CreateTestPort(kLocalAddr2, "rfrag", "rpass"));
+ lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
+ lport->SetIceTiebreaker(kTiebreaker1);
+ rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
+ rport->SetIceTiebreaker(kTiebreaker2);
+
+ lport->PrepareAddress();
+ rport->PrepareAddress();
+ ASSERT_FALSE(lport->Candidates().empty());
+ ASSERT_FALSE(rport->Candidates().empty());
+ Connection* lconn = lport->CreateConnection(rport->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+ Connection* rconn = rport->CreateConnection(lport->Candidates()[0],
+ Port::ORIGIN_MESSAGE);
+
+ EXPECT_EQ(0u, lconn->stats().total_round_trip_time_ms);
+ EXPECT_FALSE(lconn->stats().current_round_trip_time_ms);
+
+ SendPingAndReceiveResponse(
+ lconn, lport.get(), rconn, rport.get(), &clock, 10);
+ EXPECT_EQ(10u, lconn->stats().total_round_trip_time_ms);
+ ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+ EXPECT_EQ(10u, *lconn->stats().current_round_trip_time_ms);
+
+ SendPingAndReceiveResponse(
+ lconn, lport.get(), rconn, rport.get(), &clock, 20);
+ EXPECT_EQ(30u, lconn->stats().total_round_trip_time_ms);
+ ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+ EXPECT_EQ(20u, *lconn->stats().current_round_trip_time_ms);
+
+ SendPingAndReceiveResponse(
+ lconn, lport.get(), rconn, rport.get(), &clock, 30);
+ EXPECT_EQ(60u, lconn->stats().total_round_trip_time_ms);
+ ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+ EXPECT_EQ(30u, *lconn->stats().current_round_trip_time_ms);
+}
+
TEST_F(PortTest, TestUseCandidateAttribute) {
std::unique_ptr<TestPort> lport(
CreateTestPort(kLocalAddr1, "lfrag", "lpass"));
diff --git a/webrtc/pc/rtcstats_integrationtest.cc b/webrtc/pc/rtcstats_integrationtest.cc
index 746dc0d..34adb4c 100644
--- a/webrtc/pc/rtcstats_integrationtest.cc
+++ b/webrtc/pc/rtcstats_integrationtest.cc
@@ -372,7 +372,8 @@
verifier.TestMemberIsUndefined(candidate_pair.readable);
verifier.TestMemberIsNonNegative<uint64_t>(candidate_pair.bytes_sent);
verifier.TestMemberIsNonNegative<uint64_t>(candidate_pair.bytes_received);
- verifier.TestMemberIsUndefined(candidate_pair.total_round_trip_time);
+ verifier.TestMemberIsNonNegative<double>(
+ candidate_pair.total_round_trip_time);
verifier.TestMemberIsNonNegative<double>(
candidate_pair.current_round_trip_time);
if (is_selected_pair) {
diff --git a/webrtc/pc/rtcstatscollector.cc b/webrtc/pc/rtcstatscollector.cc
index 81443bb..abb4ab9 100644
--- a/webrtc/pc/rtcstatscollector.cc
+++ b/webrtc/pc/rtcstatscollector.cc
@@ -875,11 +875,14 @@
static_cast<uint64_t>(info.sent_total_bytes);
candidate_pair_stats->bytes_received =
static_cast<uint64_t>(info.recv_total_bytes);
- // TODO(hbos): The |info.rtt| measurement is smoothed. It shouldn't be
- // smoothed according to the spec. crbug.com/633550. See
- // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentrtt
- candidate_pair_stats->current_round_trip_time =
- static_cast<double>(info.rtt) / rtc::kNumMillisecsPerSec;
+ candidate_pair_stats->total_round_trip_time =
+ static_cast<double>(info.total_round_trip_time_ms) /
+ rtc::kNumMillisecsPerSec;
+ if (info.current_round_trip_time_ms) {
+ candidate_pair_stats->current_round_trip_time =
+ static_cast<double>(*info.current_round_trip_time_ms) /
+ rtc::kNumMillisecsPerSec;
+ }
if (info.best_connection && video_media_info &&
!video_media_info->bw_estimations.empty()) {
// The bandwidth estimations we have are for the selected candidate
diff --git a/webrtc/pc/rtcstatscollector_unittest.cc b/webrtc/pc/rtcstatscollector_unittest.cc
index e698cf5..eea33e0 100644
--- a/webrtc/pc/rtcstatscollector_unittest.cc
+++ b/webrtc/pc/rtcstatscollector_unittest.cc
@@ -1241,7 +1241,8 @@
connection_info.writable = true;
connection_info.sent_total_bytes = 42;
connection_info.recv_total_bytes = 1234;
- connection_info.rtt = 1337;
+ connection_info.total_round_trip_time_ms = 0;
+ connection_info.current_round_trip_time_ms = rtc::Optional<uint32_t>();
connection_info.recv_ping_requests = 2020;
connection_info.sent_ping_requests_total = 2020;
connection_info.sent_ping_requests_before_first_response = 2000;
@@ -1294,12 +1295,14 @@
expected_pair.writable = true;
expected_pair.bytes_sent = 42;
expected_pair.bytes_received = 1234;
- expected_pair.current_round_trip_time = 1.337;
+ expected_pair.total_round_trip_time = 0.0;
expected_pair.requests_received = 2020;
expected_pair.requests_sent = 2000;
expected_pair.responses_received = 4321;
expected_pair.responses_sent = 1000;
expected_pair.consent_requests_sent = (2020 - 2000);
+ // |expected_pair.current_round_trip_time| should be undefined because the
+ // current RTT is not set.
// |expected_pair.available_[outgoing/incoming]_bitrate| should be undefined
// because is is not the current pair.
@@ -1325,6 +1328,24 @@
report->Get(expected_pair.id())->cast_to<RTCIceCandidatePairStats>());
EXPECT_TRUE(report->Get(*expected_pair.transport_id));
+ // Set round trip times and "GetStats" again.
+ session_stats.transport_stats["transport"].channel_stats[0]
+ .connection_infos[0].total_round_trip_time_ms = 7331;
+ session_stats.transport_stats["transport"].channel_stats[0]
+ .connection_infos[0].current_round_trip_time_ms =
+ rtc::Optional<uint32_t>(1337);
+ EXPECT_CALL(*video_media_channel, GetStats(_))
+ .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true)));
+ collector_->ClearCachedStatsReport();
+ report = GetStatsReport();
+ expected_pair.total_round_trip_time = 7.331;
+ expected_pair.current_round_trip_time = 1.337;
+ ASSERT_TRUE(report->Get(expected_pair.id()));
+ EXPECT_EQ(
+ expected_pair,
+ report->Get(expected_pair.id())->cast_to<RTCIceCandidatePairStats>());
+ EXPECT_TRUE(report->Get(*expected_pair.transport_id));
+
// Make pair the current pair, clear bandwidth and "GetStats" again.
session_stats.transport_stats["transport"]
.channel_stats[0]