From 016bd7514df7cd77be9cbd16a10711e87141fa05 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Mon, 20 Mar 2023 15:40:45 +0100 Subject: [PATCH] Make GetNegotiatedHeaderExtensions return all header extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit so the size and order corresponds to the local capabilities. The direction may differ. BUG=chromium:1051821 Change-Id: Icf5312237b8ed137f822c9f7dd35f70a01d2df99 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298043 Reviewed-by: Harald Alvestrand Reviewed-by: Henrik Boström Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#39623} --- ...er_connection_header_extension_unittest.cc | 89 ++++++++++++++++++- pc/rtp_transceiver.cc | 15 +++- pc/rtp_transceiver_unittest.cc | 60 +++++++++---- 3 files changed, 144 insertions(+), 20 deletions(-) diff --git a/pc/peer_connection_header_extension_unittest.cc b/pc/peer_connection_header_extension_unittest.cc index 9db547fb57..73a439d8bd 100644 --- a/pc/peer_connection_header_extension_unittest.cc +++ b/pc/peer_connection_header_extension_unittest.cc @@ -225,8 +225,14 @@ TEST_P(PeerConnectionHeaderExtensionTest, NegotiatedExtensionsAreAccessible) { // PC1 has exts 2-4 unstopped and PC2 has exts 1-3 unstopped -> ext 2, 3 // survives. EXPECT_THAT(transceiver1->GetNegotiatedHeaderExtensions(), - ElementsAre(Field(&RtpHeaderExtensionCapability::uri, "uri2"), - Field(&RtpHeaderExtensionCapability::uri, "uri3"))); + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); } TEST_P(PeerConnectionHeaderExtensionTest, OfferedExtensionsArePerTransceiver) { @@ -291,6 +297,85 @@ TEST_P(PeerConnectionHeaderExtensionTest, RemovalAfterRenegotiation) { Field(&RtpExtension::uri, "uri3"))); } +TEST_P(PeerConnectionHeaderExtensionTest, + StoppedByDefaultExtensionCanBeActivatedByRemoteSdp) { + 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()); + std::string sdp; + ASSERT_TRUE(answer->ToString(&sdp)); + // We support uri1 but it is stopped by default. Let the remote reactivate it. + sdp += "a=extmap:15 uri1\r\n"; + auto modified_answer = CreateSessionDescription(SdpType::kAnswer, sdp); + pc1->SetRemoteDescription(std::move(modified_answer)); + EXPECT_THAT(transceiver1->GetNegotiatedHeaderExtensions(), + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv))); +} + +TEST_P(PeerConnectionHeaderExtensionTest, + UnknownExtensionInRemoteOfferDoesNotShowUp) { + cricket::MediaType media_type; + SdpSemantics semantics; + std::tie(media_type, semantics) = GetParam(); + if (semantics != SdpSemantics::kUnifiedPlan) + return; + std::unique_ptr pc = + CreatePeerConnection(media_type, semantics); + std::string sdp = + "v=0\r\n" + "o=- 0 3 IN IP4 127.0.0.1\r\n" + "s=-\r\n" + "t=0 0\r\n" + "a=fingerprint:sha-256 " + "A7:24:72:CA:6E:02:55:39:BA:66:DF:6E:CC:4C:D8:B0:1A:BF:1A:56:65:7D:F4:03:" + "AD:7E:77:43:2A:29:EC:93\r\n" + "a=ice-ufrag:6HHHdzzeIhkE0CKj\r\n" + "a=ice-pwd:XYDGVpfvklQIEnZ6YnyLsAew\r\n" + "m=audio 9 RTP/AVPF 111\r\n" + "c=IN IP4 0.0.0.0\r\n" + "a=rtcp-mux\r\n" + "a=sendonly\r\n" + "a=mid:audio\r\n" + "a=rtpmap:111 fake_audio_codec/0\r\n" + "a=setup:actpass\r\n" + "a=extmap:1 urn:bogus\r\n"; + RTC_LOG(LS_ERROR) << sdp; + auto offer = CreateSessionDescription(SdpType::kOffer, sdp); + pc->SetRemoteDescription(std::move(offer)); + pc->CreateAnswerAndSetAsLocal( + PeerConnectionInterface::RTCOfferAnswerOptions()); + ASSERT_GT(pc->pc()->GetTransceivers().size(), 0u); + auto transceiver = pc->pc()->GetTransceivers()[0]; + auto negotiated = transceiver->GetNegotiatedHeaderExtensions(); + EXPECT_EQ(negotiated.size(), + transceiver->GetHeaderExtensionsToNegotiate().size()); + // All extensions are stopped, the "bogus" one does not show up. + for (const auto& extension : negotiated) { + EXPECT_EQ(extension.direction, RtpTransceiverDirection::kStopped); + EXPECT_NE(extension.uri, "urn:bogus"); + } +} + INSTANTIATE_TEST_SUITE_P( , PeerConnectionHeaderExtensionTest, diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index b8fc370f69..30a16256a5 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -728,8 +728,19 @@ std::vector RtpTransceiver::GetNegotiatedHeaderExtensions() const { RTC_DCHECK_RUN_ON(thread_); std::vector result; - for (const auto& ext : negotiated_header_extensions_) { - result.emplace_back(ext.uri, ext.id, RtpTransceiverDirection::kSendRecv); + result.reserve(header_extensions_to_negotiate_.size()); + for (const auto& ext : header_extensions_to_negotiate_) { + auto negotiated = absl::c_find_if(negotiated_header_extensions_, + [&ext](const RtpExtension& negotiated) { + return negotiated.uri == ext.uri; + }); + RtpHeaderExtensionCapability capability(ext.uri); + // TODO(bugs.webrtc.org/7477): extend when header extensions support + // direction. + capability.direction = negotiated != negotiated_header_extensions_.end() + ? RtpTransceiverDirection::kSendRecv + : RtpTransceiverDirection::kStopped; + result.push_back(capability); } return result; } diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index 0ada470d11..9e47fab80f 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc @@ -30,6 +30,7 @@ using ::testing::_; using ::testing::ElementsAre; +using ::testing::Field; using ::testing::Optional; using ::testing::Property; using ::testing::Return; @@ -357,7 +358,15 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_CALL(*receiver_.get(), SetMediaChannel(_)); EXPECT_CALL(*sender_.get(), SetTransceiverAsStopped()); EXPECT_CALL(*sender_.get(), Stop()); - EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), ElementsAre()); + EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); } TEST_F(RtpTransceiverTestForHeaderExtensions, @@ -379,7 +388,15 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_CALL(*mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); transceiver_->SetChannel(std::move(mock_channel), [](const std::string&) { return nullptr; }); - EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), ElementsAre()); + EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); EXPECT_CALL(*mock_channel_ptr, SetFirstPacketReceivedCallback(_)); ClearChannel(); @@ -411,12 +428,16 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, ReturnsNegotiatedHdrExts) { transceiver_->SetChannel(std::move(mock_channel), [](const std::string&) { return nullptr; }); - EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), - ElementsAre(RtpHeaderExtensionCapability( - "uri1", 1, RtpTransceiverDirection::kSendRecv), - RtpHeaderExtensionCapability( - "uri2", 2, RtpTransceiverDirection::kSendRecv))); + EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); EXPECT_CALL(*mock_channel_ptr, SetFirstPacketReceivedCallback(_)); ClearChannel(); } @@ -435,21 +456,28 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, transceiver_->OnNegotiationUpdate(SdpType::kAnswer, &description); EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), - ElementsAre(RtpHeaderExtensionCapability( - "uri1", 1, RtpTransceiverDirection::kSendRecv), - RtpHeaderExtensionCapability( - "uri2", 2, RtpTransceiverDirection::kSendRecv))); - + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kSendRecv), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); extensions = {webrtc::RtpExtension("uri3", 4), webrtc::RtpExtension("uri5", 6)}; description.set_rtp_header_extensions(extensions); transceiver_->OnNegotiationUpdate(SdpType::kAnswer, &description); EXPECT_THAT(transceiver_->GetNegotiatedHeaderExtensions(), - ElementsAre(RtpHeaderExtensionCapability( - "uri3", 4, RtpTransceiverDirection::kSendRecv), - RtpHeaderExtensionCapability( - "uri5", 6, RtpTransceiverDirection::kSendRecv))); + ElementsAre(Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped), + Field(&RtpHeaderExtensionCapability::direction, + RtpTransceiverDirection::kStopped))); } } // namespace