From 97fc11fb86f6309f5a4b4d6c131afac08e6bce07 Mon Sep 17 00:00:00 2001 From: "Piotr (Peter) Slatala" Date: Thu, 18 Oct 2018 12:57:59 -0700 Subject: [PATCH] Fix the 'SetConfiguration(RTCConfiguration::use_media_transport)' setting. In the past, it would incorrectly set up a state for 'use_media_transport' (i.e. it could say "use_media_transport" is true, but jseptransportcontroller wouldn't know about that). Also, removes unnecessary field (unused). Bug: webrtc:9719 Change-Id: I7e5c0ce81b3b70f63c49d661d95b95b5bcbb0c68 Reviewed-on: https://webrtc-review.googlesource.com/c/106960 Reviewed-by: Steve Anton Reviewed-by: Anton Sukhanov Commit-Queue: Peter Slatala Cr-Commit-Position: refs/heads/master@{#25263} --- pc/jseptransportcontroller.cc | 9 +++++++++ pc/jseptransportcontroller.h | 6 ++++++ pc/peerconnection.cc | 3 +++ pc/peerconnection.h | 9 ++++----- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pc/jseptransportcontroller.cc b/pc/jseptransportcontroller.cc index 19a2d40275..991d769e71 100644 --- a/pc/jseptransportcontroller.cc +++ b/pc/jseptransportcontroller.cc @@ -378,6 +378,15 @@ void JsepTransportController::SetActiveResetSrtpParams( } } +void JsepTransportController::SetMediaTransportFactory( + MediaTransportFactory* media_transport_factory) { + RTC_DCHECK(media_transport_factory == config_.media_transport_factory || + jsep_transports_by_name_.empty()) + << "You can only call SetMediaTransportFactory before " + "JsepTransportController created its first transport."; + config_.media_transport_factory = media_transport_factory; +} + std::unique_ptr JsepTransportController::CreateDtlsTransport(const std::string& transport_name, bool rtcp) { diff --git a/pc/jseptransportcontroller.h b/pc/jseptransportcontroller.h index 518d31060f..5d0f5ceb23 100644 --- a/pc/jseptransportcontroller.h +++ b/pc/jseptransportcontroller.h @@ -167,6 +167,12 @@ 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); + // All of these signals are fired on the signaling thread. // If any transport failed => failed, diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index ca4d13d60c..ecdec61c4c 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2981,6 +2981,9 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration, } transport_controller_->SetIceConfig(ParseIceConfig(modified_config)); + transport_controller_->SetMediaTransportFactory( + modified_config.use_media_transport ? factory_->media_transport_factory() + : nullptr); if (configuration_.active_reset_srtp_params != modified_config.active_reset_srtp_params) { diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 0cd976a1df..c1d3d40c2a 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -927,7 +927,10 @@ class PeerConnection : public PeerConnectionInternal, MediaTransportInterface* GetMediaTransport(const std::string& mid) { auto media_transport = transport_controller_->GetMediaTransport(mid); RTC_DCHECK(configuration_.use_media_transport == - (media_transport != nullptr)); + (media_transport != nullptr)) + << "configuration_.use_media_transport=" + << configuration_.use_media_transport + << ", (media_transport != nullptr)=" << (media_transport != nullptr); return media_transport; } @@ -1043,10 +1046,6 @@ class PeerConnection : public PeerConnectionInternal, // List of content names for which the remote side triggered an ICE restart. std::set pending_ice_restarts_; - // Optional media transport for sending / receiving encoded frames. - // If available, media transport will be used instead of RTP / SRTP. - std::unique_ptr media_transport_factory_; - std::unique_ptr webrtc_session_desc_factory_; // Member variables for caching global options.