From 8f429927870fd2a999cb3e5c82e03aab5801bc7c Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Wed, 4 May 2022 10:32:30 +0000 Subject: [PATCH] Move channel creation functions into RtpTransceiver This breaks the link from sdp_offer_answer.cc to channel.h. Bug: webrtc:13931 Change-Id: I75608f75713bf4e69013ac5f5b17c19e53d07519 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261060 Commit-Queue: Harald Alvestrand Reviewed-by: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#36757} --- pc/BUILD.gn | 20 +++++ pc/channel.cc | 6 +- pc/channel.h | 6 +- pc/channel_factory_interface.h | 4 +- pc/channel_manager.cc | 4 +- pc/channel_manager.h | 4 +- pc/jsep_transport_collection.cc | 16 ++++ pc/jsep_transport_collection.h | 2 + pc/jsep_transport_controller.cc | 11 ++- pc/jsep_transport_controller.h | 6 +- pc/peer_connection_sdp_methods.h | 131 +++++++++++++++++++++++++++++++ pc/rtp_transceiver.cc | 48 ++++++++++- pc/rtp_transceiver.h | 23 +++++- pc/sdp_offer_answer.cc | 101 ++++++++---------------- pc/sdp_offer_answer.h | 5 -- 15 files changed, 294 insertions(+), 93 deletions(-) create mode 100644 pc/peer_connection_sdp_methods.h diff --git a/pc/BUILD.gn b/pc/BUILD.gn index f94643daa0..1ae3715b82 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -1009,12 +1009,26 @@ rtc_source_set("data_channel_controller") { ] } +rtc_source_set("peer_connection_sdp_methods") { + visibility = [ ":*" ] + sources = [ "peer_connection_sdp_methods.h" ] + deps = [ + ":jsep_transport_controller", + ":peer_connection_message_handler", + ":sctp_data_channel", + ":usage_pattern", + "../api:libjingle_peerconnection_api", + "../call:call_interfaces", + ] +} + rtc_source_set("peer_connection_internal") { visibility = [ ":*" ] sources = [ "peer_connection_internal.h" ] deps = [ ":jsep_transport_controller", ":peer_connection_message_handler", + ":peer_connection_sdp_methods", ":rtp_transceiver", ":rtp_transmission_manager", ":sctp_data_channel", @@ -1022,6 +1036,7 @@ rtc_source_set("peer_connection_internal") { "../call:call_interfaces", ] } + rtc_source_set("rtc_stats_collector") { visibility = [ ":*", @@ -1133,6 +1148,7 @@ rtc_source_set("sdp_offer_answer") { ":webrtc_session_description_factory", "../api:array_view", "../api:audio_options_api", + "../api:field_trials_view", "../api:libjingle_peerconnection_api", "../api:media_stream_interface", "../api:rtc_error", @@ -1579,8 +1595,10 @@ rtc_library("rtp_transceiver") { "rtp_transceiver.h", ] deps = [ + ":channel", ":channel_interface", ":channel_manager", + ":peer_connection_sdp_methods", ":proxy", ":rtp_media_utils", ":rtp_parameters_conversion", @@ -1591,6 +1609,7 @@ rtc_library("rtp_transceiver") { ":rtp_transport_internal", ":session_description", "../api:array_view", + "../api:audio_options_api", "../api:libjingle_peerconnection_api", "../api:rtc_error", "../api:rtp_parameters", @@ -1598,6 +1617,7 @@ rtc_library("rtp_transceiver") { "../api:scoped_refptr", "../api:sequence_checker", "../api/task_queue", + "../api/video:video_bitrate_allocator_factory", "../media:rtc_media_base", "../rtc_base:checks", "../rtc_base:location", diff --git a/pc/channel.cc b/pc/channel.cc index ea1ca51dd9..9b8e8f8c39 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -117,7 +117,7 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) @@ -818,7 +818,7 @@ VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) @@ -941,7 +941,7 @@ VideoChannel::VideoChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) diff --git a/pc/channel.h b/pc/channel.h index c320286f53..8e121b2b32 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -83,7 +83,7 @@ class BaseChannel : public ChannelInterface, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); @@ -361,7 +361,7 @@ class VoiceChannel : public BaseChannel { rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); @@ -407,7 +407,7 @@ class VideoChannel : public BaseChannel { rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& mid, + absl::string_view mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); diff --git a/pc/channel_factory_interface.h b/pc/channel_factory_interface.h index 62fb1aea1f..6cae0baad9 100644 --- a/pc/channel_factory_interface.h +++ b/pc/channel_factory_interface.h @@ -31,7 +31,7 @@ class ChannelFactoryInterface { virtual std::unique_ptr CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const VideoOptions& options, @@ -41,7 +41,7 @@ class ChannelFactoryInterface { virtual std::unique_ptr CreateVoiceChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const AudioOptions& options) = 0; diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 6ca7973561..e13db54806 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -152,7 +152,7 @@ ChannelManager::GetSupportedVideoRtpHeaderExtensions() const { std::unique_ptr ChannelManager::CreateVoiceChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const AudioOptions& options) { @@ -188,7 +188,7 @@ std::unique_ptr ChannelManager::CreateVoiceChannel( std::unique_ptr ChannelManager::CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const VideoOptions& options, diff --git a/pc/channel_manager.h b/pc/channel_manager.h index c9a54936f3..8d1ec28001 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -84,7 +84,7 @@ class ChannelManager : public ChannelFactoryInterface { std::unique_ptr CreateVoiceChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const AudioOptions& options) override; @@ -95,7 +95,7 @@ class ChannelManager : public ChannelFactoryInterface { std::unique_ptr CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - const std::string& mid, + absl::string_view mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, const VideoOptions& options, diff --git a/pc/jsep_transport_collection.cc b/pc/jsep_transport_collection.cc index df44a9c157..b50d303d77 100644 --- a/pc/jsep_transport_collection.cc +++ b/pc/jsep_transport_collection.cc @@ -219,6 +219,22 @@ const cricket::JsepTransport* JsepTransportCollection::GetTransportForMid( return it == mid_to_transport_.end() ? nullptr : it->second; } +cricket::JsepTransport* JsepTransportCollection::GetTransportForMid( + absl::string_view mid) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + // TODO(hta): should be a better way. + auto it = mid_to_transport_.find(std::string(mid)); + return it == mid_to_transport_.end() ? nullptr : it->second; +} + +const cricket::JsepTransport* JsepTransportCollection::GetTransportForMid( + absl::string_view mid) const { + RTC_DCHECK_RUN_ON(&sequence_checker_); + // TODO(hta): Should be a better way + auto it = mid_to_transport_.find(std::string(mid)); + return it == mid_to_transport_.end() ? nullptr : it->second; +} + bool JsepTransportCollection::SetTransportForMid( const std::string& mid, cricket::JsepTransport* jsep_transport) { diff --git a/pc/jsep_transport_collection.h b/pc/jsep_transport_collection.h index 099e24a457..0bb19a3260 100644 --- a/pc/jsep_transport_collection.h +++ b/pc/jsep_transport_collection.h @@ -118,6 +118,8 @@ class JsepTransportCollection { cricket::JsepTransport* GetTransportForMid(const std::string& mid); const cricket::JsepTransport* GetTransportForMid( const std::string& mid) const; + cricket::JsepTransport* GetTransportForMid(absl::string_view mid); + const cricket::JsepTransport* GetTransportForMid(absl::string_view mid) const; // Set transport for a MID. This may destroy a transport if it is no // longer in use. bool SetTransportForMid(const std::string& mid, diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 727a30a1ae..7b445741a2 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -107,7 +107,7 @@ RTCError JsepTransportController::SetRemoteDescription( } RtpTransportInternal* JsepTransportController::GetRtpTransport( - const std::string& mid) const { + absl::string_view mid) const { RTC_DCHECK_RUN_ON(network_thread_); auto jsep_transport = GetJsepTransportForMid(mid); if (!jsep_transport) { @@ -1000,6 +1000,15 @@ cricket::JsepTransport* JsepTransportController::GetJsepTransportForMid( const std::string& mid) { return transports_.GetTransportForMid(mid); } +const cricket::JsepTransport* JsepTransportController::GetJsepTransportForMid( + absl::string_view mid) const { + return transports_.GetTransportForMid(mid); +} + +cricket::JsepTransport* JsepTransportController::GetJsepTransportForMid( + absl::string_view mid) { + return transports_.GetTransportForMid(mid); +} const cricket::JsepTransport* JsepTransportController::GetJsepTransportByName( const std::string& transport_name) const { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 3b7832c018..ccb7426510 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -167,7 +167,7 @@ class JsepTransportController : public sigslot::has_slots<> { // Get transports to be used for the provided `mid`. If bundling is enabled, // calling GetRtpTransport for multiple MIDs may yield the same object. - RtpTransportInternal* GetRtpTransport(const std::string& mid) const; + RtpTransportInternal* GetRtpTransport(absl::string_view mid) const; cricket::DtlsTransportInternal* GetDtlsTransport(const std::string& mid); const cricket::DtlsTransportInternal* GetRtcpDtlsTransport( const std::string& mid) const; @@ -362,6 +362,10 @@ class JsepTransportController : public sigslot::has_slots<> { const std::string& mid) const RTC_RUN_ON(network_thread_); cricket::JsepTransport* GetJsepTransportForMid(const std::string& mid) RTC_RUN_ON(network_thread_); + const cricket::JsepTransport* GetJsepTransportForMid( + absl::string_view mid) const RTC_RUN_ON(network_thread_); + cricket::JsepTransport* GetJsepTransportForMid(absl::string_view mid) + RTC_RUN_ON(network_thread_); // Get the JsepTransport without considering the BUNDLE group. Return nullptr // if the JsepTransport is destroyed. diff --git a/pc/peer_connection_sdp_methods.h b/pc/peer_connection_sdp_methods.h new file mode 100644 index 0000000000..972ad9c7b4 --- /dev/null +++ b/pc/peer_connection_sdp_methods.h @@ -0,0 +1,131 @@ +/* + * Copyright 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef PC_PEER_CONNECTION_SDP_METHODS_H_ +#define PC_PEER_CONNECTION_SDP_METHODS_H_ + +#include +#include +#include +#include +#include + +#include "api/peer_connection_interface.h" +#include "pc/jsep_transport_controller.h" +#include "pc/peer_connection_message_handler.h" +#include "pc/sctp_data_channel.h" +#include "pc/usage_pattern.h" + +namespace webrtc { + +class DataChannelController; +class RtpTransmissionManager; +class StatsCollector; + +// This interface defines the functions that are needed for +// SdpOfferAnswerHandler to access PeerConnection internal state. +class PeerConnectionSdpMethods { + public: + virtual ~PeerConnectionSdpMethods() = default; + + // The SDP session ID as defined by RFC 3264. + virtual std::string session_id() const = 0; + + // Returns true if the ICE restart flag above was set, and no ICE restart has + // occurred yet for this transport (by applying a local description with + // changed ufrag/password). If the transport has been deleted as a result of + // bundling, returns false. + virtual bool NeedsIceRestart(const std::string& content_name) const = 0; + + virtual absl::optional sctp_mid() const = 0; + + // Functions below this comment are known to only be accessed + // from SdpOfferAnswerHandler. + // Return a pointer to the active configuration. + virtual const PeerConnectionInterface::RTCConfiguration* configuration() + const = 0; + + // Report the UMA metric SdpFormatReceived for the given remote description. + virtual void ReportSdpFormatReceived( + const SessionDescriptionInterface& remote_description) = 0; + + // Report the UMA metric BundleUsage for the given remote description. + virtual void ReportSdpBundleUsage( + const SessionDescriptionInterface& remote_description) = 0; + + virtual PeerConnectionMessageHandler* message_handler() = 0; + virtual RtpTransmissionManager* rtp_manager() = 0; + virtual const RtpTransmissionManager* rtp_manager() const = 0; + virtual bool dtls_enabled() const = 0; + virtual const PeerConnectionFactoryInterface::Options* options() const = 0; + + // 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. + virtual CryptoOptions GetCryptoOptions() = 0; + virtual JsepTransportController* transport_controller_s() = 0; + virtual JsepTransportController* transport_controller_n() = 0; + virtual DataChannelController* data_channel_controller() = 0; + virtual cricket::PortAllocator* port_allocator() = 0; + virtual StatsCollector* stats() = 0; + // Returns the observer. Will crash on CHECK if the observer is removed. + virtual PeerConnectionObserver* Observer() const = 0; + virtual bool GetSctpSslRole(rtc::SSLRole* role) = 0; + virtual PeerConnectionInterface::IceConnectionState + ice_connection_state_internal() = 0; + virtual void SetIceConnectionState( + PeerConnectionInterface::IceConnectionState new_state) = 0; + virtual void NoteUsageEvent(UsageEvent event) = 0; + virtual bool IsClosed() const = 0; + // 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. + virtual bool IsUnifiedPlan() const = 0; + virtual bool ValidateBundleSettings( + const cricket::SessionDescription* desc, + const std::map& + bundle_groups_by_mid) = 0; + + virtual absl::optional GetDataMid() const = 0; + // Internal implementation for AddTransceiver family of methods. If + // `fire_callback` is set, fires OnRenegotiationNeeded callback if successful. + virtual RTCErrorOr> + AddTransceiver(cricket::MediaType media_type, + rtc::scoped_refptr track, + const RtpTransceiverInit& init, + bool fire_callback = true) = 0; + // Asynchronously calls SctpTransport::Start() on the network thread for + // `sctp_mid()` if set. Called as part of setting the local description. + virtual void StartSctpTransport(int local_port, + int remote_port, + int max_message_size) = 0; + + // Asynchronously adds a remote candidate on the network thread. + virtual void AddRemoteCandidate(const std::string& mid, + const cricket::Candidate& candidate) = 0; + + virtual Call* call_ptr() = 0; + // Returns true if SRTP (either using DTLS-SRTP or SDES) is required by + // this session. + virtual bool SrtpRequired() const = 0; + virtual bool SetupDataChannelTransport_n(const std::string& mid) = 0; + virtual void TeardownDataChannelTransport_n() = 0; + virtual void SetSctpDataMid(const std::string& mid) = 0; + virtual void ResetSctpDataMid() = 0; + + virtual const FieldTrialsView& trials() const = 0; +}; + +} // namespace webrtc + +#endif // PC_PEER_CONNECTION_SDP_METHODS_H_ diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 2ada253327..caa9ba5d64 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -17,10 +17,12 @@ #include #include "absl/algorithm/container.h" +#include "api/peer_connection_interface.h" #include "api/rtp_parameters.h" #include "api/sequence_checker.h" #include "media/base/codec.h" #include "media/base/media_constants.h" +#include "pc/channel.h" #include "pc/channel_manager.h" #include "pc/rtp_media_utils.h" #include "pc/session_description.h" @@ -157,7 +159,51 @@ RtpTransceiver::~RtpTransceiver() { StopInternal(); } - RTC_CHECK(!channel_) << "Missing call to SetChannel(nullptr)?"; + RTC_CHECK(!channel_) << "Missing call to ClearChannel?"; +} + +RTCError RtpTransceiver::CreateChannel( + absl::string_view mid, + Call* call_ptr, + const cricket::MediaConfig& media_config, + bool srtp_required, + CryptoOptions crypto_options, + const cricket::AudioOptions& audio_options, + const cricket::VideoOptions& video_options, + VideoBitrateAllocatorFactory* video_bitrate_allocator_factory, + std::function transport_lookup) { + RTC_DCHECK_RUN_ON(thread_); + if (!channel_manager_->media_engine()) { + // TODO(hta): Must be a better way + return RTCError(RTCErrorType::INTERNAL_ERROR, + "No media engine for mid=" + std::string(mid)); + } + std::unique_ptr new_channel; + if (media_type() == cricket::MEDIA_TYPE_AUDIO) { + // TODO(bugs.webrtc.org/11992): CreateVideoChannel internally switches to + // the worker thread. We shouldn't be using the `call_ptr_` hack here but + // simply be on the worker thread and use `call_` (update upstream code). + new_channel = channel_manager_->CreateVoiceChannel( + call_ptr, media_config, mid, srtp_required, crypto_options, + audio_options); + + } else { + RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, media_type()); + + // TODO(bugs.webrtc.org/11992): CreateVideoChannel internally switches to + // the worker thread. We shouldn't be using the `call_ptr_` hack here but + // simply be on the worker thread and use `call_` (update upstream code). + new_channel = channel_manager_->CreateVideoChannel( + call_ptr, media_config, mid, srtp_required, crypto_options, + video_options, video_bitrate_allocator_factory); + } + if (!new_channel) { + // TODO(hta): Must be a better way + return RTCError(RTCErrorType::INTERNAL_ERROR, + "Failed to create channel for mid=" + std::string(mid)); + } + SetChannel(std::move(new_channel), transport_lookup); + return RTCError::OK(); } void RtpTransceiver::SetChannel( diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index c0c987787c..6549f9952a 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h @@ -20,6 +20,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/audio_options.h" #include "api/jsep.h" #include "api/media_types.h" #include "api/rtc_error.h" @@ -30,6 +31,8 @@ #include "api/rtp_transceiver_interface.h" #include "api/scoped_refptr.h" #include "api/task_queue/task_queue_base.h" +#include "api/video/video_bitrate_allocator_factory.h" +#include "media/base/media_channel.h" #include "pc/channel_interface.h" #include "pc/channel_manager.h" #include "pc/proxy.h" @@ -45,6 +48,8 @@ namespace webrtc { +class PeerConnectionSdpMethods; + // Implementation of the public RtpTransceiverInterface. // // The RtpTransceiverInterface is only intended to be used with a PeerConnection @@ -66,10 +71,8 @@ namespace webrtc { // with this m= section. Since the transceiver, senders, and receivers are // reference counted and can be referenced from JavaScript (in Chromium), these // objects must be ready to live for an arbitrary amount of time. The -// BaseChannel is not reference counted and is owned by the ChannelManager, so -// the PeerConnection must take care of creating/deleting the BaseChannel and -// setting the channel reference in the transceiver to null when it has been -// deleted. +// BaseChannel is not reference counted, so +// the PeerConnection must take care of creating/deleting the BaseChannel. // // The RtpTransceiver is specialized to either audio or video according to the // MediaType specified in the constructor. Audio RtpTransceivers will have @@ -102,6 +105,18 @@ class RtpTransceiver : public RtpTransceiverInterface, // the transceiver is not in the currently set local/remote description. cricket::ChannelInterface* channel() const { return channel_.get(); } + // Creates the Voice/VideoChannel and sets it. + RTCError CreateChannel( + absl::string_view mid, + Call* call_ptr, + const cricket::MediaConfig& media_config, + bool srtp_required, + CryptoOptions crypto_options, + const cricket::AudioOptions& audio_options, + const cricket::VideoOptions& video_options, + VideoBitrateAllocatorFactory* video_bitrate_allocator_factory, + std::function transport_lookup); + // Sets the Voice/VideoChannel. The caller must pass in the correct channel // implementation based on the type of the transceiver. The call must // furthermore be made on the signaling thread. diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index aa8e17a010..ff8cf0a870 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -26,6 +26,7 @@ #include "api/array_view.h" #include "api/crypto/crypto_options.h" #include "api/dtls_transport_interface.h" +#include "api/field_trials_view.h" #include "api/rtp_parameters.h" #include "api/rtp_receiver_interface.h" #include "api/rtp_sender_interface.h" @@ -39,7 +40,6 @@ #include "p2p/base/transport_description.h" #include "p2p/base/transport_description_factory.h" #include "p2p/base/transport_info.h" -#include "pc/channel.h" #include "pc/channel_interface.h" #include "pc/dtls_transport.h" #include "pc/media_stream.h" @@ -3652,24 +3652,17 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiverChannel( } } else { if (!channel) { - std::unique_ptr new_channel; - if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) { - new_channel = CreateVoiceChannel(content.name); - } else { - RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, transceiver->media_type()); - new_channel = CreateVideoChannel(content.name); - } - if (!new_channel) { - return RTCError(RTCErrorType::INTERNAL_ERROR, - "Failed to create channel for mid=" + content.name); - } - // Note: this is a thread hop; the lambda will be executed - // on the network thread. - transceiver->internal()->SetChannel( - std::move(new_channel), [&](const std::string& mid) { + auto error = transceiver->internal()->CreateChannel( + content.name, pc_->call_ptr(), pc_->configuration()->media_config, + pc_->SrtpRequired(), pc_->GetCryptoOptions(), audio_options(), + video_options(), video_bitrate_allocator_factory_.get(), + [&](absl::string_view mid) { RTC_DCHECK_RUN_ON(network_thread()); return transport_controller_n()->GetRtpTransport(mid); }); + if (!error.ok()) { + return error; + } } } return RTCError::OK(); @@ -4842,33 +4835,36 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { const cricket::ContentInfo* voice = cricket::GetFirstAudioContent(&desc); if (voice && !voice->rejected && !rtp_manager()->GetAudioTransceiver()->internal()->channel()) { - std::unique_ptr voice_channel = - CreateVoiceChannel(voice->name); - if (!voice_channel) { - return RTCError(RTCErrorType::INTERNAL_ERROR, - "Failed to create voice channel."); + auto error = + rtp_manager()->GetAudioTransceiver()->internal()->CreateChannel( + voice->name, pc_->call_ptr(), pc_->configuration()->media_config, + pc_->SrtpRequired(), pc_->GetCryptoOptions(), audio_options(), + video_options(), video_bitrate_allocator_factory_.get(), + [&](absl::string_view mid) { + RTC_DCHECK_RUN_ON(network_thread()); + return transport_controller_n()->GetRtpTransport(mid); + }); + if (!error.ok()) { + return error; } - rtp_manager()->GetAudioTransceiver()->internal()->SetChannel( - std::move(voice_channel), [&](const std::string& mid) { - RTC_DCHECK_RUN_ON(network_thread()); - return transport_controller_n()->GetRtpTransport(mid); - }); } const cricket::ContentInfo* video = cricket::GetFirstVideoContent(&desc); if (video && !video->rejected && !rtp_manager()->GetVideoTransceiver()->internal()->channel()) { - std::unique_ptr video_channel = - CreateVideoChannel(video->name); - if (!video_channel) { - return RTCError(RTCErrorType::INTERNAL_ERROR, - "Failed to create video channel."); + auto error = + rtp_manager()->GetVideoTransceiver()->internal()->CreateChannel( + video->name, pc_->call_ptr(), pc_->configuration()->media_config, + pc_->SrtpRequired(), pc_->GetCryptoOptions(), + + audio_options(), video_options(), + video_bitrate_allocator_factory_.get(), [&](absl::string_view mid) { + RTC_DCHECK_RUN_ON(network_thread()); + return transport_controller_n()->GetRtpTransport(mid); + }); + if (!error.ok()) { + return error; } - rtp_manager()->GetVideoTransceiver()->internal()->SetChannel( - std::move(video_channel), [&](const std::string& mid) { - RTC_DCHECK_RUN_ON(network_thread()); - return transport_controller_n()->GetRtpTransport(mid); - }); } const cricket::ContentInfo* data = cricket::GetFirstDataContent(&desc); @@ -4883,39 +4879,6 @@ RTCError SdpOfferAnswerHandler::CreateChannels(const SessionDescription& desc) { return RTCError::OK(); } -// TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. -std::unique_ptr -SdpOfferAnswerHandler::CreateVoiceChannel(const std::string& mid) { - TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::CreateVoiceChannel"); - RTC_DCHECK_RUN_ON(signaling_thread()); - if (!channel_manager()->media_engine()) - return nullptr; - - // TODO(bugs.webrtc.org/11992): CreateVoiceChannel internally switches to the - // worker thread. We shouldn't be using the `call_ptr_` hack here but simply - // be on the worker thread and use `call_` (update upstream code). - return channel_manager()->CreateVoiceChannel( - pc_->call_ptr(), pc_->configuration()->media_config, mid, - pc_->SrtpRequired(), pc_->GetCryptoOptions(), audio_options()); -} - -// TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. -std::unique_ptr -SdpOfferAnswerHandler::CreateVideoChannel(const std::string& mid) { - TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::CreateVideoChannel"); - RTC_DCHECK_RUN_ON(signaling_thread()); - if (!channel_manager()->media_engine()) - return nullptr; - - // TODO(bugs.webrtc.org/11992): CreateVideoChannel internally switches to the - // worker thread. We shouldn't be using the `call_ptr_` hack here but simply - // be on the worker thread and use `call_` (update upstream code). - return channel_manager()->CreateVideoChannel( - pc_->call_ptr(), pc_->configuration()->media_config, mid, - pc_->SrtpRequired(), pc_->GetCryptoOptions(), video_options(), - video_bitrate_allocator_factory_.get()); -} - bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) { RTC_DCHECK_RUN_ON(signaling_thread()); if (!context_->network_thread()->Invoke(RTC_FROM_HERE, [this, &mid] { diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 4463f05ff3..2c0cdd4078 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -527,11 +527,6 @@ class SdpOfferAnswerHandler : public SdpStateProvider, // This method will also delete any existing media channels before creating. RTCError CreateChannels(const cricket::SessionDescription& desc); - // Helper methods to create media channels. - std::unique_ptr CreateVoiceChannel( - const std::string& mid); - std::unique_ptr CreateVideoChannel( - const std::string& mid); bool CreateDataChannel(const std::string& mid); // Destroys the RTP data channel transport and/or the SCTP data channel