From 80b7c6befd39c83241ad1bdfb3902be45d0a0ccf Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 20 Jun 2022 19:59:11 +0200 Subject: Delete Call dependency on ProcessThread as unused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Last usage or ProcessThread was removed in https://webrtc-review.googlesource.com/c/src/+/265921 Bug: webrtc:7219 Change-Id: Ia46d9e2530cd0dbf56a5c0ca6e1bf0936fd62672 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/266363 Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#37287} --- api/test/create_time_controller.cc | 9 +- call/BUILD.gn | 7 -- call/call.cc | 120 +-------------------- call/call.h | 36 ------- call/call_factory.cc | 28 ++--- call/call_factory.h | 2 - call/call_unittest.cc | 56 ---------- ...p_transport_controller_send_factory_interface.h | 1 - test/scenario/call_client.cc | 19 ++-- test/scenario/call_client.h | 2 - 10 files changed, 22 insertions(+), 258 deletions(-) diff --git a/api/test/create_time_controller.cc b/api/test/create_time_controller.cc index d4ec9876e2..d198f2b0fe 100644 --- a/api/test/create_time_controller.cc +++ b/api/test/create_time_controller.cc @@ -37,22 +37,15 @@ std::unique_ptr CreateTimeControllerBasedCallFactory( explicit TimeControllerBasedCallFactory(TimeController* time_controller) : time_controller_(time_controller) {} Call* CreateCall(const Call::Config& config) override { - if (!module_thread_) { - module_thread_ = SharedModuleThread::Create( - time_controller_->CreateProcessThread("CallModules"), - [this]() { module_thread_ = nullptr; }); - } - RtpTransportConfig transportConfig = config.ExtractTransportConfig(); - return Call::Create(config, time_controller_->GetClock(), module_thread_, + return Call::Create(config, time_controller_->GetClock(), config.rtp_transport_controller_send_factory->Create( transportConfig, time_controller_->GetClock())); } private: TimeController* time_controller_; - rtc::scoped_refptr module_thread_; }; return std::make_unique(time_controller); } diff --git a/call/BUILD.gn b/call/BUILD.gn index 7e8eb6549d..c212bb09ff 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -67,7 +67,6 @@ rtc_library("call_interfaces") { "../modules/audio_processing:api", "../modules/audio_processing:audio_processing_statistics", "../modules/rtp_rtcp:rtp_rtcp_format", - "../modules/utility", "../rtc_base", "../rtc_base:audio_format_to_string", "../rtc_base:checks", @@ -121,7 +120,6 @@ rtc_library("rtp_interfaces") { "../api/units:timestamp", "../common_video:frame_counts", "../modules/rtp_rtcp:rtp_rtcp_format", - "../modules/utility", "../rtc_base:checks", "../rtc_base:rtc_task_queue", "../rtc_base:stringutils", @@ -203,7 +201,6 @@ rtc_library("rtp_sender") { "../modules/rtp_rtcp", "../modules/rtp_rtcp:rtp_rtcp_format", "../modules/rtp_rtcp:rtp_video_header", - "../modules/utility", "../modules/video_coding:chain_diff_calculator", "../modules/video_coding:codec_globals_headers", "../modules/video_coding:frame_dependencies_calculator", @@ -316,7 +313,6 @@ rtc_library("call") { "../modules/pacing", "../modules/rtp_rtcp", "../modules/rtp_rtcp:rtp_rtcp_format", - "../modules/utility", "../modules/video_coding", "../rtc_base:checks", "../rtc_base:copy_on_write_buffer", @@ -433,7 +429,6 @@ rtc_library("fake_network") { "../api:sequence_checker", "../api:simulated_network_api", "../api:transport_api", - "../modules/utility", "../rtc_base:checks", "../rtc_base:logging", "../rtc_base:macromagic", @@ -484,7 +479,6 @@ if (rtc_include_tests) { "../api/video:video_frame", "../api/video:video_rtp_headers", "../audio", - "../modules:module_api", "../modules/audio_device:mock_audio_device", "../modules/audio_mixer", "../modules/audio_mixer:audio_mixer_impl", @@ -494,7 +488,6 @@ if (rtc_include_tests) { "../modules/rtp_rtcp", "../modules/rtp_rtcp:mock_rtp_rtcp", "../modules/rtp_rtcp:rtp_rtcp_format", - "../modules/utility:mock_process_thread", "../modules/video_coding", "../modules/video_coding:codec_globals_headers", "../modules/video_coding:video_codec_interface", diff --git a/call/call.cc b/call/call.cc index b86cc10ac2..39772a2bad 100644 --- a/call/call.cc +++ b/call/call.cc @@ -50,7 +50,6 @@ #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_util.h" -#include "modules/utility/include/process_thread.h" #include "modules/video_coding/fec_controller_default.h" #include "rtc_base/checks.h" #include "rtc_base/location.h" @@ -504,32 +503,14 @@ std::string Call::Stats::ToString(int64_t time_ms) const { } Call* Call::Create(const Call::Config& config) { - rtc::scoped_refptr call_thread = - SharedModuleThread::Create(ProcessThread::Create("ModuleProcessThread"), - nullptr); - return Create(config, Clock::GetRealTimeClock(), std::move(call_thread), - ProcessThread::Create("PacerThread")); + Clock* clock = Clock::GetRealTimeClock(); + return Create(config, clock, + RtpTransportControllerSendFactory().Create( + config.ExtractTransportConfig(), clock)); } Call* Call::Create(const Call::Config& config, Clock* clock, - rtc::scoped_refptr /*call_thread*/, - std::unique_ptr pacer_thread) { - RTC_DCHECK(config.task_queue_factory); - - RtpTransportControllerSendFactory transport_controller_factory_; - - RtpTransportConfig transportConfig = config.ExtractTransportConfig(); - - return new internal::Call( - clock, config, - transport_controller_factory_.Create(transportConfig, clock), - config.task_queue_factory); -} - -Call* Call::Create(const Call::Config& config, - Clock* clock, - rtc::scoped_refptr /*call_thread*/, std::unique_ptr transportControllerSend) { RTC_DCHECK(config.task_queue_factory); @@ -537,99 +518,6 @@ Call* Call::Create(const Call::Config& config, config.task_queue_factory); } -class SharedModuleThread::Impl { - public: - Impl(std::unique_ptr process_thread, - std::function on_one_ref_remaining) - : module_thread_(std::move(process_thread)), - on_one_ref_remaining_(std::move(on_one_ref_remaining)) {} - - void EnsureStarted() { - RTC_DCHECK_RUN_ON(&sequence_checker_); - if (started_) - return; - started_ = true; - module_thread_->Start(); - } - - ProcessThread* process_thread() { - RTC_DCHECK_RUN_ON(&sequence_checker_); - return module_thread_.get(); - } - - void AddRef() const { - RTC_DCHECK_RUN_ON(&sequence_checker_); - ++ref_count_; - } - - rtc::RefCountReleaseStatus Release() const { - RTC_DCHECK_RUN_ON(&sequence_checker_); - --ref_count_; - - if (ref_count_ == 0) { - module_thread_->Stop(); - return rtc::RefCountReleaseStatus::kDroppedLastRef; - } - - if (ref_count_ == 1 && on_one_ref_remaining_) { - auto moved_fn = std::move(on_one_ref_remaining_); - // NOTE: after this function returns, chances are that `this` has been - // deleted - do not touch any member variables. - // If the owner of the last reference implements a lambda that releases - // that last reference inside of the callback (which is legal according - // to this implementation), we will recursively enter Release() above, - // call Stop() and release the last reference. - moved_fn(); - } - - return rtc::RefCountReleaseStatus::kOtherRefsRemained; - } - - private: - RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; - mutable int ref_count_ RTC_GUARDED_BY(sequence_checker_) = 0; - std::unique_ptr const module_thread_; - std::function const on_one_ref_remaining_; - bool started_ = false; -}; - -SharedModuleThread::SharedModuleThread( - std::unique_ptr process_thread, - std::function on_one_ref_remaining) - : impl_(std::make_unique(std::move(process_thread), - std::move(on_one_ref_remaining))) {} - -SharedModuleThread::~SharedModuleThread() = default; - -// static - -rtc::scoped_refptr SharedModuleThread::Create( - std::unique_ptr process_thread, - std::function on_one_ref_remaining) { - // Using `new` to access a non-public constructor. - return rtc::scoped_refptr(new SharedModuleThread( - std::move(process_thread), std::move(on_one_ref_remaining))); -} - -void SharedModuleThread::EnsureStarted() { - impl_->EnsureStarted(); -} - -ProcessThread* SharedModuleThread::process_thread() { - return impl_->process_thread(); -} - -void SharedModuleThread::AddRef() const { - impl_->AddRef(); -} - -rtc::RefCountReleaseStatus SharedModuleThread::Release() const { - auto ret = impl_->Release(); - if (ret == rtc::RefCountReleaseStatus::kDroppedLastRef) - delete this; - return ret; -} - // This method here to avoid subclasses has to implement this method. // Call perf test will use Internal::Call::CreateVideoSendStream() to inject // FecController. diff --git a/call/call.h b/call/call.h index a4d5974898..366978392e 100644 --- a/call/call.h +++ b/call/call.h @@ -27,7 +27,6 @@ #include "call/rtp_transport_controller_send_interface.h" #include "call/video_receive_stream.h" #include "call/video_send_stream.h" -#include "modules/utility/include/process_thread.h" #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/network/sent_packet.h" #include "rtc_base/network_route.h" @@ -35,36 +34,6 @@ namespace webrtc { -// A restricted way to share the module process thread across multiple instances -// of Call that are constructed on the same worker thread (which is what the -// peer connection factory guarantees). -// SharedModuleThread supports a callback that is issued when only one reference -// remains, which is used to indicate to the original owner that the thread may -// be discarded. -class SharedModuleThread final { - public: - // Allows injection of an externally created process thread. - static rtc::scoped_refptr Create( - std::unique_ptr process_thread, - std::function on_one_ref_remaining); - - void EnsureStarted(); - - ProcessThread* process_thread(); - - private: - friend class rtc::scoped_refptr; - SharedModuleThread(std::unique_ptr process_thread, - std::function on_one_ref_remaining); - ~SharedModuleThread(); - - void AddRef() const; - rtc::RefCountReleaseStatus Release() const; - - class Impl; - mutable std::unique_ptr impl_; -}; - // A Call represents a two-way connection carrying zero or more outgoing // and incoming media streams, transported over one or more RTP transports. @@ -92,11 +61,6 @@ class Call { static Call* Create(const Call::Config& config); static Call* Create(const Call::Config& config, Clock* clock, - rtc::scoped_refptr call_thread, - std::unique_ptr pacer_thread); - static Call* Create(const Call::Config& config, - Clock* clock, - rtc::scoped_refptr call_thread, std::unique_ptr transportControllerSend); diff --git a/call/call_factory.cc b/call/call_factory.cc index 7d3581d2d7..9fe4c2999e 100644 --- a/call/call_factory.cc +++ b/call/call_factory.cc @@ -17,6 +17,7 @@ #include #include +#include "absl/memory/memory.h" #include "absl/types/optional.h" #include "api/test/simulated_network.h" #include "api/units/time_delta.h" @@ -155,29 +156,18 @@ Call* CallFactory::CreateCall(const Call::Config& config) { RtpTransportConfig transportConfig = config.ExtractTransportConfig(); + Call* call = + Call::Create(config, Clock::GetRealTimeClock(), + config.rtp_transport_controller_send_factory->Create( + transportConfig, Clock::GetRealTimeClock())); + if (!send_degradation_configs.empty() || !receive_degradation_configs.empty()) { - return new DegradedCall( - std::unique_ptr(Call::Create( - config, Clock::GetRealTimeClock(), - SharedModuleThread::Create( - ProcessThread::Create("ModuleProcessThread"), nullptr), - config.rtp_transport_controller_send_factory->Create( - transportConfig, Clock::GetRealTimeClock()))), - send_degradation_configs, receive_degradation_configs); - } - - if (!module_thread_) { - module_thread_ = SharedModuleThread::Create( - ProcessThread::Create("SharedModThread"), [this]() { - RTC_DCHECK_RUN_ON(&call_thread_); - module_thread_ = nullptr; - }); + return new DegradedCall(absl::WrapUnique(call), send_degradation_configs, + receive_degradation_configs); } - return Call::Create(config, Clock::GetRealTimeClock(), module_thread_, - config.rtp_transport_controller_send_factory->Create( - transportConfig, Clock::GetRealTimeClock())); + return call; } std::unique_ptr CreateCallFactory() { diff --git a/call/call_factory.h b/call/call_factory.h index 469bec39e1..9feed7bbb6 100644 --- a/call/call_factory.h +++ b/call/call_factory.h @@ -29,8 +29,6 @@ class CallFactory : public CallFactoryInterface { Call* CreateCall(const CallConfig& config) override; RTC_NO_UNIQUE_ADDRESS SequenceChecker call_thread_; - rtc::scoped_refptr module_thread_ - RTC_GUARDED_BY(call_thread_); }; } // namespace webrtc diff --git a/call/call_unittest.cc b/call/call_unittest.cc index 5d30f45dc5..5db3f5902b 100644 --- a/call/call_unittest.cc +++ b/call/call_unittest.cc @@ -31,7 +31,6 @@ #include "call/audio_state.h" #include "modules/audio_device/include/mock_audio_device.h" #include "modules/audio_processing/include/mock_audio_processing.h" -#include "modules/include/module.h" #include "modules/rtp_rtcp/source/rtp_rtcp_interface.h" #include "test/fake_encoder.h" #include "test/gtest.h" @@ -476,59 +475,4 @@ TEST(CallTest, AddAdaptationResourceBeforeCreatingVideoSendStream) { call->DestroyVideoSendStream(stream2); } -TEST(CallTest, SharedModuleThread) { - class SharedModuleThreadUser : public Module { - public: - SharedModuleThreadUser(ProcessThread* expected_thread, - rtc::scoped_refptr thread) - : expected_thread_(expected_thread), thread_(std::move(thread)) { - thread_->EnsureStarted(); - thread_->process_thread()->RegisterModule(this, RTC_FROM_HERE); - } - - ~SharedModuleThreadUser() override { - thread_->process_thread()->DeRegisterModule(this); - EXPECT_TRUE(thread_was_checked_); - } - - private: - int64_t TimeUntilNextProcess() override { return 1000; } - void Process() override {} - void ProcessThreadAttached(ProcessThread* process_thread) override { - if (!process_thread) { - // Being detached. - return; - } - EXPECT_EQ(process_thread, expected_thread_); - thread_was_checked_ = true; - } - - bool thread_was_checked_ = false; - ProcessThread* const expected_thread_; - rtc::scoped_refptr thread_; - }; - - // Create our test instance and pass a lambda to it that gets executed when - // the reference count goes back to 1 - meaning `shared` again is the only - // reference, which means we can free the variable and deallocate the thread. - rtc::scoped_refptr shared; - shared = - SharedModuleThread::Create(ProcessThread::Create("MySharedProcessThread"), - [&shared]() { shared = nullptr; }); - ProcessThread* process_thread = shared->process_thread(); - - ASSERT_TRUE(shared.get()); - - { - // Create a couple of users of the thread. - // These instances are in a separate scope to trigger the callback to our - // lambda, which will run when these go out of scope. - SharedModuleThreadUser user1(process_thread, shared); - SharedModuleThreadUser user2(process_thread, shared); - } - - // The thread should now have been stopped and freed. - EXPECT_FALSE(shared); -} - } // namespace webrtc diff --git a/call/rtp_transport_controller_send_factory_interface.h b/call/rtp_transport_controller_send_factory_interface.h index 071af9b118..0f4c36c221 100644 --- a/call/rtp_transport_controller_send_factory_interface.h +++ b/call/rtp_transport_controller_send_factory_interface.h @@ -14,7 +14,6 @@ #include "call/rtp_transport_config.h" #include "call/rtp_transport_controller_send_interface.h" -#include "modules/utility/include/process_thread.h" namespace webrtc { // A factory used for dependency injection on the send side of the transport diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc index 3c306675a3..4ae0a640fa 100644 --- a/test/scenario/call_client.cc +++ b/test/scenario/call_client.cc @@ -16,6 +16,8 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/rtc_event_log/rtc_event_log_factory.h" #include "api/transport/network_types.h" +#include "call/call.h" +#include "call/rtp_transport_controller_send_factory.h" #include "modules/audio_mixer/audio_mixer_impl.h" #include "modules/rtp_rtcp/source/rtp_util.h" @@ -57,8 +59,7 @@ Call* CreateCall(TimeController* time_controller, RtcEventLog* event_log, CallClientConfig config, LoggingNetworkControllerFactory* network_controller_factory, - rtc::scoped_refptr audio_state, - rtc::scoped_refptr call_thread) { + rtc::scoped_refptr audio_state) { CallConfig call_config(event_log); call_config.bitrate_config.max_bitrate_bps = config.transport.rates.max_rate.bps_or(-1); @@ -70,9 +71,10 @@ Call* CreateCall(TimeController* time_controller, call_config.network_controller_factory = network_controller_factory; call_config.audio_state = audio_state; call_config.trials = config.field_trials; - return Call::Create(call_config, time_controller->GetClock(), - std::move(call_thread), - time_controller->CreateProcessThread("Pacer")); + Clock* clock = time_controller->GetClock(); + return Call::Create(call_config, clock, + RtpTransportControllerSendFactory().Create( + call_config.ExtractTransportConfig(), clock)); } std::unique_ptr CreateEventLog( @@ -222,14 +224,10 @@ CallClient::CallClient( event_log_ = CreateEventLog(time_controller_->GetTaskQueueFactory(), log_writer_factory_.get()); fake_audio_setup_ = InitAudio(time_controller_); - RTC_DCHECK(!module_thread_); - module_thread_ = SharedModuleThread::Create( - time_controller_->CreateProcessThread("CallThread"), - [this]() { module_thread_ = nullptr; }); call_.reset(CreateCall(time_controller_, event_log_.get(), config, &network_controller_factory_, - fake_audio_setup_.audio_state, module_thread_)); + fake_audio_setup_.audio_state)); transport_ = std::make_unique(clock_, call_.get()); }); } @@ -237,7 +235,6 @@ CallClient::CallClient( CallClient::~CallClient() { SendTask([&] { call_.reset(); - RTC_DCHECK(!module_thread_); // Should be set to null in the lambda above. fake_audio_setup_ = {}; rtc::Event done; event_log_->StopLogging([&done] { done.Set(); }); diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h index 489844039b..18aa67f77d 100644 --- a/test/scenario/call_client.h +++ b/test/scenario/call_client.h @@ -166,8 +166,6 @@ class CallClient : public EmulatedNetworkReceiverInterface { // Defined last so it's destroyed first. TaskQueueForTest task_queue_; - rtc::scoped_refptr module_thread_; - const FieldTrialBasedConfig field_trials_; }; -- cgit v1.2.3