From 653429c5b32e58e2a3a1a25488f8f2584fb42349 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 19 Oct 2020 16:05:20 +0000 Subject: [PATCH] Remove friendship between PeerConnection and SdpOfferAnswerHandler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add multiple accessors to PeerConnection, and make multiple formerly private functions public for access from SdpOfferAnswerHandler. Reducing the surface of PeerConnection is a job to be done iteratively. Bug: webrtc:11995 Change-Id: Iab176824ae557af84ac934e40ff674a1008a29d1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189540 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#32459} --- pc/peer_connection.h | 166 ++++++++++++++++++++++------------------- pc/sdp_offer_answer.cc | 31 ++++---- 2 files changed, 105 insertions(+), 92 deletions(-) diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 582c76d29f..dd7a3b4afe 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -371,6 +371,97 @@ class PeerConnection : public PeerConnectionInternal, const RtpTransmissionManager* rtp_manager() const { return rtp_manager_.get(); } + cricket::ChannelManager* channel_manager() const; + + JsepTransportController* transport_controller() { + return transport_controller_.get(); + } + cricket::PortAllocator* port_allocator() { return port_allocator_.get(); } + Call* call_ptr() { return call_ptr_; } + rtc::UniqueRandomIdGenerator* ssrc_generator() { return &ssrc_generator_; } + const cricket::AudioOptions& audio_options() { return audio_options_; } + const cricket::VideoOptions& video_options() { return video_options_; } + VideoBitrateAllocatorFactory* video_bitrate_allocator_factory() { + return video_bitrate_allocator_factory_.get(); + } + + cricket::DataChannelType data_channel_type() const; + void SetIceConnectionState(IceConnectionState new_state); + void NoteUsageEvent(UsageEvent event); + + // Report the UMA metric SdpFormatReceived for the given remote offer. + void ReportSdpFormatReceived(const SessionDescriptionInterface& remote_offer); + // Signals from MediaStreamObserver. + void OnAudioTrackAdded(AudioTrackInterface* track, + MediaStreamInterface* stream) + RTC_RUN_ON(signaling_thread()); + void OnAudioTrackRemoved(AudioTrackInterface* track, + MediaStreamInterface* stream) + RTC_RUN_ON(signaling_thread()); + void OnVideoTrackAdded(VideoTrackInterface* track, + MediaStreamInterface* stream) + RTC_RUN_ON(signaling_thread()); + void OnVideoTrackRemoved(VideoTrackInterface* track, + MediaStreamInterface* stream) + RTC_RUN_ON(signaling_thread()); + + // Returns true if the PeerConnection is configured to use Unified Plan + // semantics for creating offers/answers and setting local/remote + // descriptions. If this is true the RtpTransceiver API will also be available + // to the user. If this is false, Plan B semantics are assumed. + // TODO(bugs.webrtc.org/8530): Flip the default to be Unified Plan once + // sufficient time has passed. + bool IsUnifiedPlan() const { + RTC_DCHECK_RUN_ON(signaling_thread()); + return configuration_.sdp_semantics == SdpSemantics::kUnifiedPlan; + } + bool ValidateBundleSettings(const cricket::SessionDescription* desc); + + // Returns the MID for the data section associated with either the + // RtpDataChannel or SCTP data channel, if it has been set. If no data + // channels are configured this will return nullopt. + absl::optional GetDataMid() const; + + void SetSctpDataMid(const std::string& mid) { + RTC_DCHECK_RUN_ON(signaling_thread()); + sctp_mid_s_ = mid; + } + void ResetSctpDataMid() { + RTC_DCHECK_RUN_ON(signaling_thread()); + sctp_mid_s_.reset(); + } + + // Returns the CryptoOptions for this PeerConnection. This will always + // return the RTCConfiguration.crypto_options if set and will only default + // back to the PeerConnectionFactory settings if nothing was set. + CryptoOptions GetCryptoOptions(); + + // Internal implementation for AddTransceiver family of methods. If + // |fire_callback| is set, fires OnRenegotiationNeeded callback if successful. + RTCErrorOr> AddTransceiver( + cricket::MediaType media_type, + rtc::scoped_refptr track, + const RtpTransceiverInit& init, + bool fire_callback = true); + + // Returns rtp transport, result can not be nullptr. + RtpTransportInternal* GetRtpTransport(const std::string& mid) { + RTC_DCHECK_RUN_ON(signaling_thread()); + auto rtp_transport = transport_controller_->GetRtpTransport(mid); + RTC_DCHECK(rtp_transport); + return rtp_transport; + } + + // Returns true if SRTP (either using DTLS-SRTP or SDES) is required by + // this session. + bool SrtpRequired() const RTC_RUN_ON(signaling_thread()); + + void OnSentPacket_w(const rtc::SentPacket& sent_packet); + + bool SetupDataChannelTransport_n(const std::string& mid) + RTC_RUN_ON(network_thread()); + void TeardownDataChannelTransport_n() RTC_RUN_ON(network_thread()); + cricket::ChannelInterface* GetChannel(const std::string& content_name); // Functions made public for testing. void ReturnHistogramVeryQuicklyForTesting() { @@ -383,23 +474,10 @@ class PeerConnection : public PeerConnectionInternal, ~PeerConnection() override; private: - // While refactoring: Allow access from SDP negotiation - // TOOD(https://bugs.webrtc.org/11995): Remove friendship. - friend class SdpOfferAnswerHandler; - rtc::scoped_refptr> FindTransceiverBySender(rtc::scoped_refptr sender) RTC_RUN_ON(signaling_thread()); - // Internal implementation for AddTransceiver family of methods. If - // |fire_callback| is set, fires OnRenegotiationNeeded callback if successful. - RTCErrorOr> AddTransceiver( - cricket::MediaType media_type, - rtc::scoped_refptr track, - const RtpTransceiverInit& init, - bool fire_callback = true); - - void SetIceConnectionState(IceConnectionState new_state); void SetStandardizedIceConnectionState( PeerConnectionInterface::IceConnectionState new_state) RTC_RUN_ON(signaling_thread()); @@ -428,39 +506,10 @@ class PeerConnection : public PeerConnectionInternal, const cricket::CandidatePairChangeEvent& event) RTC_RUN_ON(signaling_thread()); - // Signals from MediaStreamObserver. - void OnAudioTrackAdded(AudioTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); - void OnAudioTrackRemoved(AudioTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); - void OnVideoTrackAdded(VideoTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); - void OnVideoTrackRemoved(VideoTrackInterface* track, - MediaStreamInterface* stream) - RTC_RUN_ON(signaling_thread()); void OnNegotiationNeeded(); - // Returns the MID for the data section associated with either the - // RtpDataChannel or SCTP data channel, if it has been set. If no data - // channels are configured this will return nullopt. - absl::optional GetDataMid() const; - - // Returns true if the PeerConnection is configured to use Unified Plan - // semantics for creating offers/answers and setting local/remote - // descriptions. If this is true the RtpTransceiver API will also be available - // to the user. If this is false, Plan B semantics are assumed. - // TODO(bugs.webrtc.org/8530): Flip the default to be Unified Plan once - // sufficient time has passed. - bool IsUnifiedPlan() const { - RTC_DCHECK_RUN_ON(signaling_thread()); - return configuration_.sdp_semantics == SdpSemantics::kUnifiedPlan; - } - // Returns the specified SCTP DataChannel in sctp_data_channels_, // or nullptr if not found. SctpDataChannel* FindDataChannelBySid(int sid) const @@ -501,14 +550,9 @@ class PeerConnection : public PeerConnectionInternal, // Returns RTCError::OK() if there are no issues. RTCError ValidateConfiguration(const RTCConfiguration& config) const; - cricket::ChannelManager* channel_manager() const; - - cricket::ChannelInterface* GetChannel(const std::string& content_name); - cricket::IceConfig ParseIceConfig( const PeerConnectionInterface::RTCConfiguration& config) const; - cricket::DataChannelType data_channel_type() const; // Called when an RTCCertificate is generated or retrieved by // WebRTCSessionDescriptionFactory. Should happen before setLocalDescription. @@ -529,21 +573,12 @@ class PeerConnection : public PeerConnectionInternal, int* sdp_mline_index) RTC_RUN_ON(signaling_thread()); - bool SetupDataChannelTransport_n(const std::string& mid) - RTC_RUN_ON(network_thread()); - void TeardownDataChannelTransport_n() RTC_RUN_ON(network_thread()); - - bool ValidateBundleSettings(const cricket::SessionDescription* desc); bool HasRtcpMuxEnabled(const cricket::ContentInfo* content); // Verifies a=setup attribute as per RFC 5763. bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc, SdpType type); - // Returns true if SRTP (either using DTLS-SRTP or SDES) is required by - // this session. - bool SrtpRequired() const RTC_RUN_ON(signaling_thread()); - // JsepTransportController signal handlers. void OnTransportControllerConnectionState(cricket::IceConnectionState state) RTC_RUN_ON(signaling_thread()); @@ -564,9 +599,6 @@ class PeerConnection : public PeerConnectionInternal, RTC_RUN_ON(signaling_thread()); void OnTransportControllerDtlsHandshakeError(rtc::SSLHandshakeError error); - // Report the UMA metric SdpFormatReceived for the given remote offer. - void ReportSdpFormatReceived(const SessionDescriptionInterface& remote_offer); - // Invoked when TransportController connection completion is signaled. // Reports stats for all transports in use. void ReportTransportStats() RTC_RUN_ON(signaling_thread()); @@ -580,11 +612,8 @@ class PeerConnection : public PeerConnectionInternal, void ReportIceCandidateCollected(const cricket::Candidate& candidate) RTC_RUN_ON(signaling_thread()); - void NoteUsageEvent(UsageEvent event); void ReportUsagePattern() const RTC_RUN_ON(signaling_thread()); - void OnSentPacket_w(const rtc::SentPacket& sent_packet); - // JsepTransportController::Observer override. // // Called by |transport_controller_| when processing transport information @@ -597,19 +626,6 @@ class PeerConnection : public PeerConnectionInternal, rtc::scoped_refptr dtls_transport, DataChannelTransportInterface* data_channel_transport) override; - // Returns the CryptoOptions for this PeerConnection. This will always - // return the RTCConfiguration.crypto_options if set and will only default - // back to the PeerConnectionFactory settings if nothing was set. - CryptoOptions GetCryptoOptions(); - - // Returns rtp transport, result can not be nullptr. - RtpTransportInternal* GetRtpTransport(const std::string& mid) { - RTC_DCHECK_RUN_ON(signaling_thread()); - auto rtp_transport = transport_controller_->GetRtpTransport(mid); - RTC_DCHECK(rtp_transport); - return rtp_transport; - } - std::function InitializeRtcpCallback(); diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index b40d00c72a..81df99a9d0 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -962,20 +962,20 @@ const TransceiverList* SdpOfferAnswerHandler::transceivers() const { return pc_->rtp_manager()->transceivers(); } JsepTransportController* SdpOfferAnswerHandler::transport_controller() { - return pc_->transport_controller_.get(); + return pc_->transport_controller(); } DataChannelController* SdpOfferAnswerHandler::data_channel_controller() { - return &pc_->data_channel_controller_; + return pc_->data_channel_controller(); } const DataChannelController* SdpOfferAnswerHandler::data_channel_controller() const { - return &pc_->data_channel_controller_; + return pc_->data_channel_controller(); } cricket::PortAllocator* SdpOfferAnswerHandler::port_allocator() { - return pc_->port_allocator_.get(); + return pc_->port_allocator(); } const cricket::PortAllocator* SdpOfferAnswerHandler::port_allocator() const { - return pc_->port_allocator_.get(); + return pc_->port_allocator(); } RtpTransmissionManager* SdpOfferAnswerHandler::rtp_manager() { return pc_->rtp_manager(); @@ -4509,9 +4509,9 @@ cricket::VoiceChannel* SdpOfferAnswerHandler::CreateVoiceChannel( { RTC_DCHECK_RUN_ON(pc_->signaling_thread()); voice_channel = channel_manager()->CreateVoiceChannel( - pc_->call_ptr_, pc_->configuration_.media_config, rtp_transport, + pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport, signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), - &pc_->ssrc_generator_, pc_->audio_options_); + pc_->ssrc_generator(), pc_->audio_options()); } if (!voice_channel) { return nullptr; @@ -4536,10 +4536,10 @@ cricket::VideoChannel* SdpOfferAnswerHandler::CreateVideoChannel( { RTC_DCHECK_RUN_ON(pc_->signaling_thread()); video_channel = channel_manager()->CreateVideoChannel( - pc_->call_ptr_, pc_->configuration_.media_config, rtp_transport, + pc_->call_ptr(), pc_->configuration()->media_config, rtp_transport, signaling_thread(), mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), - &pc_->ssrc_generator_, pc_->video_options_, - pc_->video_bitrate_allocator_factory_.get()); + pc_->ssrc_generator(), pc_->video_options(), + pc_->video_bitrate_allocator_factory()); } if (!video_channel) { return nullptr; @@ -4559,10 +4559,7 @@ bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetupDataChannelTransport_n, pc_, mid))) { - { - RTC_DCHECK_RUN_ON(pc_->signaling_thread()); - pc_->sctp_mid_s_ = mid; - } + pc_->SetSctpDataMid(mid); } else { return false; } @@ -4576,9 +4573,9 @@ bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { RTC_DCHECK_RUN_ON(pc_->signaling_thread()); data_channel_controller()->set_rtp_data_channel( channel_manager()->CreateRtpDataChannel( - pc_->configuration_.media_config, rtp_transport, + pc_->configuration()->media_config, rtp_transport, signaling_thread(), mid, pc_->SrtpRequired(), - pc_->GetCryptoOptions(), &pc_->ssrc_generator_)); + pc_->GetCryptoOptions(), pc_->ssrc_generator())); } if (!data_channel_controller()->rtp_data_channel()) { return false; @@ -4626,7 +4623,7 @@ void SdpOfferAnswerHandler::DestroyDataChannelTransport() { RTC_DCHECK_RUN_ON(pc_->network_thread()); pc_->TeardownDataChannelTransport_n(); }); - pc_->sctp_mid_s_.reset(); + pc_->ResetSctpDataMid(); } }