From 51dbe82fedb1af2c74cd1215869bad709d1c0913 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Thu, 2 Feb 2023 13:20:37 +0100 Subject: [PATCH] setOfferedHeaderExtensions: stop any filtered extension addressing feedback from https://github.com/w3c/webrtc-extensions/issues/130 and aligning the behavior with setCodecPreferences. BUG=chromium:1051821 Change-Id: If0c29e1e16781b6898814e2f888ad08a079fc609 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/286780 Reviewed-by: Harald Alvestrand Reviewed-by: Markus Handell Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#39264} --- pc/rtp_transceiver.cc | 22 +++++++++++++++++----- pc/rtp_transceiver_unittest.cc | 6 ------ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 5f9e876b4c..76549d1735 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -677,6 +677,15 @@ RtpTransceiver::HeaderExtensionsNegotiated() const { return result; } +// Helper function to determine mandatory-to-negotiate extensions. +// See https://www.rfc-editor.org/rfc/rfc8834#name-header-extensions +// and https://w3c.github.io/webrtc-extensions/#rtcrtptransceiver-interface +// Since BUNDLE is offered by default, MID is mandatory and can not be turned +// off via this API. +bool IsMandatoryHeaderExtension(const std::string& uri) { + return uri == RtpExtension::kMidUri; +} + RTCError RtpTransceiver::SetOfferedRtpHeaderExtensions( rtc::ArrayView header_extensions_to_offer) { @@ -699,17 +708,20 @@ RTCError RtpTransceiver::SetOfferedRtpHeaderExtensions( } // Step 2.4-2.5. - // - Use of the transceiver interface indicates unified plan is in effect, - // hence the MID extension needs to be enabled. - // - Also handle the mandatory video orientation extensions. - if ((entry.uri == RtpExtension::kMidUri || - entry.uri == RtpExtension::kVideoRotationUri) && + if (IsMandatoryHeaderExtension(entry.uri) && entry.direction != RtpTransceiverDirection::kSendRecv) { return RTCError(RTCErrorType::INVALID_MODIFICATION, "Attempted to stop a mandatory extension."); } } + // Set all current extensions but the mandatory ones to stopped. + // This means that anything filtered from the input will not show up. + for (auto& entry : header_extensions_to_offer_) { + if (!IsMandatoryHeaderExtension(entry.uri)) { + entry.direction = RtpTransceiverDirection::kStopped; + } + } // Apply mutation after error checking. for (const auto& entry : header_extensions_to_offer) { auto it = std::find_if( diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index a2f2c362dd..cdbd5511cb 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc @@ -314,12 +314,6 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_THAT(transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions), Property(&RTCError::type, RTCErrorType::INVALID_MODIFICATION)); EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); - modified_extensions = extensions_; - // Attempting to stop the mandatory video orientation extension. - modified_extensions[3].direction = RtpTransceiverDirection::kStopped; - EXPECT_THAT(transceiver_->SetOfferedRtpHeaderExtensions(modified_extensions), - Property(&RTCError::type, RTCErrorType::INVALID_MODIFICATION)); - EXPECT_EQ(transceiver_->HeaderExtensionsToOffer(), extensions_); } TEST_F(RtpTransceiverTestForHeaderExtensions,