From b396e2b159b060791954495d68278a55e8f72092 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Wed, 22 Mar 2023 17:01:26 +0100 Subject: [PATCH] Revert "Only serialize non-stopped RTP header extensions" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit be03c0971863c9e6807fcbdb4175754e8242a652. Causes regression in web projects that 1/ add a stopped-by-default extension in SRD 2/ call createAnswer 3/ munge the stopped-by-default extension back in SLD 4/ create a subsequent offer and expect the extension to be present BUG=chromium:1051821 Change-Id: If77f77c2c0ac19625f502fb8a07facd151475807 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298744 Commit-Queue: Markus Handell Reviewed-by: Henrik Boström Reviewed-by: Markus Handell Cr-Commit-Position: refs/heads/main@{#39655} --- pc/media_session.cc | 7 +-- pc/media_session_unittest.cc | 5 +- ...er_connection_header_extension_unittest.cc | 62 ------------------- 3 files changed, 5 insertions(+), 69 deletions(-) diff --git a/pc/media_session.cc b/pc/media_session.cc index 92fe80f3d1..e703b44101 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -684,11 +684,8 @@ static bool CreateContentOffer( 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. 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); - } + // capability. + extensions.push_back(extension_with_id); } } } diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 6dc6d76021..8f38f6a1b4 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -2018,7 +2018,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, } TEST_F(MediaSessionDescriptionFactoryTest, - AllowsStoppedExtensionsToBeRemovedFromSubsequentOffer) { + AppendsStoppedExtensionIfKnownAndPresentInTheOffer) { MediaSessionOptions opts; AddMediaDescriptionOptions(MEDIA_TYPE_VIDEO, "video", RtpTransceiverDirection::kSendRecv, kActive, @@ -2043,7 +2043,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, ElementsAre(Property( &ContentInfo::media_description, Pointee(Property(&MediaContentDescription::rtp_header_extensions, - ElementsAre(Field(&RtpExtension::uri, "uri1"))))))); + ElementsAre(Field(&RtpExtension::uri, "uri1"), + Field(&RtpExtension::uri, "uri2"))))))); } TEST_F(MediaSessionDescriptionFactoryTest, diff --git a/pc/peer_connection_header_extension_unittest.cc b/pc/peer_connection_header_extension_unittest.cc index 73a439d8bd..94d4401ca4 100644 --- a/pc/peer_connection_header_extension_unittest.cc +++ b/pc/peer_connection_header_extension_unittest.cc @@ -235,68 +235,6 @@ TEST_P(PeerConnectionHeaderExtensionTest, NegotiatedExtensionsAreAccessible) { RtpTransceiverDirection::kStopped))); } -TEST_P(PeerConnectionHeaderExtensionTest, OfferedExtensionsArePerTransceiver) { - cricket::MediaType media_type; - SdpSemantics semantics; - std::tie(media_type, semantics) = GetParam(); - if (semantics != SdpSemantics::kUnifiedPlan) - return; - std::unique_ptr pc1 = - CreatePeerConnection(media_type, semantics); - auto transceiver1 = pc1->AddTransceiver(media_type); - auto modified_extensions = transceiver1->GetHeaderExtensionsToNegotiate(); - modified_extensions[3].direction = RtpTransceiverDirection::kStopped; - transceiver1->SetHeaderExtensionsToNegotiate(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 pc1 = - CreatePeerConnection(media_type, semantics); - std::unique_ptr 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->GetHeaderExtensionsToNegotiate(); - modified_extensions[3].direction = RtpTransceiverDirection::kStopped; - modified_extensions[3].direction = RtpTransceiverDirection::kStopped; - transceiver1->SetHeaderExtensionsToNegotiate(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"))); -} - TEST_P(PeerConnectionHeaderExtensionTest, StoppedByDefaultExtensionCanBeActivatedByRemoteSdp) { cricket::MediaType media_type;