diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 278a95ee87..6864dfc45d 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -38,16 +38,12 @@ JsepTransportDescription::JsepTransportDescription( const std::vector& cryptos, const std::vector& encrypted_header_extension_ids, int rtp_abs_sendtime_extn_id, - const TransportDescription& transport_desc, - absl::optional media_alt_protocol, - absl::optional data_alt_protocol) + const TransportDescription& transport_desc) : rtcp_mux_enabled(rtcp_mux_enabled), cryptos(cryptos), encrypted_header_extension_ids(encrypted_header_extension_ids), rtp_abs_sendtime_extn_id(rtp_abs_sendtime_extn_id), - transport_desc(transport_desc), - media_alt_protocol(media_alt_protocol), - data_alt_protocol(data_alt_protocol) {} + transport_desc(transport_desc) {} JsepTransportDescription::JsepTransportDescription( const JsepTransportDescription& from) @@ -55,9 +51,7 @@ JsepTransportDescription::JsepTransportDescription( cryptos(from.cryptos), encrypted_header_extension_ids(from.encrypted_header_extension_ids), rtp_abs_sendtime_extn_id(from.rtp_abs_sendtime_extn_id), - transport_desc(from.transport_desc), - media_alt_protocol(from.media_alt_protocol), - data_alt_protocol(from.data_alt_protocol) {} + transport_desc(from.transport_desc) {} JsepTransportDescription::~JsepTransportDescription() = default; @@ -71,8 +65,6 @@ JsepTransportDescription& JsepTransportDescription::operator=( encrypted_header_extension_ids = from.encrypted_header_extension_ids; rtp_abs_sendtime_extn_id = from.rtp_abs_sendtime_extn_id; transport_desc = from.transport_desc; - media_alt_protocol = from.media_alt_protocol; - data_alt_protocol = from.data_alt_protocol; return *this; } diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index 8116ebf53e..27c30a5202 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -53,9 +53,7 @@ struct JsepTransportDescription { const std::vector& cryptos, const std::vector& encrypted_header_extension_ids, int rtp_abs_sendtime_extn_id, - const TransportDescription& transport_description, - absl::optional media_alt_protocol, - absl::optional data_alt_protocol); + const TransportDescription& transport_description); JsepTransportDescription(const JsepTransportDescription& from); ~JsepTransportDescription(); @@ -68,14 +66,6 @@ struct JsepTransportDescription { // TODO(zhihuang): Add the ICE and DTLS related variables and methods from // TransportDescription and remove this extra layer of abstraction. TransportDescription transport_desc; - - // Alt-protocols that apply to this JsepTransport. Presence indicates a - // request to use an alternative protocol for media and/or data. The - // alt-protocol is handled by a datagram transport. If one or both of these - // values are present, JsepTransport will attempt to negotiate use of the - // datagram transport for media and/or data. - absl::optional media_alt_protocol; - absl::optional data_alt_protocol; }; // Helper class used by JsepTransportController that processes diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 5c249e013c..dbe7435c02 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -576,16 +576,9 @@ RTCError JsepTransportController::ApplyDescription_n( } std::vector merged_encrypted_extension_ids; - absl::optional bundle_media_alt_protocol; - absl::optional bundle_data_alt_protocol; if (bundle_group_) { merged_encrypted_extension_ids = MergeEncryptedHeaderExtensionIdsForBundle(description); - error = GetAltProtocolsForBundle(description, &bundle_media_alt_protocol, - &bundle_data_alt_protocol); - if (!error.ok()) { - return error; - } } for (const cricket::ContentInfo& content_info : description->contents()) { @@ -604,8 +597,6 @@ RTCError JsepTransportController::ApplyDescription_n( description->transport_infos().size()); for (size_t i = 0; i < description->contents().size(); ++i) { const cricket::ContentInfo& content_info = description->contents()[i]; - const cricket::MediaContentDescription* media_description = - content_info.media_description(); const cricket::TransportInfo& transport_info = description->transport_infos()[i]; if (content_info.rejected) { @@ -628,23 +619,10 @@ RTCError JsepTransportController::ApplyDescription_n( } std::vector extension_ids; - absl::optional media_alt_protocol; - absl::optional data_alt_protocol; if (bundled_mid() && content_info.name == *bundled_mid()) { extension_ids = merged_encrypted_extension_ids; - media_alt_protocol = bundle_media_alt_protocol; - data_alt_protocol = bundle_data_alt_protocol; } else { extension_ids = GetEncryptedHeaderExtensionIds(content_info); - switch (media_description->type()) { - case cricket::MEDIA_TYPE_AUDIO: - case cricket::MEDIA_TYPE_VIDEO: - media_alt_protocol = media_description->alt_protocol(); - break; - case cricket::MEDIA_TYPE_DATA: - data_alt_protocol = media_description->alt_protocol(); - break; - } } int rtp_abs_sendtime_extn_id = @@ -658,8 +636,7 @@ RTCError JsepTransportController::ApplyDescription_n( cricket::JsepTransportDescription jsep_description = CreateJsepTransportDescription(content_info, transport_info, - extension_ids, rtp_abs_sendtime_extn_id, - media_alt_protocol, data_alt_protocol); + extension_ids, rtp_abs_sendtime_extn_id); if (local) { error = transport->SetLocalJsepTransportDescription(jsep_description, type); @@ -860,9 +837,7 @@ JsepTransportController::CreateJsepTransportDescription( const cricket::ContentInfo& content_info, const cricket::TransportInfo& transport_info, const std::vector& encrypted_extension_ids, - int rtp_abs_sendtime_extn_id, - absl::optional media_alt_protocol, - absl::optional data_alt_protocol) { + int rtp_abs_sendtime_extn_id) { const cricket::MediaContentDescription* content_desc = content_info.media_description(); RTC_DCHECK(content_desc); @@ -872,8 +847,7 @@ JsepTransportController::CreateJsepTransportDescription( return cricket::JsepTransportDescription( rtcp_mux_enabled, content_desc->cryptos(), encrypted_extension_ids, - rtp_abs_sendtime_extn_id, transport_info.description, media_alt_protocol, - data_alt_protocol); + rtp_abs_sendtime_extn_id, transport_info.description); } bool JsepTransportController::ShouldUpdateBundleGroup( @@ -939,55 +913,6 @@ JsepTransportController::MergeEncryptedHeaderExtensionIdsForBundle( return merged_ids; } -RTCError JsepTransportController::GetAltProtocolsForBundle( - const cricket::SessionDescription* description, - absl::optional* media_alt_protocol, - absl::optional* data_alt_protocol) { - RTC_DCHECK(description); - RTC_DCHECK(bundle_group_); - RTC_DCHECK(media_alt_protocol); - RTC_DCHECK(data_alt_protocol); - - bool found_media = false; - bool found_data = false; - for (const cricket::ContentInfo& content : description->contents()) { - if (bundle_group_->HasContentName(content.name)) { - const cricket::MediaContentDescription* media_description = - content.media_description(); - switch (media_description->type()) { - case cricket::MEDIA_TYPE_AUDIO: - case cricket::MEDIA_TYPE_VIDEO: - if (found_media && - *media_alt_protocol != media_description->alt_protocol()) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "The BUNDLE group contains conflicting " - "alt-protocols for media ('" + - media_alt_protocol->value_or("") + "' and '" + - media_description->alt_protocol().value_or("") + - "')"); - } - found_media = true; - *media_alt_protocol = media_description->alt_protocol(); - break; - case cricket::MEDIA_TYPE_DATA: - if (found_data && - *data_alt_protocol != media_description->alt_protocol()) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "The BUNDLE group contains conflicting " - "alt-protocols for data ('" + - data_alt_protocol->value_or("") + "' and '" + - media_description->alt_protocol().value_or("") + - "')"); - } - found_data = true; - *data_alt_protocol = media_description->alt_protocol(); - break; - } - } - } - return RTCError::OK(); -} - int JsepTransportController::GetRtpAbsSendTimeHeaderExtensionId( const cricket::ContentInfo& content_info) { if (!config_.enable_external_auth) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 5335abccdc..250501fd05 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -246,9 +246,7 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::ContentInfo& content_info, const cricket::TransportInfo& transport_info, const std::vector& encrypted_extension_ids, - int rtp_abs_sendtime_extn_id, - absl::optional media_alt_protocol, - absl::optional data_alt_protocol); + int rtp_abs_sendtime_extn_id); absl::optional bundled_mid() const { absl::optional bundled_mid; @@ -270,12 +268,6 @@ class JsepTransportController : public sigslot::has_slots<> { std::vector GetEncryptedHeaderExtensionIds( const cricket::ContentInfo& content_info); - // Extracts the alt-protocol settings that apply to the bundle group. - RTCError GetAltProtocolsForBundle( - const cricket::SessionDescription* description, - absl::optional* media_alt_protocol, - absl::optional* data_alt_protocol); - int GetRtpAbsSendTimeHeaderExtensionId( const cricket::ContentInfo& content_info); diff --git a/pc/media_session.cc b/pc/media_session.cc index 45af823d93..38e74fb2db 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -645,8 +645,6 @@ static bool CreateContentOffer( } } - offer->set_alt_protocol(media_description_options.alt_protocol); - if (secure_policy == SEC_REQUIRED && offer->cryptos().empty()) { return false; } @@ -1202,9 +1200,6 @@ static bool CreateMediaContentAnswer( answer->set_direction(NegotiateRtpTransceiverDirection( offer->direction(), media_description_options.direction)); - if (offer->alt_protocol() == media_description_options.alt_protocol) { - answer->set_alt_protocol(media_description_options.alt_protocol); - } return true; } diff --git a/pc/media_session.h b/pc/media_session.h index ef83834318..1c4b61858a 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -78,7 +78,6 @@ struct MediaDescriptionOptions { // stream information goes in the local descriptions. std::vector sender_options; std::vector codec_preferences; - absl::optional alt_protocol; private: // Doesn't DCHECK on |type|. diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index d9ad3795ea..ab3fc84031 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -3624,124 +3624,6 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfo(false, options, true); } -TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolAddedToOffer) { - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - AddDataSection(cricket::DCT_RTP, RtpTransceiverDirection::kRecvOnly, - &options); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz"; - - std::unique_ptr offer = f1_.CreateOffer(options, nullptr); - - EXPECT_EQ(offer->GetContentDescriptionByName("audio")->alt_protocol(), "foo"); - EXPECT_EQ(offer->GetContentDescriptionByName("video")->alt_protocol(), "bar"); - EXPECT_EQ(offer->GetContentDescriptionByName("data")->alt_protocol(), "baz"); -} - -TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolAddedToAnswer) { - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly, - &options); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz"; - - std::unique_ptr offer = f1_.CreateOffer(options, nullptr); - std::unique_ptr answer = - f1_.CreateAnswer(offer.get(), options, nullptr); - - EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(), - "foo"); - EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(), - "bar"); - EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(), "baz"); -} - -TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolNotInOffer) { - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly, - &options); - - std::unique_ptr offer = f1_.CreateOffer(options, nullptr); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz"; - - std::unique_ptr answer = - f1_.CreateAnswer(offer.get(), options, nullptr); - - EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(), - absl::nullopt); -} - -TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolDifferentInOffer) { - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly, - &options); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "not-foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "not-bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "not-baz"; - - std::unique_ptr offer = f1_.CreateOffer(options, nullptr); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz"; - - std::unique_ptr answer = - f1_.CreateAnswer(offer.get(), options, nullptr); - - EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(), - absl::nullopt); -} - -TEST_F(MediaSessionDescriptionFactoryTest, AltProtocolNotInAnswer) { - MediaSessionOptions options; - AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); - AddDataSection(cricket::DCT_SCTP, RtpTransceiverDirection::kRecvOnly, - &options); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = "foo"; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = "bar"; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = "baz"; - - std::unique_ptr offer = f1_.CreateOffer(options, nullptr); - - FindFirstMediaDescriptionByMid("audio", &options)->alt_protocol = - absl::nullopt; - FindFirstMediaDescriptionByMid("video", &options)->alt_protocol = - absl::nullopt; - FindFirstMediaDescriptionByMid("data", &options)->alt_protocol = - absl::nullopt; - - std::unique_ptr answer = - f1_.CreateAnswer(offer.get(), options, nullptr); - - EXPECT_EQ(answer->GetContentDescriptionByName("audio")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("video")->alt_protocol(), - absl::nullopt); - EXPECT_EQ(answer->GetContentDescriptionByName("data")->alt_protocol(), - absl::nullopt); -} - // Create an offer with bundle enabled and verify the crypto parameters are // the common set of the available cryptos. TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoWithOfferBundle) { diff --git a/pc/session_description.h b/pc/session_description.h index bfd19b8c7a..27142e1c72 100644 --- a/pc/session_description.h +++ b/pc/session_description.h @@ -253,13 +253,6 @@ class MediaContentDescription { receive_rids_ = rids; } - virtual const absl::optional& alt_protocol() const { - return alt_protocol_; - } - virtual void set_alt_protocol(const absl::optional& protocol) { - alt_protocol_ = protocol; - } - protected: bool rtcp_mux_ = false; bool rtcp_reduced_size_ = false; @@ -282,8 +275,6 @@ class MediaContentDescription { SimulcastDescription simulcast_; std::vector receive_rids_; - absl::optional alt_protocol_; - private: // Copy function that returns a raw pointer. Caller will assert ownership. // Should only be called by the Clone() function. Must be implemented diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 6d65309769..90a00c1479 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -229,9 +229,6 @@ static const char kApplicationSpecificMaximum[] = "AS"; static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel"; -// This is a non-standardized setting for plugin transports. -static const char kAltProtocolLine[] = "x-alt-protocol"; - // RTP payload type is in the 0-127 range. Use -1 to indicate "all" payload // types. const int kWildcardPayloadType = -1; @@ -520,14 +517,6 @@ static void InitAttrLine(const std::string& attribute, rtc::StringBuilder* os) { InitLine(kLineTypeAttributes, attribute, os); } -static void AddAltProtocolLine(const std::string& protocol, - std::string* message) { - rtc::StringBuilder os; - InitAttrLine(kAltProtocolLine, &os); - os << kSdpDelimiterColon << protocol; - AddLine(os.str(), message); -} - // Writes a SDP attribute line based on |attribute| and |value| to |message|. static void AddAttributeLine(const std::string& attribute, int value, @@ -1520,10 +1509,6 @@ void BuildMediaDescription(const ContentInfo* content_info, } } - if (media_desc->alt_protocol()) { - AddAltProtocolLine(*media_desc->alt_protocol(), message); - } - // RFC 3388 // mid-attribute = "a=mid:" identification-tag // identification-tag = token @@ -2086,12 +2071,6 @@ bool ParseConnectionData(const std::string& line, return true; } -bool ParseAltProtocolLine(const std::string& line, - std::string* protocol, - SdpParseError* error) { - return GetValue(line, kAltProtocolLine, protocol, error); -} - bool ParseSessionDescription(const std::string& message, size_t* pos, std::string* session_id, @@ -3098,12 +3077,6 @@ bool ParseContent(const std::string& message, if (!ParseIceOptions(line, &transport->transport_options, error)) { return false; } - } else if (HasAttribute(line, kAltProtocolLine)) { - std::string alt_protocol; - if (!ParseAltProtocolLine(line, &alt_protocol, error)) { - return false; - } - media_desc->set_alt_protocol(alt_protocol); } else if (HasAttribute(line, kAttributeFmtp)) { if (!ParseFmtpAttributes(line, media_type, media_desc, error)) { return false; diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 41bf18cbc9..4279de67ee 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -1528,8 +1528,6 @@ class WebRtcSdpTest : public ::testing::Test { CompareSimulcastDescription( c1.media_description()->simulcast_description(), c2.media_description()->simulcast_description()); - EXPECT_EQ(c1.media_description()->alt_protocol(), - c2.media_description()->alt_protocol()); } // group @@ -1677,14 +1675,6 @@ class WebRtcSdpTest : public ::testing::Test { desc_.AddTransportInfo(transport_info); } - void AddAltProtocol(const std::string& content_name, - const std::string& alt_protocol) { - ASSERT_TRUE(desc_.GetTransportInfoByName(content_name) != NULL); - cricket::MediaContentDescription* description = - desc_.GetContentDescriptionByName(content_name); - description->set_alt_protocol(alt_protocol); - } - void AddFingerprint() { desc_.RemoveTransportInfoByName(kAudioContentName); desc_.RemoveTransportInfoByName(kVideoContentName); @@ -2225,22 +2215,6 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithIceOptions) { EXPECT_EQ(sdp_with_ice_options, message); } -TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithAltProtocol) { - AddAltProtocol(kAudioContentName, "foo"); - AddAltProtocol(kVideoContentName, "bar"); - - ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), - jdesc_.session_version())); - std::string message = webrtc::SdpSerialize(jdesc_); - - std::string sdp_with_alt_protocol = kSdpFullString; - InjectAfter(kAttributeIcePwdVoice, "a=x-alt-protocol:foo\r\n", - &sdp_with_alt_protocol); - InjectAfter(kAttributeIcePwdVideo, "a=x-alt-protocol:bar\r\n", - &sdp_with_alt_protocol); - EXPECT_EQ(message, sdp_with_alt_protocol); -} - TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRecvOnlyContent) { EXPECT_TRUE(TestSerializeDirection(RtpTransceiverDirection::kRecvOnly)); } @@ -2653,24 +2627,6 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithIceOptions) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_ice_options)); } -TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithAltProtocol) { - std::string sdp_with_alt_protocol = kSdpFullString; - InjectAfter(kAttributeIcePwdVoice, "a=x-alt-protocol:foo\r\n", - &sdp_with_alt_protocol); - InjectAfter(kAttributeIcePwdVideo, "a=x-alt-protocol:bar\r\n", - &sdp_with_alt_protocol); - - JsepSessionDescription jdesc_with_alt_protocol(kDummyType); - EXPECT_TRUE(SdpDeserialize(sdp_with_alt_protocol, &jdesc_with_alt_protocol)); - - AddAltProtocol(kAudioContentName, "foo"); - AddAltProtocol(kVideoContentName, "bar"); - - ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), - jdesc_.session_version())); - EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_alt_protocol)); -} - TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithUfragPwd) { // Remove the original ice-ufrag and ice-pwd JsepSessionDescription jdesc_with_ufrag_pwd(kDummyType);