Integrate RTP Header extension API with SDP munging
in order to not regress existing use-cases while following rules
described by the specification. This change now makes the existing
regression test pass after the spec-compliant modifications.
BUG=chromium:1051821
Change-Id: Ia384adf9a172ed88b5ec6a3cc5c478764a686cb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299002
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39726}
diff --git a/pc/media_session.cc b/pc/media_session.cc
index e703b44..8aff4d9 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -685,7 +685,9 @@
// TODO(crbug.com/1051821): Configure the extension direction from
// the information in the media_description_options extension
// capability.
- extensions.push_back(extension_with_id);
+ 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..6ba5032 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,
@@ -2026,12 +2026,12 @@
opts.media_description_options.back().header_extensions = {
webrtc::RtpHeaderExtensionCapability("uri1", 1,
RtpTransceiverDirection::kSendRecv),
- webrtc::RtpHeaderExtensionCapability("uri2", 1,
+ webrtc::RtpHeaderExtensionCapability("uri2", 2,
RtpTransceiverDirection::kSendRecv)};
auto offer = f1_.CreateOffer(opts, nullptr);
- // Now add "uri2" as stopped to the options verify that the offer contains
- // uri2 since it's already present since before.
+ // Check that a subsequent offer after setting "uri2" to stopped no longer
+ // contains the extension.
opts.media_description_options.back().header_extensions = {
webrtc::RtpHeaderExtensionCapability("uri1", 1,
RtpTransceiverDirection::kSendRecv),
@@ -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 42c58fd..405b7e0 100644
--- a/pc/peer_connection_header_extension_unittest.cc
+++ b/pc/peer_connection_header_extension_unittest.cc
@@ -321,11 +321,11 @@
}
}
-// This test is a regression test for behavior that the API
+// These tests are regression tests for behavior that the API
// enables in a proper way. It conflicts with the behavior
// of the API to only offer non-stopped extensions.
TEST_P(PeerConnectionHeaderExtensionTest,
- SdpMungingWithoutApiUsageEnablesExtensions) {
+ SdpMungingAnswerWithoutApiUsageEnablesExtensions) {
cricket::MediaType media_type;
SdpSemantics semantics;
std::tie(media_type, semantics) = GetParam();
@@ -371,7 +371,6 @@
ASSERT_TRUE(pc->SetLocalDescription(std::move(modified_answer)));
auto session_description = pc->CreateOffer();
- ASSERT_TRUE(session_description->ToString(&sdp));
EXPECT_THAT(session_description->description()
->contents()[0]
.media_description()
@@ -382,6 +381,37 @@
Field(&RtpExtension::uri, "uri4")));
}
+TEST_P(PeerConnectionHeaderExtensionTest,
+ SdpMungingOfferWithoutApiUsageEnablesExtensions) {
+ cricket::MediaType media_type;
+ SdpSemantics semantics;
+ std::tie(media_type, semantics) = GetParam();
+ if (semantics != SdpSemantics::kUnifiedPlan)
+ return;
+ std::unique_ptr<PeerConnectionWrapper> pc =
+ CreatePeerConnection(media_type, semantics);
+ pc->AddTransceiver(media_type);
+
+ auto offer =
+ pc->CreateOffer(PeerConnectionInterface::RTCOfferAnswerOptions());
+ std::string modified_sdp;
+ ASSERT_TRUE(offer->ToString(&modified_sdp));
+ modified_sdp += "a=extmap:1 uri1\r\n";
+ auto modified_offer = CreateSessionDescription(SdpType::kOffer, modified_sdp);
+ ASSERT_TRUE(pc->SetLocalDescription(std::move(modified_offer)));
+
+ auto offer2 =
+ pc->CreateOffer(PeerConnectionInterface::RTCOfferAnswerOptions());
+ EXPECT_THAT(offer2->description()
+ ->contents()[0]
+ .media_description()
+ ->rtp_header_extensions(),
+ ElementsAre(Field(&RtpExtension::uri, "uri2"),
+ Field(&RtpExtension::uri, "uri3"),
+ Field(&RtpExtension::uri, "uri4"),
+ Field(&RtpExtension::uri, "uri1")));
+}
+
TEST_P(PeerConnectionHeaderExtensionTest, EnablingExtensionsAfterRemoteOffer) {
cricket::MediaType media_type;
SdpSemantics semantics;
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 9c0e7eb..c382d61 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -815,8 +815,37 @@
} // namespace
+void UpdateRtpHeaderExtensionPreferencesFromSdpMunging(
+ const cricket::SessionDescription* description,
+ TransceiverList* transceivers) {
+ // This integrates the RTP Header Extension Control API and local SDP munging
+ // for backward compability reasons. If something was enabled in the local
+ // description via SDP munging, consider it non-stopped in the API as well
+ // so that is shows up in subsequent offers/answers.
+ RTC_DCHECK(description);
+ RTC_DCHECK(transceivers);
+ for (const auto& content : description->contents()) {
+ auto transceiver = transceivers->FindByMid(content.name);
+ if (!transceiver) {
+ continue;
+ }
+ auto extension_capabilities = transceiver->GetHeaderExtensionsToNegotiate();
+ // Set the capability of every extension we see here to "sendrecv".
+ for (auto& ext : content.media_description()->rtp_header_extensions()) {
+ auto it = absl::c_find_if(extension_capabilities,
+ [&ext](const RtpHeaderExtensionCapability c) {
+ return ext.uri == c.uri;
+ });
+ if (it != extension_capabilities.end()) {
+ it->direction = RtpTransceiverDirection::kSendRecv;
+ }
+ }
+ transceiver->SetHeaderExtensionsToNegotiate(extension_capabilities);
+ }
+}
+
// This class stores state related to a SetRemoteDescription operation, captures
-// and reports potential errors that migth occur and makes sure to notify the
+// and reports potential errors that might occur and makes sure to notify the
// observer of the operation and the operations chain of completion.
class SdpOfferAnswerHandler::RemoteDescriptionOperation {
public:
@@ -1816,6 +1845,11 @@
local_ice_credentials_to_replace_->ClearIceCredentials();
}
+ if (IsUnifiedPlan()) {
+ UpdateRtpHeaderExtensionPreferencesFromSdpMunging(
+ local_description()->description(), transceivers());
+ }
+
return RTCError::OK();
}
