diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index bdf56162af..1cfd340765 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -27,8 +27,13 @@ namespace webrtc { // could unit test audio / video integration. class FakeMediaTransport : public MediaTransportInterface { public: - explicit FakeMediaTransport(const MediaTransportSettings& settings) - : settings_(settings) {} + explicit FakeMediaTransport( + const MediaTransportSettings& settings, + const absl::optional& transport_offer = "", + const absl::optional& remote_transport_parameters = "") + : settings_(settings), + transport_offer_(transport_offer), + remote_transport_parameters_(remote_transport_parameters) {} ~FakeMediaTransport() = default; RTCError SendAudioFrame(uint64_t channel_id, @@ -103,16 +108,39 @@ class FakeMediaTransport : public MediaTransportInterface { // Settings that were passed down to fake media transport. const MediaTransportSettings& settings() { return settings_; } + absl::optional GetTransportParametersOffer() const override { + // At least right now, we intend to use GetTransportParametersOffer before + // the transport is connected. This may change in the future. + RTC_CHECK(!is_connected_); + return transport_offer_; + } + + const absl::optional& remote_transport_parameters() { + return remote_transport_parameters_; + } + + void Connect(rtc::PacketTransportInternal* packet_transport) { + RTC_CHECK(!is_connected_) << "::Connect was called twice"; + is_connected_ = true; + } + + bool is_connected() { return is_connected_; } + private: const MediaTransportSettings settings_; - MediaTransportStateCallback* state_callback_; + MediaTransportStateCallback* state_callback_ = nullptr; std::vector target_rate_observers_; + const absl::optional transport_offer_; + const absl::optional remote_transport_parameters_; + bool is_connected_ = false; }; // Fake media transport factory creates fake media transport. class FakeMediaTransportFactory : public MediaTransportFactory { public: - FakeMediaTransportFactory() = default; + explicit FakeMediaTransportFactory( + const absl::optional& transport_offer = "") + : transport_offer_(transport_offer) {} ~FakeMediaTransportFactory() = default; std::string GetTransportName() const override { return "fake"; } @@ -122,9 +150,22 @@ class FakeMediaTransportFactory : public MediaTransportFactory { rtc::Thread* network_thread, const MediaTransportSettings& settings) override { std::unique_ptr media_transport = - absl::make_unique(settings); + absl::make_unique(settings, transport_offer_); + media_transport->Connect(packet_transport); return std::move(media_transport); } + + RTCErrorOr> CreateMediaTransport( + rtc::Thread* network_thread, + const MediaTransportSettings& settings) override { + std::unique_ptr media_transport = + absl::make_unique( + settings, transport_offer_, settings.remote_transport_parameters); + return std::move(media_transport); + } + + private: + const absl::optional transport_offer_; }; } // namespace webrtc diff --git a/api/test/loopback_media_transport.cc b/api/test/loopback_media_transport.cc index 490fc46d7c..9b43da2015 100644 --- a/api/test/loopback_media_transport.cc +++ b/api/test/loopback_media_transport.cc @@ -88,6 +88,10 @@ class WrapperMediaTransport : public MediaTransportInterface { void SetAllocatedBitrateLimits( const MediaTransportAllocatedBitrateLimits& limits) override {} + absl::optional GetTransportParametersOffer() const override { + return wrapped_->GetTransportParametersOffer(); + } + private: MediaTransportInterface* wrapped_; }; @@ -98,20 +102,62 @@ WrapperMediaTransportFactory::WrapperMediaTransportFactory( MediaTransportInterface* wrapped) : wrapped_(wrapped) {} +WrapperMediaTransportFactory::WrapperMediaTransportFactory( + MediaTransportFactory* wrapped) + : wrapped_factory_(wrapped) {} + RTCErrorOr> WrapperMediaTransportFactory::CreateMediaTransport( rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, const MediaTransportSettings& settings) { + created_transport_count_++; + if (wrapped_factory_) { + return wrapped_factory_->CreateMediaTransport(packet_transport, + network_thread, settings); + } return {absl::make_unique(wrapped_)}; } +std::string WrapperMediaTransportFactory::GetTransportName() const { + if (wrapped_factory_) { + return wrapped_factory_->GetTransportName(); + } + return "wrapped-transport"; +} + +int WrapperMediaTransportFactory::created_transport_count() const { + return created_transport_count_; +} + +RTCErrorOr> +WrapperMediaTransportFactory::CreateMediaTransport( + rtc::Thread* network_thread, + const MediaTransportSettings& settings) { + created_transport_count_++; + if (wrapped_factory_) { + return wrapped_factory_->CreateMediaTransport(network_thread, settings); + } + return {absl::make_unique(wrapped_)}; +} + +MediaTransportPair::MediaTransportPair(rtc::Thread* thread) + : first_(thread, &second_), + second_(thread, &first_), + first_factory_(&first_), + second_factory_(&second_) {} + +MediaTransportPair::~MediaTransportPair() = default; + MediaTransportPair::LoopbackMediaTransport::LoopbackMediaTransport( rtc::Thread* thread, LoopbackMediaTransport* other) - : thread_(thread), other_(other) {} + : thread_(thread), other_(other) { + RTC_LOG(LS_INFO) << "LoopbackMediaTransport"; +} MediaTransportPair::LoopbackMediaTransport::~LoopbackMediaTransport() { + RTC_LOG(LS_INFO) << "~LoopbackMediaTransport"; rtc::CritScope lock(&sink_lock_); RTC_CHECK(audio_sink_ == nullptr); RTC_CHECK(video_sink_ == nullptr); @@ -120,6 +166,12 @@ MediaTransportPair::LoopbackMediaTransport::~LoopbackMediaTransport() { RTC_CHECK(rtt_observers_.empty()); } +absl::optional +MediaTransportPair::LoopbackMediaTransport::GetTransportParametersOffer() + const { + return "loopback-media-transport-parameters"; +} + RTCError MediaTransportPair::LoopbackMediaTransport::SendAudioFrame( uint64_t channel_id, MediaTransportEncodedAudioFrame frame) { diff --git a/api/test/loopback_media_transport.h b/api/test/loopback_media_transport.h index 280dc9aaa9..2972b49e0e 100644 --- a/api/test/loopback_media_transport.h +++ b/api/test/loopback_media_transport.h @@ -12,6 +12,7 @@ #define API_TEST_LOOPBACK_MEDIA_TRANSPORT_H_ #include +#include #include #include @@ -24,19 +25,43 @@ namespace webrtc { -// Wrapper used to hand out unique_ptrs to loopback media transports without -// ownership changes to the underlying transport. +// Wrapper used to hand out unique_ptrs to loopback media +// transport without ownership changes to the underlying +// transport. +// It works in two modes: +// It can either wrap a factory, or it can wrap an existing interface. +// In the former mode, it delegates the work to the wrapped factory. +// In the latter mode, it always returns static instance of the transport +// interface. +// +// Example use: +// Factory wrap_static_interface = Wrapper(media_transport_interface); +// Factory wrap_factory = Wrapper(wrap_static_interface); +// The second factory may be created multiple times, and ownership may be passed +// to the client. The first factory counts the number of invocations of +// CreateMediaTransport(); class WrapperMediaTransportFactory : public MediaTransportFactory { public: explicit WrapperMediaTransportFactory(MediaTransportInterface* wrapped); + explicit WrapperMediaTransportFactory(MediaTransportFactory* wrapped); RTCErrorOr> CreateMediaTransport( rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, const MediaTransportSettings& settings) override; + RTCErrorOr> CreateMediaTransport( + rtc::Thread* network_thread, + const MediaTransportSettings& settings) override; + + std::string GetTransportName() const override; + + int created_transport_count() const; + private: MediaTransportInterface* wrapped_; + MediaTransportFactory* wrapped_factory_ = nullptr; + int created_transport_count_ = 0; }; // Contains two MediaTransportsInterfaces that are connected to each other. @@ -50,19 +75,19 @@ class MediaTransportPair { int received_video_frames = 0; }; - explicit MediaTransportPair(rtc::Thread* thread) - : first_(thread, &second_), second_(thread, &first_) {} + explicit MediaTransportPair(rtc::Thread* thread); + ~MediaTransportPair(); // Ownership stays with MediaTransportPair MediaTransportInterface* first() { return &first_; } MediaTransportInterface* second() { return &second_; } std::unique_ptr first_factory() { - return absl::make_unique(&first_); + return absl::make_unique(&first_factory_); } std::unique_ptr second_factory() { - return absl::make_unique(&second_); + return absl::make_unique(&second_factory_); } void SetState(MediaTransportState state) { @@ -78,6 +103,14 @@ class MediaTransportPair { Stats FirstStats() { return first_.GetStats(); } Stats SecondStats() { return second_.GetStats(); } + int first_factory_transport_count() const { + return first_factory_.created_transport_count(); + } + + int second_factory_transport_count() const { + return second_factory_.created_transport_count(); + } + private: class LoopbackMediaTransport : public MediaTransportInterface { public: @@ -132,6 +165,8 @@ class MediaTransportPair { void SetAllocatedBitrateLimits( const MediaTransportAllocatedBitrateLimits& limits) override; + absl::optional GetTransportParametersOffer() const override; + private: void OnData(uint64_t channel_id, MediaTransportEncodedAudioFrame frame); @@ -180,6 +215,8 @@ class MediaTransportPair { LoopbackMediaTransport first_; LoopbackMediaTransport second_; + WrapperMediaTransportFactory first_factory_; + WrapperMediaTransportFactory second_factory_; }; } // namespace webrtc diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 90cb11febb..24834321e5 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -275,6 +275,7 @@ if (rtc_include_tests) { "../api:fake_media_transport", "../api:ice_transport_factory", "../api:libjingle_peerconnection_api", + "../api:loopback_media_transport", "../call:rtp_interfaces", "../call:rtp_receiver", "../logging:rtc_event_log_api", diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 97f6bdde46..73455d5991 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -959,8 +959,7 @@ std::unique_ptr JsepTransportController::MaybeCreateMediaTransport( const cricket::ContentInfo& content_info, const cricket::SessionDescription& description, - bool local, - cricket::IceTransportInternal* ice_transport) { + bool local) { if (!is_media_transport_factory_enabled_) { return nullptr; } @@ -973,75 +972,40 @@ JsepTransportController::MaybeCreateMediaTransport( return nullptr; } - absl::optional selected_crypto_for_media_transport; - if (content_info.media_description() && - !content_info.media_description()->cryptos().empty()) { - // Order of cryptos is deterministic (rfc4568, 5.1.1), so we just select the - // first one (in fact the first one should be the most preferred one.) We - // ignore the HMAC size, as media transport crypto settings currently don't - // expose HMAC size, nor crypto protocol for that matter. - selected_crypto_for_media_transport = - content_info.media_description()->cryptos()[0]; + // Caller (offerer) media transport. + if (local) { + if (offer_media_transport_) { + RTC_LOG(LS_INFO) << "Offered media transport has now been activated."; + return std::move(offer_media_transport_); + } else { + RTC_LOG(LS_INFO) + << "Not returning media transport. Either SDES wasn't enabled, or " + "media transport didn't return an offer earlier."; + // Offer wasn't generated. Either because media transport didn't want it, + // or because SDES wasn't enabled. + return nullptr; + } } - if (!selected_crypto_for_media_transport.has_value()) { - RTC_LOG(LS_WARNING) << "a=cryto line was not found in the offer. Most " - "likely you did not enable SDES. " - "Make sure to pass config.enable_dtls_srtp=false " - "to RTCConfiguration. " - "Cannot continue with media transport. Falling " - "back to RTP. is_local=" - << local; - - // Remove media_transport_factory from config, because we don't want to - // use it on the subsequent call (for the other side of the offer). - is_media_transport_factory_enabled_ = false; + // Remote offer. If no x-mt lines, do not create media transport. + if (description.MediaTransportSettings().empty()) { return nullptr; } - // TODO(psla): This code will be removed in favor of media transport settings. - // Note that we ignore here lifetime and length. - // In fact we take those bits (inline, lifetime and length) and keep it as - // part of key derivation. - // - // Technically, we are also not following rfc4568, which requires us to - // send and answer with the key that we chose. In practice, for media - // transport, the current approach should be sufficient (we take the key - // that sender offered, and caller assumes we will use it. We are not - // signaling back that we indeed used it.) - std::unique_ptr key_derivation = - rtc::KeyDerivation::Create(rtc::KeyDerivationAlgorithm::HKDF_SHA256); - const std::string label = "MediaTransportLabel"; - constexpr int kDerivedKeyByteSize = 32; + // When bundle is enabled, two JsepTransports are created, and then + // the second transport is destroyed (right away). + // For media transport, we don't want to create the second + // media transport in the first place. + RTC_LOG(LS_INFO) << "Returning new, client media transport."; - int key_len, salt_len; - if (!rtc::GetSrtpKeyAndSaltLengths( - rtc::SrtpCryptoSuiteFromName( - selected_crypto_for_media_transport.value().cipher_suite), - &key_len, &salt_len)) { - RTC_CHECK(false) << "Cannot set up secure media transport"; - } - rtc::ZeroOnFreeBuffer raw_key(key_len + salt_len); - - cricket::SrtpFilter::ParseKeyParams( - selected_crypto_for_media_transport.value().key_params, raw_key.data(), - raw_key.size()); - absl::optional> key = - key_derivation->DeriveKey( - raw_key, - /*salt=*/nullptr, - rtc::ArrayView( - reinterpret_cast(label.data()), label.size()), - kDerivedKeyByteSize); - // TODO(psla): End of the code to be removed. - - // We want to crash the app if we don't have a key, and not silently fall - // back to the unsecure communication. - RTC_CHECK(key.has_value()); + RTC_DCHECK(!local) + << "If media transport is used, you must call " + "GenerateOrGetLastMediaTransportOffer before SetLocalDescription. You " + "also " + "must use kRtcpMuxPolicyRequire and kBundlePolicyMaxBundle with media " + "transport."; MediaTransportSettings settings; settings.is_caller = local; - settings.pre_shared_key = std::string( - reinterpret_cast(key.value().data()), key.value().size()); if (config_.use_media_transport_for_media) { settings.event_log = config_.event_log; } @@ -1055,8 +1019,8 @@ JsepTransportController::MaybeCreateMediaTransport( } auto media_transport_result = - config_.media_transport_factory->CreateMediaTransport( - ice_transport, network_thread_, settings); + config_.media_transport_factory->CreateMediaTransport(network_thread_, + settings); // TODO(sukhanov): Proper error handling. RTC_CHECK(media_transport_result.ok()); @@ -1086,7 +1050,11 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( CreateIceTransport(content_info.name, /*rtcp=*/false); std::unique_ptr media_transport = - MaybeCreateMediaTransport(content_info, description, local, ice.get()); + MaybeCreateMediaTransport(content_info, description, local); + if (media_transport) { + media_transport_created_once_ = true; + media_transport->Connect(ice.get()); + } std::unique_ptr rtp_dtls_transport = CreateDtlsTransport(std::move(ice)); @@ -1527,4 +1495,58 @@ void JsepTransportController::OnDtlsHandshakeError( SignalDtlsHandshakeError(error); } +absl::optional +JsepTransportController::GenerateOrGetLastMediaTransportOffer() { + 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_; + } + + RTC_LOG(LS_INFO) << "Generating media transport offer!"; + // Check that media transport is supposed to be used. + if (config_.use_media_transport_for_media || + config_.use_media_transport_for_data_channels) { + RTC_DCHECK(config_.media_transport_factory != nullptr); + // ICE is not available when media transport is created. It will only be + // available in 'Connect'. This may be a potential server config, if we + // decide to use this peer connection as a caller, not as a callee. + webrtc::MediaTransportSettings settings; + settings.is_caller = true; + settings.pre_shared_key = rtc::CreateRandomString(32); + settings.event_log = config_.event_log; + auto media_transport_or_error = + config_.media_transport_factory->CreateMediaTransport(network_thread_, + settings); + + if (media_transport_or_error.ok()) { + offer_media_transport_ = std::move(media_transport_or_error.value()); + } else { + RTC_LOG(LS_INFO) << "Unable to create media transport, error=" + << media_transport_or_error.error().message(); + } + } + + if (!offer_media_transport_) { + RTC_LOG(LS_INFO) << "Media transport doesn't exist"; + return absl::nullopt; + } + + absl::optional transport_parameters = + offer_media_transport_->GetTransportParametersOffer(); + if (!transport_parameters) { + RTC_LOG(LS_INFO) << "Media transport didn't generate the offer"; + // Media transport didn't generate the offer, and is not supposed to be + // used. Destroy the temporary media transport. + offer_media_transport_ = nullptr; + return absl::nullopt; + } + + cricket::SessionDescription::MediaTransportSetting setting; + setting.transport_name = config_.media_transport_factory->GetTransportName(); + setting.transport_setting = *transport_parameters; + media_transport_offer_settings_ = setting; + return setting; +} + } // namespace webrtc diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index c98ff25cc3..537b2b1d86 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -188,6 +188,13 @@ class JsepTransportController : public sigslot::has_slots<> { void SetMediaTransportSettings(bool use_media_transport_for_media, bool use_media_transport_for_data_channels); + // If media transport is present enabled and supported, + // when this method is called, it creates a media transport and generates its + // offer. The new offer is then returned, and the created media transport will + // subsequently be used. + absl::optional + GenerateOrGetLastMediaTransportOffer(); + // All of these signals are fired on the signaling thread. // If any transport failed => failed, @@ -288,14 +295,15 @@ class JsepTransportController : public sigslot::has_slots<> { const cricket::ContentInfo& content_info, const cricket::SessionDescription& description); - // Creates media transport if config wants to use it, and pre-shared key is - // provided in content info. It modifies the config to disable media transport - // if pre-shared key is not provided. + // Creates media transport if config wants to use it, and a=x-mt line is + // present for the current media transport. Returned MediaTransportInterface + // is not connected, and must be connected to ICE. You must call + // |GenerateOrGetLastMediaTransportOffer| on the caller before calling + // MaybeCreateMediaTransport. std::unique_ptr MaybeCreateMediaTransport( const cricket::ContentInfo& content_info, const cricket::SessionDescription& description, - bool local, - cricket::IceTransportInternal* ice_transport); + bool local); void MaybeDestroyJsepTransport(const std::string& mid); void DestroyAllJsepTransports_n(); @@ -377,6 +385,35 @@ class JsepTransportController : public sigslot::has_slots<> { // (the factory needs to be provided in the config, and config must allow for // media transport). bool is_media_transport_factory_enabled_ = true; + + // Early on in the call we don't know if media transport is going to be used, + // but we need to get the server-supported parameters to add to an SDP. + // This server media transport will be promoted to the used media transport + // after the local description is set, and the ownership will be transferred + // to the actual JsepTransport. + // This "offer" media transport is not created if it's done on the party that + // provides answer. This offer media transport is only created once at the + // beginning of the connection, and never again. + std::unique_ptr offer_media_transport_ = nullptr; + + // Contains the offer of the |offer_media_transport_|, in case if it needs to + // be repeated. + absl::optional + media_transport_offer_settings_; + + // When the new offer is regenerated (due to upgrade), we don't want to + // re-create media transport. New streams might be created; but media + // transport stays the same. This flag prevents re-creation of the transport + // on the offerer. + // The first media transport is created in jsep transport controller as the + // |offer_media_transport_|, and then the ownership is moved to the + // appropriate JsepTransport, at which point |offer_media_transport_| is + // zeroed out. On the callee (answerer), the first media transport is not even + // assigned to |offer_media_transport_|. Both offerer and answerer can + // 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; + const cricket::SessionDescription* local_desc_ = nullptr; const cricket::SessionDescription* remote_desc_ = nullptr; absl::optional initial_offerer_; diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 0bd55b53b7..64da95b9b7 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -14,6 +14,7 @@ #include "absl/memory/memory.h" #include "api/media_transport_interface.h" #include "api/test/fake_media_transport.h" +#include "api/test/loopback_media_transport.h" #include "p2p/base/fake_dtls_transport.h" #include "p2p/base/fake_ice_transport.h" #include "p2p/base/no_op_dtls_transport.h" @@ -425,12 +426,16 @@ TEST_F(JsepTransportControllerTest, JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_data_channels = true; CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); + EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -458,6 +463,7 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_data_channels = true; config.use_media_transport_for_media = true; @@ -465,6 +471,9 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); + EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -476,7 +485,9 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { // After SetLocalDescription, media transport should be created as caller. EXPECT_TRUE(media_transport->is_caller()); + // We set the pre-shared key on the caller. EXPECT_TRUE(media_transport->pre_shared_key().has_value()); + EXPECT_TRUE(media_transport->is_connected()); // Return nullptr for non-existing mids. EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); @@ -486,6 +497,43 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { << "Because media transport is used, expected no-op DTLS transport."; } +TEST_F(JsepTransportControllerTest, + GetMediaTransportOfferInTheConfigOnSubsequentCalls) { + FakeMediaTransportFactory fake_media_transport_factory; + WrapperMediaTransportFactory wrapping_factory(&fake_media_transport_factory); + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + config.media_transport_factory = &wrapping_factory; + config.use_media_transport_for_data_channels = true; + config.use_media_transport_for_media = true; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + + absl::optional settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + ASSERT_NE(absl::nullopt, settings); + + EXPECT_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + absl::optional + new_settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + ASSERT_NE(absl::nullopt, new_settings); + EXPECT_EQ(settings->transport_name, new_settings->transport_name); + EXPECT_EQ(settings->transport_setting, new_settings->transport_setting); + EXPECT_EQ(1, wrapping_factory.created_transport_count()); +} + TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; @@ -497,6 +545,7 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + description->AddMediaTransportSetting("fake", "fake-remote-settings"); EXPECT_TRUE(transport_controller_ ->SetRemoteDescription(SdpType::kOffer, description.get()) .ok()); @@ -508,8 +557,13 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { // After SetRemoteDescription, media transport should be created as callee. EXPECT_FALSE(media_transport->is_caller()); - EXPECT_TRUE(media_transport->pre_shared_key().has_value()); + // We do not set pre-shared key on the callee, it comes in media transport + // settings. + EXPECT_EQ(absl::nullopt, media_transport->settings().pre_shared_key); + EXPECT_TRUE(media_transport->is_connected()); + EXPECT_EQ("fake-remote-settings", + media_transport->remote_transport_parameters()); // Return nullptr for non-existing mids. EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); @@ -543,6 +597,75 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCalleePassesSdp) { media_transport->settings().remote_transport_parameters); } +// Caller generates the offer if media transport returns empty offer (no +// parameters). +TEST_F(JsepTransportControllerTest, MediaTransportGeneratesSessionDescription) { + FakeMediaTransportFactory fake_media_transport_factory( + /*transport_offer=*/""); + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_data_channels = true; + config.use_media_transport_for_media = true; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + absl::optional settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + + ASSERT_TRUE(settings.has_value()); + EXPECT_EQ("fake", settings->transport_name); + // Fake media transport returns empty settings (but not nullopt settings!) + EXPECT_EQ("", settings->transport_setting); +} + +// Caller generates the offer if media transport returns offer with parameters. +TEST_F(JsepTransportControllerTest, + MediaTransportGeneratesSessionDescriptionWithOfferParams) { + FakeMediaTransportFactory fake_media_transport_factory( + /*transport_offer=*/"offer-params"); + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_data_channels = true; + config.use_media_transport_for_media = true; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + absl::optional settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + + ASSERT_TRUE(settings.has_value()); + EXPECT_EQ("fake", settings->transport_name); + EXPECT_EQ("offer-params", settings->transport_setting); +} + +// Caller skips the offer if media transport requests it. +TEST_F(JsepTransportControllerTest, + MediaTransportGeneratesSkipsSessionDescription) { + FakeMediaTransportFactory fake_media_transport_factory( + /*transport_offer=*/absl::nullopt); + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_data_channels = true; + config.use_media_transport_for_media = true; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + absl::optional settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + + // Fake media transport returns nullopt settings + ASSERT_EQ(absl::nullopt, settings); +} + // Caller ignores its own outgoing parameters. TEST_F(JsepTransportControllerTest, GetMediaTransportInCallerIgnoresXmtSection) { @@ -550,13 +673,15 @@ TEST_F(JsepTransportControllerTest, JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_data_channels = true; config.use_media_transport_for_media = true; CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); - description->AddMediaTransportSetting("fake", "this-is-a-test-setting"); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -578,6 +703,7 @@ TEST_F(JsepTransportControllerTest, JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_data_channels = true; config.use_media_transport_for_media = true; @@ -604,10 +730,11 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_media = true; CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithoutBundle(); + auto description = CreateSessionDescriptionWithBundleGroup(); EXPECT_TRUE(transport_controller_ ->SetRemoteDescription(SdpType::kOffer, description.get()) .ok()); @@ -616,7 +743,7 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { // Even if we set local description with crypto now (after the remote offer // was set), media transport won't be provided. - auto description2 = CreateSessionDescriptionWithoutBundle(); + auto description2 = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description2.get()); EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kAnswer, description2.get()) @@ -630,28 +757,31 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { } TEST_F(JsepTransportControllerTest, - AfterSettingAnswerTheSameMediaTransportIsReturned) { + AfterSettingAnswerTheSameMediaTransportIsReturnedCallee) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.media_transport_factory = &fake_media_transport_factory; config.use_media_transport_for_media = true; CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithoutBundle(); + auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + description->AddMediaTransportSetting("fake", "fake-settings"); EXPECT_TRUE(transport_controller_ ->SetRemoteDescription(SdpType::kOffer, description.get()) .ok()); - FakeMediaTransport* media_transport = static_cast( transport_controller_->GetMediaTransport(kAudioMid1)); EXPECT_NE(nullptr, media_transport); - EXPECT_TRUE(media_transport->pre_shared_key().has_value()); + EXPECT_FALSE(media_transport->pre_shared_key().has_value()) + << "On the callee, preshared key is passed through the media-transport " + "settings (x-mt)"; // Even if we set local description with crypto now (after the remote offer // was set), media transport won't be provided. - auto description2 = CreateSessionDescriptionWithoutBundle(); + auto description2 = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description2.get()); RTCError result = transport_controller_->SetLocalDescription( @@ -946,10 +1076,12 @@ TEST_F(JsepTransportControllerTest, } TEST_F(JsepTransportControllerTest, - SignalConnectionStateConnectedWithMediaTransportAndNoDtls) { + SignalConnectionStateConnectedWithMediaTransportAndNoDtlsCaller) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; config.media_transport_factory = &fake_media_transport_factory; + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.use_media_transport_for_data_channels = true; config.use_media_transport_for_media = true; CreateJsepTransportController(config); @@ -957,6 +1089,8 @@ TEST_F(JsepTransportControllerTest, // Media Transport is only used with bundle. auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -965,6 +1099,7 @@ TEST_F(JsepTransportControllerTest, transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); auto fake_video_ice = static_cast( transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport()); + EXPECT_EQ(fake_audio_ice, fake_video_ice); fake_audio_ice->SetConnectionCount(2); fake_audio_ice->SetConnectionCount(1); fake_video_ice->SetConnectionCount(2); @@ -979,29 +1114,28 @@ TEST_F(JsepTransportControllerTest, FakeMediaTransport* media_transport = static_cast( transport_controller_->GetMediaTransport(kAudioMid1)); - media_transport->SetState(webrtc::MediaTransportState::kWritable); - EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, - kTimeout); + ASSERT_NE(nullptr, media_transport); - // Still waiting for the second media transport. - media_transport = static_cast( - transport_controller_->GetMediaTransport(kVideoMid1)); media_transport->SetState(webrtc::MediaTransportState::kWritable); - + // Only one media transport. EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); } TEST_F(JsepTransportControllerTest, - SignalConnectionStateConnectedWithMediaTransport) { + SignalConnectionStateConnectedWithMediaTransportCaller) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; config.media_transport_factory = &fake_media_transport_factory; + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.use_media_transport_for_media = true; CreateJsepTransportController(config); // Media Transport is only used with bundle. auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -1031,6 +1165,8 @@ TEST_F(JsepTransportControllerTest, FakeMediaTransport* media_transport = static_cast( transport_controller_->GetMediaTransport(kAudioMid1)); + ASSERT_NE(nullptr, media_transport); + media_transport->SetState(webrtc::MediaTransportState::kWritable); EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, kTimeout); @@ -1044,14 +1180,18 @@ TEST_F(JsepTransportControllerTest, } TEST_F(JsepTransportControllerTest, - SignalConnectionStateFailedWhenMediaTransportClosed) { + SignalConnectionStateFailedWhenMediaTransportClosedCaller) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; config.media_transport_factory = &fake_media_transport_factory; + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; config.use_media_transport_for_media = true; CreateJsepTransportController(config); - auto description = CreateSessionDescriptionWithoutBundle(); + auto description = CreateSessionDescriptionWithBundleGroup(); AddCryptoSettings(description.get()); + EXPECT_NE(absl::nullopt, + transport_controller_->GenerateOrGetLastMediaTransportOffer()); EXPECT_TRUE(transport_controller_ ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); @@ -1078,11 +1218,12 @@ TEST_F(JsepTransportControllerTest, FakeMediaTransport* media_transport = static_cast( transport_controller_->GetMediaTransport(kAudioMid1)); - + ASSERT_NE(nullptr, media_transport); media_transport->SetState(webrtc::MediaTransportState::kWritable); media_transport = static_cast( transport_controller_->GetMediaTransport(kVideoMid1)); + ASSERT_NE(nullptr, media_transport); media_transport->SetState(webrtc::MediaTransportState::kWritable); diff --git a/pc/media_session.cc b/pc/media_session.cc index 5e6b97bb43..ca4434e0d5 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1542,6 +1542,12 @@ std::unique_ptr MediaSessionDescriptionFactory::CreateOffer( offer->set_extmap_allow_mixed(session_options.offer_extmap_allow_mixed); + if (session_options.media_transport_settings.has_value()) { + offer->AddMediaTransportSetting( + session_options.media_transport_settings->transport_name, + session_options.media_transport_settings->transport_setting); + } + return offer; } diff --git a/pc/media_session.h b/pc/media_session.h index a459c4c07c..33c8c17d37 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -110,6 +110,12 @@ struct MediaSessionOptions { // descriptions will be generated. std::vector media_description_options; std::vector pooled_ice_credentials; + + // An optional media transport settings. + // In the future we may consider using a vector here, to indicate multiple + // supported transports. + absl::optional + media_transport_settings; }; // Creates media session descriptions according to the supplied codecs and diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 44617dd822..9c4461d9ec 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1001,6 +1001,23 @@ bool PeerConnection::Initialize( return false; } + if (configuration.use_media_transport || + configuration.use_media_transport_for_data_channels) { + // TODO(bugs.webrtc.org/9719): This check will eventually go away, when + // RTP media transport is introduced. But until then, we require SDES to + // be enabled. + if (configuration.enable_dtls_srtp.has_value() && + configuration.enable_dtls_srtp.value()) { + RTC_LOG(LS_WARNING) + << "When media transport is used, SDES must be enabled. Set " + "configuration.enable_dtls_srtp to false. use_media_transport=" + << configuration.use_media_transport + << ", use_media_transport_for_data_channels=" + << configuration.use_media_transport_for_data_channels; + return false; + } + } + config.use_media_transport_for_media = configuration.use_media_transport; config.use_media_transport_for_data_channels = configuration.use_media_transport_for_data_channels; @@ -4140,6 +4157,12 @@ void PeerConnection::GetOptionsForOffer( port_allocator_.get())); session_options->offer_extmap_allow_mixed = configuration_.offer_extmap_allow_mixed; + + if (configuration_.enable_dtls_srtp && + !configuration_.enable_dtls_srtp.value()) { + session_options->media_transport_settings = + transport_controller_->GenerateOrGetLastMediaTransportOffer(); + } } void PeerConnection::GetOptionsForPlanBOffer( @@ -6352,7 +6375,8 @@ bool PeerConnection::SetupMediaTransportForDataChannels_n( const std::string& mid) { media_transport_ = transport_controller_->GetMediaTransport(mid); if (!media_transport_) { - RTC_LOG(LS_ERROR) << "Media transport is not available for data channels"; + RTC_LOG(LS_ERROR) + << "Media transport is not available for data channels, mid=" << mid; return false; } diff --git a/pc/peer_connection_data_channel_unittest.cc b/pc/peer_connection_data_channel_unittest.cc index e0ceb72560..ad3817e5b5 100644 --- a/pc/peer_connection_data_channel_unittest.cc +++ b/pc/peer_connection_data_channel_unittest.cc @@ -403,17 +403,15 @@ TEST_P(PeerConnectionDataChannelTest, EXPECT_EQ(CreatePeerConnection(config), nullptr); } -TEST_P(PeerConnectionDataChannelTest, - MediaTransportDataChannelFailsWithoutSdes) { +// This test now DCHECKs, instead of failing to SetLocalDescription. +TEST_P(PeerConnectionDataChannelTest, MediaTransportWithoutSdesFails) { RTCConfiguration config; config.use_media_transport_for_data_channels = true; config.enable_dtls_srtp = true; // Disables SDES for data sections. + auto caller = CreatePeerConnectionWithDataChannel(config); - std::string error; - ASSERT_FALSE(caller->SetLocalDescription(caller->CreateOffer(), &error)); - EXPECT_EQ(error, - "Failed to set local offer sdp: Failed to create data channel."); + EXPECT_EQ(nullptr, caller); } INSTANTIATE_TEST_SUITE_P(PeerConnectionDataChannelTest, diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 7927c1ee6e..f87a4476f2 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -3465,6 +3465,8 @@ TEST_P(PeerConnectionIntegrationTest, // channel. TEST_P(PeerConnectionIntegrationTest, MediaTransportDataChannelEndToEnd) { PeerConnectionInterface::RTCConfiguration rtc_config; + rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; rtc_config.use_media_transport_for_data_channels = true; rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( @@ -3566,8 +3568,86 @@ TEST_P(PeerConnectionIntegrationTest, EXPECT_FALSE(callee()->data_channel()->negotiated()); } +TEST_P(PeerConnectionIntegrationTest, MediaTransportOfferUpgrade) { + PeerConnectionInterface::RTCConfiguration rtc_config; + rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + rtc_config.use_media_transport = true; + rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. + ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( + rtc_config, rtc_config, loopback_media_transports()->first_factory(), + loopback_media_transports()->second_factory())); + ConnectFakeSignaling(); + + // Do initial offer/answer with just a video track. + caller()->AddVideoTrack(); + callee()->AddVideoTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Ensure that the media transport is ready. + loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); + loopback_media_transports()->FlushAsyncInvokes(); + + // Now add an audio track and do another offer/answer. + caller()->AddAudioTrack(); + callee()->AddAudioTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Ensure both audio and video frames are received end-to-end. + MediaExpectations media_expectations; + media_expectations.ExpectBidirectionalAudioAndVideo(); + ASSERT_TRUE(ExpectNewFrames(media_expectations)); + + // The second offer should not have generated another media transport. + // Media transport was kept alive, and was not recreated. + EXPECT_EQ(1, loopback_media_transports()->first_factory_transport_count()); + EXPECT_EQ(1, loopback_media_transports()->second_factory_transport_count()); +} + +TEST_P(PeerConnectionIntegrationTest, MediaTransportOfferUpgradeOnTheCallee) { + PeerConnectionInterface::RTCConfiguration rtc_config; + rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; + rtc_config.use_media_transport = true; + rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. + ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( + rtc_config, rtc_config, loopback_media_transports()->first_factory(), + loopback_media_transports()->second_factory())); + ConnectFakeSignaling(); + + // Do initial offer/answer with just a video track. + caller()->AddVideoTrack(); + callee()->AddVideoTrack(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Ensure that the media transport is ready. + loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable); + loopback_media_transports()->FlushAsyncInvokes(); + + // Now add an audio track and do another offer/answer. + caller()->AddAudioTrack(); + callee()->AddAudioTrack(); + callee()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + // Ensure both audio and video frames are received end-to-end. + MediaExpectations media_expectations; + media_expectations.ExpectBidirectionalAudioAndVideo(); + ASSERT_TRUE(ExpectNewFrames(media_expectations)); + + // The second offer should not have generated another media transport. + // Media transport was kept alive, and was not recreated. + EXPECT_EQ(1, loopback_media_transports()->first_factory_transport_count()); + EXPECT_EQ(1, loopback_media_transports()->second_factory_transport_count()); +} + TEST_P(PeerConnectionIntegrationTest, MediaTransportBidirectionalAudio) { PeerConnectionInterface::RTCConfiguration rtc_config; + rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; rtc_config.use_media_transport = true; rtc_config.enable_dtls_srtp = false; // SDES is required for media transport. ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory( diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc index 7d4b464937..006e4a1c38 100644 --- a/pc/webrtc_sdp.cc +++ b/pc/webrtc_sdp.cc @@ -532,6 +532,17 @@ static void InitAttrLine(const std::string& attribute, rtc::StringBuilder* os) { InitLine(kLineTypeAttributes, attribute, os); } +// Writes an x-mt SDP attribute line based on the media transport settings. +static void AddMediaTransportLine( + const cricket::SessionDescription::MediaTransportSetting& setting, + std::string* message) { + rtc::StringBuilder os; + InitAttrLine(kMediaTransportSettingLine, &os); + os << kSdpDelimiterColon << setting.transport_name << kSdpDelimiterColon + << rtc::Base64::Encode(setting.transport_setting); + 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, @@ -873,6 +884,11 @@ std::string SdpSerialize(const JsepSessionDescription& jdesc) { // Time Description. AddLine(kTimeDescription, &message); + for (const cricket::SessionDescription::MediaTransportSetting& settings : + desc->MediaTransportSettings()) { + AddMediaTransportLine(settings, &message); + } + // Group if (desc->HasGroup(cricket::GROUP_TYPE_BUNDLE)) { std::string group_line = kAttrGroup; diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc index 117269a80a..6b3868978f 100644 --- a/pc/webrtc_sdp_unittest.cc +++ b/pc/webrtc_sdp_unittest.cc @@ -18,6 +18,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/memory/memory.h" #include "absl/strings/str_replace.h" #include "api/array_view.h" #include "api/crypto_params.h" @@ -4418,3 +4419,25 @@ TEST_F(WebRtcSdpTest, ParseMediaTransportIgnoreNonsenseAttributeLines) { << error.description; EXPECT_TRUE(output.description()->MediaTransportSettings().empty()); } + +TEST_F(WebRtcSdpTest, SerializeMediaTransportSettings) { + cricket::SessionDescription* description = new cricket::SessionDescription(); + + JsepSessionDescription output(SdpType::kOffer); + // JsepSessionDescription takes ownership of the description. + output.Initialize(description, "session_id", "session_version"); + output.description()->AddMediaTransportSetting("foo", "bar"); + std::string serialized_out; + output.ToString(&serialized_out); + ASSERT_THAT(serialized_out, ::testing::HasSubstr("\r\na=x-mt:foo:YmFy\r\n")); +} + +TEST_F(WebRtcSdpTest, SerializeMediaTransportSettingsTestCopy) { + cricket::SessionDescription description; + description.AddMediaTransportSetting("name", "setting"); + std::unique_ptr copy = + absl::WrapUnique(description.Copy()); + ASSERT_EQ(1u, copy->MediaTransportSettings().size()); + EXPECT_EQ("name", copy->MediaTransportSettings()[0].transport_name); + EXPECT_EQ("setting", copy->MediaTransportSettings()[0].transport_setting); +}