diff --git a/api/test/fake_media_transport.h b/api/test/fake_media_transport.h index 16cd1bad87..6f87dfcb01 100644 --- a/api/test/fake_media_transport.h +++ b/api/test/fake_media_transport.h @@ -98,6 +98,9 @@ class FakeMediaTransport : public MediaTransportInterface { int target_rate_observers_size() { return target_rate_observers_.size(); } + // Settings that were passed down to fake media transport. + const MediaTransportSettings& settings() { return settings_; } + private: const MediaTransportSettings settings_; MediaTransportStateCallback* state_callback_; @@ -110,6 +113,8 @@ class FakeMediaTransportFactory : public MediaTransportFactory { FakeMediaTransportFactory() = default; ~FakeMediaTransportFactory() = default; + std::string GetTransportName() const override { return "fake"; } + RTCErrorOr> CreateMediaTransport( rtc::PacketTransportInternal* packet_transport, rtc::Thread* network_thread, diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index eb78f3473a..c1b8e4ed88 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -596,7 +596,7 @@ RTCError JsepTransportController::ApplyDescription_n( (IsBundled(content_info.name) && content_info.name != *bundled_mid())) { continue; } - error = MaybeCreateJsepTransport(local, content_info); + error = MaybeCreateJsepTransport(local, content_info, *description); if (!error.ok()) { return error; } @@ -958,6 +958,7 @@ cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( std::unique_ptr JsepTransportController::MaybeCreateMediaTransport( const cricket::ContentInfo& content_info, + const cricket::SessionDescription& description, bool local, cricket::IceTransportInternal* ice_transport) { if (!is_media_transport_factory_enabled_) { @@ -998,6 +999,7 @@ JsepTransportController::MaybeCreateMediaTransport( 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. @@ -1031,6 +1033,7 @@ JsepTransportController::MaybeCreateMediaTransport( 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. @@ -1042,6 +1045,15 @@ JsepTransportController::MaybeCreateMediaTransport( if (config_.use_media_transport_for_media) { 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 media_transport_result = config_.media_transport_factory->CreateMediaTransport( ice_transport, network_thread_, settings); @@ -1054,7 +1066,8 @@ JsepTransportController::MaybeCreateMediaTransport( RTCError JsepTransportController::MaybeCreateJsepTransport( bool local, - const cricket::ContentInfo& content_info) { + const cricket::ContentInfo& content_info, + const cricket::SessionDescription& description) { RTC_DCHECK(network_thread_->IsCurrent()); cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); if (transport) { @@ -1073,7 +1086,7 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( CreateIceTransport(content_info.name, /*rtcp=*/false); std::unique_ptr media_transport = - MaybeCreateMediaTransport(content_info, local, ice.get()); + MaybeCreateMediaTransport(content_info, description, local, ice.get()); std::unique_ptr rtp_dtls_transport = CreateDtlsTransport(std::move(ice)); diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index f28c10c2db..1363907377 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -283,14 +283,17 @@ class JsepTransportController : public sigslot::has_slots<> { // Transport is created either during SetLocalDescription (|local| == true) or // during SetRemoteDescription (|local| == false). Passing |local| helps to // differentiate initiator (caller) from answerer (callee). - RTCError MaybeCreateJsepTransport(bool local, - const cricket::ContentInfo& content_info); + RTCError MaybeCreateJsepTransport( + bool local, + 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. std::unique_ptr MaybeCreateMediaTransport( const cricket::ContentInfo& content_info, + const cricket::SessionDescription& description, bool local, cricket::IceTransportInternal* ice_transport); void MaybeDestroyJsepTransport(const std::string& mid); diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 43ba4ad6b4..590f57dab2 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -514,6 +514,87 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { << "Because media transport is used, expected no-op DTLS transport."; } +TEST_F(JsepTransportControllerTest, GetMediaTransportInCalleePassesSdp) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + 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_TRUE(transport_controller_ + ->SetRemoteDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + EXPECT_EQ("this-is-a-test-setting", + media_transport->settings().remote_transport_parameters); +} + +// Caller ignores its own outgoing parameters. +TEST_F(JsepTransportControllerTest, + GetMediaTransportInCallerIgnoresXmtSection) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + 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_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + // Remote parameters are nullopt, because we are the offerer (we don't) + // have the remote transport parameters, only ours. + EXPECT_EQ(absl::nullopt, + media_transport->settings().remote_transport_parameters); +} + +TEST_F(JsepTransportControllerTest, + GetMediaTransportInCalleeIgnoresDifferentTransport) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + + config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; + 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("not-a-fake-transport", + "this-is-a-test-setting"); + EXPECT_TRUE(transport_controller_ + ->SetRemoteDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + EXPECT_EQ(absl::nullopt, + media_transport->settings().remote_transport_parameters); +} + TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config;