Add transport_name to CandidatePairChangeEvent
The native Candidate objects within a CandidatePairChangeEvent do not
always contain a transport_name (aka mid). That creates issues where the
transport name is required to properly construct IceCandidate objects
for compatibility with other APIs.
This change adds the transport_name to the CandidatePairChangeEvent and
setting it in P2pTransportChannel. Downstream consumers, including the
JNI and Objective-C layers, are updated to use the new field instead of
the one on the candidate.
Bug: webrtc:42233526
Change-Id: Icb411331af61fada3770b4d65002520ae1d18c7d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/403761
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45330}
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 3a04b25..c2afc27 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -1879,23 +1879,17 @@
SignalNetworkRouteChanged(network_route_);
// Create event for candidate pair change.
- if (selected_connection_) {
- CandidatePairChangeEvent pair_change;
- pair_change.reason = IceSwitchReasonToString(reason);
- 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(TimeMillis(),
- old_selected_connection);
- } else {
- pair_change.estimated_disconnected_time_ms = 0;
- }
- if (candidate_pair_change_callback_) {
- candidate_pair_change_callback_(pair_change);
- }
+ if (selected_connection_ && candidate_pair_change_callback_) {
+ CandidatePairChangeEvent pair_change = {
+ .transport_name = transport_name(),
+ .selected_candidate_pair = *GetSelectedCandidatePair(),
+ .last_data_received_ms = selected_connection_->last_data_received(),
+ .reason = IceSwitchReasonToString(reason),
+ .estimated_disconnected_time_ms =
+ old_selected_connection ? ComputeEstimatedDisconnectedTimeMs(
+ TimeMillis(), old_selected_connection)
+ : 0};
+ candidate_pair_change_callback_(pair_change);
}
++selected_candidate_pair_changes_;
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 823e6d6..83d2d1c 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -3727,6 +3727,7 @@
channel_state_ = channel->GetState();
}
void OnCandidatePairChanged(const CandidatePairChangeEvent& event) {
+ RTC_DCHECK(!event.transport_name.empty());
last_candidate_change_event_ = event;
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index bae110f..0d774db 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -146,6 +146,7 @@
};
struct CandidatePairChangeEvent {
+ std::string transport_name;
CandidatePair selected_candidate_pair;
int64_t last_data_received_ms;
std::string reason;
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 6c7e539..5a708b8 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -1302,6 +1302,7 @@
}
void JsepTransportController::OnTransportCandidatePairChanged_n(
const CandidatePairChangeEvent& event) {
+ RTC_DCHECK(!event.transport_name.empty());
signal_ice_candidate_pair_changed_.Send(event);
}
diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn
index f3db81c..07ea51f 100644
--- a/sdk/android/BUILD.gn
+++ b/sdk/android/BUILD.gn
@@ -866,6 +866,7 @@
"../../stats:rtc_stats",
"../../system_wrappers:field_trial",
"//third_party/abseil-cpp/absl/memory",
+ "//third_party/abseil-cpp/absl/strings:string_view",
"//third_party/jni_zero",
]
}
@@ -1018,6 +1019,7 @@
"../../api:array_view",
"../../api:sequence_checker",
"../../rtc_base:checks",
+ "//third_party/abseil-cpp/absl/strings:string_view",
"//third_party/jni_zero",
]
}
diff --git a/sdk/android/native_api/jni/java_types.cc b/sdk/android/native_api/jni/java_types.cc
index 3f7558c..f1abe91 100644
--- a/sdk/android/native_api/jni/java_types.cc
+++ b/sdk/android/native_api/jni/java_types.cc
@@ -211,6 +211,11 @@
return NativeToJavaString(jni, str.c_str());
}
+ScopedJavaLocalRef<jstring> NativeToJavaString(JNIEnv* jni,
+ absl::string_view str) {
+ return NativeToJavaString(jni, str.data());
+}
+
ScopedJavaLocalRef<jobject> NativeToJavaDouble(
JNIEnv* jni,
const std::optional<double>& optional_double) {
diff --git a/sdk/android/native_api/jni/java_types.h b/sdk/android/native_api/jni/java_types.h
index 83bf93d..cf54ef9 100644
--- a/sdk/android/native_api/jni/java_types.h
+++ b/sdk/android/native_api/jni/java_types.h
@@ -26,6 +26,7 @@
#include <string>
#include <vector>
+#include "absl/strings/string_view.h"
#include "api/array_view.h"
#include "api/sequence_checker.h"
#include "rtc_base/checks.h"
@@ -210,6 +211,8 @@
ScopedJavaLocalRef<jstring> NativeToJavaString(JNIEnv* jni, const char* str);
ScopedJavaLocalRef<jstring> NativeToJavaString(JNIEnv* jni,
const std::string& str);
+ScopedJavaLocalRef<jstring> NativeToJavaString(JNIEnv* jni,
+ absl::string_view str);
ScopedJavaLocalRef<jobject> NativeToJavaDouble(
JNIEnv* jni,
diff --git a/sdk/android/src/jni/pc/ice_candidate.cc b/sdk/android/src/jni/pc/ice_candidate.cc
index 356a8eb..e28a4dd 100644
--- a/sdk/android/src/jni/pc/ice_candidate.cc
+++ b/sdk/android/src/jni/pc/ice_candidate.cc
@@ -12,6 +12,7 @@
#include <string>
+#include "absl/strings/string_view.h"
#include "api/jsep.h"
#include "pc/webrtc_sdp.h"
#include "sdk/android/generated_peerconnection_jni/IceCandidate_jni.h"
@@ -25,7 +26,7 @@
namespace {
ScopedJavaLocalRef<jobject> CreateJavaIceCandidate(JNIEnv* env,
- const std::string& sdp_mid,
+ absl::string_view sdp_mid,
int sdp_mline_index,
const std::string& sdp,
const std::string server_url,
@@ -51,11 +52,17 @@
ScopedJavaLocalRef<jobject> NativeToJavaCandidate(JNIEnv* env,
const Candidate& candidate) {
+ return NativeToJavaIceCandidate(env, candidate.transport_name(), candidate);
+}
+
+ScopedJavaLocalRef<jobject> NativeToJavaIceCandidate(
+ JNIEnv* env,
+ absl::string_view mid,
+ const Candidate& candidate) {
std::string sdp = SdpSerializeCandidate(candidate);
RTC_CHECK(!sdp.empty()) << "got an empty ICE candidate";
// sdp_mline_index is not used, pass an invalid value -1.
- return CreateJavaIceCandidate(env, candidate.transport_name(),
- -1 /* sdp_mline_index */, sdp,
+ return CreateJavaIceCandidate(env, mid, -1 /* sdp_mline_index */, sdp,
"" /* server_url */, candidate.network_type());
}
diff --git a/sdk/android/src/jni/pc/ice_candidate.h b/sdk/android/src/jni/pc/ice_candidate.h
index a499ef2..2267f30 100644
--- a/sdk/android/src/jni/pc/ice_candidate.h
+++ b/sdk/android/src/jni/pc/ice_candidate.h
@@ -29,6 +29,10 @@
ScopedJavaLocalRef<jobject> NativeToJavaCandidate(JNIEnv* env,
const Candidate& candidate);
+ScopedJavaLocalRef<jobject> NativeToJavaIceCandidate(
+ JNIEnv* env,
+ absl::string_view mid,
+ const Candidate& candidate);
ScopedJavaLocalRef<jobject> NativeToJavaIceCandidate(
JNIEnv* env,
diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc
index d5c5ba6..4d51c41 100644
--- a/sdk/android/src/jni/pc/peer_connection.cc
+++ b/sdk/android/src/jni/pc/peer_connection.cc
@@ -131,8 +131,11 @@
const CandidatePairChangeEvent& event) {
const auto& selected_pair = event.selected_candidate_pair;
return Java_CandidatePairChangeEvent_Constructor(
- env, NativeToJavaCandidate(env, selected_pair.local_candidate()),
- NativeToJavaCandidate(env, selected_pair.remote_candidate()),
+ env,
+ NativeToJavaIceCandidate(env, event.transport_name,
+ selected_pair.local_candidate()),
+ NativeToJavaIceCandidate(env, event.transport_name,
+ selected_pair.remote_candidate()),
static_cast<int>(event.last_data_received_ms),
NativeToJavaString(env, event.reason),
static_cast<int>(event.estimated_disconnected_time_ms));
diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
index dbac2ab..d092b5f 100644
--- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm
+++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm
@@ -391,16 +391,12 @@
}
const auto &selected_pair = event.selected_candidate_pair;
IceCandidate local_candidate_wrapper(
- selected_pair.local_candidate().transport_name(),
- -1,
- selected_pair.local_candidate());
+ event.transport_name, -1, selected_pair.local_candidate());
RTC_OBJC_TYPE(RTCIceCandidate) *local_candidate =
[[RTC_OBJC_TYPE(RTCIceCandidate) alloc]
initWithNativeCandidate:&local_candidate_wrapper];
IceCandidate remote_candidate_wrapper(
- selected_pair.remote_candidate().transport_name(),
- -1,
- selected_pair.remote_candidate());
+ event.transport_name, -1, selected_pair.remote_candidate());
RTC_OBJC_TYPE(RTCIceCandidate) *remote_candidate =
[[RTC_OBJC_TYPE(RTCIceCandidate) alloc]
initWithNativeCandidate:&remote_candidate_wrapper];
