diff --git a/webrtc/api/stats/rtcstats_objects.h b/webrtc/api/stats/rtcstats_objects.h index 29e2d7633c..baf0d28386 100644 --- a/webrtc/api/stats/rtcstats_objects.h +++ b/webrtc/api/stats/rtcstats_objects.h @@ -144,14 +144,11 @@ class RTCIceCandidatePairStats final : public RTCStats { RTCStatsMember readable; RTCStatsMember bytes_sent; RTCStatsMember bytes_received; - // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7062 RTCStatsMember 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 current_round_trip_time; RTCStatsMember 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 available_incoming_bitrate; RTCStatsMember requests_received; diff --git a/webrtc/p2p/base/jseptransport.cc b/webrtc/p2p/base/jseptransport.cc index 86dc6a3584..4bc24fd07b 100644 --- a/webrtc/p2p/base/jseptransport.cc +++ b/webrtc/p2p/base/jseptransport.cc @@ -61,7 +61,8 @@ ConnectionInfo::ConnectionInfo() 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 2d68543d5c..e29dd0d02a 100644 --- a/webrtc/p2p/base/jseptransport.h +++ b/webrtc/p2p/base/jseptransport.h @@ -115,6 +115,10 @@ struct ConnectionInfo { 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 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 80b09c5034..7410b20bf4 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1251,6 +1251,7 @@ void Connection::ReceivedPing() { } 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 @@ void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) { acked_nomination_ = iter->nomination; } + total_round_trip_time_ms_ += rtt; + current_round_trip_time_ms_ = rtc::Optional( + static_cast(rtt)); + pings_since_last_response_.clear(); last_ping_response_received_ = rtc::TimeMillis(); UpdateReceiving(last_ping_response_received_); @@ -1504,6 +1509,8 @@ ConnectionInfo Connection::stats() { 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 a80fde79a9..4d15656776 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 @@ class Connection : public CandidatePairInterface, 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 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 ecd5c416c7..4029118892 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -222,6 +222,23 @@ class TestPort : public Port { 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(), + 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(), + 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 @@ TEST_F(PortTest, TestNomination) { EXPECT_EQ(rconn->nominated(), rconn->stats().nominated); } +TEST_F(PortTest, TestRoundTripTime) { + rtc::ScopedFakeClock clock; + + std::unique_ptr lport( + CreateTestPort(kLocalAddr1, "lfrag", "lpass")); + std::unique_ptr 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 lport( CreateTestPort(kLocalAddr1, "lfrag", "lpass")); diff --git a/webrtc/pc/rtcstats_integrationtest.cc b/webrtc/pc/rtcstats_integrationtest.cc index 746dc0d6c6..34adb4cb13 100644 --- a/webrtc/pc/rtcstats_integrationtest.cc +++ b/webrtc/pc/rtcstats_integrationtest.cc @@ -372,7 +372,8 @@ class RTCStatsReportVerifier { verifier.TestMemberIsUndefined(candidate_pair.readable); verifier.TestMemberIsNonNegative(candidate_pair.bytes_sent); verifier.TestMemberIsNonNegative(candidate_pair.bytes_received); - verifier.TestMemberIsUndefined(candidate_pair.total_round_trip_time); + verifier.TestMemberIsNonNegative( + candidate_pair.total_round_trip_time); verifier.TestMemberIsNonNegative( candidate_pair.current_round_trip_time); if (is_selected_pair) { diff --git a/webrtc/pc/rtcstatscollector.cc b/webrtc/pc/rtcstatscollector.cc index 81443bb82c..abb4ab94c7 100644 --- a/webrtc/pc/rtcstatscollector.cc +++ b/webrtc/pc/rtcstatscollector.cc @@ -875,11 +875,14 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( static_cast(info.sent_total_bytes); candidate_pair_stats->bytes_received = static_cast(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(info.rtt) / rtc::kNumMillisecsPerSec; + candidate_pair_stats->total_round_trip_time = + static_cast(info.total_round_trip_time_ms) / + rtc::kNumMillisecsPerSec; + if (info.current_round_trip_time_ms) { + candidate_pair_stats->current_round_trip_time = + static_cast(*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 e698cf513a..eea33e0abb 100644 --- a/webrtc/pc/rtcstatscollector_unittest.cc +++ b/webrtc/pc/rtcstatscollector_unittest.cc @@ -1241,7 +1241,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { 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(); 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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { 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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { report->Get(expected_pair.id())->cast_to()); 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(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()); + 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]