diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index b04b3db0e9..56339ff8a8 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -39,7 +39,6 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/strings/string_builder.h" -#include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" @@ -704,7 +703,7 @@ WebRtcVideoChannel::WebRtcVideoChannel( webrtc::VideoDecoderFactory* decoder_factory, webrtc::VideoBitrateAllocatorFactory* bitrate_allocator_factory) : VideoMediaChannel(config), - worker_thread_(rtc::Thread::Current()), + worker_thread_(call->worker_thread()), call_(call), unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), video_config_(config.video), @@ -2066,7 +2065,7 @@ WebRtcVideoChannel::WebRtcVideoSendStream::WebRtcVideoSendStream( // TODO(deadbeef): Don't duplicate information between send_params, // rtp_extensions, options, etc. const VideoSendParameters& send_params) - : worker_thread_(rtc::Thread::Current()), + : worker_thread_(call->worker_thread()), ssrcs_(sp.ssrcs), ssrc_groups_(sp.ssrc_groups), call_(call), diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 0ec7216e6d..c5c06998fa 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -43,10 +43,6 @@ class VideoEncoderFactory; struct MediaConfig; } // namespace webrtc -namespace rtc { -class Thread; -} // namespace rtc - namespace cricket { class WebRtcVideoChannel; @@ -402,7 +398,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, RTC_EXCLUSIVE_LOCKS_REQUIRED(&thread_checker_); webrtc::SequenceChecker thread_checker_; - rtc::Thread* worker_thread_; + webrtc::TaskQueueBase* const worker_thread_; const std::vector ssrcs_ RTC_GUARDED_BY(&thread_checker_); const std::vector ssrc_groups_ RTC_GUARDED_BY(&thread_checker_); webrtc::Call* const call_; @@ -553,7 +549,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, void FillSendAndReceiveCodecStats(VideoMediaInfo* video_media_info) RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_); - rtc::Thread* const worker_thread_; + webrtc::TaskQueueBase* const worker_thread_; webrtc::ScopedTaskSafety task_safety_; webrtc::SequenceChecker network_thread_checker_; webrtc::SequenceChecker thread_checker_; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 717fb89c61..8c5b83dcbc 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -50,7 +50,6 @@ #include "rtc_base/task_utils/pending_task_safety_flag.h" #include "rtc_base/task_utils/to_queued_task.h" #include "rtc_base/third_party/base64/base64.h" -#include "rtc_base/thread.h" #include "rtc_base/trace_event.h" #include "system_wrappers/include/metrics.h" @@ -377,7 +376,7 @@ VoiceMediaChannel* WebRtcVoiceEngine::CreateMediaChannel( const MediaConfig& config, const AudioOptions& options, const webrtc::CryptoOptions& crypto_options) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); + RTC_DCHECK_RUN_ON(call->worker_thread()); return new WebRtcVoiceMediaChannel(this, config, options, crypto_options, call); } @@ -626,19 +625,6 @@ WebRtcVoiceEngine::GetRtpHeaderExtensions() const { return result; } -void WebRtcVoiceEngine::RegisterChannel(WebRtcVoiceMediaChannel* channel) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - RTC_DCHECK(channel); - channels_.push_back(channel); -} - -void WebRtcVoiceEngine::UnregisterChannel(WebRtcVoiceMediaChannel* channel) { - RTC_DCHECK_RUN_ON(&worker_thread_checker_); - auto it = absl::c_find(channels_, channel); - RTC_DCHECK(it != channels_.end()); - channels_.erase(it); -} - bool WebRtcVoiceEngine::StartAecDump(webrtc::FileWrapper file, int64_t max_size_bytes) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); @@ -1396,18 +1382,16 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel( const webrtc::CryptoOptions& crypto_options, webrtc::Call* call) : VoiceMediaChannel(config), - worker_thread_(rtc::Thread::Current()), + worker_thread_(call->worker_thread()), engine_(engine), call_(call), audio_config_(config.audio), crypto_options_(crypto_options), audio_red_for_opus_trial_enabled_( IsEnabled(call->trials(), "WebRTC-Audio-Red-For-Opus")) { - RTC_DCHECK_RUN_ON(worker_thread_); network_thread_checker_.Detach(); RTC_LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel"; RTC_DCHECK(call); - engine->RegisterChannel(this); SetOptions(options); } @@ -1423,7 +1407,6 @@ WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { while (!recv_streams_.empty()) { RemoveRecvStream(recv_streams_.begin()->first); } - engine()->UnregisterChannel(this); } bool WebRtcVoiceMediaChannel::SetSendParameters( diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 5a1cb57ff6..18cbbba29e 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -80,12 +80,6 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { std::vector GetRtpHeaderExtensions() const override; - // For tracking WebRtc channels. Needed because we have to pause them - // all when switching devices. - // May only be called by WebRtcVoiceMediaChannel. - void RegisterChannel(WebRtcVoiceMediaChannel* channel); - void UnregisterChannel(WebRtcVoiceMediaChannel* channel); - // Starts AEC dump using an existing file. A maximum file size in bytes can be // specified. When the maximum file size is reached, logging is stopped and // the file is closed. If max_size_bytes is set to <= 0, no limit will be @@ -129,7 +123,6 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { rtc::scoped_refptr audio_state_; std::vector send_codecs_; std::vector recv_codecs_; - std::vector channels_; bool is_dumping_aec_ = false; bool initialized_ = false; diff --git a/pc/channel.cc b/pc/channel.cc index 897b1aeeb1..53933c33ed 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -295,7 +295,11 @@ void BaseChannel::Enable(bool enable) { bool BaseChannel::SetLocalContent(const MediaContentDescription* content, SdpType type, std::string* error_desc) { + RTC_DCHECK_RUN_ON(signaling_thread()); TRACE_EVENT0("webrtc", "BaseChannel::SetLocalContent"); + + SetContent_s(content, type); + return InvokeOnWorker(RTC_FROM_HERE, [this, content, type, error_desc] { RTC_DCHECK_RUN_ON(worker_thread()); return SetLocalContent_w(content, type, error_desc); @@ -305,13 +309,24 @@ bool BaseChannel::SetLocalContent(const MediaContentDescription* content, bool BaseChannel::SetRemoteContent(const MediaContentDescription* content, SdpType type, std::string* error_desc) { + RTC_DCHECK_RUN_ON(signaling_thread()); TRACE_EVENT0("webrtc", "BaseChannel::SetRemoteContent"); + + SetContent_s(content, type); + return InvokeOnWorker(RTC_FROM_HERE, [this, content, type, error_desc] { RTC_DCHECK_RUN_ON(worker_thread()); return SetRemoteContent_w(content, type, error_desc); }); } +void BaseChannel::SetContent_s(const MediaContentDescription* content, + SdpType type) { + RTC_DCHECK(content); + if (type == SdpType::kAnswer) + negotiated_header_extensions_ = content->rtp_header_extensions(); +} + bool BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) { TRACE_EVENT0("webrtc", "BaseChannel::SetPayloadTypeDemuxingEnabled"); return InvokeOnWorker(RTC_FROM_HERE, [this, enabled] { @@ -853,16 +868,8 @@ void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) { media_channel()->OnPacketSent(sent_packet); } -void BaseChannel::SetNegotiatedHeaderExtensions_w( - const RtpHeaderExtensions& extensions) { - TRACE_EVENT0("webrtc", __func__); - webrtc::MutexLock lock(&negotiated_header_extensions_lock_); - negotiated_header_extensions_ = extensions; -} - RtpHeaderExtensions BaseChannel::GetNegotiatedRtpHeaderExtensions() const { RTC_DCHECK_RUN_ON(signaling_thread()); - webrtc::MutexLock lock(&negotiated_header_extensions_lock_); return negotiated_header_extensions_; } @@ -913,26 +920,19 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, RTC_DCHECK_RUN_ON(worker_thread()); RTC_LOG(LS_INFO) << "Setting local voice description for " << ToString(); - RTC_DCHECK(content); - if (!content) { - SafeSetError("Can't find audio content in local description.", error_desc); - return false; - } - - const AudioContentDescription* audio = content->as_audio(); - - if (type == SdpType::kAnswer) - SetNegotiatedHeaderExtensions_w(audio->rtp_header_extensions()); - RtpHeaderExtensions rtp_header_extensions = - GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(content->rtp_header_extensions()); + // TODO(tommi): There's a hop to the network thread here. + // some of the below is also network thread related. UpdateRtpHeaderExtensionMap(rtp_header_extensions); - media_channel()->SetExtmapAllowMixed(audio->extmap_allow_mixed()); + media_channel()->SetExtmapAllowMixed(content->extmap_allow_mixed()); AudioRecvParameters recv_params = last_recv_params_; RtpParametersFromMediaDescription( - audio, rtp_header_extensions, - webrtc::RtpTransceiverDirectionHasRecv(audio->direction()), &recv_params); + content->as_audio(), rtp_header_extensions, + webrtc::RtpTransceiverDirectionHasRecv(content->direction()), + &recv_params); + if (!media_channel()->SetRecvParameters(recv_params)) { SafeSetError( "Failed to set local audio description recv parameters for m-section " @@ -942,8 +942,8 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, return false; } - if (webrtc::RtpTransceiverDirectionHasRecv(audio->direction())) { - for (const AudioCodec& codec : audio->codecs()) { + if (webrtc::RtpTransceiverDirectionHasRecv(content->direction())) { + for (const AudioCodec& codec : content->as_audio()->codecs()) { MaybeAddHandledPayloadType(codec.id); } // Need to re-register the sink to update the handled payload. @@ -959,7 +959,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, // only give it to the media channel once we have a remote // description too (without a remote description, we won't be able // to send them anyway). - if (!UpdateLocalStreams_w(audio->streams(), type, error_desc)) { + if (!UpdateLocalStreams_w(content->as_audio()->streams(), type, error_desc)) { SafeSetError( "Failed to set local audio description streams for m-section with " "mid='" + @@ -980,17 +980,8 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, RTC_DCHECK_RUN_ON(worker_thread()); RTC_LOG(LS_INFO) << "Setting remote voice description for " << ToString(); - RTC_DCHECK(content); - if (!content) { - SafeSetError("Can't find audio content in remote description.", error_desc); - return false; - } - const AudioContentDescription* audio = content->as_audio(); - if (type == SdpType::kAnswer) - SetNegotiatedHeaderExtensions_w(audio->rtp_header_extensions()); - RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(audio->rtp_header_extensions()); @@ -1091,26 +1082,17 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, RTC_DCHECK_RUN_ON(worker_thread()); RTC_LOG(LS_INFO) << "Setting local video description for " << ToString(); - RTC_DCHECK(content); - if (!content) { - SafeSetError("Can't find video content in local description.", error_desc); - return false; - } - - const VideoContentDescription* video = content->as_video(); - - if (type == SdpType::kAnswer) - SetNegotiatedHeaderExtensions_w(video->rtp_header_extensions()); - RtpHeaderExtensions rtp_header_extensions = - GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); + GetFilteredRtpHeaderExtensions(content->rtp_header_extensions()); UpdateRtpHeaderExtensionMap(rtp_header_extensions); - media_channel()->SetExtmapAllowMixed(video->extmap_allow_mixed()); + media_channel()->SetExtmapAllowMixed(content->extmap_allow_mixed()); VideoRecvParameters recv_params = last_recv_params_; + RtpParametersFromMediaDescription( - video, rtp_header_extensions, - webrtc::RtpTransceiverDirectionHasRecv(video->direction()), &recv_params); + content->as_video(), rtp_header_extensions, + webrtc::RtpTransceiverDirectionHasRecv(content->direction()), + &recv_params); VideoSendParameters send_params = last_send_params_; @@ -1143,8 +1125,8 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, return false; } - if (webrtc::RtpTransceiverDirectionHasRecv(video->direction())) { - for (const VideoCodec& codec : video->codecs()) { + if (webrtc::RtpTransceiverDirectionHasRecv(content->direction())) { + for (const VideoCodec& codec : content->as_video()->codecs()) { MaybeAddHandledPayloadType(codec.id); } // Need to re-register the sink to update the handled payload. @@ -1170,7 +1152,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, // only give it to the media channel once we have a remote // description too (without a remote description, we won't be able // to send them anyway). - if (!UpdateLocalStreams_w(video->streams(), type, error_desc)) { + if (!UpdateLocalStreams_w(content->as_video()->streams(), type, error_desc)) { SafeSetError( "Failed to set local video description streams for m-section with " "mid='" + @@ -1191,17 +1173,8 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, RTC_DCHECK_RUN_ON(worker_thread()); RTC_LOG(LS_INFO) << "Setting remote video description for " << ToString(); - RTC_DCHECK(content); - if (!content) { - SafeSetError("Can't find video content in remote description.", error_desc); - return false; - } - const VideoContentDescription* video = content->as_video(); - if (type == SdpType::kAnswer) - SetNegotiatedHeaderExtensions_w(video->rtp_header_extensions()); - RtpHeaderExtensions rtp_header_extensions = GetFilteredRtpHeaderExtensions(video->rtp_header_extensions()); diff --git a/pc/channel.h b/pc/channel.h index 24d609144e..528e7d0ac6 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -59,7 +59,6 @@ #include "rtc_base/network/sent_packet.h" #include "rtc_base/network_route.h" #include "rtc_base/socket.h" -#include "rtc_base/synchronization/mutex.h" #include "rtc_base/task_utils/pending_task_safety_flag.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread.h" @@ -306,9 +305,6 @@ class BaseChannel : public ChannelInterface, // Return description of media channel to facilitate logging std::string ToString() const; - void SetNegotiatedHeaderExtensions_w(const RtpHeaderExtensions& extensions) - RTC_RUN_ON(worker_thread()); - // ChannelInterface overrides RtpHeaderExtensions GetNegotiatedRtpHeaderExtensions() const override; @@ -316,6 +312,8 @@ class BaseChannel : public ChannelInterface, bool ConnectToRtpTransport() RTC_RUN_ON(network_thread()); void DisconnectFromRtpTransport() RTC_RUN_ON(network_thread()); void SignalSentPacket_n(const rtc::SentPacket& sent_packet); + void SetContent_s(const MediaContentDescription* content, + webrtc::SdpType type) RTC_RUN_ON(signaling_thread()); rtc::Thread* const worker_thread_; rtc::Thread* const network_thread_; @@ -382,13 +380,11 @@ class BaseChannel : public ChannelInterface, // This object is not owned by the channel so it must outlive it. rtc::UniqueRandomIdGenerator* const ssrc_generator_; - // |negotiated_header_extensions_| is read on the signaling thread, but - // written on the worker thread while being sync-invoked from the signal - // thread in SdpOfferAnswerHandler::PushdownMediaDescription(). Hence the lock - // isn't strictly needed, but it's anyway placed here for future safeness. - mutable webrtc::Mutex negotiated_header_extensions_lock_; + // |negotiated_header_extensions_| is read and written to on the signaling + // thread from the SdpOfferAnswerHandler class (e.g. + // PushdownMediaDescription(). RtpHeaderExtensions negotiated_header_extensions_ - RTC_GUARDED_BY(negotiated_header_extensions_lock_); + RTC_GUARDED_BY(signaling_thread()); }; // VoiceChannel is a specialization that adds support for early media, DTMF,