p2p_transport_channel: Add estimated disconnected time to CandidatePairChangeEvent
This patch adds a computed estimate on how long the ice stack
was disconnected before switching to a new connection.
The metric is currently computed as now - max(connection->last_data_recevied())
and has resonably good precision.
Bug: webrtc:11862
Change-Id: I8950d55f0eadcf164de089cdb715b4f7eed0a4c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182002
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31969}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 6f0df04..090d5a4 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -1784,6 +1784,15 @@
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 @@
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 @@
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 @@
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 4f891be..69a32e4 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -358,6 +358,9 @@
return const_cast<Connection*>(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 @@
// 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 cfdee81..ad62920 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -3268,6 +3268,14 @@
}
}
+ 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<rtc::VirtualSocketServer> vss_;
rtc::AutoSocketServerThread thread_;
@@ -4088,6 +4096,64 @@
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 893e80b..c7be445 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -150,6 +150,8 @@
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<rtc::SocketAddress> ServerAddresses;
diff --git a/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java b/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java
index 395b629..b8e6685 100644
--- a/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java
+++ b/sdk/android/api/org/webrtc/CandidatePairChangeEvent.java
@@ -20,12 +20,20 @@
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 1165333..6706782 100644
--- a/sdk/android/src/jni/pc/peer_connection.cc
+++ b/sdk/android/src/jni/pc/peer_connection.cc
@@ -128,7 +128,8 @@
env, NativeToJavaCandidate(env, selected_pair.local_candidate()),
NativeToJavaCandidate(env, selected_pair.remote_candidate()),
static_cast<int>(event.last_data_received_ms),
- NativeToJavaString(env, event.reason));
+ NativeToJavaString(env, event.reason),
+ static_cast<int>(event.estimated_disconnected_time_ms));
}
} // namespace
