diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 536df8e22b..99f3898e68 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -343,6 +343,7 @@ void WebRtcVoiceEngine::Init() { RTC_LOG(LS_INFO) << "WebRtcVoiceEngine::Init"; // TaskQueue expects to be created/destroyed on the same thread. + RTC_DCHECK(!low_priority_worker_queue_); low_priority_worker_queue_.reset( new rtc::TaskQueue(task_queue_factory_->CreateTaskQueue( "rtc-low-prio", webrtc::TaskQueueFactory::Priority::LOW))); diff --git a/pc/channel.cc b/pc/channel.cc index dac0cbcf68..5210ae4237 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& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) @@ -131,7 +131,7 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, ? webrtc::RtpExtension::kPreferEncryptedExtension : webrtc::RtpExtension::kDiscardEncryptedExtension), media_channel_(std::move(media_channel)), - demuxer_criteria_(content_name), + demuxer_criteria_(mid), ssrc_generator_(ssrc_generator) { RTC_DCHECK_RUN_ON(worker_thread_); RTC_DCHECK(media_channel_); @@ -151,7 +151,7 @@ BaseChannel::~BaseChannel() { } std::string BaseChannel::ToString() const { - return StringFormat("{mid: %s, media_type: %s}", content_name().c_str(), + return StringFormat("{mid: %s, media_type: %s}", mid().c_str(), MediaTypeToString(media_channel_->media_type()).c_str()); } @@ -618,7 +618,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, error_desc = StringFormat( "Failed to remove send stream with ssrc %u from m-section with " "mid='%s'.", - old_stream.first_ssrc(), content_name().c_str()); + old_stream.first_ssrc(), mid().c_str()); ret = false; } } @@ -644,7 +644,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, error_desc = StringFormat( "Failed to add send stream: %u into m-section with mid='%s'. Stream " "has both SSRCs and RIDs.", - new_stream.first_ssrc(), content_name().c_str()); + new_stream.first_ssrc(), mid().c_str()); ret = false; continue; } @@ -663,7 +663,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector& streams, } else { error_desc = StringFormat( "Failed to add send stream ssrc: %u into m-section with mid='%s'", - new_stream.first_ssrc(), content_name().c_str()); + new_stream.first_ssrc(), mid().c_str()); ret = false; } } @@ -706,7 +706,7 @@ bool BaseChannel::UpdateRemoteStreams_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to remove remote stream with ssrc %u from m-section with " "mid='%s'.", - old_stream.first_ssrc(), content_name().c_str()); + old_stream.first_ssrc(), mid().c_str()); return false; } } @@ -750,7 +750,7 @@ bool BaseChannel::UpdateRemoteStreams_w(const MediaContentDescription* content, // Re-register the sink to update after changing the demuxer criteria. if (needs_re_registration && !RegisterRtpDemuxerSink_w()) { error_desc = StringFormat("Failed to set up audio demuxing for mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } @@ -800,7 +800,7 @@ VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) @@ -808,7 +808,7 @@ VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, network_thread, signaling_thread, std::move(media_channel), - content_name, + mid, srtp_required, crypto_options, ssrc_generator) {} @@ -858,7 +858,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to set local audio description recv parameters for m-section " "with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } @@ -903,14 +903,14 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content, AudioSendParameters send_params = last_send_params_; RtpSendParametersFromMediaDescription(content->as_audio(), extensions_filter(), &send_params); - send_params.mid = content_name(); + send_params.mid = mid(); bool parameters_applied = media_channel()->SetSendParameters(send_params); if (!parameters_applied) { error_desc = StringFormat( "Failed to set remote audio description send parameters for m-section " "with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } last_send_params_ = send_params; @@ -922,7 +922,7 @@ VideoChannel::VideoChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, UniqueRandomIdGenerator* ssrc_generator) @@ -930,7 +930,7 @@ VideoChannel::VideoChannel(rtc::Thread* worker_thread, network_thread, signaling_thread, std::move(media_channel), - content_name, + mid, srtp_required, crypto_options, ssrc_generator) {} @@ -990,7 +990,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to set local answer due to invalid codec packetization " "specified in m-section with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } } @@ -1001,7 +1001,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to set local video description recv parameters for m-section " "with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } @@ -1019,7 +1019,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, if (!media_channel()->SetSendParameters(send_params)) { error_desc = StringFormat( "Failed to set send parameters for m-section with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } last_send_params_ = send_params; @@ -1057,7 +1057,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, VideoSendParameters send_params = last_send_params_; RtpSendParametersFromMediaDescription(video, extensions_filter(), &send_params); - send_params.mid = content_name(); + send_params.mid = mid(); send_params.conference_mode = video->conference_mode(); VideoRecvParameters recv_params = last_recv_params_; @@ -1074,7 +1074,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to set remote answer due to invalid codec packetization " "specifid in m-section with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } } @@ -1085,7 +1085,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, error_desc = StringFormat( "Failed to set remote video description send parameters for m-section " "with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } last_send_params_ = send_params; @@ -1094,7 +1094,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, if (!media_channel()->SetRecvParameters(recv_params)) { error_desc = StringFormat( "Failed to set recv parameters for m-section with mid='%s'.", - content_name().c_str()); + mid().c_str()); return false; } last_recv_params_ = recv_params; diff --git a/pc/channel.h b/pc/channel.h index fd5495c152..829d29eefa 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -105,7 +105,7 @@ class BaseChannel : public ChannelInterface, rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); @@ -113,9 +113,7 @@ class BaseChannel : public ChannelInterface, rtc::Thread* worker_thread() const { return worker_thread_; } rtc::Thread* network_thread() const { return network_thread_; } - const std::string& content_name() const override { - return demuxer_criteria_.mid(); - } + const std::string& mid() const override { return demuxer_criteria_.mid(); } // TODO(deadbeef): This is redundant; remove this. absl::string_view transport_name() const override { RTC_DCHECK_RUN_ON(network_thread()); @@ -374,7 +372,7 @@ class VoiceChannel : public BaseChannel { rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr channel, - const std::string& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); @@ -416,7 +414,7 @@ class VideoChannel : public BaseChannel { rtc::Thread* network_thread, rtc::Thread* signaling_thread, std::unique_ptr media_channel, - const std::string& content_name, + const std::string& mid, bool srtp_required, webrtc::CryptoOptions crypto_options, rtc::UniqueRandomIdGenerator* ssrc_generator); diff --git a/pc/channel_interface.h b/pc/channel_interface.h index 6185b69e6a..6033046b92 100644 --- a/pc/channel_interface.h +++ b/pc/channel_interface.h @@ -20,9 +20,17 @@ #include "media/base/media_channel.h" #include "pc/rtp_transport_internal.h" +namespace webrtc { +class Call; +class VideoBitrateAllocatorFactory; +} // namespace webrtc + namespace cricket { class MediaContentDescription; +class VideoChannel; +class VoiceChannel; +struct MediaConfig; // ChannelInterface contains methods common to voice, video and data channels. // As more methods are added to BaseChannel, they should be included in the @@ -39,7 +47,8 @@ class ChannelInterface { // TODO(deadbeef): This is redundant; remove this. virtual absl::string_view transport_name() const = 0; - virtual const std::string& content_name() const = 0; + // TODO(tommi): Change return type to string_view. + virtual const std::string& mid() const = 0; // Enables or disables this channel virtual void Enable(bool enable) = 0; @@ -72,6 +81,32 @@ class ChannelInterface { virtual ~ChannelInterface() = default; }; +class ChannelFactoryInterface { + public: + virtual VideoChannel* CreateVideoChannel( + webrtc::Call* call, + const MediaConfig& media_config, + const std::string& mid, + bool srtp_required, + const webrtc::CryptoOptions& crypto_options, + const VideoOptions& options, + webrtc::VideoBitrateAllocatorFactory* + video_bitrate_allocator_factory) = 0; + + virtual VoiceChannel* CreateVoiceChannel( + webrtc::Call* call, + const MediaConfig& media_config, + const std::string& mid, + bool srtp_required, + const webrtc::CryptoOptions& crypto_options, + const AudioOptions& options) = 0; + + virtual void DestroyChannel(ChannelInterface* channel) = 0; + + protected: + virtual ~ChannelFactoryInterface() = default; +}; + } // namespace cricket #endif // PC_CHANNEL_INTERFACE_H_ diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 304283eca4..b583b94e3d 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -31,13 +31,9 @@ std::unique_ptr ChannelManager::Create( bool enable_rtx, rtc::Thread* worker_thread, rtc::Thread* network_thread) { - RTC_DCHECK_RUN_ON(worker_thread); RTC_DCHECK(network_thread); RTC_DCHECK(worker_thread); - if (media_engine) - media_engine->Init(); - return absl::WrapUnique(new ChannelManager( std::move(media_engine), enable_rtx, worker_thread, network_thread)); } @@ -48,16 +44,33 @@ ChannelManager::ChannelManager( rtc::Thread* worker_thread, rtc::Thread* network_thread) : media_engine_(std::move(media_engine)), + signaling_thread_(rtc::Thread::Current()), worker_thread_(worker_thread), network_thread_(network_thread), enable_rtx_(enable_rtx) { + RTC_DCHECK_RUN_ON(signaling_thread_); RTC_DCHECK(worker_thread_); RTC_DCHECK(network_thread_); - RTC_DCHECK_RUN_ON(worker_thread_); + + if (media_engine_) { + // TODO(tommi): Change VoiceEngine to do ctor time initialization so that + // this isn't necessary. + worker_thread_->Invoke(RTC_FROM_HERE, [&] { media_engine_->Init(); }); + } } ChannelManager::~ChannelManager() { - RTC_DCHECK_RUN_ON(worker_thread_); + RTC_DCHECK_RUN_ON(signaling_thread_); + if (media_engine_) { + // While `media_engine_` is const throughout the ChannelManager's lifetime, + // it requires destruction to happen on the worker thread. Instead of + // marking the pointer as non-const, we live with this const_cast<> in the + // destructor. + worker_thread_->Invoke(RTC_FROM_HERE, [&] { + const_cast&>(media_engine_).reset(); + }); + } + RTC_DCHECK(voice_channels_.empty()); RTC_DCHECK(video_channels_.empty()); } @@ -143,11 +156,9 @@ ChannelManager::GetSupportedVideoRtpHeaderExtensions() const { VoiceChannel* ChannelManager::CreateVoiceChannel( webrtc::Call* call, const MediaConfig& media_config, - rtc::Thread* signaling_thread, - const std::string& content_name, + const std::string& mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, const AudioOptions& options) { RTC_DCHECK(call); RTC_DCHECK(media_engine_); @@ -156,9 +167,8 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( // thread. if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - return CreateVoiceChannel(call, media_config, signaling_thread, - content_name, srtp_required, crypto_options, - ssrc_generator, options); + return CreateVoiceChannel(call, media_config, mid, srtp_required, + crypto_options, options); }); } @@ -171,41 +181,28 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( } auto voice_channel = std::make_unique( - worker_thread_, network_thread_, signaling_thread, - absl::WrapUnique(media_channel), content_name, srtp_required, - crypto_options, ssrc_generator); + worker_thread_, network_thread_, signaling_thread_, + absl::WrapUnique(media_channel), mid, srtp_required, crypto_options, + &ssrc_generator_); VoiceChannel* voice_channel_ptr = voice_channel.get(); voice_channels_.push_back(std::move(voice_channel)); return voice_channel_ptr; } -void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel) { +void ChannelManager::DestroyVoiceChannel(VoiceChannel* channel) { TRACE_EVENT0("webrtc", "ChannelManager::DestroyVoiceChannel"); - RTC_DCHECK(voice_channel); - - if (!worker_thread_->IsCurrent()) { - worker_thread_->Invoke(RTC_FROM_HERE, - [&] { DestroyVoiceChannel(voice_channel); }); - return; - } - RTC_DCHECK_RUN_ON(worker_thread_); - voice_channels_.erase(absl::c_find_if( - voice_channels_, [&](const std::unique_ptr& p) { - return p.get() == voice_channel; - })); + voice_channels_, [&](const auto& p) { return p.get() == channel; })); } VideoChannel* ChannelManager::CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - rtc::Thread* signaling_thread, - const std::string& content_name, + const std::string& mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, const VideoOptions& options, webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) { RTC_DCHECK(call); @@ -215,9 +212,8 @@ VideoChannel* ChannelManager::CreateVideoChannel( // thread. if (!worker_thread_->IsCurrent()) { return worker_thread_->Invoke(RTC_FROM_HERE, [&] { - return CreateVideoChannel(call, media_config, signaling_thread, - content_name, srtp_required, crypto_options, - ssrc_generator, options, + return CreateVideoChannel(call, media_config, mid, srtp_required, + crypto_options, options, video_bitrate_allocator_factory); }); } @@ -232,30 +228,40 @@ VideoChannel* ChannelManager::CreateVideoChannel( } auto video_channel = std::make_unique( - worker_thread_, network_thread_, signaling_thread, - absl::WrapUnique(media_channel), content_name, srtp_required, - crypto_options, ssrc_generator); + worker_thread_, network_thread_, signaling_thread_, + absl::WrapUnique(media_channel), mid, srtp_required, crypto_options, + &ssrc_generator_); VideoChannel* video_channel_ptr = video_channel.get(); video_channels_.push_back(std::move(video_channel)); return video_channel_ptr; } -void ChannelManager::DestroyVideoChannel(VideoChannel* video_channel) { +void ChannelManager::DestroyVideoChannel(VideoChannel* channel) { TRACE_EVENT0("webrtc", "ChannelManager::DestroyVideoChannel"); - RTC_DCHECK(video_channel); - - if (!worker_thread_->IsCurrent()) { - worker_thread_->Invoke(RTC_FROM_HERE, - [&] { DestroyVideoChannel(video_channel); }); - return; - } RTC_DCHECK_RUN_ON(worker_thread_); video_channels_.erase(absl::c_find_if( - video_channels_, [&](const std::unique_ptr& p) { - return p.get() == video_channel; - })); + video_channels_, [&](const auto& p) { return p.get() == channel; })); +} + +void ChannelManager::DestroyChannel(ChannelInterface* channel) { + RTC_DCHECK(channel); + + // TODO(bugs.webrtc.org/11992): Change to either be called on the worker + // thread, or do this asynchronously on the worker. + if (!worker_thread_->IsCurrent()) { + worker_thread_->Invoke(RTC_FROM_HERE, + [&] { DestroyChannel(channel); }); + return; + } + + if (channel->media_type() == MEDIA_TYPE_AUDIO) { + DestroyVoiceChannel(static_cast(channel)); + } else { + RTC_DCHECK_EQ(channel->media_type(), MEDIA_TYPE_VIDEO); + DestroyVideoChannel(static_cast(channel)); + } } bool ChannelManager::StartAecDump(webrtc::FileWrapper file, diff --git a/pc/channel_manager.h b/pc/channel_manager.h index 0952099e82..9e4e1bf131 100644 --- a/pc/channel_manager.h +++ b/pc/channel_manager.h @@ -43,7 +43,7 @@ namespace cricket { // voice or just video channels. // ChannelManager also allows the application to discover what devices it has // using device manager. -class ChannelManager final { +class ChannelManager final : public ChannelFactoryInterface { public: // Returns an initialized instance of ChannelManager. // If media_engine is non-nullptr, then the returned ChannelManager instance @@ -55,11 +55,12 @@ class ChannelManager final { rtc::Thread* network_thread); ChannelManager() = delete; - ~ChannelManager(); + ~ChannelManager() override; rtc::Thread* worker_thread() const { return worker_thread_; } rtc::Thread* network_thread() const { return network_thread_; } MediaEngineInterface* media_engine() { return media_engine_.get(); } + rtc::UniqueRandomIdGenerator& ssrc_generator() { return ssrc_generator_; } // Retrieves the list of supported audio & video codec types. // Can be called before starting the media engine. @@ -81,14 +82,10 @@ class ChannelManager final { // Creates a voice channel, to be associated with the specified session. VoiceChannel* CreateVoiceChannel(webrtc::Call* call, const MediaConfig& media_config, - rtc::Thread* signaling_thread, - const std::string& content_name, + const std::string& mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, - const AudioOptions& options); - // Destroys a voice channel created by CreateVoiceChannel. - void DestroyVoiceChannel(VoiceChannel* voice_channel); + const AudioOptions& options) override; // Creates a video channel, synced with the specified voice channel, and // associated with the specified session. @@ -96,15 +93,14 @@ class ChannelManager final { VideoChannel* CreateVideoChannel( webrtc::Call* call, const MediaConfig& media_config, - rtc::Thread* signaling_thread, - const std::string& content_name, + const std::string& mid, bool srtp_required, const webrtc::CryptoOptions& crypto_options, - rtc::UniqueRandomIdGenerator* ssrc_generator, const VideoOptions& options, - webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory); - // Destroys a video channel created by CreateVideoChannel. - void DestroyVideoChannel(VideoChannel* video_channel); + webrtc::VideoBitrateAllocatorFactory* video_bitrate_allocator_factory) + override; + + void DestroyChannel(ChannelInterface* channel) override; // Starts AEC dump using existing file, with a specified maximum file size in // bytes. When the limit is reached, logging will stop and the file will be @@ -120,10 +116,23 @@ class ChannelManager final { rtc::Thread* worker_thread, rtc::Thread* network_thread); + // Destroys a voice channel created by CreateVoiceChannel. + void DestroyVoiceChannel(VoiceChannel* voice_channel); + + // Destroys a video channel created by CreateVideoChannel. + void DestroyVideoChannel(VideoChannel* video_channel); + const std::unique_ptr media_engine_; // Nullable. + rtc::Thread* const signaling_thread_; rtc::Thread* const worker_thread_; rtc::Thread* const network_thread_; + // This object should be used to generate any SSRC that is not explicitly + // specified by the user (or by the remote party). + // TODO(bugs.webrtc.org/12666): This variable is used from both the signaling + // and worker threads. See if we can't restrict usage to a single thread. + rtc::UniqueRandomIdGenerator ssrc_generator_; + // Vector contents are non-null. std::vector> voice_channels_ RTC_GUARDED_BY(worker_thread_); diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc index 80f30c1c71..765e8e144d 100644 --- a/pc/channel_manager_unittest.cc +++ b/pc/channel_manager_unittest.cc @@ -69,20 +69,18 @@ class ChannelManagerTest : public ::testing::Test { void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) { RTC_DCHECK_RUN_ON(worker_); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), - cricket::CN_AUDIO, kDefaultSrtpRequired, webrtc::CryptoOptions(), - &ssrc_generator_, AudioOptions()); + &fake_call_, cricket::MediaConfig(), cricket::CN_AUDIO, + kDefaultSrtpRequired, webrtc::CryptoOptions(), AudioOptions()); ASSERT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( - &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), - cricket::CN_VIDEO, kDefaultSrtpRequired, webrtc::CryptoOptions(), - &ssrc_generator_, VideoOptions(), + &fake_call_, cricket::MediaConfig(), cricket::CN_VIDEO, + kDefaultSrtpRequired, webrtc::CryptoOptions(), VideoOptions(), video_bitrate_allocator_factory_.get()); ASSERT_TRUE(video_channel != nullptr); - cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel); + cm_->DestroyChannel(video_channel); + cm_->DestroyChannel(voice_channel); } std::unique_ptr network_; @@ -91,7 +89,6 @@ class ChannelManagerTest : public ::testing::Test { video_bitrate_allocator_factory_; std::unique_ptr cm_; cricket::FakeCall fake_call_; - rtc::UniqueRandomIdGenerator ssrc_generator_; }; TEST_F(ChannelManagerTest, SetVideoRtxEnabled) { diff --git a/pc/connection_context.cc b/pc/connection_context.cc index 6c6da4e476..558702a72e 100644 --- a/pc/connection_context.cc +++ b/pc/connection_context.cc @@ -129,11 +129,9 @@ ConnectionContext::ConnectionContext( default_socket_factory_ = std::make_unique( network_thread()->socketserver()); - worker_thread_->Invoke(RTC_FROM_HERE, [&]() { - channel_manager_ = cricket::ChannelManager::Create( - std::move(dependencies->media_engine), - /*enable_rtx=*/true, worker_thread(), network_thread()); - }); + channel_manager_ = cricket::ChannelManager::Create( + std::move(dependencies->media_engine), + /*enable_rtx=*/true, worker_thread(), network_thread()); // Set warning levels on the threads, to give warnings when response // may be slower than is expected of the thread. @@ -147,8 +145,7 @@ ConnectionContext::ConnectionContext( ConnectionContext::~ConnectionContext() { RTC_DCHECK_RUN_ON(signaling_thread_); - worker_thread_->Invoke(RTC_FROM_HERE, - [&]() { channel_manager_.reset(nullptr); }); + channel_manager_.reset(nullptr); // Make sure `worker_thread()` and `signaling_thread()` outlive // `default_socket_factory_` and `default_network_manager_`. diff --git a/pc/media_session.cc b/pc/media_session.cc index ed292cf5de..637dbec73b 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -1534,9 +1534,9 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( MediaSessionDescriptionFactory::MediaSessionDescriptionFactory( ChannelManager* channel_manager, - const TransportDescriptionFactory* transport_desc_factory, - rtc::UniqueRandomIdGenerator* ssrc_generator) - : MediaSessionDescriptionFactory(transport_desc_factory, ssrc_generator) { + const TransportDescriptionFactory* transport_desc_factory) + : MediaSessionDescriptionFactory(transport_desc_factory, + &channel_manager->ssrc_generator()) { channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_); channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_); channel_manager->GetSupportedVideoSendCodecs(&video_send_codecs_); diff --git a/pc/media_session.h b/pc/media_session.h index bb97f42b27..aa24f015de 100644 --- a/pc/media_session.h +++ b/pc/media_session.h @@ -141,8 +141,7 @@ class MediaSessionDescriptionFactory { // This helper automatically sets up the factory to get its configuration // from the specified ChannelManager. MediaSessionDescriptionFactory(ChannelManager* cmanager, - const TransportDescriptionFactory* factory, - rtc::UniqueRandomIdGenerator* ssrc_generator); + const TransportDescriptionFactory* factory); const AudioCodecs& audio_sendrecv_codecs() const; const AudioCodecs& audio_send_codecs() const; diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 3c6402cead..3e1a09bb50 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -2111,11 +2111,10 @@ void PeerConnection::StopRtcEventLog_w() { } } -cricket::ChannelInterface* PeerConnection::GetChannel( - const std::string& content_name) { +cricket::ChannelInterface* PeerConnection::GetChannel(const std::string& mid) { for (const auto& transceiver : rtp_manager()->transceivers()->UnsafeList()) { cricket::ChannelInterface* channel = transceiver->internal()->channel(); - if (channel && channel->content_name() == content_name) { + if (channel && channel->mid() == mid) { return channel; } } diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 59f43c7d9a..20d946dfdc 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -432,7 +432,7 @@ class PeerConnection : public PeerConnectionInternal, 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) + cricket::ChannelInterface* GetChannel(const std::string& mid) RTC_RUN_ON(network_thread()); // Functions made public for testing. diff --git a/pc/peer_connection_ice_unittest.cc b/pc/peer_connection_ice_unittest.cc index 49a79a2556..59f5b5620d 100644 --- a/pc/peer_connection_ice_unittest.cc +++ b/pc/peer_connection_ice_unittest.cc @@ -224,7 +224,7 @@ class PeerConnectionIceBaseTest : public ::testing::Test { for (const auto& transceiver : pc->GetTransceiversInternal()) { if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) { auto dtls_transport = pc->LookupDtlsTransportByMidInternal( - transceiver->internal()->channel()->content_name()); + transceiver->internal()->channel()->mid()); return dtls_transport->ice_transport()->internal()->GetIceRole(); } } diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 6ebbe3deba..ca1671a39e 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -2152,7 +2152,7 @@ void RTCStatsCollector::PrepareTransceiverStatsInfosAndCallStats_s_w_n() { continue; } - stats.mid = channel->content_name(); + stats.mid = channel->mid(); stats.transport_name = std::string(channel->transport_name()); if (media_type == cricket::MEDIA_TYPE_AUDIO) { diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 6127622482..2c6bc7b71a 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -122,13 +122,11 @@ class RtpSenderReceiverTest rtp_transport_ = CreateDtlsSrtpTransport(); voice_channel_ = channel_manager_->CreateVoiceChannel( - &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), - cricket::CN_AUDIO, srtp_required, webrtc::CryptoOptions(), - &ssrc_generator_, cricket::AudioOptions()); + &fake_call_, cricket::MediaConfig(), cricket::CN_AUDIO, srtp_required, + webrtc::CryptoOptions(), cricket::AudioOptions()); video_channel_ = channel_manager_->CreateVideoChannel( - &fake_call_, cricket::MediaConfig(), rtc::Thread::Current(), - cricket::CN_VIDEO, srtp_required, webrtc::CryptoOptions(), - &ssrc_generator_, cricket::VideoOptions(), + &fake_call_, cricket::MediaConfig(), cricket::CN_VIDEO, srtp_required, + webrtc::CryptoOptions(), cricket::VideoOptions(), video_bitrate_allocator_factory_.get()); voice_channel_->SetRtpTransport(rtp_transport_.get()); @@ -177,11 +175,8 @@ class RtpSenderReceiverTest voice_channel_->SetRtpTransport(nullptr); video_channel_->SetRtpTransport(nullptr); - channel_manager_->DestroyVoiceChannel(voice_channel_); - channel_manager_->DestroyVideoChannel(video_channel_); - - worker_thread_->Invoke(RTC_FROM_HERE, - [&]() { channel_manager_.reset(); }); + channel_manager_->DestroyChannel(voice_channel_); + channel_manager_->DestroyChannel(video_channel_); } std::unique_ptr CreateDtlsSrtpTransport() { @@ -542,7 +537,6 @@ class RtpSenderReceiverTest rtc::scoped_refptr video_track_; rtc::scoped_refptr audio_track_; bool audio_sender_destroyed_signal_fired_ = false; - rtc::UniqueRandomIdGenerator ssrc_generator_; }; // Test that `voice_channel_` is updated when an audio track is associated diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index e152d76bf5..becf8f2cca 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -198,7 +198,7 @@ void RtpTransceiver::SetChannel( channel_ = channel; if (channel_) { - channel_->SetRtpTransport(transport_lookup(channel_->content_name())); + channel_->SetRtpTransport(transport_lookup(channel_->mid())); channel_->SetFirstPacketReceivedCallback( [thread = thread_, flag = signaling_thread_safety_, this]() mutable { thread->PostTask(ToQueuedTask( diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index a1a2a197fb..0122c0d64d 100644 --- a/pc/rtp_transceiver_unittest.cc +++ b/pc/rtp_transceiver_unittest.cc @@ -41,7 +41,7 @@ TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) { cricket::MockChannelInterface channel1; EXPECT_CALL(channel1, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); - EXPECT_CALL(channel1, content_name()).WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(channel1, mid()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(channel1, SetFirstPacketReceivedCallback(_)); EXPECT_CALL(channel1, SetRtpTransport(_)).WillRepeatedly(Return(true)); @@ -73,7 +73,7 @@ TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) { cricket::MockChannelInterface channel; EXPECT_CALL(channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_VIDEO)); - EXPECT_CALL(channel, content_name()).WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(channel, mid()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(channel, SetFirstPacketReceivedCallback(_)) .WillRepeatedly(testing::Return()); EXPECT_CALL(channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); @@ -302,8 +302,7 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, EXPECT_CALL(mock_channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); EXPECT_CALL(mock_channel, media_channel()).WillRepeatedly(Return(nullptr)); - EXPECT_CALL(mock_channel, content_name()) - .WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(mock_channel, mid()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); transceiver_.SetChannel(&mock_channel, [](const std::string&) { return nullptr; }); @@ -323,8 +322,7 @@ TEST_F(RtpTransceiverTestForHeaderExtensions, ReturnsNegotiatedHdrExts) { EXPECT_CALL(mock_channel, media_type()) .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO)); EXPECT_CALL(mock_channel, media_channel()).WillRepeatedly(Return(nullptr)); - EXPECT_CALL(mock_channel, content_name()) - .WillRepeatedly(ReturnRef(content_name)); + EXPECT_CALL(mock_channel, mid()).WillRepeatedly(ReturnRef(content_name)); EXPECT_CALL(mock_channel, SetRtpTransport(_)).WillRepeatedly(Return(true)); cricket::RtpHeaderExtensions extensions = {webrtc::RtpExtension("uri1", 1), diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index ea1c4c4d23..f80e7191da 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -1223,7 +1223,7 @@ void SdpOfferAnswerHandler::Initialize( std::make_unique( signaling_thread(), channel_manager(), this, pc_->session_id(), pc_->dtls_enabled(), std::move(dependencies.cert_generator), - certificate, &ssrc_generator_, + certificate, [this](const rtc::scoped_refptr& certificate) { transport_controller()->SetLocalCertificate(certificate); }); @@ -3523,7 +3523,7 @@ RTCError SdpOfferAnswerHandler::UpdateTransceiverChannel( if (content.rejected) { if (channel) { transceiver->internal()->SetChannel(nullptr, nullptr); - DestroyChannelInterface(channel); + channel_manager()->DestroyChannel(channel); } } else { if (!channel) { @@ -4760,9 +4760,8 @@ cricket::VoiceChannel* SdpOfferAnswerHandler::CreateVoiceChannel( // 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, signaling_thread(), - mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), &ssrc_generator_, - audio_options()); + pc_->call_ptr(), pc_->configuration()->media_config, mid, + pc_->SrtpRequired(), pc_->GetCryptoOptions(), audio_options()); } // TODO(steveanton): Perhaps this should be managed by the RtpTransceiver. @@ -4777,9 +4776,9 @@ cricket::VideoChannel* SdpOfferAnswerHandler::CreateVideoChannel( // 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, signaling_thread(), - mid, pc_->SrtpRequired(), pc_->GetCryptoOptions(), &ssrc_generator_, - video_options(), video_bitrate_allocator_factory_.get()); + 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) { @@ -4806,13 +4805,6 @@ void SdpOfferAnswerHandler::DestroyTransceiverChannel( RTC_DCHECK(transceiver); RTC_LOG_THREAD_BLOCK_COUNT(); - // TODO(tommi): We're currently on the signaling thread. - // There are multiple hops to the worker ahead. - // Consider if we can make the call to SetChannel() on the worker thread - // (and require that to be the context it's always called in) and also - // call DestroyChannelInterface there, since it also needs to hop to the - // worker. - cricket::ChannelInterface* channel = transceiver->internal()->channel(); RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0); if (channel) { @@ -4825,11 +4817,7 @@ void SdpOfferAnswerHandler::DestroyTransceiverChannel( // un-set the channel pointer and uninitialize/destruct the channel object // at the same time, rather than in separate steps. transceiver->internal()->SetChannel(nullptr, nullptr); - // TODO(tommi): All channel objects end up getting deleted on the - // worker thread (ideally should be on the network thread but the - // MediaChannel objects are tied to the worker. Can the teardown be done - // asynchronously across the threads rather than blocking? - DestroyChannelInterface(channel); + channel_manager()->DestroyChannel(channel); } } @@ -4849,45 +4837,6 @@ void SdpOfferAnswerHandler::DestroyDataChannelTransport(RTCError error) { pc_->ResetSctpDataMid(); } -void SdpOfferAnswerHandler::DestroyChannelInterface( - cricket::ChannelInterface* channel) { - TRACE_EVENT0("webrtc", "SdpOfferAnswerHandler::DestroyChannelInterface"); - RTC_DCHECK_RUN_ON(signaling_thread()); - RTC_DCHECK(channel_manager()->media_engine()); - RTC_DCHECK(channel); - - // TODO(bugs.webrtc.org/11992): All the below methods should be called on the - // worker thread. (they switch internally anyway). Change - // DestroyChannelInterface to either be called on the worker thread, or do - // this asynchronously on the worker. - RTC_LOG_THREAD_BLOCK_COUNT(); - - switch (channel->media_type()) { - case cricket::MEDIA_TYPE_AUDIO: - channel_manager()->DestroyVoiceChannel( - static_cast(channel)); - break; - case cricket::MEDIA_TYPE_VIDEO: - channel_manager()->DestroyVideoChannel( - static_cast(channel)); - break; - case cricket::MEDIA_TYPE_DATA: - RTC_DCHECK_NOTREACHED() - << "Trying to destroy datachannel through DestroyChannelInterface"; - break; - default: - RTC_DCHECK_NOTREACHED() - << "Unknown media type: " << channel->media_type(); - break; - } - - // TODO(tommi): Figure out why we can get 2 blocking calls when running - // PeerConnectionCryptoTest.CreateAnswerWithDifferentSslRoles. - // and 3 when running - // PeerConnectionCryptoTest.CreateAnswerWithDifferentSslRoles - // RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1); -} - void SdpOfferAnswerHandler::DestroyAllChannels() { RTC_DCHECK_RUN_ON(signaling_thread()); if (!transceivers()) { @@ -5142,7 +5091,7 @@ bool SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState( local_direction = RtpTransceiverDirectionReversed(local_direction); } - auto bundle_it = bundle_groups_by_mid.find(channel->content_name()); + auto bundle_it = bundle_groups_by_mid.find(channel->mid()); const cricket::ContentGroup* bundle_group = bundle_it != bundle_groups_by_mid.end() ? bundle_it->second : nullptr; bool pt_demux_enabled = RtpTransceiverDirectionHasRecv(local_direction); diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h index 7349999807..e7851d8d6c 100644 --- a/pc/sdp_offer_answer.h +++ b/pc/sdp_offer_answer.h @@ -543,12 +543,8 @@ class SdpOfferAnswerHandler : public SdpStateProvider, // transport and clears it. void DestroyDataChannelTransport(RTCError error); - // Destroys the given ChannelInterface. - // The channel cannot be accessed after this method is called. - void DestroyChannelInterface(cricket::ChannelInterface* channel); // Generates MediaDescriptionOptions for the `session_opts` based on existing // local description or remote description. - void GenerateMediaDescriptionOptions( const SessionDescriptionInterface* session_desc, RtpTransceiverDirection audio_direction, @@ -682,14 +678,6 @@ class SdpOfferAnswerHandler : public SdpStateProvider, cricket::AudioOptions audio_options_ RTC_GUARDED_BY(signaling_thread()); cricket::VideoOptions video_options_ RTC_GUARDED_BY(signaling_thread()); - // This object should be used to generate any SSRC that is not explicitly - // specified by the user (or by the remote party). - // The generator is not used directly, instead it is passed on to the - // channel manager and the session description factory. - // TODO(bugs.webrtc.org/12666): This variable is used from both the signaling - // and worker threads. See if we can't restrict usage to a single thread. - rtc::UniqueRandomIdGenerator ssrc_generator_; - // A video bitrate allocator factory. // This can be injected using the PeerConnectionDependencies, // or else the CreateBuiltinVideoBitrateAllocatorFactory() will be called. diff --git a/pc/stats_collector.cc b/pc/stats_collector.cc index c2d44bb5f4..493c26cfbd 100644 --- a/pc/stats_collector.cc +++ b/pc/stats_collector.cc @@ -886,7 +886,7 @@ StatsCollector::SessionStats StatsCollector::ExtractSessionInfo_n( for (auto& transceiver : transceivers) { cricket::ChannelInterface* channel = transceiver->internal()->channel(); if (channel) { - stats.transport_names_by_mid[channel->content_name()] = + stats.transport_names_by_mid[channel->mid()] = std::string(channel->transport_name()); } } @@ -1182,7 +1182,7 @@ void StatsCollector::ExtractMediaInfo( } std::unique_ptr gatherer = CreateMediaChannelStatsGatherer(channel->media_channel()); - gatherer->mid = channel->content_name(); + gatherer->mid = channel->mid(); gatherer->transport_name = transport_names_by_mid.at(gatherer->mid); for (const auto& sender : transceiver->internal()->senders()) { @@ -1209,7 +1209,7 @@ void StatsCollector::ExtractMediaInfo( if (!channel) continue; MediaChannelStatsGatherer* gatherer = gatherers[i++].get(); - RTC_DCHECK_EQ(gatherer->mid, channel->content_name()); + RTC_DCHECK_EQ(gatherer->mid, channel->mid()); for (const auto& receiver : transceiver->internal()->receivers()) { gatherer->receiver_track_id_by_ssrc.insert(std::make_pair( diff --git a/pc/test/mock_channel_interface.h b/pc/test/mock_channel_interface.h index de59ebd75b..844a36e2d0 100644 --- a/pc/test/mock_channel_interface.h +++ b/pc/test/mock_channel_interface.h @@ -27,7 +27,7 @@ class MockChannelInterface : public cricket::ChannelInterface { MOCK_METHOD(cricket::MediaType, media_type, (), (const, override)); MOCK_METHOD(MediaChannel*, media_channel, (), (const, override)); MOCK_METHOD(absl::string_view, transport_name, (), (const, override)); - MOCK_METHOD(const std::string&, content_name, (), (const, override)); + MOCK_METHOD(const std::string&, mid, (), (const, override)); MOCK_METHOD(void, Enable, (bool), (override)); MOCK_METHOD(void, SetFirstPacketReceivedCallback, diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index ac20308df9..36130758a3 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -132,13 +132,10 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( bool dtls_enabled, std::unique_ptr cert_generator, const rtc::scoped_refptr& certificate, - UniqueRandomIdGenerator* ssrc_generator, std::function&)> on_certificate_ready) : signaling_thread_(signaling_thread), - session_desc_factory_(channel_manager, - &transport_desc_factory_, - ssrc_generator), + session_desc_factory_(channel_manager, &transport_desc_factory_), // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp // as the session id and session version. To simplify, it should be fine // to just use a random number as session id and start version from diff --git a/pc/webrtc_session_description_factory.h b/pc/webrtc_session_description_factory.h index efa208ff78..6a6e8efa51 100644 --- a/pc/webrtc_session_description_factory.h +++ b/pc/webrtc_session_description_factory.h @@ -86,7 +86,6 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, bool dtls_enabled, std::unique_ptr cert_generator, const rtc::scoped_refptr& certificate, - rtc::UniqueRandomIdGenerator* ssrc_generator, std::function&)> on_certificate_ready); virtual ~WebRtcSessionDescriptionFactory();