Only serialize non-stopped RTP header extensions
as described in https://w3c.github.io/webrtc-extensions/#modifications-to-existing-procedures-0
"For each RTP header extension "e" listed in
[[HeaderExtensionsToOffer]] where direction is not "stopped", an
"a=extmap" line, as specified in [RFC5285], section 5
This avoids including them in case they are stopped on one
transceiver but not the other. Also, this allows extensions to
be removed from a subsequent offer.
See also
https://github.com/w3c/webrtc-extensions/issues/140
BUG=chromium:1051821
Change-Id: I4d7462f939ce4cd5d8c2331bc038200fe18f70e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291703
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39242}
diff --git a/pc/media_session.cc b/pc/media_session.cc
index e703b44..92fe80f 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -684,8 +684,11 @@
if (extension_with_id.uri == extension.uri) {
// TODO(crbug.com/1051821): Configure the extension direction from
// the information in the media_description_options extension
- // capability.
- extensions.push_back(extension_with_id);
+ // capability. For now, do not include stopped extensions.
+ // See also crbug.com/webrtc/7477 about the general lack of direction.
+ if (extension.direction != RtpTransceiverDirection::kStopped) {
+ extensions.push_back(extension_with_id);
+ }
}
}
}
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index 8f38f6a..6dc6d76 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -2018,7 +2018,7 @@
}
TEST_F(MediaSessionDescriptionFactoryTest,
- AppendsStoppedExtensionIfKnownAndPresentInTheOffer) {
+ AllowsStoppedExtensionsToBeRemovedFromSubsequentOffer) {
MediaSessionOptions opts;
AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video",
RtpTransceiverDirection::kSendRecv, kActive,
@@ -2043,8 +2043,7 @@
ElementsAre(Property(
&ContentInfo::media_description,
Pointee(Property(&MediaContentDescription::rtp_header_extensions,
- ElementsAre(Field(&RtpExtension::uri, "uri1"),
- Field(&RtpExtension::uri, "uri2")))))));
+ ElementsAre(Field(&RtpExtension::uri, "uri1")))))));
}
TEST_F(MediaSessionDescriptionFactoryTest,
diff --git a/pc/peer_connection_header_extension_unittest.cc b/pc/peer_connection_header_extension_unittest.cc
index 1a452b0..9bbc32b 100644
--- a/pc/peer_connection_header_extension_unittest.cc
+++ b/pc/peer_connection_header_extension_unittest.cc
@@ -229,6 +229,68 @@
Field(&RtpHeaderExtensionCapability::uri, "uri3")));
}
+TEST_P(PeerConnectionHeaderExtensionTest, OfferedExtensionsArePerTransceiver) {
+ cricket::MediaType media_type;
+ SdpSemantics semantics;
+ std::tie(media_type, semantics) = GetParam();
+ if (semantics != SdpSemantics::kUnifiedPlan)
+ return;
+ std::unique_ptr<PeerConnectionWrapper> pc1 =
+ CreatePeerConnection(media_type, semantics);
+ auto transceiver1 = pc1->AddTransceiver(media_type);
+ auto modified_extensions = transceiver1->HeaderExtensionsToOffer();
+ modified_extensions[3].direction = RtpTransceiverDirection::kStopped;
+ transceiver1->SetOfferedRtpHeaderExtensions(modified_extensions);
+ auto transceiver2 = pc1->AddTransceiver(media_type);
+
+ auto session_description = pc1->CreateOffer();
+ EXPECT_THAT(session_description->description()
+ ->contents()[0]
+ .media_description()
+ ->rtp_header_extensions(),
+ ElementsAre(Field(&RtpExtension::uri, "uri2"),
+ Field(&RtpExtension::uri, "uri3")));
+ EXPECT_THAT(session_description->description()
+ ->contents()[1]
+ .media_description()
+ ->rtp_header_extensions(),
+ ElementsAre(Field(&RtpExtension::uri, "uri2"),
+ Field(&RtpExtension::uri, "uri3"),
+ Field(&RtpExtension::uri, "uri4")));
+}
+
+TEST_P(PeerConnectionHeaderExtensionTest, RemovalAfterRenegotiation) {
+ cricket::MediaType media_type;
+ SdpSemantics semantics;
+ std::tie(media_type, semantics) = GetParam();
+ if (semantics != SdpSemantics::kUnifiedPlan)
+ return;
+ std::unique_ptr<PeerConnectionWrapper> pc1 =
+ CreatePeerConnection(media_type, semantics);
+ std::unique_ptr<PeerConnectionWrapper> pc2 =
+ CreatePeerConnection(media_type, semantics);
+ auto transceiver1 = pc1->AddTransceiver(media_type);
+
+ auto offer = pc1->CreateOfferAndSetAsLocal(
+ PeerConnectionInterface::RTCOfferAnswerOptions());
+ pc2->SetRemoteDescription(std::move(offer));
+ auto answer = pc2->CreateAnswerAndSetAsLocal(
+ PeerConnectionInterface::RTCOfferAnswerOptions());
+ pc1->SetRemoteDescription(std::move(answer));
+
+ auto modified_extensions = transceiver1->HeaderExtensionsToOffer();
+ modified_extensions[3].direction = RtpTransceiverDirection::kStopped;
+ modified_extensions[3].direction = RtpTransceiverDirection::kStopped;
+ transceiver1->SetOfferedRtpHeaderExtensions(modified_extensions);
+ auto session_description = pc1->CreateOffer();
+ EXPECT_THAT(session_description->description()
+ ->contents()[0]
+ .media_description()
+ ->rtp_header_extensions(),
+ ElementsAre(Field(&RtpExtension::uri, "uri2"),
+ Field(&RtpExtension::uri, "uri3")));
+}
+
INSTANTIATE_TEST_SUITE_P(
,
PeerConnectionHeaderExtensionTest,
