diff --git a/webrtc/base/event_tracer.cc b/webrtc/base/event_tracer.cc index 4393bff298..cb7554a891 100644 --- a/webrtc/base/event_tracer.cc +++ b/webrtc/base/event_tracer.cc @@ -80,7 +80,7 @@ namespace rtc { namespace tracing { namespace { -static bool EventTracingThreadFunc(void* params); +static void EventTracingThreadFunc(void* params); // Atomic-int fast path for avoiding logging when disabled. static volatile int g_event_logging_active = 0; @@ -89,7 +89,10 @@ static volatile int g_event_logging_active = 0; class EventLogger final { public: EventLogger() - : logging_thread_(EventTracingThreadFunc, this, "EventTracingThread"), + : logging_thread_(EventTracingThreadFunc, + this, + "EventTracingThread", + kLowPriority), shutdown_event_(false, false) {} ~EventLogger() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); } @@ -210,7 +213,6 @@ class EventLogger final { // Finally start, everything should be set up now. logging_thread_.Start(); TRACE_EVENT_INSTANT0("webrtc", "EventLogger::Start"); - logging_thread_.SetPriority(kLowPriority); } void Stop() { @@ -326,11 +328,8 @@ class EventLogger final { bool output_file_owned_ = false; }; -static bool EventTracingThreadFunc(void* params) { +static void EventTracingThreadFunc(void* params) { static_cast(params)->Log(); - // False indicates that the thread function has done its job and doesn't need - // to be restarted again. Log() runs its own internal loop. - return false; } static EventLogger* volatile g_event_logger = nullptr; diff --git a/webrtc/base/platform_thread.cc b/webrtc/base/platform_thread.cc index d1bd509bf1..525f0ddf08 100644 --- a/webrtc/base/platform_thread.cc +++ b/webrtc/base/platform_thread.cc @@ -92,14 +92,27 @@ struct ThreadAttributes { #endif // defined(WEBRTC_WIN) } -PlatformThread::PlatformThread(ThreadRunFunction func, +PlatformThread::PlatformThread(ThreadRunFunctionDeprecated func, void* obj, const char* thread_name) - : run_function_(func), + : run_function_deprecated_(func), obj_(obj), name_(thread_name ? thread_name : "webrtc") { RTC_DCHECK(func); RTC_DCHECK(name_.length() < 64); + spawned_thread_checker_.DetachFromThread(); +} + +PlatformThread::PlatformThread(ThreadRunFunction func, + void* obj, + const char* thread_name, + ThreadPriority priority /*= kNormalPriority*/) + : run_function_(func), priority_(priority), obj_(obj), name_(thread_name) { + RTC_DCHECK(func); + RTC_DCHECK(!name_.empty()); + // TODO(tommi): Consider lowering the limit to 15 (limit on Linux). + RTC_DCHECK(name_.length() < 64); + spawned_thread_checker_.DetachFromThread(); } PlatformThread::~PlatformThread() { @@ -180,11 +193,14 @@ void PlatformThread::Stop() { thread_ = nullptr; thread_id_ = 0; #else - RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_)); + if (!run_function_) + RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_)); RTC_CHECK_EQ(0, pthread_join(thread_, nullptr)); - AtomicOps::ReleaseStore(&stop_flag_, 0); + if (!run_function_) + AtomicOps::ReleaseStore(&stop_flag_, 0); thread_ = 0; #endif // defined(WEBRTC_WIN) + spawned_thread_checker_.DetachFromThread(); } // TODO(tommi): Deprecate the loop behavior in PlatformThread. @@ -195,8 +211,16 @@ void PlatformThread::Stop() { // All implementations will need to be aware of how the thread should be stopped // and encouraging a busy polling loop, can be costly in terms of power and cpu. void PlatformThread::Run() { - if (!name_.empty()) - rtc::SetCurrentThreadName(name_.c_str()); + // Attach the worker thread checker to this thread. + RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread()); + rtc::SetCurrentThreadName(name_.c_str()); + + if (run_function_) { + SetPriority(priority_); + run_function_(obj_); + return; + } +// TODO(tommi): Delete the below. #if !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN) const struct timespec ts_null = {0}; #endif @@ -204,7 +228,7 @@ void PlatformThread::Run() { // The interface contract of Start/Stop is that for a successful call to // Start, there should be at least one call to the run function. So we // call the function before checking |stop_|. - if (!run_function_(obj_)) + if (!run_function_deprecated_(obj_)) break; #if defined(WEBRTC_WIN) // Alertable sleep to permit RaiseFlag to run and update |stop_|. @@ -221,8 +245,19 @@ void PlatformThread::Run() { } bool PlatformThread::SetPriority(ThreadPriority priority) { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - RTC_DCHECK(IsRunning()); +#if RTC_DCHECK_IS_ON + if (run_function_) { + // The non-deprecated way of how this function gets called, is that it must + // be called on the worker thread itself. + RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread()); + } else { + // In the case of deprecated use of this method, it must be called on the + // same thread as the PlatformThread object is constructed on. + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + RTC_DCHECK(IsRunning()); + } +#endif + #if defined(WEBRTC_WIN) return SetThreadPriority(thread_, priority) != FALSE; #elif defined(__native_client__) diff --git a/webrtc/base/platform_thread.h b/webrtc/base/platform_thread.h index 667407fabc..ed26ca03cc 100644 --- a/webrtc/base/platform_thread.h +++ b/webrtc/base/platform_thread.h @@ -32,7 +32,8 @@ void SetCurrentThreadName(const char* name); // Callback function that the spawned thread will enter once spawned. // A return value of false is interpreted as that the function has no // more work to do and that the thread can be released. -typedef bool (*ThreadRunFunction)(void*); +typedef bool (*ThreadRunFunctionDeprecated)(void*); +typedef void (*ThreadRunFunction)(void*); enum ThreadPriority { #ifdef WEBRTC_WIN @@ -55,7 +56,13 @@ enum ThreadPriority { // called from the same thread, including instantiation. class PlatformThread { public: - PlatformThread(ThreadRunFunction func, void* obj, const char* thread_name); + PlatformThread(ThreadRunFunctionDeprecated func, + void* obj, + const char* thread_name); + PlatformThread(ThreadRunFunction func, + void* obj, + const char* thread_name, + ThreadPriority priority = kNormalPriority); virtual ~PlatformThread(); const std::string& name() const { return name_; } @@ -74,6 +81,7 @@ class PlatformThread { void Stop(); // Set the priority of the thread. Must be called when thread is running. + // TODO(tommi): Make private and only allow public support via ctor. bool SetPriority(ThreadPriority priority); protected: @@ -85,12 +93,15 @@ class PlatformThread { private: void Run(); - ThreadRunFunction const run_function_; + ThreadRunFunctionDeprecated const run_function_deprecated_ = nullptr; + ThreadRunFunction const run_function_ = nullptr; + const ThreadPriority priority_ = kNormalPriority; void* const obj_; // TODO(pbos): Make sure call sites use string literals and update to a const // char* instead of a std::string. const std::string name_; rtc::ThreadChecker thread_checker_; + rtc::ThreadChecker spawned_thread_checker_; #if defined(WEBRTC_WIN) static DWORD WINAPI StartThread(void* param); diff --git a/webrtc/base/platform_thread_unittest.cc b/webrtc/base/platform_thread_unittest.cc index 322eb35ffc..d6d35e40e4 100644 --- a/webrtc/base/platform_thread_unittest.cc +++ b/webrtc/base/platform_thread_unittest.cc @@ -16,20 +16,65 @@ namespace rtc { namespace { // Function that does nothing, and reports success. -bool NullRunFunction(void* obj) { +bool NullRunFunctionDeprecated(void* obj) { webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping. return true; } +void NullRunFunction(void* obj) {} + // Function that sets a boolean. -bool SetFlagRunFunction(void* obj) { +bool SetFlagRunFunctionDeprecated(void* obj) { bool* obj_as_bool = static_cast(obj); *obj_as_bool = true; webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping. return true; } + +void SetFlagRunFunction(void* obj) { + bool* obj_as_bool = static_cast(obj); + *obj_as_bool = true; +} + } // namespace +TEST(PlatformThreadTest, StartStopDeprecated) { + PlatformThread thread(&NullRunFunctionDeprecated, nullptr, + "PlatformThreadTest"); + EXPECT_TRUE(thread.name() == "PlatformThreadTest"); + EXPECT_TRUE(thread.GetThreadRef() == 0); + thread.Start(); + EXPECT_TRUE(thread.GetThreadRef() != 0); + thread.Stop(); + EXPECT_TRUE(thread.GetThreadRef() == 0); +} + +TEST(PlatformThreadTest, StartStop2Deprecated) { + PlatformThread thread1(&NullRunFunctionDeprecated, nullptr, + "PlatformThreadTest1"); + PlatformThread thread2(&NullRunFunctionDeprecated, nullptr, + "PlatformThreadTest2"); + EXPECT_TRUE(thread1.GetThreadRef() == thread2.GetThreadRef()); + thread1.Start(); + thread2.Start(); + EXPECT_TRUE(thread1.GetThreadRef() != thread2.GetThreadRef()); + thread2.Stop(); + thread1.Stop(); +} + +TEST(PlatformThreadTest, RunFunctionIsCalledDeprecated) { + bool flag = false; + PlatformThread thread(&SetFlagRunFunctionDeprecated, &flag, + "RunFunctionIsCalled"); + thread.Start(); + + // At this point, the flag may be either true or false. + thread.Stop(); + + // We expect the thread to have run at least once. + EXPECT_TRUE(flag); +} + TEST(PlatformThreadTest, StartStop) { PlatformThread thread(&NullRunFunction, nullptr, "PlatformThreadTest"); EXPECT_TRUE(thread.name() == "PlatformThreadTest"); @@ -62,4 +107,5 @@ TEST(PlatformThreadTest, RunFunctionIsCalled) { // We expect the thread to have run at least once. EXPECT_TRUE(flag); } + } // rtc diff --git a/webrtc/base/rate_limiter_unittest.cc b/webrtc/base/rate_limiter_unittest.cc index 3c1b541f5e..6d925670bf 100644 --- a/webrtc/base/rate_limiter_unittest.cc +++ b/webrtc/base/rate_limiter_unittest.cc @@ -130,9 +130,8 @@ class ThreadTask { rtc::Event end_signal_; }; -bool RunTask(void* thread_task) { +void RunTask(void* thread_task) { reinterpret_cast(thread_task)->Run(); - return false; } TEST_F(RateLimitTest, MultiThreadedUsage) { diff --git a/webrtc/base/sequenced_task_checker_unittest.cc b/webrtc/base/sequenced_task_checker_unittest.cc index 8588648316..ae6e09dff7 100644 --- a/webrtc/base/sequenced_task_checker_unittest.cc +++ b/webrtc/base/sequenced_task_checker_unittest.cc @@ -35,14 +35,13 @@ class CallCalledSequentiallyOnThread { } private: - static bool Run(void* obj) { + static void Run(void* obj) { CallCalledSequentiallyOnThread* call_stuff_on_thread = static_cast(obj); EXPECT_EQ( call_stuff_on_thread->expect_true_, call_stuff_on_thread->sequenced_task_checker_->CalledSequentially()); call_stuff_on_thread->thread_has_run_event_.Set(); - return false; } const bool expect_true_; diff --git a/webrtc/base/task_queue.h b/webrtc/base/task_queue.h index a6a70364ac..5fcf00fcd2 100644 --- a/webrtc/base/task_queue.h +++ b/webrtc/base/task_queue.h @@ -232,7 +232,7 @@ class LOCKABLE TaskQueue { private: #if defined(WEBRTC_BUILD_LIBEVENT) - static bool ThreadMain(void* context); + static void ThreadMain(void* context); static void OnWakeup(int socket, short flags, void* context); // NOLINT static void RunTask(int fd, short flags, void* context); // NOLINT static void RunTimer(int fd, short flags, void* context); // NOLINT @@ -263,7 +263,7 @@ class LOCKABLE TaskQueue { class MultimediaTimer; typedef std::unordered_map> DelayedTasks; - static bool ThreadMain(void* context); + static void ThreadMain(void* context); static bool ProcessQueuedMessages(DelayedTasks* delayed_tasks, std::vector* timers); diff --git a/webrtc/base/task_queue_libevent.cc b/webrtc/base/task_queue_libevent.cc index f3ab89be80..c80258811f 100644 --- a/webrtc/base/task_queue_libevent.cc +++ b/webrtc/base/task_queue_libevent.cc @@ -256,7 +256,7 @@ void TaskQueue::PostTaskAndReply(std::unique_ptr task, } // static -bool TaskQueue::ThreadMain(void* context) { +void TaskQueue::ThreadMain(void* context) { TaskQueue* me = static_cast(context); QueueContext queue_context(me); @@ -269,8 +269,6 @@ bool TaskQueue::ThreadMain(void* context) { for (TimerEvent* timer : queue_context.pending_timers_) delete timer; - - return false; } // static diff --git a/webrtc/base/task_queue_win.cc b/webrtc/base/task_queue_win.cc index 11aa81de03..c8ef7211e8 100644 --- a/webrtc/base/task_queue_win.cc +++ b/webrtc/base/task_queue_win.cc @@ -228,7 +228,7 @@ void TaskQueue::PostTaskAndReply(std::unique_ptr task, } // static -bool TaskQueue::ThreadMain(void* context) { +void TaskQueue::ThreadMain(void* context) { HANDLE timer_handles[MultimediaTimer::kMaxTimers]; // Active multimedia timers. std::vector mm_timers; @@ -283,8 +283,6 @@ bool TaskQueue::ThreadMain(void* context) { RTC_DCHECK_EQ(WAIT_IO_COMPLETION, result); } } - - return false; } // static diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index 94153b08ae..6ec3d836ab 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -44,7 +44,7 @@ RampUpTester::RampUpTester(size_t num_video_streams, bool red, bool report_perf_stats) : EndToEndTest(test::CallTest::kLongTimeoutMs), - event_(false, false), + stop_event_(false, false), clock_(Clock::GetRealTimeClock()), num_video_streams_(num_video_streams), num_audio_streams_(num_audio_streams), @@ -72,7 +72,6 @@ RampUpTester::RampUpTester(size_t num_video_streams, } RampUpTester::~RampUpTester() { - event_.Set(); } Call::Config RampUpTester::GetSenderCallConfig() { @@ -282,25 +281,25 @@ void RampUpTester::OnCallsCreated(Call* sender_call, Call* receiver_call) { sender_call_ = sender_call; } -bool RampUpTester::BitrateStatsPollingThread(void* obj) { - return static_cast(obj)->PollStats(); +void RampUpTester::BitrateStatsPollingThread(void* obj) { + static_cast(obj)->PollStats(); } -bool RampUpTester::PollStats() { - if (sender_call_) { - Call::Stats stats = sender_call_->GetStats(); +void RampUpTester::PollStats() { + do { + if (sender_call_) { + Call::Stats stats = sender_call_->GetStats(); - EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_); - EXPECT_GE(expected_bitrate_bps_, 0); - if (stats.send_bandwidth_bps >= expected_bitrate_bps_ && - (min_run_time_ms_ == -1 || - clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) { - ramp_up_finished_ms_ = clock_->TimeInMilliseconds(); - observation_complete_.Set(); + EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_); + EXPECT_GE(expected_bitrate_bps_, 0); + if (stats.send_bandwidth_bps >= expected_bitrate_bps_ && + (min_run_time_ms_ == -1 || + clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) { + ramp_up_finished_ms_ = clock_->TimeInMilliseconds(); + observation_complete_.Set(); + } } - } - - return !event_.Wait(kPollIntervalMs); + } while (!stop_event_.Wait(kPollIntervalMs)); } void RampUpTester::ReportResult(const std::string& measurement, @@ -380,6 +379,7 @@ void RampUpTester::PerformTest() { poller_thread_.Start(); EXPECT_TRUE(Wait()) << "Timed out while waiting for ramp-up to complete."; TriggerTestDone(); + stop_event_.Set(); poller_thread_.Stop(); } @@ -415,22 +415,22 @@ RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams, RampUpDownUpTester::~RampUpDownUpTester() {} -bool RampUpDownUpTester::PollStats() { - if (send_stream_) { - webrtc::VideoSendStream::Stats stats = send_stream_->GetStats(); - int transmit_bitrate_bps = 0; - for (auto it : stats.substreams) { - transmit_bitrate_bps += it.second.total_bitrate_bps; +void RampUpDownUpTester::PollStats() { + do { + if (send_stream_) { + webrtc::VideoSendStream::Stats stats = send_stream_->GetStats(); + int transmit_bitrate_bps = 0; + for (auto it : stats.substreams) { + transmit_bitrate_bps += it.second.total_bitrate_bps; + } + EvolveTestState(transmit_bitrate_bps, stats.suspended); + } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) { + // An audio send stream doesn't have bitrate stats, so the call send BW is + // currently used instead. + int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps; + EvolveTestState(transmit_bitrate_bps, false); } - EvolveTestState(transmit_bitrate_bps, stats.suspended); - } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) { - // An audio send stream doesn't have bitrate stats, so the call send BW is - // currently used instead. - int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps; - EvolveTestState(transmit_bitrate_bps, false); - } - - return !event_.Wait(kPollIntervalMs); + } while (!stop_event_.Wait(kPollIntervalMs)); } Call::Config RampUpDownUpTester::GetReceiverCallConfig() { diff --git a/webrtc/call/rampup_tests.h b/webrtc/call/rampup_tests.h index 65276ea265..b9fd5974ee 100644 --- a/webrtc/call/rampup_tests.h +++ b/webrtc/call/rampup_tests.h @@ -49,7 +49,7 @@ class RampUpTester : public test::EndToEndTest { void PerformTest() override; protected: - virtual bool PollStats(); + virtual void PollStats(); void AccumulateStats(const VideoSendStream::StreamStats& stream, size_t* total_packets_sent, @@ -63,7 +63,7 @@ class RampUpTester : public test::EndToEndTest { void TriggerTestDone(); webrtc::RtcEventLogNullImpl event_log_; - rtc::Event event_; + rtc::Event stop_event_; Clock* const clock_; FakeNetworkPipe::Config forward_transport_config_; const size_t num_video_streams_; @@ -95,7 +95,7 @@ class RampUpTester : public test::EndToEndTest { std::vector* receive_configs) override; void OnCallsCreated(Call* sender_call, Call* receiver_call) override; - static bool BitrateStatsPollingThread(void* obj); + static void BitrateStatsPollingThread(void* obj); const int start_bitrate_bps_; const int64_t min_run_time_ms_; @@ -125,7 +125,7 @@ class RampUpDownUpTester : public RampUpTester { ~RampUpDownUpTester() override; protected: - bool PollStats() override; + void PollStats() override; private: enum TestStates { diff --git a/webrtc/common_video/include/incoming_video_stream.h b/webrtc/common_video/include/incoming_video_stream.h index 250bed8dbf..2ea80eace7 100644 --- a/webrtc/common_video/include/incoming_video_stream.h +++ b/webrtc/common_video/include/incoming_video_stream.h @@ -31,14 +31,10 @@ class IncomingVideoStream : public rtc::VideoSinkInterface { ~IncomingVideoStream() override; protected: - static bool IncomingVideoStreamThreadFun(void* obj); - bool IncomingVideoStreamProcess(); + static void IncomingVideoStreamThreadFun(void* obj); + void IncomingVideoStreamProcess(); private: - enum { kEventStartupTimeMs = 10 }; - enum { kEventMaxWaitTimeMs = 100 }; - enum { kFrameRatePeriodMs = 1000 }; - void OnFrame(const VideoFrame& video_frame) override; rtc::ThreadChecker main_thread_checker_; diff --git a/webrtc/common_video/incoming_video_stream.cc b/webrtc/common_video/incoming_video_stream.cc index ed7b9ea29d..c1f61d1cfb 100644 --- a/webrtc/common_video/incoming_video_stream.cc +++ b/webrtc/common_video/incoming_video_stream.cc @@ -16,13 +16,18 @@ #include "webrtc/system_wrappers/include/event_wrapper.h" namespace webrtc { +namespace { +const int kEventStartupTimeMs = 10; +const int kEventMaxWaitTimeMs = 100; +} // namespace IncomingVideoStream::IncomingVideoStream( int32_t delay_ms, rtc::VideoSinkInterface* callback) : incoming_render_thread_(&IncomingVideoStreamThreadFun, this, - "IncomingVideoStreamThread"), + "IncomingVideoStreamThread", + rtc::kRealtimePriority), deliver_buffer_event_(EventTimerWrapper::Create()), external_callback_(callback), render_buffers_(new VideoRenderFrames(delay_ms)) { @@ -32,7 +37,6 @@ IncomingVideoStream::IncomingVideoStream( deliver_buffer_event_->StartTimer(false, kEventStartupTimeMs); incoming_render_thread_.Start(); - incoming_render_thread_.SetPriority(rtc::kRealtimePriority); } IncomingVideoStream::~IncomingVideoStream() { @@ -57,39 +61,44 @@ void IncomingVideoStream::OnFrame(const VideoFrame& video_frame) { } } -bool IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) { - return static_cast(obj)->IncomingVideoStreamProcess(); +// static +void IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) { + static_cast(obj)->IncomingVideoStreamProcess(); } -bool IncomingVideoStream::IncomingVideoStreamProcess() { +void IncomingVideoStream::IncomingVideoStreamProcess() { RTC_DCHECK_RUN_ON(&render_thread_checker_); - if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) { - // Get a new frame to render and the time for the frame after this one. - rtc::Optional frame_to_render; - uint32_t wait_time; - { - rtc::CritScope cs(&buffer_critsect_); - if (!render_buffers_.get()) { - // Terminating - return false; + while (true) { + if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) { + // Get a new frame to render and the time for the frame after this one. + rtc::Optional frame_to_render; + uint32_t wait_time; + { + rtc::CritScope cs(&buffer_critsect_); + if (!render_buffers_.get()) { + // Terminating + return; + } + + frame_to_render = render_buffers_->FrameToRender(); + wait_time = render_buffers_->TimeToNextFrameRelease(); } - frame_to_render = render_buffers_->FrameToRender(); - wait_time = render_buffers_->TimeToNextFrameRelease(); - } - // Set timer for next frame to render. - if (wait_time > kEventMaxWaitTimeMs) { - wait_time = kEventMaxWaitTimeMs; - } + // Set timer for next frame to render. + if (wait_time > kEventMaxWaitTimeMs) { + wait_time = kEventMaxWaitTimeMs; + } - deliver_buffer_event_->StartTimer(false, wait_time); + deliver_buffer_event_->StartTimer(false, wait_time); - if (frame_to_render) { - external_callback_->OnFrame(*frame_to_render); + if (frame_to_render) { + external_callback_->OnFrame(*frame_to_render); + } + } else { + RTC_NOTREACHED(); } } - return true; } } // namespace webrtc diff --git a/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc b/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc index e7edd9a93c..f8fda49223 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc +++ b/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc @@ -307,10 +307,9 @@ void RtcEventLogHelperThread::ProcessEvents() { } } -bool RtcEventLogHelperThread::ThreadOutputFunction(void* obj) { +void RtcEventLogHelperThread::ThreadOutputFunction(void* obj) { RtcEventLogHelperThread* helper = static_cast(obj); helper->ProcessEvents(); - return false; } } // namespace webrtc diff --git a/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.h b/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.h index 7d41f60db2..420e5c529f 100644 --- a/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.h +++ b/webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.h @@ -81,7 +81,7 @@ class RtcEventLogHelperThread final { void SignalNewEvent(); private: - static bool ThreadOutputFunction(void* obj); + static void ThreadOutputFunction(void* obj); bool AppendEventToString(rtclog::Event* event); bool LogToMemory(); diff --git a/webrtc/modules/video_coding/frame_buffer2_unittest.cc b/webrtc/modules/video_coding/frame_buffer2_unittest.cc index c3d8ce7fab..b67d99376f 100644 --- a/webrtc/modules/video_coding/frame_buffer2_unittest.cc +++ b/webrtc/modules/video_coding/frame_buffer2_unittest.cc @@ -197,7 +197,7 @@ class TestFrameBuffer2 : public ::testing::Test { ASSERT_FALSE(frames_[index]); } - static bool ExtractLoop(void* obj) { + static void ExtractLoop(void* obj) { TestFrameBuffer2* tfb = static_cast(obj); while (true) { tfb->trigger_extract_event_.Wait(rtc::Event::kForever); @@ -205,7 +205,7 @@ class TestFrameBuffer2 : public ::testing::Test { rtc::CritScope lock(&tfb->crit_); tfb->crit_acquired_event_.Set(); if (tfb->tear_down_) - return false; + return; std::unique_ptr frame; FrameBuffer::ReturnReason res = diff --git a/webrtc/system_wrappers/source/critical_section_unittest.cc b/webrtc/system_wrappers/source/critical_section_unittest.cc index 6d73242f6a..f6ceff7c2f 100644 --- a/webrtc/system_wrappers/source/critical_section_unittest.cc +++ b/webrtc/system_wrappers/source/critical_section_unittest.cc @@ -68,10 +68,9 @@ public: } }; -bool LockUnlockThenStopRunFunction(void* obj) { +void LockUnlockThenStopRunFunction(void* obj) { ProtectedCount* the_count = static_cast(obj); the_count->Increment(); - return false; } TEST_F(CritSectTest, ThreadWakesOnce) NO_THREAD_SAFETY_ANALYSIS { diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 6137308f6c..f427471193 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -2142,45 +2142,45 @@ TEST_P(EndToEndTest, RembWithSendSideBwe) { sender_call_ = sender_call; } - static bool BitrateStatsPollingThread(void* obj) { - return static_cast(obj)->PollStats(); + static void BitrateStatsPollingThread(void* obj) { + static_cast(obj)->PollStats(); } - bool PollStats() { - if (sender_call_) { - Call::Stats stats = sender_call_->GetStats(); - switch (state_) { - case kWaitForFirstRampUp: - if (stats.send_bandwidth_bps >= remb_bitrate_bps_) { - state_ = kWaitForRemb; - remb_bitrate_bps_ /= 2; - rtp_rtcp_->SetREMBData( - remb_bitrate_bps_, - std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); - rtp_rtcp_->SendRTCP(kRtcpRr); - } - break; + void PollStats() { + do { + if (sender_call_) { + Call::Stats stats = sender_call_->GetStats(); + switch (state_) { + case kWaitForFirstRampUp: + if (stats.send_bandwidth_bps >= remb_bitrate_bps_) { + state_ = kWaitForRemb; + remb_bitrate_bps_ /= 2; + rtp_rtcp_->SetREMBData( + remb_bitrate_bps_, + std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); + rtp_rtcp_->SendRTCP(kRtcpRr); + } + break; - case kWaitForRemb: - if (stats.send_bandwidth_bps == remb_bitrate_bps_) { - state_ = kWaitForSecondRampUp; - remb_bitrate_bps_ *= 2; - rtp_rtcp_->SetREMBData( - remb_bitrate_bps_, - std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); - rtp_rtcp_->SendRTCP(kRtcpRr); - } - break; + case kWaitForRemb: + if (stats.send_bandwidth_bps == remb_bitrate_bps_) { + state_ = kWaitForSecondRampUp; + remb_bitrate_bps_ *= 2; + rtp_rtcp_->SetREMBData( + remb_bitrate_bps_, + std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); + rtp_rtcp_->SendRTCP(kRtcpRr); + } + break; - case kWaitForSecondRampUp: - if (stats.send_bandwidth_bps == remb_bitrate_bps_) { - observation_complete_.Set(); - } - break; + case kWaitForSecondRampUp: + if (stats.send_bandwidth_bps == remb_bitrate_bps_) { + observation_complete_.Set(); + } + break; + } } - } - - return !stop_event_.Wait(1000); + } while (!stop_event_.Wait(1000)); } void PerformTest() override { diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc index b754d29548..c8e379e4e9 100644 --- a/webrtc/video/video_quality_test.cc +++ b/webrtc/video/video_quality_test.cc @@ -555,37 +555,34 @@ class VideoAnalyzer : public PacketReceiver, comparison_available_event_.Set(); } - static bool PollStatsThread(void* obj) { - return static_cast(obj)->PollStats(); + static void PollStatsThread(void* obj) { + static_cast(obj)->PollStats(); } - bool PollStats() { - if (done_.Wait(kSendStatsPollingIntervalMs)) - return false; + void PollStats() { + while (!done_.Wait(kSendStatsPollingIntervalMs)) { + rtc::CritScope crit(&comparison_lock_); - rtc::CritScope crit(&comparison_lock_); + VideoSendStream::Stats send_stats = send_stream_->GetStats(); + // It's not certain that we yet have estimates for any of these stats. + // Check that they are positive before mixing them in. + if (send_stats.encode_frame_rate > 0) + encode_frame_rate_.AddSample(send_stats.encode_frame_rate); + if (send_stats.avg_encode_time_ms > 0) + encode_time_ms_.AddSample(send_stats.avg_encode_time_ms); + if (send_stats.encode_usage_percent > 0) + encode_usage_percent_.AddSample(send_stats.encode_usage_percent); + if (send_stats.media_bitrate_bps > 0) + media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps); - VideoSendStream::Stats send_stats = send_stream_->GetStats(); - // It's not certain that we yet have estimates for any of these stats. Check - // that they are positive before mixing them in. - if (send_stats.encode_frame_rate > 0) - encode_frame_rate_.AddSample(send_stats.encode_frame_rate); - if (send_stats.avg_encode_time_ms > 0) - encode_time_ms_.AddSample(send_stats.avg_encode_time_ms); - if (send_stats.encode_usage_percent > 0) - encode_usage_percent_.AddSample(send_stats.encode_usage_percent); - if (send_stats.media_bitrate_bps > 0) - media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps); - - if (receive_stream_ != nullptr) { - VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats(); - if (receive_stats.decode_ms > 0) - decode_time_ms_.AddSample(receive_stats.decode_ms); - if (receive_stats.max_decode_ms > 0) - decode_time_max_ms_.AddSample(receive_stats.max_decode_ms); + if (receive_stream_ != nullptr) { + VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats(); + if (receive_stats.decode_ms > 0) + decode_time_ms_.AddSample(receive_stats.decode_ms); + if (receive_stats.max_decode_ms > 0) + decode_time_max_ms_.AddSample(receive_stats.max_decode_ms); + } } - - return true; } static bool FrameComparisonThread(void* obj) { diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index 86501409e2..5538d9ee9e 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -155,8 +155,12 @@ bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) { } void ChannelProxy::SetBitrate(int bitrate_bps, int64_t probing_interval_ms) { - RTC_DCHECK(worker_thread_checker_.CalledOnValidThread() || - module_process_thread_checker_.CalledOnValidThread()); + // This method can be called on the worker thread, module process thread + // or on a TaskQueue via VideoSendStreamImpl::OnEncoderConfigurationChanged. + // TODO(solenberg): Figure out a good way to check this or enforce calling + // rules. + // RTC_DCHECK(worker_thread_checker_.CalledOnValidThread() || + // module_process_thread_checker_.CalledOnValidThread()); channel()->SetBitRate(bitrate_bps, probing_interval_ms); }