From 71c6482baf0ff17141c635e6a7639493db68a65c Mon Sep 17 00:00:00 2001 From: Bjorn A Mellem Date: Thu, 6 Jun 2019 17:26:32 -0700 Subject: [PATCH] Implement true negotiation for DatagramTransport with fallback to RTP. In short, the caller places a x-opaque line in SDP for each m= section that uses datagram transport. If the answerer supports datagram transport, it will parse this line and create a datagram transport. It will then echo the x-opaque line into the answer (to indicate that it accepted use of datagram transport). If the offer and answer contain exactly the same x-opaque line, both peers will use datagram transport. If the x-opaque line is omitted from the answer (or is different in the answer) they will fall back to RTP. Note that a different x-opaque line in the answer means the answerer did not understand something in the negotiation proto. Since WebRTC cannot know what was misunderstood, or whether it's still possible to use the datagram transport, it must fall back to RTP. This may change in the future, possibly by passing the answer to the datagram transport, but it's good enough for now. Negotiation consists of four parts: 1. DatagramTransport exposes transport parameters for both client and server perspectives. The client just echoes what it received from the server (modulo any fields it might not have understood). 2. SDP adds a x-opaque line for opaque transport parameters. Identical to x-mt, but this is specific to datagram transport and goes in each m= section, and appears in the answer as well as the offer. - This is propagated to Jsep as part of the TransportDescription. - SDP files: transport_description.h,cc, transport_description_factory.h,cc, media_session.cc, webrtc_sdp.cc 3. JsepTransport/Controller: - Exposes opaque parameters for each mid (m= section). On offerer, this means pre-allocating a datagram transport and getting its parameters. On the answerer, this means echoing the offerer's parameters. - Uses a composite RTP transport to receive from either default RTP or datagram transport until both offer and answer arrive. - If a provisional answer arrives, sets the composite to send on the provisionally selected transport. - Once both offer and answer are set, deletes the unneeded transports and keeps whichever transport is selected. 4. PeerConnection pulls transport parameters out of Jsep and adds them to SDP. Bug: webrtc:9719 Change-Id: Id8996eb1871e79d93b7923a5d7eb3431548c798d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/140700 Commit-Queue: Bjorn Mellem Reviewed-by: Steve Anton Reviewed-by: Anton Sukhanov Cr-Commit-Position: refs/heads/master@{#28182} --- api/BUILD.gn | 1 + api/datagram_transport_interface.h | 16 +- api/test/fake_datagram_transport.h | 85 +++++ api/test/fake_media_transport.h | 10 + p2p/base/transport_description.cc | 4 +- p2p/base/transport_description.h | 24 ++ p2p/base/transport_description_factory.cc | 9 + p2p/base/transport_description_factory.h | 3 + .../transport_description_factory_unittest.cc | 92 ++++++ pc/composite_rtp_transport.cc | 21 ++ pc/composite_rtp_transport.h | 11 + pc/composite_rtp_transport_test.cc | 13 + pc/jsep_transport.cc | 187 ++++++++--- pc/jsep_transport.h | 61 +++- pc/jsep_transport_controller.cc | 136 +++++--- pc/jsep_transport_controller.h | 8 +- pc/jsep_transport_controller_unittest.cc | 309 ++++++++++++++++++ pc/jsep_transport_unittest.cc | 4 +- pc/media_session.cc | 3 + pc/media_session_unittest.cc | 52 ++- pc/peer_connection.cc | 21 +- pc/webrtc_sdp.cc | 46 +++ pc/webrtc_sdp_unittest.cc | 54 +++ 23 files changed, 1049 insertions(+), 121 deletions(-) create mode 100644 api/test/fake_datagram_transport.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 2617a19f7b..063767a555 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -794,6 +794,7 @@ if (rtc_include_tests) { testonly = true sources = [ + "test/fake_datagram_transport.h", "test/fake_media_transport.h", ] diff --git a/api/datagram_transport_interface.h b/api/datagram_transport_interface.h index c908dfb3d5..6205f0043f 100644 --- a/api/datagram_transport_interface.h +++ b/api/datagram_transport_interface.h @@ -120,10 +120,24 @@ class DatagramTransportInterface { // that the binary blob goes through). This should only be called for the // caller's perspective. // - // TODO(sukhanov): Make pure virtual. + // TODO(mellem): Delete. virtual absl::optional GetTransportParametersOffer() const { return absl::nullopt; } + + // Retrieves transport parameters for this datagram transport. May be called + // on either client- or server-perspective transports. + // + // For servers, the parameters represent what kind of connections and data the + // server is prepared to accept. This is generally a superset of acceptable + // parameters. + // + // For clients, the parameters echo the server configuration used to create + // the client, possibly removing any fields or parameters which the client + // does not understand. + // + // TODO(mellem): Make pure virtual. + virtual std::string GetTransportParameters() const { return ""; } }; } // namespace webrtc diff --git a/api/test/fake_datagram_transport.h b/api/test/fake_datagram_transport.h new file mode 100644 index 0000000000..a73a7e8d1c --- /dev/null +++ b/api/test/fake_datagram_transport.h @@ -0,0 +1,85 @@ +/* + * Copyright 2019 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_TEST_FAKE_DATAGRAM_TRANSPORT_H_ +#define API_TEST_FAKE_DATAGRAM_TRANSPORT_H_ + +#include +#include + +#include "api/datagram_transport_interface.h" + +namespace webrtc { + +// Maxmum size of datagrams sent by |FakeDatagramTransport|. +constexpr size_t kMaxFakeDatagramSize = 1000; + +// Fake datagram transport. Does not support making an actual connection +// or sending data. Only used for tests that need to stub out a transport. +class FakeDatagramTransport : public DatagramTransportInterface { + public: + FakeDatagramTransport(const MediaTransportSettings& settings, + std::string transport_parameters) + : settings_(settings), transport_parameters_(transport_parameters) {} + + ~FakeDatagramTransport() override { RTC_DCHECK(!state_callback_); } + + void Connect(rtc::PacketTransportInternal* packet_transport) override { + packet_transport_ = packet_transport; + } + + CongestionControlInterface* congestion_control() override { + return nullptr; // Datagram interface doesn't provide this yet. + } + + void SetTransportStateCallback( + MediaTransportStateCallback* callback) override { + state_callback_ = callback; + } + + RTCError SendDatagram(rtc::ArrayView data, + DatagramId datagram_id) override { + return RTCError::OK(); + } + + size_t GetLargestDatagramSize() const override { + return kMaxFakeDatagramSize; + } + + void SetDatagramSink(DatagramSinkInterface* sink) override {} + + std::string GetTransportParameters() const override { + if (settings_.remote_transport_parameters) { + return *settings_.remote_transport_parameters; + } + return transport_parameters_; + } + + rtc::PacketTransportInternal* packet_transport() { return packet_transport_; } + + void set_state(webrtc::MediaTransportState state) { + if (state_callback_) { + state_callback_->OnStateChanged(state); + } + } + + const MediaTransportSettings& settings() { return settings_; } + + private: + const MediaTransportSettings settings_; + const std::string transport_parameters_; + + rtc::PacketTransportInternal* packet_transport_ = nullptr; + MediaTransportStateCallback* state_callback_ = nullptr; +}; + +} // namespace webrtc + +#endif // API_TEST_FAKE_DATAGRAM_TRANSPORT_H_ diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index ef8284ca93..38b94c9143 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -19,6 +19,7 @@ #include "absl/algorithm/container.h" #include "absl/memory/memory.h" #include "api/media_transport_interface.h" +#include "api/test/fake_datagram_transport.h" namespace webrtc { @@ -146,6 +147,8 @@ class FakeMediaTransport : public MediaTransportInterface { }; // Fake media transport factory creates fake media transport. +// Also creates fake datagram transport, since both media and datagram +// transports are created by |MediaTransportFactory|. class FakeMediaTransportFactory : public MediaTransportFactory { public: explicit FakeMediaTransportFactory( @@ -174,6 +177,13 @@ class FakeMediaTransportFactory : public MediaTransportFactory { return std::move(media_transport); } + RTCErrorOr> + CreateDatagramTransport(rtc::Thread* network_thread, + const MediaTransportSettings& settings) override { + return std::unique_ptr( + new FakeDatagramTransport(settings, transport_offer_.value_or(""))); + } + private: const absl::optional transport_offer_; }; diff --git a/p2p/base/transport_description.cc b/p2p/base/transport_description.cc index 1847eec975..b0a21d6d71 100644 --- a/p2p/base/transport_description.cc +++ b/p2p/base/transport_description.cc @@ -79,7 +79,8 @@ TransportDescription::TransportDescription(const TransportDescription& from) ice_pwd(from.ice_pwd), ice_mode(from.ice_mode), connection_role(from.connection_role), - identity_fingerprint(CopyFingerprint(from.identity_fingerprint.get())) {} + identity_fingerprint(CopyFingerprint(from.identity_fingerprint.get())), + opaque_parameters(from.opaque_parameters) {} TransportDescription::~TransportDescription() = default; @@ -96,6 +97,7 @@ TransportDescription& TransportDescription::operator=( connection_role = from.connection_role; identity_fingerprint.reset(CopyFingerprint(from.identity_fingerprint.get())); + opaque_parameters = from.opaque_parameters; return *this; } diff --git a/p2p/base/transport_description.h b/p2p/base/transport_description.h index b13defbc2b..15e2e919f3 100644 --- a/p2p/base/transport_description.h +++ b/p2p/base/transport_description.h @@ -16,6 +16,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/types/optional.h" #include "p2p/base/p2p_constants.h" #include "rtc_base/ssl_fingerprint.h" @@ -87,6 +88,28 @@ constexpr auto* ICE_OPTION_RENOMINATION = "renomination"; bool StringToConnectionRole(const std::string& role_str, ConnectionRole* role); bool ConnectionRoleToString(const ConnectionRole& role, std::string* role_str); +// Parameters for an opaque transport protocol which may be plugged into WebRTC. +struct OpaqueTransportParameters { + // Protocol used by this opaque transport. Two endpoints that support the + // same protocol are expected to be able to understand the contents of each + // others' |parameters| fields. If those parameters are compatible, the + // endpoints are expected to use this transport protocol. + std::string protocol; + + // Opaque parameters for this transport. These parameters are serialized in a + // manner determined by the |protocol|. They can be parsed and understood by + // the plugin that supports |protocol|. + std::string parameters; + + bool operator==(const OpaqueTransportParameters& other) const { + return protocol == other.protocol && parameters == other.parameters; + } + + bool operator!=(const OpaqueTransportParameters& other) const { + return !(*this == other); + } +}; + struct TransportDescription { TransportDescription(); TransportDescription(const std::vector& transport_options, @@ -133,6 +156,7 @@ struct TransportDescription { ConnectionRole connection_role; std::unique_ptr identity_fingerprint; + absl::optional opaque_parameters; }; } // namespace cricket diff --git a/p2p/base/transport_description_factory.cc b/p2p/base/transport_description_factory.cc index d95328b121..4d4a1383ac 100644 --- a/p2p/base/transport_description_factory.cc +++ b/p2p/base/transport_description_factory.cc @@ -55,6 +55,8 @@ std::unique_ptr TransportDescriptionFactory::CreateOffer( } } + desc->opaque_parameters = options.opaque_parameters; + return desc; } @@ -108,6 +110,13 @@ std::unique_ptr TransportDescriptionFactory::CreateAnswer( return NULL; } + // Answers may only attach opaque parameters that exactly match parameters + // present in the offer. If the answerer cannot fully understand or accept + // the offered transport, it must reject it and fall back. + if (offer->opaque_parameters == options.opaque_parameters) { + desc->opaque_parameters = options.opaque_parameters; + } + return desc; } diff --git a/p2p/base/transport_description_factory.h b/p2p/base/transport_description_factory.h index c1656a0fac..d0813dc541 100644 --- a/p2p/base/transport_description_factory.h +++ b/p2p/base/transport_description_factory.h @@ -29,6 +29,9 @@ struct TransportOptions { // If true, ICE renomination is supported and will be used if it is also // supported by the remote side. bool enable_ice_renomination = false; + + // Opaque parameters for plug-in transports. + absl::optional opaque_parameters; }; // Creates transport descriptions according to the supplied configuration. diff --git a/p2p/base/transport_description_factory_unittest.cc b/p2p/base/transport_description_factory_unittest.cc index 875d140ab6..3819e81a6d 100644 --- a/p2p/base/transport_description_factory_unittest.cc +++ b/p2p/base/transport_description_factory_unittest.cc @@ -24,6 +24,7 @@ #include "test/gmock.h" #include "test/gtest.h" +using cricket::OpaqueTransportParameters; using cricket::TransportDescription; using cricket::TransportDescriptionFactory; using cricket::TransportOptions; @@ -207,6 +208,97 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) { CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, digest_alg); } +TEST_F(TransportDescriptionFactoryTest, TestOfferOpaqueTransportParameters) { + OpaqueTransportParameters params; + params.protocol = "fake"; + params.parameters = "foobar"; + + TransportOptions options; + options.opaque_parameters = params; + + std::unique_ptr desc = + f1_.CreateOffer(options, NULL, &ice_credentials_); + + CheckDesc(desc.get(), "", "", "", ""); + EXPECT_EQ(desc->opaque_parameters, params); +} + +TEST_F(TransportDescriptionFactoryTest, TestAnswerOpaqueTransportParameters) { + OpaqueTransportParameters params; + params.protocol = "fake"; + params.parameters = "foobar"; + + TransportOptions options; + options.opaque_parameters = params; + + std::unique_ptr offer = + f1_.CreateOffer(options, NULL, &ice_credentials_); + std::unique_ptr answer = + f2_.CreateAnswer(offer.get(), options, true, NULL, &ice_credentials_); + + CheckDesc(answer.get(), "", "", "", ""); + EXPECT_EQ(answer->opaque_parameters, params); +} + +TEST_F(TransportDescriptionFactoryTest, TestAnswerNoOpaqueTransportParameters) { + OpaqueTransportParameters params; + params.protocol = "fake"; + params.parameters = "foobar"; + + TransportOptions options; + options.opaque_parameters = params; + + std::unique_ptr offer = + f1_.CreateOffer(options, NULL, &ice_credentials_); + std::unique_ptr answer = f2_.CreateAnswer( + offer.get(), TransportOptions(), true, NULL, &ice_credentials_); + + CheckDesc(answer.get(), "", "", "", ""); + EXPECT_EQ(answer->opaque_parameters, absl::nullopt); +} + +TEST_F(TransportDescriptionFactoryTest, + TestAnswerDifferentOpaqueTransportParameters) { + OpaqueTransportParameters offer_params; + offer_params.protocol = "fake"; + offer_params.parameters = "foobar"; + + TransportOptions options; + options.opaque_parameters = offer_params; + + std::unique_ptr offer = + f1_.CreateOffer(options, NULL, &ice_credentials_); + + OpaqueTransportParameters answer_params; + answer_params.protocol = "fake"; + answer_params.parameters = "baz"; + + options.opaque_parameters = answer_params; + std::unique_ptr answer = + f2_.CreateAnswer(offer.get(), options, true, NULL, &ice_credentials_); + + CheckDesc(answer.get(), "", "", "", ""); + EXPECT_EQ(answer->opaque_parameters, absl::nullopt); +} + +TEST_F(TransportDescriptionFactoryTest, + TestAnswerNoOpaqueTransportParametersInOffer) { + std::unique_ptr offer = + f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); + + OpaqueTransportParameters params; + params.protocol = "fake"; + params.parameters = "foobar"; + + TransportOptions options; + options.opaque_parameters = params; + std::unique_ptr answer = + f2_.CreateAnswer(offer.get(), options, true, NULL, &ice_credentials_); + + CheckDesc(answer.get(), "", "", "", ""); + EXPECT_EQ(answer->opaque_parameters, absl::nullopt); +} + TEST_F(TransportDescriptionFactoryTest, TestAnswerDefault) { std::unique_ptr offer = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_); diff --git a/pc/composite_rtp_transport.cc b/pc/composite_rtp_transport.cc index 1209fa5c11..bdeaacd116 100644 --- a/pc/composite_rtp_transport.cc +++ b/pc/composite_rtp_transport.cc @@ -68,6 +68,25 @@ void CompositeRtpTransport::SetSendTransport( } } +void CompositeRtpTransport::RemoveTransport(RtpTransportInternal* transport) { + RTC_DCHECK(transport != send_transport_) << "Cannot remove send transport"; + + auto it = absl::c_find(transports_, transport); + if (it == transports_.end()) { + RTC_NOTREACHED() << "Callers should not remove transports they did not " + "include in the composite"; + return; + } + + transport->SignalNetworkRouteChanged.disconnect(this); + transport->SignalRtcpPacketReceived.disconnect(this); + for (auto sink : rtp_demuxer_sinks_) { + transport->UnregisterRtpDemuxerSink(sink); + } + + transports_.erase(it); +} + const std::string& CompositeRtpTransport::transport_name() const { return transports_.front()->transport_name(); } @@ -145,6 +164,7 @@ bool CompositeRtpTransport::RegisterRtpDemuxerSink( for (RtpTransportInternal* transport : transports_) { transport->RegisterRtpDemuxerSink(criteria, sink); } + rtp_demuxer_sinks_.insert(sink); return true; } @@ -153,6 +173,7 @@ bool CompositeRtpTransport::UnregisterRtpDemuxerSink( for (RtpTransportInternal* transport : transports_) { transport->UnregisterRtpDemuxerSink(sink); } + rtp_demuxer_sinks_.erase(sink); return true; } diff --git a/pc/composite_rtp_transport.h b/pc/composite_rtp_transport.h index deb315dcf6..35f9382571 100644 --- a/pc/composite_rtp_transport.h +++ b/pc/composite_rtp_transport.h @@ -12,6 +12,7 @@ #define PC_COMPOSITE_RTP_TRANSPORT_H_ #include +#include #include #include @@ -46,6 +47,12 @@ class CompositeRtpTransport : public RtpTransportInternal { // state of |send_tranpsort|. void SetSendTransport(RtpTransportInternal* send_transport); + // Removes |transport| from the composite. No-op if |transport| is null or + // not found in the composite. Removing a transport disconnects all signals + // and RTP demux sinks from that transport. The send transport may not be + // removed. + void RemoveTransport(RtpTransportInternal* transport); + // All transports within a composite must have the same name. const std::string& transport_name() const override; @@ -101,6 +108,10 @@ class CompositeRtpTransport : public RtpTransportInternal { std::vector transports_; RtpTransportInternal* send_transport_ = nullptr; + + // Record of registered RTP demuxer sinks. Used to unregister sinks when a + // transport is removed. + std::set rtp_demuxer_sinks_; }; } // namespace webrtc diff --git a/pc/composite_rtp_transport_test.cc b/pc/composite_rtp_transport_test.cc index 5264c73c63..77512d9929 100644 --- a/pc/composite_rtp_transport_test.cc +++ b/pc/composite_rtp_transport_test.cc @@ -243,6 +243,19 @@ TEST_F(CompositeRtpTransportTest, NetworkRouteChange) { EXPECT_EQ(8, last_network_route_->local_network_id); } +TEST_F(CompositeRtpTransportTest, RemoveTransport) { + SetupRtpTransports(/*rtcp_mux=*/true); + + composite_->RemoveTransport(transport_1_.get()); + + // Check that signals are disconnected. + rtc::NetworkRoute route; + route.local_network_id = 7; + packet_transport_1_->SetNetworkRoute(route); + + EXPECT_EQ(0, network_route_count_); +} + TEST_F(CompositeRtpTransportTest, SendRtcpBeforeSendTransportSet) { SetupRtpTransports(/*rtcp_mux=*/true); diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index 8c614a9db5..9380341a57 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -98,8 +98,10 @@ JsepTransport::JsepTransport( std::unique_ptr unencrypted_rtp_transport, std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, + std::unique_ptr datagram_rtp_transport, std::unique_ptr rtp_dtls_transport, std::unique_ptr rtcp_dtls_transport, + std::unique_ptr datagram_dtls_transport, std::unique_ptr media_transport, std::unique_ptr datagram_transport) : network_thread_(rtc::Thread::Current()), @@ -110,6 +112,7 @@ JsepTransport::JsepTransport( unencrypted_rtp_transport_(std::move(unencrypted_rtp_transport)), sdes_transport_(std::move(sdes_transport)), dtls_srtp_transport_(std::move(dtls_srtp_transport)), + datagram_rtp_transport_(std::move(datagram_rtp_transport)), rtp_dtls_transport_( rtp_dtls_transport ? new rtc::RefCountedObject( std::move(rtp_dtls_transport)) @@ -119,6 +122,11 @@ JsepTransport::JsepTransport( ? new rtc::RefCountedObject( std::move(rtcp_dtls_transport)) : nullptr), + datagram_dtls_transport_( + datagram_dtls_transport + ? new rtc::RefCountedObject( + std::move(datagram_dtls_transport)) + : nullptr), media_transport_(std::move(media_transport)), datagram_transport_(std::move(datagram_transport)) { RTC_DCHECK(ice_transport_); @@ -141,6 +149,12 @@ JsepTransport::JsepTransport( RTC_DCHECK(!sdes_transport); } + if (datagram_rtp_transport_ && default_rtp_transport()) { + composite_rtp_transport_ = absl::make_unique( + std::vector{ + datagram_rtp_transport_.get(), default_rtp_transport()}); + } + if (media_transport_) { media_transport_->SetMediaTransportStateCallback(this); } @@ -161,6 +175,12 @@ JsepTransport::~JsepTransport() { rtcp_dtls_transport_->Clear(); } + // Datagram dtls transport must be disconnected before the datagram transport + // is released. + if (datagram_dtls_transport_) { + datagram_dtls_transport_->Clear(); + } + // Delete datagram transport before ICE, but after DTLS transport. datagram_transport_.reset(); @@ -185,20 +205,23 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( } // If doing SDES, setup the SDES crypto parameters. - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_LOCAL)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); + { + rtc::CritScope scope(&accessor_lock_); + if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + if (!SetSdes(jsep_description.cryptos, + jsep_description.encrypted_header_extension_ids, type, + ContentSource::CS_LOCAL)) { + return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, + "Failed to setup SDES crypto parameters."); + } + } else if (dtls_srtp_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->UpdateRecvEncryptedHeaderExtensionIds( + jsep_description.encrypted_header_extension_ids); } - } else if (dtls_srtp_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->UpdateRecvEncryptedHeaderExtensionIds( - jsep_description.encrypted_header_extension_ids); } bool ice_restarting = local_description_ != nullptr && @@ -233,6 +256,7 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription( // If PRANSWER/ANSWER is set, we should decide transport protocol type. if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { error = NegotiateAndSetDtlsParameters(type); + NegotiateRtpTransport(type); } if (!error.ok()) { local_description_.reset(); @@ -269,24 +293,27 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription( } // If doing SDES, setup the SDES crypto parameters. - if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - if (!SetSdes(jsep_description.cryptos, - jsep_description.encrypted_header_extension_ids, type, - ContentSource::CS_REMOTE)) { - return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, - "Failed to setup SDES crypto parameters."); + { + rtc::CritScope lock(&accessor_lock_); + if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + if (!SetSdes(jsep_description.cryptos, + jsep_description.encrypted_header_extension_ids, type, + ContentSource::CS_REMOTE)) { + return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER, + "Failed to setup SDES crypto parameters."); + } + sdes_transport_->CacheRtpAbsSendTimeHeaderExtension( + jsep_description.rtp_abs_sendtime_extn_id); + } else if (dtls_srtp_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->UpdateSendEncryptedHeaderExtensionIds( + jsep_description.encrypted_header_extension_ids); + dtls_srtp_transport_->CacheRtpAbsSendTimeHeaderExtension( + jsep_description.rtp_abs_sendtime_extn_id); } - sdes_transport_->CacheRtpAbsSendTimeHeaderExtension( - jsep_description.rtp_abs_sendtime_extn_id); - } else if (dtls_srtp_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->UpdateSendEncryptedHeaderExtensionIds( - jsep_description.encrypted_header_extension_ids); - dtls_srtp_transport_->CacheRtpAbsSendTimeHeaderExtension( - jsep_description.rtp_abs_sendtime_extn_id); } remote_description_.reset(new JsepTransportDescription(jsep_description)); @@ -300,6 +327,7 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription( // If PRANSWER/ANSWER is set, we should decide transport protocol type. if (type == SdpType::kPrAnswer || type == SdpType::kAnswer) { error = NegotiateAndSetDtlsParameters(SdpType::kOffer); + NegotiateRtpTransport(type); } if (!error.ok()) { remote_description_.reset(); @@ -356,6 +384,18 @@ absl::optional JsepTransport::GetDtlsRole() const { return absl::optional(dtls_role); } +absl::optional +JsepTransport::GetTransportParameters() const { + rtc::CritScope scope(&accessor_lock_); + if (!datagram_transport()) { + return absl::nullopt; + } + + OpaqueTransportParameters params; + params.parameters = datagram_transport()->GetTransportParameters(); + return params; +} + bool JsepTransport::GetStats(TransportStats* stats) { RTC_DCHECK_RUN_ON(network_thread_); rtc::CritScope scope(&accessor_lock_); @@ -490,23 +530,26 @@ void JsepTransport::ActivateRtcpMux() { // TODO(https://crbug.com/webrtc/10318): Simplify when possible. RTC_DCHECK_RUN_ON(network_thread_); } - if (unencrypted_rtp_transport_) { - RTC_DCHECK(!sdes_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - unencrypted_rtp_transport_->SetRtcpPacketTransport(nullptr); - } else if (sdes_transport_) { - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!dtls_srtp_transport_); - sdes_transport_->SetRtcpPacketTransport(nullptr); - } else { - RTC_DCHECK(dtls_srtp_transport_); - RTC_DCHECK(!unencrypted_rtp_transport_); - RTC_DCHECK(!sdes_transport_); - dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(), - /*rtcp_dtls_transport=*/nullptr); - } { rtc::CritScope scope(&accessor_lock_); + if (datagram_rtp_transport_) { + datagram_rtp_transport_->SetRtcpPacketTransport(nullptr); + } + if (unencrypted_rtp_transport_) { + RTC_DCHECK(!sdes_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + unencrypted_rtp_transport_->SetRtcpPacketTransport(nullptr); + } else if (sdes_transport_) { + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!dtls_srtp_transport_); + sdes_transport_->SetRtcpPacketTransport(nullptr); + } else if (dtls_srtp_transport_) { + RTC_DCHECK(dtls_srtp_transport_); + RTC_DCHECK(!unencrypted_rtp_transport_); + RTC_DCHECK(!sdes_transport_); + dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(), + /*rtcp_dtls_transport=*/nullptr); + } rtcp_dtls_transport_ = nullptr; // Destroy this reference. } // Notify the JsepTransportController to update the aggregate states. @@ -526,9 +569,9 @@ bool JsepTransport::SetSdes(const std::vector& cryptos, } if (source == ContentSource::CS_LOCAL) { - recv_extension_ids_ = std::move(encrypted_extension_ids); + recv_extension_ids_ = encrypted_extension_ids; } else { - send_extension_ids_ = std::move(encrypted_extension_ids); + send_extension_ids_ = encrypted_extension_ids; } // If setting an SDES answer succeeded, apply the negotiated parameters @@ -735,4 +778,54 @@ void JsepTransport::OnStateChanged(webrtc::MediaTransportState state) { } SignalMediaTransportStateChanged(); } + +void JsepTransport::NegotiateRtpTransport(SdpType type) { + RTC_DCHECK(type == SdpType::kAnswer || type == SdpType::kPrAnswer); + rtc::CritScope lock(&accessor_lock_); + if (!composite_rtp_transport_) { + return; // No need to negotiate which RTP transport to use. + } + + bool use_datagram_transport = + remote_description_->transport_desc.opaque_parameters && + remote_description_->transport_desc.opaque_parameters == + local_description_->transport_desc.opaque_parameters; + + if (use_datagram_transport) { + RTC_LOG(INFO) << "Datagram transport provisionally activated"; + composite_rtp_transport_->SetSendTransport(datagram_rtp_transport_.get()); + } else { + RTC_LOG(INFO) << "Datagram transport provisionally rejected"; + composite_rtp_transport_->SetSendTransport(default_rtp_transport()); + } + + if (type != SdpType::kAnswer) { + // A provisional answer lets the peer start sending on the chosen + // transport, but does not allow it to destroy other transports yet. + return; + } + + // A full answer lets the peer send on the chosen transport and delete the + // rest. + if (use_datagram_transport) { + RTC_LOG(INFO) << "Datagram transport activated"; + composite_rtp_transport_->RemoveTransport(default_rtp_transport()); + if (unencrypted_rtp_transport_) { + unencrypted_rtp_transport_ = nullptr; + } else if (sdes_transport_) { + sdes_transport_ = nullptr; + } else { + dtls_srtp_transport_ = nullptr; + } + } else { + RTC_LOG(INFO) << "Datagram transport rejected"; + composite_rtp_transport_->RemoveTransport(datagram_rtp_transport_.get()); + datagram_rtp_transport_ = nullptr; + // This one is ref-counted, so it can't be deleted directly. + datagram_dtls_transport_->Clear(); + datagram_dtls_transport_ = nullptr; + datagram_transport_ = nullptr; + } +} + } // namespace cricket diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h index 8e00793bf9..65800270ab 100644 --- a/pc/jsep_transport.h +++ b/pc/jsep_transport.h @@ -24,6 +24,7 @@ #include "p2p/base/dtls_transport.h" #include "p2p/base/p2p_constants.h" #include "p2p/base/transport_info.h" +#include "pc/composite_rtp_transport.h" #include "pc/dtls_srtp_transport.h" #include "pc/dtls_transport.h" #include "pc/rtcp_mux_filter.h" @@ -92,8 +93,10 @@ class JsepTransport : public sigslot::has_slots<>, std::unique_ptr unencrypted_rtp_transport, std::unique_ptr sdes_transport, std::unique_ptr dtls_srtp_transport, + std::unique_ptr datagram_rtp_transport, std::unique_ptr rtp_dtls_transport, std::unique_ptr rtcp_dtls_transport, + std::unique_ptr datagram_dtls_transport, std::unique_ptr media_transport, std::unique_ptr datagram_transport); @@ -146,6 +149,8 @@ class JsepTransport : public sigslot::has_slots<>, // negotiated yet. absl::optional GetDtlsRole() const; + absl::optional GetTransportParameters() const; + // TODO(deadbeef): Make this const. See comment in transportcontroller.h. bool GetStats(TransportStats* stats); @@ -160,15 +165,13 @@ class JsepTransport : public sigslot::has_slots<>, } webrtc::RtpTransportInternal* rtp_transport() const { - // This method is called from the signaling thread, which means - // that a race is possible, making safety analysis complex. - // After fixing, this method should be marked "network thread only". - if (dtls_srtp_transport_) { - return dtls_srtp_transport_.get(); - } else if (sdes_transport_) { - return sdes_transport_.get(); + rtc::CritScope scope(&accessor_lock_); + if (composite_rtp_transport_) { + return composite_rtp_transport_.get(); + } else if (datagram_rtp_transport_) { + return datagram_rtp_transport_.get(); } else { - return unencrypted_rtp_transport_.get(); + return default_rtp_transport(); } } @@ -299,6 +302,26 @@ class JsepTransport : public sigslot::has_slots<>, // Invoked whenever the state of the media transport changes. void OnStateChanged(webrtc::MediaTransportState state) override; + // Deactivates, signals removal, and deletes |composite_rtp_transport_| if the + // current state of negotiation is sufficient to determine which rtp_transport + // to use. + void NegotiateRtpTransport(webrtc::SdpType type) RTC_RUN_ON(network_thread_); + + // Returns the default (non-datagram) rtp transport, if any. + webrtc::RtpTransportInternal* default_rtp_transport() const + RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_) { + if (dtls_srtp_transport_) { + return dtls_srtp_transport_.get(); + } else if (sdes_transport_) { + return sdes_transport_.get(); + } else if (unencrypted_rtp_transport_) { + return unencrypted_rtp_transport_.get(); + } else { + RTC_DCHECK(media_transport_); + return nullptr; + } + } + // Owning thread, for safety checks const rtc::Thread* const network_thread_; // Critical scope for fields accessed off-thread @@ -321,16 +344,28 @@ class JsepTransport : public sigslot::has_slots<>, // To avoid downcasting and make it type safe, keep three unique pointers for // different SRTP mode and only one of these is non-nullptr. - // Since these are const, the variables don't need locks; - // accessing the objects depends on the objects' thread safety contract. - const std::unique_ptr unencrypted_rtp_transport_; - const std::unique_ptr sdes_transport_; - const std::unique_ptr dtls_srtp_transport_; + std::unique_ptr unencrypted_rtp_transport_ + RTC_GUARDED_BY(accessor_lock_); + std::unique_ptr sdes_transport_ + RTC_GUARDED_BY(accessor_lock_); + std::unique_ptr dtls_srtp_transport_ + RTC_GUARDED_BY(accessor_lock_); + std::unique_ptr datagram_rtp_transport_ + RTC_GUARDED_BY(accessor_lock_); + + // If multiple RTP transports are in use, |composite_rtp_transport_| will be + // passed to callers. This is only valid for offer-only, receive-only + // scenarios, as it is not possible for the composite to correctly choose + // which transport to use for sending. + std::unique_ptr composite_rtp_transport_ + RTC_GUARDED_BY(accessor_lock_); rtc::scoped_refptr rtp_dtls_transport_ RTC_GUARDED_BY(accessor_lock_); rtc::scoped_refptr rtcp_dtls_transport_ RTC_GUARDED_BY(accessor_lock_); + rtc::scoped_refptr datagram_dtls_transport_ + RTC_GUARDED_BY(accessor_lock_); SrtpFilter sdes_negotiator_ RTC_GUARDED_BY(network_thread_); RtcpMuxFilter rtcp_mux_negotiator_ RTC_GUARDED_BY(network_thread_); diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 56d9a47ed7..597ab4a1db 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -1082,46 +1082,45 @@ JsepTransportController::MaybeCreateDatagramTransport( } // Caller (offerer) datagram transport. - if (local) { - if (offer_datagram_transport_) { - RTC_LOG(LS_INFO) << "Offered datagram transport has now been activated."; - return std::move(offer_datagram_transport_); - } else { - RTC_LOG(LS_INFO) - << "Not returning datagram transport. Either SDES wasn't enabled, or " - "datagram transport didn't return an offer earlier."; - return nullptr; - } + if (offer_datagram_transport_) { + RTC_DCHECK(local); + RTC_LOG(LS_INFO) << "Offered datagram transport has now been activated."; + return std::move(offer_datagram_transport_); } - // Remote offer. If no x-mt lines, do not create datagram transport. - if (description.MediaTransportSettings().empty()) { + const cricket::TransportDescription* transport_description = + description.GetTransportDescriptionByName(content_info.mid()); + RTC_DCHECK(transport_description) + << "Missing transport description for mid=" << content_info.mid(); + + if (!transport_description->opaque_parameters) { + RTC_LOG(LS_INFO) + << "No opaque transport parameters, not creating datagram transport"; return nullptr; } + if (transport_description->opaque_parameters->protocol != + config_.media_transport_factory->GetTransportName()) { + RTC_LOG(LS_INFO) << "Opaque transport parameters for protocol=" + << transport_description->opaque_parameters->protocol + << ", which does not match supported protocol=" + << config_.media_transport_factory->GetTransportName(); + return nullptr; + } + + RTC_DCHECK(!local); // When bundle is enabled, two JsepTransports are created, and then // the second transport is destroyed (right away). // For datagram transport, we don't want to create the second // datagram transport in the first place. RTC_LOG(LS_INFO) << "Returning new, client datagram transport."; - RTC_DCHECK(!local) - << "If datagram transport is used, you must call " - "GenerateOrGetLastMediaTransportOffer before SetLocalDescription. You " - "also must use kRtcpMuxPolicyRequire and kBundlePolicyMaxBundle with " - "datagram transport."; MediaTransportSettings settings; settings.is_caller = local; + settings.remote_transport_parameters = + transport_description->opaque_parameters->parameters; settings.event_log = config_.event_log; - // Assume there is only one media transport (or if more, use the first one). - if (!local && !description.MediaTransportSettings().empty() && - config_.media_transport_factory->GetTransportName() == - description.MediaTransportSettings()[0].transport_name) { - settings.remote_transport_parameters = - description.MediaTransportSettings()[0].transport_setting; - } - auto datagram_transport_result = config_.media_transport_factory->CreateDatagramTransport(network_thread_, settings); @@ -1161,18 +1160,21 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( std::unique_ptr datagram_transport = MaybeCreateDatagramTransport(content_info, description, local); + std::unique_ptr datagram_dtls_transport; if (datagram_transport) { - datagram_transport_created_once_ = true; datagram_transport->Connect(ice.get()); + datagram_dtls_transport = + CreateDtlsTransport(ice.get(), datagram_transport.get()); } std::unique_ptr rtp_dtls_transport = - CreateDtlsTransport(ice.get(), datagram_transport.get()); + CreateDtlsTransport(ice.get(), nullptr); std::unique_ptr rtcp_dtls_transport; std::unique_ptr unencrypted_rtp_transport; std::unique_ptr sdes_transport; std::unique_ptr dtls_srtp_transport; + std::unique_ptr datagram_rtp_transport; std::unique_ptr rtcp_ice; if (config_.rtcp_mux_policy != @@ -1196,9 +1198,11 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( RTC_LOG(LS_INFO) << "Creating UnencryptedRtpTransport, because datagram " "transport is used."; RTC_DCHECK(!rtcp_dtls_transport); - unencrypted_rtp_transport = CreateUnencryptedRtpTransport( - content_info.name, rtp_dtls_transport.get(), rtcp_dtls_transport.get()); - } else if (config_.disable_encryption) { + datagram_rtp_transport = CreateUnencryptedRtpTransport( + content_info.name, datagram_dtls_transport.get(), nullptr); + } + + if (config_.disable_encryption) { RTC_LOG(LS_INFO) << "Creating UnencryptedRtpTransport, becayse encryption is disabled."; unencrypted_rtp_transport = CreateUnencryptedRtpTransport( @@ -1217,8 +1221,9 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( absl::make_unique( content_info.name, certificate_, std::move(ice), std::move(rtcp_ice), std::move(unencrypted_rtp_transport), std::move(sdes_transport), - std::move(dtls_srtp_transport), std::move(rtp_dtls_transport), - std::move(rtcp_dtls_transport), std::move(media_transport), + std::move(dtls_srtp_transport), std::move(datagram_rtp_transport), + std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), + std::move(datagram_dtls_transport), std::move(media_transport), std::move(datagram_transport)); jsep_transport->SignalRtcpMuxActive.connect( @@ -1650,7 +1655,7 @@ void JsepTransportController::OnDtlsHandshakeError( absl::optional JsepTransportController::GenerateOrGetLastMediaTransportOffer() { - if (media_transport_created_once_ || datagram_transport_created_once_) { + if (media_transport_created_once_) { RTC_LOG(LS_INFO) << "Not regenerating media transport for the new offer in " "existing session."; return media_transport_offer_settings_; @@ -1685,27 +1690,9 @@ JsepTransportController::GenerateOrGetLastMediaTransportOffer() { RTC_LOG(LS_INFO) << "Unable to create media transport, error=" << media_transport_or_error.error().message(); } - } else if (config_.use_datagram_transport) { - webrtc::MediaTransportSettings settings; - settings.is_caller = true; - settings.pre_shared_key = rtc::CreateRandomString(32); - settings.event_log = config_.event_log; - auto datagram_transport_or_error = - config_.media_transport_factory->CreateDatagramTransport( - network_thread_, settings); - - if (datagram_transport_or_error.ok()) { - offer_datagram_transport_ = - std::move(datagram_transport_or_error.value()); - transport_parameters = - offer_datagram_transport_->GetTransportParametersOffer(); - } else { - RTC_LOG(LS_INFO) << "Unable to create media transport, error=" - << datagram_transport_or_error.error().message(); - } } - if (!offer_media_transport_ && !offer_datagram_transport_) { + if (!offer_media_transport_) { RTC_LOG(LS_INFO) << "Media and data transports do not exist"; return absl::nullopt; } @@ -1725,4 +1712,49 @@ JsepTransportController::GenerateOrGetLastMediaTransportOffer() { return setting; } +absl::optional +JsepTransportController::GetTransportParameters(const std::string& mid) { + if (!config_.use_datagram_transport) { + return absl::nullopt; + } + + cricket::JsepTransport* transport = GetJsepTransportForMid(mid); + if (transport) { + absl::optional params = + transport->GetTransportParameters(); + if (params) { + params->protocol = config_.media_transport_factory->GetTransportName(); + } + return params; + } + + RTC_DCHECK(!local_desc_ && !remote_desc_) + << "JsepTransport should exist for every mid once any description is set"; + + // Need to generate a transport for the offer. + if (!offer_datagram_transport_) { + webrtc::MediaTransportSettings settings; + settings.is_caller = true; + settings.pre_shared_key = rtc::CreateRandomString(32); + settings.event_log = config_.event_log; + auto datagram_transport_or_error = + config_.media_transport_factory->CreateDatagramTransport( + network_thread_, settings); + + if (datagram_transport_or_error.ok()) { + offer_datagram_transport_ = + std::move(datagram_transport_or_error.value()); + } else { + RTC_LOG(LS_INFO) << "Unable to create datagram transport, error=" + << datagram_transport_or_error.error().message(); + } + } + + // We have prepared a transport for the offer, and can now use its parameters. + cricket::OpaqueTransportParameters params; + params.parameters = offer_datagram_transport_->GetTransportParameters(); + params.protocol = config_.media_transport_factory->GetTransportName(); + return params; +} + } // namespace webrtc diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index fcae15373f..23d4485a6d 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -213,6 +213,13 @@ class JsepTransportController : public sigslot::has_slots<> { absl::optional GenerateOrGetLastMediaTransportOffer(); + // Gets the transport parameters for the transport identified by |mid|. + // If |mid| is bundled, returns the parameters for the bundled transport. + // If the transport for |mid| has not been created yet, it may be allocated in + // order to generate transport parameters. + absl::optional GetTransportParameters( + const std::string& mid); + // All of these signals are fired on the signaling thread. // If any transport failed => failed, @@ -459,7 +466,6 @@ class JsepTransportController : public sigslot::has_slots<> { // recreate the Offer (e.g. after adding streams in Plan B), and so we want to // prevent recreation of the media transport when that happens. bool media_transport_created_once_ = false; - bool datagram_transport_created_once_ = false; const cricket::SessionDescription* local_desc_ = nullptr; const cricket::SessionDescription* remote_desc_ = nullptr; diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 3bde0e7c0b..353a659712 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -2115,4 +2115,313 @@ TEST_F(JsepTransportControllerTest, ChangeTaggedMediaSectionMaxBundle) { .ok()); } +constexpr char kFakeTransportParameters[] = "fake-params"; + +// Test fixture that provides common setup and helpers for tests related to the +// datagram transport. +class JsepTransportControllerDatagramTest + : public JsepTransportControllerTest, + public testing::WithParamInterface { + public: + JsepTransportControllerDatagramTest() + : JsepTransportControllerTest(), + fake_media_transport_factory_(kFakeTransportParameters) { + JsepTransportController::Config config; + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + config.media_transport_factory = &fake_media_transport_factory_; + config.use_datagram_transport = true; + CreateJsepTransportController(config); + } + + // Whether the JsepTransportController under test acts as the offerer or + // answerer in this test. + bool IsOfferer() { return GetParam(); } + + // Sets a description as local or remote based on type and current + // perspective. + RTCError SetDescription(SdpType type, + const cricket::SessionDescription* description) { + if (IsOfferer() == (type == SdpType::kOffer)) { + return transport_controller_->SetLocalDescription(type, description); + } else { + return transport_controller_->SetRemoteDescription(type, description); + } + } + + // Creates a session description with the settings necessary for datagram + // transport (bundle + crypto) and the given |transport_params|. + std::unique_ptr + CreateSessionDescriptionForDatagramTransport( + absl::optional transport_params) { + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + + for (auto& info : description->transport_infos()) { + info.description.opaque_parameters = transport_params; + } + return description; + } + + // Creates transport parameters with |protocol| and |parameters| + // matching what |fake_media_transport_factory_| provides. + cricket::OpaqueTransportParameters CreateTransportParameters() { + cricket::OpaqueTransportParameters params; + params.protocol = fake_media_transport_factory_.GetTransportName(); + params.parameters = "fake-params"; + return params; + } + + protected: + FakeMediaTransportFactory fake_media_transport_factory_; +}; + +TEST_P(JsepTransportControllerDatagramTest, InitDatagramTransport) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + // Getting transport parameters is allowed before setting a description. + // This is necessary so that the offerer can include these params. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + // Setting a description activates the datagram transport without changing + // transport parameters. + auto description = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, description.get()).ok()); + + // After setting an offer with transport parameters, those parameters are + // reflected by the controller. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); +} + +TEST_P(JsepTransportControllerDatagramTest, + OfferMissingDatagramTransportParams) { + if (IsOfferer()) { + // This test doesn't make sense from the offerer's perspective, as the offer + // must contain datagram transport params if the offerer supports it. + return; + } + + auto description = + CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kOffer, description.get()).ok()); + + // The offer didn't contain any datagram transport parameters, so the answer + // won't either. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); +} + +TEST_P(JsepTransportControllerDatagramTest, OfferHasWrongTransportName) { + if (IsOfferer()) { + // This test doesn't make sense from the offerer's perspective, as the + // offerer cannot offer itself the wrong transport. + return; + } + + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + fake_params.protocol = "wrong-name"; + + auto description = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, description.get()).ok()); + + // The offerer and answerer support different datagram transports, so the + // answerer rejects the offered parameters. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); +} + +TEST_P(JsepTransportControllerDatagramTest, AnswerRejectsDatagram) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + auto offer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + auto answer = CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kAnswer, answer.get()).ok()); + + // The answer rejected datagram transport, so its parameters are empty. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); +} + +TEST_P(JsepTransportControllerDatagramTest, AnswerAcceptsDatagram) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + auto offer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + auto answer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kAnswer, answer.get()).ok()); + + // The answer accepted datagram transport, so it is present. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); +} + +TEST_P(JsepTransportControllerDatagramTest, PrAnswerRejectsDatagram) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + auto offer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + auto answer = CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kPrAnswer, answer.get()).ok()); + + // The answer rejected datagram transport, but it's provisional, so the + // transport is kept around for now. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); +} + +TEST_P(JsepTransportControllerDatagramTest, PrAnswerAcceptsDatagram) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + auto offer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + auto answer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kPrAnswer, answer.get()).ok()); + + // The answer provisionally accepted datagram transport, so it's kept. + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); +} + +TEST_P(JsepTransportControllerDatagramTest, RenegotiationCannotAddDatagram) { + auto offer = CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); + + auto answer = CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kAnswer, answer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); + + // Attempting to add a datagram transport on a re-offer does not cause an + // error, but also does not add a datagram transport. + auto reoffer = + CreateSessionDescriptionForDatagramTransport(CreateTransportParameters()); + EXPECT_TRUE(SetDescription(SdpType::kOffer, reoffer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + absl::nullopt); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + absl::nullopt); +} + +TEST_P(JsepTransportControllerDatagramTest, RenegotiationCannotRemoveDatagram) { + cricket::OpaqueTransportParameters fake_params = CreateTransportParameters(); + if (IsOfferer()) { + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + } + + auto offer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kOffer, offer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + auto answer = CreateSessionDescriptionForDatagramTransport(fake_params); + EXPECT_TRUE(SetDescription(SdpType::kAnswer, answer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); + + // Attempting to remove a datagram transport on a re-offer does not cause an + // error, but also does not remove the datagram transport. + auto reoffer = CreateSessionDescriptionForDatagramTransport(absl::nullopt); + EXPECT_TRUE(SetDescription(SdpType::kOffer, reoffer.get()).ok()); + + EXPECT_EQ(transport_controller_->GetTransportParameters(kAudioMid1), + fake_params); + EXPECT_EQ(transport_controller_->GetTransportParameters(kVideoMid1), + fake_params); +} + +INSTANTIATE_TEST_SUITE_P( + JsepTransportControllerDatagramTests, + JsepTransportControllerDatagramTest, + testing::Values(true, false), + // The parameter value is the local perspective (offerer or answerer). + [](const testing::TestParamInfo& info) { + return info.param ? "Offerer" : "Answerer"; + }); + } // namespace webrtc diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc index 7e2f80ef5e..31a4e926e3 100644 --- a/pc/jsep_transport_unittest.cc +++ b/pc/jsep_transport_unittest.cc @@ -108,7 +108,9 @@ class JsepTransport2Test : public ::testing::Test, public sigslot::has_slots<> { kTransportName, /*local_certificate=*/nullptr, std::move(ice), std::move(rtcp_ice), std::move(unencrypted_rtp_transport), std::move(sdes_transport), std::move(dtls_srtp_transport), - std::move(rtp_dtls_transport), std::move(rtcp_dtls_transport), + /*datagram_rtp_transport=*/nullptr, std::move(rtp_dtls_transport), + std::move(rtcp_dtls_transport), + /*datagram_dtls_transport=*/nullptr, /*media_transport=*/nullptr, /*datagram_transport=*/nullptr); diff --git a/pc/media_session.cc b/pc/media_session.cc index fe11d44702..f989d28f72 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -540,12 +540,15 @@ static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group, selected_transport_info->description.ice_pwd; ConnectionRole selected_connection_role = selected_transport_info->description.connection_role; + const absl::optional& selected_opaque_parameters = + selected_transport_info->description.opaque_parameters; for (TransportInfo& transport_info : sdesc->transport_infos()) { if (bundle_group.HasContentName(transport_info.content_name) && transport_info.content_name != selected_content_name) { transport_info.description.ice_ufrag = selected_ufrag; transport_info.description.ice_pwd = selected_pwd; transport_info.description.connection_role = selected_connection_role; + transport_info.description.opaque_parameters = selected_opaque_parameters; } } return true; diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 76a33d0745..0a756f3462 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -519,6 +519,8 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { EXPECT_EQ( media_desc_options_it->transport_options.enable_ice_renomination, GetIceRenomination(ti_audio)); + EXPECT_EQ(media_desc_options_it->transport_options.opaque_parameters, + ti_audio->description.opaque_parameters); } else { EXPECT_TRUE(ti_audio == NULL); @@ -526,10 +528,14 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { const TransportInfo* ti_video = desc->GetTransportInfoByName("video"); if (options.has_video()) { EXPECT_TRUE(ti_video != NULL); + auto media_desc_options_it = + FindFirstMediaDescriptionByMid("video", options); if (options.bundle_enabled) { EXPECT_EQ(ti_audio->description.ice_ufrag, ti_video->description.ice_ufrag); EXPECT_EQ(ti_audio->description.ice_pwd, ti_video->description.ice_pwd); + EXPECT_EQ(ti_audio->description.opaque_parameters, + ti_video->description.opaque_parameters); } else { if (has_current_desc) { EXPECT_EQ(current_video_ufrag, ti_video->description.ice_ufrag); @@ -540,9 +546,9 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { EXPECT_EQ(static_cast(cricket::ICE_PWD_LENGTH), ti_video->description.ice_pwd.size()); } + EXPECT_EQ(media_desc_options_it->transport_options.opaque_parameters, + ti_video->description.opaque_parameters); } - auto media_desc_options_it = - FindFirstMediaDescriptionByMid("video", options); EXPECT_EQ( media_desc_options_it->transport_options.enable_ice_renomination, GetIceRenomination(ti_video)); @@ -574,7 +580,7 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { GetIceRenomination(ti_data)); } else { - EXPECT_TRUE(ti_video == NULL); + EXPECT_TRUE(ti_data == NULL); } } @@ -3433,6 +3439,46 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfo(false, options, true); } +TEST_F(MediaSessionDescriptionFactoryTest, + TestTransportInfoOfferBundlesTransportOptions) { + MediaSessionOptions options; + AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); + + cricket::OpaqueTransportParameters audio_params; + audio_params.protocol = "audio-transport"; + audio_params.parameters = "audio-params"; + FindFirstMediaDescriptionByMid("audio", &options) + ->transport_options.opaque_parameters = audio_params; + + cricket::OpaqueTransportParameters video_params; + video_params.protocol = "video-transport"; + video_params.parameters = "video-params"; + FindFirstMediaDescriptionByMid("video", &options) + ->transport_options.opaque_parameters = video_params; + + TestTransportInfo(/*offer=*/true, options, /*has_current_desc=*/false); +} + +TEST_F(MediaSessionDescriptionFactoryTest, + TestTransportInfoAnswerBundlesTransportOptions) { + MediaSessionOptions options; + AddAudioVideoSections(RtpTransceiverDirection::kRecvOnly, &options); + + cricket::OpaqueTransportParameters audio_params; + audio_params.protocol = "audio-transport"; + audio_params.parameters = "audio-params"; + FindFirstMediaDescriptionByMid("audio", &options) + ->transport_options.opaque_parameters = audio_params; + + cricket::OpaqueTransportParameters video_params; + video_params.protocol = "video-transport"; + video_params.parameters = "video-params"; + FindFirstMediaDescriptionByMid("video", &options) + ->transport_options.opaque_parameters = video_params; + + TestTransportInfo(/*offer=*/false, options, /*has_current_desc=*/false); +} + // 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/peer_connection.cc b/pc/peer_connection.cc index b94b883989..2de3075061 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -4314,11 +4314,20 @@ void PeerConnection::GetOptionsForOffer( session_options->offer_extmap_allow_mixed = configuration_.offer_extmap_allow_mixed; - if (configuration_.enable_dtls_srtp && - !configuration_.enable_dtls_srtp.value()) { + if (configuration_.use_media_transport || + configuration_.use_media_transport_for_data_channels) { session_options->media_transport_settings = transport_controller_->GenerateOrGetLastMediaTransportOffer(); } + + // If datagram transport is in use, add opaque transport parameters. + if (configuration_.use_datagram_transport) { + for (auto& options : session_options->media_description_options) { + options.transport_options.opaque_parameters = + transport_controller_->GetTransportParameters(options.mid); + } + } + // Allow fallback for using obsolete SCTP syntax. // Note that the default in |session_options| is true, while // the default in |options| is false. @@ -4616,6 +4625,14 @@ void PeerConnection::GetOptionsForAnswer( RTC_FROM_HERE, rtc::Bind(&cricket::PortAllocator::GetPooledIceCredentials, port_allocator_.get())); + + // If datagram transport is in use, add opaque transport parameters. + if (configuration_.use_datagram_transport) { + for (auto& options : session_options->media_description_options) { + options.transport_options.opaque_parameters = + transport_controller_->GetTransportParameters(options.mid); + } + } } void PeerConnection::GetOptionsForPlanBAnswer( diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 83bd3782bd..5784defc77 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -249,6 +249,9 @@ static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel"; // |MediaTransportInterface::GetTransportParametersOffer|. static const char kMediaTransportSettingLine[] = "x-mt"; +// This is a non-standardized setting for plugin transports. +static const char kOpaqueTransportParametersLine[] = "x-opaque"; + // RTP payload type is in the 0-127 range. Use -1 to indicate "all" payload // types. const int kWildcardPayloadType = -1; @@ -548,6 +551,17 @@ static void AddMediaTransportLine( AddLine(os.str(), message); } +// Adds an x-otp SDP attribute line based on opaque transport parameters. +static void AddOpaqueTransportLine( + const cricket::OpaqueTransportParameters params, + std::string* message) { + rtc::StringBuilder os; + InitAttrLine(kOpaqueTransportParametersLine, &os); + os << kSdpDelimiterColon << params.protocol << kSdpDelimiterColon + << rtc::Base64::Encode(params.parameters); + 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, @@ -1536,6 +1550,11 @@ void BuildMediaDescription(const ContentInfo* content_info, AddLine(os.str(), message); } } + + if (transport_info->description.opaque_parameters) { + AddOpaqueTransportLine(*transport_info->description.opaque_parameters, + message); + } } // RFC 3388 @@ -2115,6 +2134,26 @@ bool ParseMediaTransportLine(const std::string& line, return true; } +bool ParseOpaqueTransportLine(const std::string& line, + std::string* protocol, + std::string* transport_parameters, + SdpParseError* error) { + std::string value; + if (!GetValue(line, kOpaqueTransportParametersLine, &value, error)) { + return false; + } + std::string tmp_parameters; + if (!rtc::tokenize_first(value, kSdpDelimiterColonChar, protocol, + &tmp_parameters)) { + return ParseFailedGetValue(line, kOpaqueTransportParametersLine, error); + } + if (!rtc::Base64::Decode(tmp_parameters, rtc::Base64::DO_STRICT, + transport_parameters, nullptr)) { + return ParseFailedGetValue(line, kOpaqueTransportParametersLine, error); + } + return true; +} + bool ParseSessionDescription(const std::string& message, size_t* pos, std::string* session_id, @@ -3137,6 +3176,13 @@ bool ParseContent(const std::string& message, if (!ParseIceOptions(line, &transport->transport_options, error)) { return false; } + } else if (HasAttribute(line, kOpaqueTransportParametersLine)) { + transport->opaque_parameters = cricket::OpaqueTransportParameters(); + if (!ParseOpaqueTransportLine( + line, &transport->opaque_parameters->protocol, + &transport->opaque_parameters->parameters, error)) { + return false; + } } 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 5fdf5f61ab..71f5153ea5 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -1577,6 +1577,8 @@ class WebRtcSdpTest : public ::testing::Test { } EXPECT_EQ(transport1.description.transport_options, transport2.description.transport_options); + EXPECT_EQ(transport1.description.opaque_parameters, + transport2.description.opaque_parameters); } // global attributes @@ -1670,6 +1672,15 @@ class WebRtcSdpTest : public ::testing::Test { desc_.AddTransportInfo(transport_info); } + void AddOpaqueTransportParameters(const std::string& content_name, + cricket::OpaqueTransportParameters params) { + ASSERT_TRUE(desc_.GetTransportInfoByName(content_name) != NULL); + cricket::TransportInfo info = *(desc_.GetTransportInfoByName(content_name)); + desc_.RemoveTransportInfoByName(content_name); + info.description.opaque_parameters = params; + desc_.AddTransportInfo(info); + } + void AddFingerprint() { desc_.RemoveTransportInfoByName(kAudioContentName); desc_.RemoveTransportInfoByName(kVideoContentName); @@ -2203,6 +2214,25 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithIceOptions) { EXPECT_EQ(sdp_with_ice_options, message); } +TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithOpaqueTransportParams) { + cricket::OpaqueTransportParameters params; + params.protocol = "foo"; + params.parameters = "test64"; + AddOpaqueTransportParameters(kAudioContentName, params); + AddOpaqueTransportParameters(kVideoContentName, params); + + ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), + jdesc_.session_version())); + std::string message = webrtc::SdpSerialize(jdesc_); + + std::string sdp_with_transport_parameters = kSdpFullString; + InjectAfter(kAttributeIcePwdVoice, "a=x-opaque:foo:dGVzdDY0\r\n", + &sdp_with_transport_parameters); + InjectAfter(kAttributeIcePwdVideo, "a=x-opaque:foo:dGVzdDY0\r\n", + &sdp_with_transport_parameters); + EXPECT_EQ(message, sdp_with_transport_parameters); +} + TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithRecvOnlyContent) { EXPECT_TRUE(TestSerializeDirection(RtpTransceiverDirection::kRecvOnly)); } @@ -2591,6 +2621,30 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithIceOptions) { EXPECT_TRUE(CompareSessionDescription(jdesc_, jdesc_with_ice_options)); } +TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithOpaqueTransportParams) { + std::string sdp_with_transport_parameters = kSdpFullString; + InjectAfter(kAttributeIcePwdVoice, "a=x-opaque:foo:dGVzdDY0\r\n", + &sdp_with_transport_parameters); + InjectAfter(kAttributeIcePwdVideo, "a=x-opaque:foo:dGVzdDY0\r\n", + &sdp_with_transport_parameters); + + JsepSessionDescription jdesc_with_transport_parameters(kDummyType); + EXPECT_TRUE(SdpDeserialize(sdp_with_transport_parameters, + &jdesc_with_transport_parameters)); + + cricket::OpaqueTransportParameters params; + params.protocol = "foo"; + params.parameters = "test64"; + + AddOpaqueTransportParameters(kAudioContentName, params); + AddOpaqueTransportParameters(kVideoContentName, params); + + ASSERT_TRUE(jdesc_.Initialize(desc_.Clone(), jdesc_.session_id(), + jdesc_.session_version())); + EXPECT_TRUE( + CompareSessionDescription(jdesc_, jdesc_with_transport_parameters)); +} + TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithUfragPwd) { // Remove the original ice-ufrag and ice-pwd JsepSessionDescription jdesc_with_ufrag_pwd(kDummyType);