From 47dfdca8dd82546cf3172ceb4e6ccfee0ccccd5c Mon Sep 17 00:00:00 2001 From: "Piotr (Peter) Slatala" Date: Fri, 16 Nov 2018 14:13:58 -0800 Subject: [PATCH] Create 'MaybeCreateMediaTransport' function JsepTransportController got a bit ugly with one super long method. Splitting it to two, so that MediaTransport creation is separated. Bug: webrtc:9719 Change-Id: I0b5aead2f96d79d6fc369a16810be58c8a661e71 Reviewed-on: https://webrtc-review.googlesource.com/c/111288 Reviewed-by: Anton Sukhanov Reviewed-by: Steve Anton Commit-Queue: Peter Slatala Cr-Commit-Position: refs/heads/master@{#25732} --- pc/jseptransportcontroller.cc | 79 +++++++++++++++++++---------------- pc/jseptransportcontroller.h | 8 ++++ 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 74a9ab6218..78ecaf31de 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -922,39 +922,11 @@ cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( return (it == jsep_transports_by_name_.end()) ? nullptr : it->second.get(); } -RTCError JsepTransportController::MaybeCreateJsepTransport( +std::unique_ptr +JsepTransportController::MaybeCreateMediaTransport( + const cricket::ContentInfo& content_info, bool local, - const cricket::ContentInfo& content_info) { - RTC_DCHECK(network_thread_->IsCurrent()); - cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); - if (transport) { - return RTCError::OK(); - } - - const cricket::MediaContentDescription* content_desc = - static_cast( - content_info.description); - if (certificate_ && !content_desc->cryptos().empty()) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "SDES and DTLS-SRTP cannot be enabled at the same time."); - } - - std::unique_ptr rtp_dtls_transport = - CreateDtlsTransport(content_info.name, /*rtcp =*/false); - - 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 media_transport; - - if (config_.rtcp_mux_policy != - PeerConnectionInterface::kRtcpMuxPolicyRequire && - content_info.type == cricket::MediaProtocolType::kRtp) { - rtcp_dtls_transport = - CreateDtlsTransport(content_info.name, /*rtcp =*/true); - } - + cricket::IceTransportInternal* ice_transport) { absl::optional selected_crypto_for_media_transport; if (content_info.media_description() && !content_info.media_description()->cryptos().empty()) { @@ -1024,16 +996,53 @@ RTCError JsepTransportController::MaybeCreateJsepTransport( key.value().size()); auto media_transport_result = config_.media_transport_factory->CreateMediaTransport( - rtp_dtls_transport->ice_transport(), network_thread_, settings); + ice_transport, network_thread_, settings); // TODO(sukhanov): Proper error handling. RTC_CHECK(media_transport_result.ok()); - RTC_DCHECK(media_transport == nullptr); - media_transport = std::move(media_transport_result.value()); + return media_transport_result.MoveValue(); } } + return nullptr; +} + +RTCError JsepTransportController::MaybeCreateJsepTransport( + bool local, + const cricket::ContentInfo& content_info) { + RTC_DCHECK(network_thread_->IsCurrent()); + cricket::JsepTransport* transport = GetJsepTransportByName(content_info.name); + if (transport) { + return RTCError::OK(); + } + + const cricket::MediaContentDescription* content_desc = + static_cast( + content_info.description); + if (certificate_ && !content_desc->cryptos().empty()) { + return RTCError(RTCErrorType::INVALID_PARAMETER, + "SDES and DTLS-SRTP cannot be enabled at the same time."); + } + + std::unique_ptr rtp_dtls_transport = + CreateDtlsTransport(content_info.name, /*rtcp =*/false); + + 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 media_transport; + + if (config_.rtcp_mux_policy != + PeerConnectionInterface::kRtcpMuxPolicyRequire && + content_info.type == cricket::MediaProtocolType::kRtp) { + rtcp_dtls_transport = + CreateDtlsTransport(content_info.name, /*rtcp =*/true); + } + media_transport = MaybeCreateMediaTransport( + content_info, local, rtp_dtls_transport->ice_transport()); + // TODO(sukhanov): Do not create RTP/RTCP transports if media transport is // used. if (config_.disable_encryption) { diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h index 42b28c220d..3ed7f5f433 100644 --- a/pc/jseptransportcontroller.h +++ b/pc/jseptransportcontroller.h @@ -272,6 +272,14 @@ class JsepTransportController : public sigslot::has_slots<> { // differentiate initiator (caller) from answerer (callee). RTCError MaybeCreateJsepTransport(bool local, const cricket::ContentInfo& content_info); + + // 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, + bool local, + cricket::IceTransportInternal* ice_transport); void MaybeDestroyJsepTransport(const std::string& mid); void DestroyAllJsepTransports_n();