diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 3115e25a4a..a429519459 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -602,9 +602,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { bool active_reset_srtp_params = false; // If MediaTransportFactory is provided in PeerConnectionFactory, this flag - // informs PeerConnection that it should use the MediaTransportInterface. - // It's invalid to set it to |true| if the MediaTransportFactory wasn't - // provided. + // informs PeerConnection that it should use the MediaTransportInterface for + // media (audio/video). It's invalid to set it to |true| if the + // MediaTransportFactory wasn't provided. bool use_media_transport = false; // If MediaTransportFactory is provided in PeerConnectionFactory, this flag diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 084cec5a34..9cecc105f0 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -402,13 +402,24 @@ void JsepTransportController::SetActiveResetSrtpParams( } } -void JsepTransportController::SetMediaTransportFactory( - MediaTransportFactory* media_transport_factory) { - RTC_DCHECK(media_transport_factory == config_.media_transport_factory || +void JsepTransportController::SetMediaTransportSettings( + bool use_media_transport_for_media, + bool use_media_transport_for_data_channels) { + RTC_DCHECK(use_media_transport_for_media == + config_.use_media_transport_for_media || jsep_transports_by_name_.empty()) - << "You can only call SetMediaTransportFactory before " - "JsepTransportController created its first transport."; - config_.media_transport_factory = media_transport_factory; + << "You can only change media transport configuration before creating " + "the first transport."; + + RTC_DCHECK(use_media_transport_for_data_channels == + config_.use_media_transport_for_data_channels || + jsep_transports_by_name_.empty()) + << "You can only change media transport configuration before creating " + "the first transport."; + + config_.use_media_transport_for_media = use_media_transport_for_media; + config_.use_media_transport_for_data_channels = + use_media_transport_for_data_channels; } std::unique_ptr @@ -433,7 +444,12 @@ JsepTransportController::CreateDtlsTransport( RTC_DCHECK(network_thread_->IsCurrent()); std::unique_ptr dtls; - if (is_media_transport_factory_enabled_ && config_.media_transport_factory) { + // If media transport is used for both media and data channels, + // then we don't need to create DTLS. + // Otherwise, DTLS is still created. + if (is_media_transport_factory_enabled_ && config_.media_transport_factory && + config_.use_media_transport_for_media && + config_.use_media_transport_for_data_channels) { dtls = absl::make_unique( std::move(ice), config_.crypto_options); } else if (config_.external_transport_factory) { @@ -954,6 +970,11 @@ JsepTransportController::MaybeCreateMediaTransport( return nullptr; } + if (!config_.use_media_transport_for_media && + !config_.use_media_transport_for_data_channels) { + return nullptr; + } + absl::optional selected_crypto_for_media_transport; if (content_info.media_description() && !content_info.media_description()->cryptos().empty()) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 08dea61414..f28c10c2db 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -83,10 +83,19 @@ class JsepTransportController : public sigslot::has_slots<> { bool active_reset_srtp_params = false; RtcEventLog* event_log = nullptr; + // Whether media transport is used for media. + bool use_media_transport_for_media = false; + + // Whether media transport is used for data channels. + bool use_media_transport_for_data_channels = false; + // Optional media transport factory (experimental). If provided it will be - // used to create media_transport and will be used to send / receive - // audio and video frames instead of RTP. Note that currently - // media_transport co-exists with RTP / RTCP transports and uses the same + // used to create media_transport (as long as either + // |use_media_transport_for_media| or + // |use_media_transport_for_data_channels| is set to true). However, whether + // it will be used to send / receive audio and video frames instead of RTP + // is determined by |use_media_transport_for_media|. Note that currently + // media_transport co-exists with RTP / RTCP transports and may use the same // underlying ICE transport. MediaTransportFactory* media_transport_factory = nullptr; }; @@ -173,10 +182,11 @@ class JsepTransportController : public sigslot::has_slots<> { void SetActiveResetSrtpParams(bool active_reset_srtp_params); // Allows to overwrite the settings from config. You may set or reset the - // media transport factory on the jsep transport controller, as long as you - // did not call 'GetMediaTransport' or 'MaybeCreateJsepTransport'. Once Jsep - // transport is created, you can't change this setting. - void SetMediaTransportFactory(MediaTransportFactory* media_transport_factory); + // media transport configuration on the jsep transport controller, as long as + // you did not call 'GetMediaTransport' or 'MaybeCreateJsepTransport'. Once + // Jsep transport is created, you can't change this setting. + void SetMediaTransportSettings(bool use_media_transport_for_media, + bool use_media_transport_for_data_channels); // All of these signals are fired on the signaling thread. diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index d6a31c81b7..43ba4ad6b4 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -415,12 +415,48 @@ TEST_F(JsepTransportControllerTest, GetDtlsTransportWithRtcpMux) { EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kAudioMid1)); } +TEST_F(JsepTransportControllerTest, + DtlsIsStillCreatedIfMediaTransportIsOnlyUsedForDataChannels) { + 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; + CreateJsepTransportController(config); + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + + EXPECT_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + ASSERT_NE(nullptr, media_transport); + + // After SetLocalDescription, media transport should be created as caller. + EXPECT_TRUE(media_transport->is_caller()); + EXPECT_TRUE(media_transport->pre_shared_key().has_value()); + + // Return nullptr for non-existing mids. + EXPECT_EQ(nullptr, transport_controller_->GetMediaTransport(kVideoMid2)); + + EXPECT_EQ(cricket::ICE_CANDIDATE_COMPONENT_RTP, + transport_controller_->GetDtlsTransport(kAudioMid1)->component()) + << "Media transport for media was not enabled, and so DTLS transport " + "should be created."; +} + TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { 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()); @@ -452,6 +488,8 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportInCallee) { 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()); @@ -482,6 +520,7 @@ TEST_F(JsepTransportControllerTest, GetMediaTransportIsNotSetIfNoSdes) { config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyNegotiate; config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_media = true; CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithoutBundle(); EXPECT_TRUE(transport_controller_ @@ -512,6 +551,7 @@ TEST_F(JsepTransportControllerTest, config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire; config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_media = true; CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithoutBundle(); AddCryptoSettings(description.get()); @@ -821,10 +861,12 @@ TEST_F(JsepTransportControllerTest, } TEST_F(JsepTransportControllerTest, - SignalConnectionStateConnectedWithMediaTransport) { + SignalConnectionStateConnectedWithMediaTransportAndNoDtls) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; 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); // Media Transport is only used with bundle. @@ -864,11 +906,64 @@ TEST_F(JsepTransportControllerTest, EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); } +TEST_F(JsepTransportControllerTest, + SignalConnectionStateConnectedWithMediaTransport) { + FakeMediaTransportFactory fake_media_transport_factory; + JsepTransportController::Config config; + config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_media = true; + CreateJsepTransportController(config); + + // Media Transport is only used with bundle. + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + EXPECT_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + + auto fake_audio_dtls = static_cast( + transport_controller_->GetDtlsTransport(kAudioMid1)); + auto fake_video_dtls = static_cast( + transport_controller_->GetDtlsTransport(kVideoMid1)); + + auto fake_audio_ice = static_cast( + transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); + auto fake_video_ice = static_cast( + transport_controller_->GetDtlsTransport(kVideoMid1)->ice_transport()); + fake_audio_ice->SetConnectionCount(2); + fake_audio_ice->SetConnectionCount(1); + fake_video_ice->SetConnectionCount(2); + fake_video_ice->SetConnectionCount(1); + fake_audio_ice->SetWritable(true); + fake_video_ice->SetWritable(true); + fake_audio_dtls->SetWritable(true); + fake_video_dtls->SetWritable(true); + + // Still not connected, because we are waiting for media transport. + EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, + kTimeout); + + FakeMediaTransport* media_transport = static_cast( + transport_controller_->GetMediaTransport(kAudioMid1)); + + media_transport->SetState(webrtc::MediaTransportState::kWritable); + EXPECT_EQ_WAIT(cricket::kIceConnectionConnecting, connection_state_, + kTimeout); + + // Still waiting for the second media transport. + media_transport = static_cast( + transport_controller_->GetMediaTransport(kVideoMid1)); + media_transport->SetState(webrtc::MediaTransportState::kWritable); + + EXPECT_EQ_WAIT(cricket::kIceConnectionConnected, connection_state_, kTimeout); +} + TEST_F(JsepTransportControllerTest, SignalConnectionStateFailedWhenMediaTransportClosed) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config; config.media_transport_factory = &fake_media_transport_factory; + config.use_media_transport_for_media = true; CreateJsepTransportController(config); auto description = CreateSessionDescriptionWithoutBundle(); AddCryptoSettings(description.get()); @@ -876,6 +971,11 @@ TEST_F(JsepTransportControllerTest, ->SetLocalDescription(SdpType::kOffer, description.get()) .ok()); + auto fake_audio_dtls = static_cast( + transport_controller_->GetDtlsTransport(kAudioMid1)); + auto fake_video_dtls = static_cast( + transport_controller_->GetDtlsTransport(kVideoMid1)); + auto fake_audio_ice = static_cast( transport_controller_->GetDtlsTransport(kAudioMid1)->ice_transport()); auto fake_video_ice = static_cast( @@ -888,6 +988,9 @@ TEST_F(JsepTransportControllerTest, fake_video_ice->SetConnectionCount(2); fake_video_ice->SetConnectionCount(1); + fake_audio_dtls->SetWritable(true); + fake_video_dtls->SetWritable(true); + FakeMediaTransport* media_transport = static_cast( transport_controller_->GetMediaTransport(kAudioMid1)); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 4cdab68474..2b4a48db7a 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -989,6 +989,9 @@ bool PeerConnection::Initialize( 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; config.media_transport_factory = factory_->media_transport_factory(); } @@ -3207,11 +3210,9 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, } transport_controller_->SetIceConfig(ParseIceConfig(modified_config)); - transport_controller_->SetMediaTransportFactory( - modified_config.use_media_transport || - modified_config.use_media_transport_for_data_channels - ? factory_->media_transport_factory() - : nullptr); + transport_controller_->SetMediaTransportSettings( + modified_config.use_media_transport, + modified_config.use_media_transport_for_data_channels); if (configuration_.active_reset_srtp_params != modified_config.active_reset_srtp_params) { @@ -6748,7 +6749,11 @@ bool PeerConnection::OnTransportChanged( sctp_transport_->SetDtlsTransport(dtls_transport); } - call_->MediaTransportChange(media_transport); + if (configuration_.use_media_transport) { + // Only pass media transport to call object if media transport is used + // for media (and not data channel). + call_->MediaTransportChange(media_transport); + } return ret; } diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index dd7f1d83cd..4091006656 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -3638,6 +3638,28 @@ TEST_P(PeerConnectionIntegrationTest, MediaTransportBidirectionalVideo) { EXPECT_GE(first_stats.sent_video_frames, second_stats.received_video_frames); } +TEST_P(PeerConnectionIntegrationTest, + MediaTransportDataChannelUsesRtpBidirectionalVideo) { + PeerConnectionInterface::RTCConfiguration rtc_config; + rtc_config.use_media_transport = false; + rtc_config.use_media_transport_for_data_channels = 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(); + + caller()->AddVideoTrack(); + callee()->AddVideoTrack(); + // Start offer/answer exchange and wait for it to complete. + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + + MediaExpectations media_expectations; + media_expectations.ExpectBidirectionalVideo(); + ASSERT_TRUE(ExpectNewFrames(media_expectations)); +} + // Test that the ICE connection and gathering states eventually reach // "complete". TEST_P(PeerConnectionIntegrationTest, IceStatesReachCompletion) {