[Unified Plan] Fix issues with recycling m= sections
Previously, the PeerConnection would look at the pending local
and remote descriptions also to determine if an m= section is
recycled. That is not quite spec compliant and breaks down under
some edge cases. This changes the PeerConnection to look only at
the *current* local or remote description (i.e., the descriptions
from the last time the PeerConnection was in a stable signaling
state) to determine if an m= section is recycled.
Additionally, the MediaSessionFactory only looked at the local
description to determine if an m= section is recycled. The full
criteria requires looking at the current local and current remote
m= sections. This change adds a state enum to the
MediaDescriptionOptions so that the MediaSessionFactory knows if
a media section is being recycled without duplicating the logic
in PeerConnection.
Tests are added to cover additional edge cases.
Bug: chromium:899680
Change-Id: I5bcf0f88957a61653269ed8bb50b2018500bc1d5
Reviewed-on: https://webrtc-review.googlesource.com/c/111293
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25959}diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index e166d1e..c341888 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -3947,6 +3947,17 @@
return media_description_options;
}
+// Returns the ContentInfo at mline index |i|, or null if none exists.
+static const ContentInfo* GetContentByIndex(
+ const SessionDescriptionInterface* sdesc,
+ size_t i) {
+ if (!sdesc) {
+ return nullptr;
+ }
+ const ContentInfos& contents = sdesc->description()->contents();
+ return (i < contents.size() ? &contents[i] : nullptr);
+}
+
void PeerConnection::GetOptionsForUnifiedPlanOffer(
const RTCOfferAnswerOptions& offer_answer_options,
cricket::MediaSessionOptions* session_options) {
@@ -3974,10 +3985,15 @@
// Either |local_content| or |remote_content| is non-null.
const ContentInfo* local_content =
(i < local_contents.size() ? &local_contents[i] : nullptr);
+ const ContentInfo* current_local_content =
+ GetContentByIndex(current_local_description(), i);
const ContentInfo* remote_content =
(i < remote_contents.size() ? &remote_contents[i] : nullptr);
- bool had_been_rejected = (local_content && local_content->rejected) ||
- (remote_content && remote_content->rejected);
+ const ContentInfo* current_remote_content =
+ GetContentByIndex(current_remote_description(), i);
+ bool had_been_rejected =
+ (current_local_content && current_local_content->rejected) ||
+ (current_remote_content && current_remote_content->rejected);
const std::string& mid =
(local_content ? local_content->name : remote_content->name);
cricket::MediaType media_type =
@@ -3988,7 +4004,7 @@
auto transceiver = GetAssociatedTransceiver(mid);
RTC_CHECK(transceiver);
// A media section is considered eligible for recycling if it is marked as
- // rejected in either the local or remote description.
+ // rejected in either the current local or current remote description.
if (had_been_rejected && transceiver->stopped()) {
session_options->media_description_options.push_back(
cricket::MediaDescriptionOptions(transceiver->media_type(), mid,
@@ -4180,12 +4196,13 @@
session_options->media_description_options.push_back(
cricket::MediaDescriptionOptions(
cricket::MEDIA_TYPE_AUDIO, content.name,
- RtpTransceiverDirection::kInactive, true));
+ RtpTransceiverDirection::kInactive, /*stopped=*/true));
} else {
+ bool stopped = (audio_direction == RtpTransceiverDirection::kInactive);
session_options->media_description_options.push_back(
- cricket::MediaDescriptionOptions(
- cricket::MEDIA_TYPE_AUDIO, content.name, audio_direction,
- audio_direction == RtpTransceiverDirection::kInactive));
+ cricket::MediaDescriptionOptions(cricket::MEDIA_TYPE_AUDIO,
+ content.name, audio_direction,
+ stopped));
*audio_index = session_options->media_description_options.size() - 1;
}
} else if (IsVideoContent(&content)) {
@@ -4194,12 +4211,13 @@
session_options->media_description_options.push_back(
cricket::MediaDescriptionOptions(
cricket::MEDIA_TYPE_VIDEO, content.name,
- RtpTransceiverDirection::kInactive, true));
+ RtpTransceiverDirection::kInactive, /*stopped=*/true));
} else {
+ bool stopped = (video_direction == RtpTransceiverDirection::kInactive);
session_options->media_description_options.push_back(
- cricket::MediaDescriptionOptions(
- cricket::MEDIA_TYPE_VIDEO, content.name, video_direction,
- video_direction == RtpTransceiverDirection::kInactive));
+ cricket::MediaDescriptionOptions(cricket::MEDIA_TYPE_VIDEO,
+ content.name, video_direction,
+ stopped));
*video_index = session_options->media_description_options.size() - 1;
}
} else {
