[PeerConnection::AddIceCandidate()] Use mid to look up contents in remote descriptions
Prior to this CL, only the mline index of an ice candidate was used to
look up contents. However, due to recent changes, it is possible that
no mline index is specified, but that only a mid is specified.
No mline index is indicated with a -1 value.
This CL makes sure the mid is used if no mline index is given.
Bug: chromium:965483
Change-Id: I8962e71acb386f7b50349802f27358ba24c11921
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/138075
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28045}
diff --git a/pc/jsep_session_description.cc b/pc/jsep_session_description.cc
index 90b12a4..d9c1fad 100644
--- a/pc/jsep_session_description.cc
+++ b/pc/jsep_session_description.cc
@@ -207,7 +207,7 @@
bool JsepSessionDescription::AddCandidate(
const IceCandidateInterface* candidate) {
- if (!candidate || candidate->sdp_mline_index() < 0)
+ if (!candidate)
return false;
size_t mediasection_index = 0;
if (!GetMediasectionIndex(candidate, &mediasection_index)) {
@@ -291,7 +291,18 @@
if (!candidate || !index) {
return false;
}
- *index = static_cast<size_t>(candidate->sdp_mline_index());
+
+ // If the candidate has no valid mline index or sdp_mid, it is impossible
+ // to find a match.
+ if (candidate->sdp_mid().empty() &&
+ (candidate->sdp_mline_index() < 0 ||
+ static_cast<size_t>(candidate->sdp_mline_index()) >=
+ description_->contents().size())) {
+ return false;
+ }
+
+ if (candidate->sdp_mline_index() >= 0)
+ *index = static_cast<size_t>(candidate->sdp_mline_index());
if (description_ && !candidate->sdp_mid().empty()) {
bool found = false;
// Try to match the sdp_mid with content name.
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index deaff5d..ca66a09 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -6172,21 +6172,18 @@
}
bool PeerConnection::UseCandidate(const IceCandidateInterface* candidate) {
- size_t mediacontent_index = static_cast<size_t>(candidate->sdp_mline_index());
- size_t remote_content_size =
- remote_description()->description()->contents().size();
- if (mediacontent_index >= remote_content_size) {
- RTC_LOG(LS_ERROR) << "UseCandidate: Invalid candidate media index.";
+ RTCErrorOr<const cricket::ContentInfo*> result =
+ FindContentInfo(remote_description(), candidate);
+ if (!result.ok()) {
+ RTC_LOG(LS_ERROR) << "UseCandidate: Invalid candidate. "
+ << result.error().message();
return false;
}
-
- cricket::ContentInfo content =
- remote_description()->description()->contents()[mediacontent_index];
std::vector<cricket::Candidate> candidates;
candidates.push_back(candidate->candidate());
// Invoking BaseSession method to handle remote candidates.
- RTCError error =
- transport_controller_->AddRemoteCandidates(content.name, candidates);
+ RTCError error = transport_controller_->AddRemoteCandidates(
+ result.value()->name, candidates);
if (error.ok()) {
// Candidates successfully submitted for checking.
if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew ||
@@ -6209,6 +6206,42 @@
return true;
}
+RTCErrorOr<const cricket::ContentInfo*> PeerConnection::FindContentInfo(
+ const SessionDescriptionInterface* description,
+ const IceCandidateInterface* candidate) {
+ if (candidate->sdp_mline_index() >= 0) {
+ size_t mediacontent_index =
+ static_cast<size_t>(candidate->sdp_mline_index());
+ size_t content_size = description->description()->contents().size();
+ if (mediacontent_index < content_size) {
+ return &description->description()->contents()[mediacontent_index];
+ } else {
+ return RTCError(RTCErrorType::INVALID_RANGE,
+ "Media line index (" +
+ rtc::ToString(candidate->sdp_mline_index()) +
+ ") out of range (number of mlines: " +
+ rtc::ToString(content_size) + ").");
+ }
+ } else if (!candidate->sdp_mid().empty()) {
+ auto& contents = description->description()->contents();
+ auto it = absl::c_find_if(
+ contents, [candidate](const cricket::ContentInfo& content_info) {
+ return content_info.mid() == candidate->sdp_mid();
+ });
+ if (it == contents.end()) {
+ return RTCError(
+ RTCErrorType::INVALID_PARAMETER,
+ "Mid " + candidate->sdp_mid() +
+ " specified but no media section with that mid found.");
+ } else {
+ return &*it;
+ }
+ }
+
+ return RTCError(RTCErrorType::INVALID_PARAMETER,
+ "Neither sdp_mline_index nor sdp_mid specified.");
+}
+
void PeerConnection::RemoveUnusedChannels(const SessionDescription* desc) {
// Destroy video channel first since it may have a pointer to the
// voice channel.
@@ -6838,26 +6871,18 @@
return false;
}
- size_t mediacontent_index = static_cast<size_t>(candidate->sdp_mline_index());
- size_t remote_content_size =
- current_remote_desc->description()->contents().size();
- if (mediacontent_index >= remote_content_size) {
- RTC_LOG(LS_ERROR)
- << "ReadyToUseRemoteCandidate: Invalid candidate media index "
- << mediacontent_index;
+ RTCErrorOr<const cricket::ContentInfo*> result =
+ FindContentInfo(current_remote_desc, candidate);
+ if (!result.ok()) {
+ RTC_LOG(LS_ERROR) << "ReadyToUseRemoteCandidate: Invalid candidate. "
+ << result.error().message();
*valid = false;
return false;
}
- cricket::ContentInfo content =
- current_remote_desc->description()->contents()[mediacontent_index];
-
- const std::string transport_name = GetTransportName(content.name);
- if (transport_name.empty()) {
- return false;
- }
- return true;
+ std::string transport_name = GetTransportName(result.value()->name);
+ return !transport_name.empty();
}
bool PeerConnection::SrtpRequired() const {
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 86fc1a8..d9c625c 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -911,6 +911,9 @@
// Uses |candidate| in this session.
bool UseCandidate(const IceCandidateInterface* candidate)
RTC_RUN_ON(signaling_thread());
+ RTCErrorOr<const cricket::ContentInfo*> FindContentInfo(
+ const SessionDescriptionInterface* description,
+ const IceCandidateInterface* candidate) RTC_RUN_ON(signaling_thread());
// Deletes the corresponding channel of contents that don't exist in |desc|.
// |desc| can be null. This means that all channels are deleted.
void RemoveUnusedChannels(const cricket::SessionDescription* desc)
diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc
index 03c1128..afc8594 100644
--- a/pc/peer_connection_ice_unittest.cc
+++ b/pc/peer_connection_ice_unittest.cc
@@ -58,7 +58,7 @@
const auto& first_content = desc->contents()[0];
candidate->set_transport_name(first_content.name);
std::unique_ptr<IceCandidateInterface> jsep_candidate =
- CreateIceCandidate(first_content.name, 0, *candidate);
+ CreateIceCandidate(first_content.name, -1, *candidate);
return pc()->AddIceCandidate(jsep_candidate.get());
}
