Add Stable Writable Connection Ping Interval parameter to RTCConfiguration.
Bug: webrtc:12642
Change-Id: I543760d49f87130d717c7cf0eca7d2d2f45e8eac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215242
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Derek Bailey <derekbailey@google.com>
Cr-Commit-Position: refs/heads/master@{#33751}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 1c147f7..f5f69f85 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -634,6 +634,10 @@
// The delay before doing a usage histogram report for long-lived
// PeerConnections. Used for testing only.
absl::optional<int> report_usage_pattern_delay_ms;
+
+ // The ping interval (ms) when the connection is stable and writable. This
+ // parameter overrides the default value in the ICE implementation if set.
+ absl::optional<int> stable_writable_connection_ping_interval_ms;
//
// Don't forget to update operator== if adding something.
//
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 19ba370..b217a74 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -2937,6 +2937,53 @@
DestroyChannels();
}
+// Test that the connection is pinged at a rate no faster than
+// what was configured when stable and writable.
+TEST_F(P2PTransportChannelMultihomedTest, TestStableWritableRate) {
+ AddAddress(0, kPublicAddrs[0]);
+ // Adding alternate address will make sure |kPublicAddrs| has the higher
+ // priority than others. This is due to FakeNetwork::AddInterface method.
+ AddAddress(1, kAlternateAddrs[1]);
+ AddAddress(1, kPublicAddrs[1]);
+
+ // Use only local ports for simplicity.
+ SetAllocatorFlags(0, kOnlyLocalPorts);
+ SetAllocatorFlags(1, kOnlyLocalPorts);
+
+ // Create channels and let them go writable, as usual.
+ CreateChannels();
+ EXPECT_TRUE_WAIT_MARGIN(CheckConnected(ep1_ch1(), ep2_ch1()), 1000, 1000);
+ // Set a value larger than the default value of 2500 ms
+ int ping_interval_ms = 3456;
+ IceConfig config = CreateIceConfig(2 * ping_interval_ms, GATHER_ONCE);
+ config.stable_writable_connection_ping_interval = ping_interval_ms;
+ ep2_ch1()->SetIceConfig(config);
+ // After the state becomes COMPLETED and is stable and writable, the
+ // connection will be pinged once every |ping_interval_ms| milliseconds.
+ ASSERT_TRUE_WAIT(ep2_ch1()->GetState() == IceTransportState::STATE_COMPLETED,
+ 1000);
+ auto connections = ep2_ch1()->connections();
+ ASSERT_EQ(2U, connections.size());
+ Connection* conn = connections[0];
+ EXPECT_TRUE_WAIT(conn->writable(), kMediumTimeout);
+
+ int64_t last_ping_response_ms;
+ // Burn through some pings so the connection is stable.
+ for (int i = 0; i < 5; i++) {
+ last_ping_response_ms = conn->last_ping_response_received();
+ EXPECT_TRUE_WAIT(
+ last_ping_response_ms < conn->last_ping_response_received(),
+ kDefaultTimeout);
+ }
+ EXPECT_TRUE(conn->stable(last_ping_response_ms)) << "Connection not stable";
+ int time_elapsed =
+ conn->last_ping_response_received() - last_ping_response_ms;
+ RTC_LOG(LS_INFO) << "Time elapsed: " << time_elapsed;
+ EXPECT_GE(time_elapsed, ping_interval_ms);
+
+ DestroyChannels();
+}
+
TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
rtc::ScopedFakeClock clock;
AddAddress(0, kAlternateAddrs[0]);
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 0da0c37..3eb5aeb 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -180,7 +180,6 @@
return kIceCandidatePairMax;
}
-
absl::optional<int> RTCConfigurationToIceConfigOptionalInt(
int rtc_configuration_parameter) {
if (rtc_configuration_parameter ==
@@ -248,6 +247,8 @@
ice_config.ice_inactive_timeout = config.ice_inactive_timeout;
ice_config.stun_keepalive_interval = config.stun_candidate_keepalive_interval;
ice_config.network_preference = config.network_preference;
+ ice_config.stable_writable_connection_ping_interval =
+ config.stable_writable_connection_ping_interval_ms;
return ice_config;
}
@@ -334,6 +335,7 @@
bool enable_implicit_rollback;
absl::optional<bool> allow_codec_switching;
absl::optional<int> report_usage_pattern_delay_ms;
+ absl::optional<int> stable_writable_connection_ping_interval_ms;
};
static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this),
"Did you add something to RTCConfiguration and forget to "
@@ -393,7 +395,9 @@
turn_logging_id == o.turn_logging_id &&
enable_implicit_rollback == o.enable_implicit_rollback &&
allow_codec_switching == o.allow_codec_switching &&
- report_usage_pattern_delay_ms == o.report_usage_pattern_delay_ms;
+ report_usage_pattern_delay_ms == o.report_usage_pattern_delay_ms &&
+ stable_writable_connection_ping_interval_ms ==
+ o.stable_writable_connection_ping_interval_ms;
}
bool PeerConnectionInterface::RTCConfiguration::operator!=(
@@ -1423,6 +1427,8 @@
configuration.active_reset_srtp_params;
modified_config.turn_logging_id = configuration.turn_logging_id;
modified_config.allow_codec_switching = configuration.allow_codec_switching;
+ modified_config.stable_writable_connection_ping_interval_ms =
+ configuration.stable_writable_connection_ping_interval_ms;
if (configuration != modified_config) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION,
"Modifying the configuration in an unsupported way.");
diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc
index 9a9e2e6..b9b8966 100644
--- a/pc/peer_connection_ice_unittest.cc
+++ b/pc/peer_connection_ice_unittest.cc
@@ -1406,6 +1406,36 @@
EXPECT_EQ(actual_stun_keepalive_interval.value_or(-1), 321);
}
+TEST_F(PeerConnectionIceConfigTest, SetStableWritableConnectionInterval) {
+ RTCConfiguration config;
+ config.stable_writable_connection_ping_interval_ms = 3500;
+ CreatePeerConnection(config);
+ EXPECT_TRUE(pc_->SetConfiguration(config).ok());
+ EXPECT_EQ(pc_->GetConfiguration().stable_writable_connection_ping_interval_ms,
+ config.stable_writable_connection_ping_interval_ms);
+}
+
+TEST_F(PeerConnectionIceConfigTest,
+ SetStableWritableConnectionInterval_FailsValidation) {
+ RTCConfiguration config;
+ CreatePeerConnection(config);
+ ASSERT_TRUE(pc_->SetConfiguration(config).ok());
+ config.stable_writable_connection_ping_interval_ms = 5000;
+ config.ice_check_interval_strong_connectivity = 7500;
+ EXPECT_FALSE(pc_->SetConfiguration(config).ok());
+}
+
+TEST_F(PeerConnectionIceConfigTest,
+ SetStableWritableConnectionInterval_DefaultValue_FailsValidation) {
+ RTCConfiguration config;
+ CreatePeerConnection(config);
+ ASSERT_TRUE(pc_->SetConfiguration(config).ok());
+ config.ice_check_interval_strong_connectivity = 2500;
+ EXPECT_TRUE(pc_->SetConfiguration(config).ok());
+ config.ice_check_interval_strong_connectivity = 2501;
+ EXPECT_FALSE(pc_->SetConfiguration(config).ok());
+}
+
TEST_P(PeerConnectionIceTest, IceCredentialsCreateOffer) {
RTCConfiguration config;
config.ice_candidate_pool_size = 1;
diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java
index e174641..38d2d38 100644
--- a/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/sdk/android/api/org/webrtc/PeerConnection.java
@@ -501,6 +501,9 @@
// to keep NAT bindings open.
// The default value in the implementation is used if this field is null.
@Nullable public Integer stunCandidateKeepaliveIntervalMs;
+ // The interval in milliseconds of pings sent when the connection is stable and writable.
+ // The default value in the implementation is used if this field is null.
+ @Nullable public Integer stableWritableConnectionPingIntervalMs;
public boolean disableIPv6OnWifi;
// By default, PeerConnection will use a limited number of IPv6 network
// interfaces, in order to avoid too many ICE candidate pairs being created
@@ -589,6 +592,7 @@
iceUnwritableTimeMs = null;
iceUnwritableMinChecks = null;
stunCandidateKeepaliveIntervalMs = null;
+ stableWritableConnectionPingIntervalMs = null;
disableIPv6OnWifi = false;
maxIPv6Networks = 5;
disableIpv6 = false;
@@ -735,6 +739,12 @@
return stunCandidateKeepaliveIntervalMs;
}
+ @Nullable
+ @CalledByNative("RTCConfiguration")
+ Integer getStableWritableConnectionPingIntervalMs() {
+ return stableWritableConnectionPingIntervalMs;
+ }
+
@CalledByNative("RTCConfiguration")
boolean getDisableIPv6OnWifi() {
return disableIPv6OnWifi;
diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc
index 19bb61b..35f6ad5 100644
--- a/sdk/android/src/jni/pc/peer_connection.cc
+++ b/sdk/android/src/jni/pc/peer_connection.cc
@@ -238,6 +238,12 @@
j_rtc_config);
rtc_config->stun_candidate_keepalive_interval =
JavaToNativeOptionalInt(jni, j_stun_candidate_keepalive_interval);
+ ScopedJavaLocalRef<jobject> j_stable_writable_connection_ping_interval_ms =
+ Java_RTCConfiguration_getStableWritableConnectionPingIntervalMs(
+ jni, j_rtc_config);
+ rtc_config->stable_writable_connection_ping_interval_ms =
+ JavaToNativeOptionalInt(jni,
+ j_stable_writable_connection_ping_interval_ms);
rtc_config->disable_ipv6_on_wifi =
Java_RTCConfiguration_getDisableIPv6OnWifi(jni, j_rtc_config);
rtc_config->max_ipv6_networks =
@@ -468,9 +474,7 @@
return jlongFromPointer(new PeerConnectionObserverJni(jni, j_observer));
}
-static void JNI_PeerConnection_FreeOwnedPeerConnection(
- JNIEnv*,
- jlong j_p) {
+static void JNI_PeerConnection_FreeOwnedPeerConnection(JNIEnv*, jlong j_p) {
delete reinterpret_cast<OwnedPeerConnection*>(j_p);
}
