diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 6f0df04150..090d5a4e22 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -1784,6 +1784,15 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, pair_change.selected_candidate_pair = *GetSelectedCandidatePair(); pair_change.last_data_received_ms = selected_connection_->last_data_received(); + + if (old_selected_connection) { + pair_change.estimated_disconnected_time_ms = + ComputeEstimatedDisconnectedTimeMs(rtc::TimeMillis(), + old_selected_connection); + } else { + pair_change.estimated_disconnected_time_ms = 0; + } + SignalCandidatePairChanged(pair_change); } @@ -1792,6 +1801,16 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn, ice_controller_->SetSelectedConnection(selected_connection_); } +int64_t P2PTransportChannel::ComputeEstimatedDisconnectedTimeMs( + int64_t now_ms, + Connection* old_connection) { + // TODO(jonaso): nicer keeps estimate of how frequently data _should_ be + // received, this could be used to give better estimate (if needed). + int64_t last_data_or_old_ping = + std::max(old_connection->last_received(), last_data_received_ms_); + return (now_ms - last_data_or_old_ping); +} + // Warning: UpdateState should eventually be called whenever a connection // is added, deleted, or the write state of any connection changes so that the // transport controller will get the up-to-date channel state. However it @@ -2110,6 +2129,9 @@ void P2PTransportChannel::OnReadPacket(Connection* connection, if (connection == selected_connection_) { // Let the client know of an incoming packet + RTC_DCHECK(connection->last_data_received() >= last_data_received_ms_); + last_data_received_ms_ = + std::max(last_data_received_ms_, connection->last_data_received()); SignalReadPacket(this, data, len, packet_time_us, 0); return; } @@ -2118,6 +2140,10 @@ void P2PTransportChannel::OnReadPacket(Connection* connection, if (!FindConnection(connection)) return; + RTC_DCHECK(connection->last_data_received() >= last_data_received_ms_); + last_data_received_ms_ = + std::max(last_data_received_ms_, connection->last_data_received()); + // Let the client know of an incoming packet SignalReadPacket(this, data, len, packet_time_us, 0); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 4f891beb1e..69a32e462c 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -358,6 +358,9 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { return const_cast(conn); } + int64_t ComputeEstimatedDisconnectedTimeMs(int64_t now, + Connection* old_connection); + std::string transport_name_ RTC_GUARDED_BY(network_thread_); int component_ RTC_GUARDED_BY(network_thread_); PortAllocator* allocator_ RTC_GUARDED_BY(network_thread_); @@ -440,6 +443,10 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { // Number of times the selected_connection_ has been modified. uint32_t selected_candidate_pair_changes_ = 0; + // When was last data received on a existing connection, + // from connection->last_data_received() that uses rtc::TimeMillis(). + int64_t last_data_received_ms_ = 0; + IceFieldTrials field_trials_; RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index cfdee81403..ad62920c5a 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -3268,6 +3268,14 @@ class P2PTransportChannelPingTest : public ::testing::Test, } } + int64_t LastEstimatedDisconnectedTimeMs() const { + if (!last_candidate_change_event_.has_value()) { + return 0; + } else { + return last_candidate_change_event_->estimated_disconnected_time_ms; + } + } + private: std::unique_ptr vss_; rtc::AutoSocketServerThread thread_; @@ -4088,6 +4096,64 @@ TEST_F(P2PTransportChannelPingTest, EXPECT_EQ(0, reset_selected_candidate_pair_switches()); } +TEST_F(P2PTransportChannelPingTest, TestEstimatedDisconnectedTime) { + rtc::ScopedFakeClock clock; + clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); + + FakePortAllocator pa(rtc::Thread::Current(), nullptr); + P2PTransportChannel ch("test", 1, &pa); + PrepareChannel(&ch); + ch.SetIceRole(ICEROLE_CONTROLLED); + ch.MaybeStartGathering(); + // The connections have decreasing priority. + Connection* conn1 = + CreateConnectionWithCandidate(&ch, &clock, "1.1.1.1", /* port= */ 1, + /* priority= */ 10, /* writable= */ true); + ASSERT_TRUE(conn1 != nullptr); + Connection* conn2 = + CreateConnectionWithCandidate(&ch, &clock, "2.2.2.2", /* port= */ 2, + /* priority= */ 9, /* writable= */ true); + ASSERT_TRUE(conn2 != nullptr); + + // conn1 is the selected connection because it has a higher priority, + EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout, + clock); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); + // No estimateded disconnect time at first connect <=> value is 0. + EXPECT_EQ(LastEstimatedDisconnectedTimeMs(), 0); + + // Use nomination to force switching of selected connection. + int nomination = 1; + + { + clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); + // This will not parse as STUN, and is considered data + conn1->OnReadPacket("XYZ", 3, rtc::TimeMicros()); + clock.AdvanceTime(webrtc::TimeDelta::Seconds(2)); + + // conn2 is nominated; it becomes selected. + NominateConnection(conn2, nomination++); + EXPECT_EQ(conn2, ch.selected_connection()); + // We got data 2s ago...guess that we lost 2s of connectivity. + EXPECT_EQ(LastEstimatedDisconnectedTimeMs(), 2000); + } + + { + clock.AdvanceTime(webrtc::TimeDelta::Seconds(1)); + conn2->OnReadPacket("XYZ", 3, rtc::TimeMicros()); + + clock.AdvanceTime(webrtc::TimeDelta::Seconds(2)); + ReceivePingOnConnection(conn2, kIceUfrag[1], 1, nomination++); + + clock.AdvanceTime(webrtc::TimeDelta::Millis(500)); + + ReceivePingOnConnection(conn1, kIceUfrag[1], 1, nomination++); + EXPECT_EQ(conn1, ch.selected_connection()); + // We got ping 500ms ago...guess that we lost 500ms of connectivity. + EXPECT_EQ(LastEstimatedDisconnectedTimeMs(), 500); + } +} + TEST_F(P2PTransportChannelPingTest, TestControlledAgentIgnoresSmallerNomination) { rtc::ScopedFakeClock clock; diff --git a/p2p/base/port.h b/p2p/base/port.h index 893e80b20f..c7be445a94 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -150,6 +150,8 @@ struct CandidatePairChangeEvent { CandidatePair selected_candidate_pair; int64_t last_data_received_ms; std::string reason; + // How long do we estimate that we've been disconnected. + int64_t estimated_disconnected_time_ms; }; typedef std::set ServerAddresses; diff --git a/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java b/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java index 395b629c54..b8e6685a87 100644 --- a/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java +++ b/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java @@ -20,12 +20,20 @@ public final class CandidatePairChangeEvent { public final int lastDataReceivedMs; public final String reason; + /** + * An estimate from the ICE stack on how long it was disconnected before + * changing to the new candidate pair in this event. + * The first time an candidate pair is signaled the value will be 0. + */ + public final int estimatedDisconnectedTimeMs; + @CalledByNative - CandidatePairChangeEvent( - IceCandidate local, IceCandidate remote, int lastDataReceivedMs, String reason) { + CandidatePairChangeEvent(IceCandidate local, IceCandidate remote, int lastDataReceivedMs, + String reason, int estimatedDisconnectedTimeMs) { this.local = local; this.remote = remote; this.lastDataReceivedMs = lastDataReceivedMs; this.reason = reason; + this.estimatedDisconnectedTimeMs = estimatedDisconnectedTimeMs; } } diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index 1165333644..6706782e34 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -128,7 +128,8 @@ ScopedJavaLocalRef NativeToJavaCandidatePairChange( env, NativeToJavaCandidate(env, selected_pair.local_candidate()), NativeToJavaCandidate(env, selected_pair.remote_candidate()), static_cast(event.last_data_received_ms), - NativeToJavaString(env, event.reason)); + NativeToJavaString(env, event.reason), + static_cast(event.estimated_disconnected_time_ms)); } } // namespace