diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc index c800232879..82be5338a5 100644 --- a/pc/jsep_transport.cc +++ b/pc/jsep_transport.cc @@ -769,8 +769,8 @@ void JsepTransport::OnStateChanged(webrtc::MediaTransportState state) { void JsepTransport::NegotiateDatagramTransport(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. + if (!datagram_transport_) { + return; // No need to negotiate the use of datagram transport. } bool use_datagram_transport = @@ -778,14 +778,16 @@ void JsepTransport::NegotiateDatagramTransport(SdpType type) { remote_description_->transport_desc.opaque_parameters == local_description_->transport_desc.opaque_parameters; + RTC_LOG(LS_INFO) << "Negotiating datagram transport, use_datagram_transport=" + << use_datagram_transport << " answer type=" + << (type == SdpType::kAnswer ? "answer" : "pr_answer"); + // A provisional or full or answer lets the peer start sending on one of the // transports. - 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 (composite_rtp_transport_) { + composite_rtp_transport_->SetSendTransport( + use_datagram_transport ? datagram_rtp_transport_.get() + : default_rtp_transport()); } if (type != SdpType::kAnswer) { @@ -805,18 +807,22 @@ void JsepTransport::NegotiateDatagramTransport(SdpType type) { /*provisional=*/false); 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; + if (composite_rtp_transport_) { + // Remove and delete the non-datagram RTP transport. + 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()); + // Remove and delete the datagram transport. + if (composite_rtp_transport_) { + composite_rtp_transport_->RemoveTransport(datagram_rtp_transport_.get()); + } datagram_rtp_transport_ = nullptr; datagram_transport_ = nullptr; } diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 1818858942..0395835cf5 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -1196,7 +1196,9 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( /*datagram_transport=*/nullptr); } - if (datagram_transport) { + // Only create a datagram RTP transport if the datagram transport should be + // used for RTP. + if (datagram_transport && config_.use_datagram_transport) { // TODO(sukhanov): We use unencrypted RTP transport over DatagramTransport, // because MediaTransport encrypts. In the future we may want to // implement our own version of RtpTransport over MediaTransport, because diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 458e09c38a..887f12b7fd 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -463,6 +463,56 @@ TEST_F(JsepTransportControllerTest, "should be created."; } +TEST_F(JsepTransportControllerTest, + DtlsIsStillCreatedIfDatagramTransportIsOnlyUsedForDataChannels) { + FakeMediaTransportFactory fake_media_transport_factory("transport_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_datagram_transport_for_data_channels = true; + CreateJsepTransportController(config); + + auto description = CreateSessionDescriptionWithBundleGroup(); + AddCryptoSettings(description.get()); + absl::optional params = + transport_controller_->GetTransportParameters(kAudioMid1); + for (auto& info : description->transport_infos()) { + info.description.opaque_parameters = params; + } + + EXPECT_TRUE(transport_controller_ + ->SetLocalDescription(SdpType::kOffer, description.get()) + .ok()); + EXPECT_TRUE(transport_controller_ + ->SetRemoteDescription(SdpType::kAnswer, description.get()) + .ok()); + + FakeDatagramTransport* datagram_transport = + static_cast( + transport_controller_->GetDataChannelTransport(kAudioMid1)); + + ASSERT_NE(nullptr, datagram_transport); + + EXPECT_EQ(cricket::ICE_CANDIDATE_COMPONENT_RTP, + transport_controller_->GetDtlsTransport(kAudioMid1)->component()) + << "Datagram transport for media was not enabled, and so DTLS transport " + "should be created."; + + // Datagram transport is not used for media, so no max packet size is + // specified. + EXPECT_EQ(transport_controller_->GetMediaTransportConfig(kAudioMid1) + .rtp_max_packet_size, + absl::nullopt); + + // Since datagram transport is not used for RTP, setting it to writable should + // not make the RTP transport writable. + datagram_transport->set_state(MediaTransportState::kWritable); + EXPECT_FALSE(transport_controller_->GetRtpTransport(kAudioMid1) + ->IsWritable(/*rtcp=*/false)); +} + TEST_F(JsepTransportControllerTest, GetMediaTransportInCaller) { FakeMediaTransportFactory fake_media_transport_factory; JsepTransportController::Config config;