From ea4091816912e42f0d99e8eff663b9eda9afac0f Mon Sep 17 00:00:00 2001 From: Alexander Yee Date: Sun, 15 Dec 2024 01:03:50 -0800 Subject: [PATCH] More fine-grained locking to reduce locking time on critical paths. --- .../Source/CommonFramework/Globals.cpp | 2 +- .../Backends/CameraWidgetQt6.5.cpp | 59 ++++++++++++------- .../Backends/CameraWidgetQt6.5.h | 22 +++++-- 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/SerialPrograms/Source/CommonFramework/Globals.cpp b/SerialPrograms/Source/CommonFramework/Globals.cpp index b05924cf6..f4e0559c4 100644 --- a/SerialPrograms/Source/CommonFramework/Globals.cpp +++ b/SerialPrograms/Source/CommonFramework/Globals.cpp @@ -25,7 +25,7 @@ namespace PokemonAutomation{ const bool IS_BETA_VERSION = true; const int PROGRAM_VERSION_MAJOR = 0; const int PROGRAM_VERSION_MINOR = 50; -const int PROGRAM_VERSION_PATCH = 13; +const int PROGRAM_VERSION_PATCH = 14; const std::string PROGRAM_VERSION_BASE = "v" + std::to_string(PROGRAM_VERSION_MAJOR) + diff --git a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.cpp b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.cpp index de8e10031..03f386376 100644 --- a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.cpp +++ b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.cpp @@ -17,7 +17,7 @@ #include //#include "Common/Cpp/Exceptions.h" //#include "Common/Cpp/Time.h" -#include "Common/Cpp/PrettyPrint.h" +//#include "Common/Cpp/PrettyPrint.h" #include "CommonFramework/GlobalSettingsPanel.h" #include "CommonFramework/GlobalServices.h" #include "CommonFramework/VideoPipeline/CameraOption.h" @@ -75,22 +75,22 @@ std::unique_ptr CameraBackend::make_camera(Log void CameraSession::add_state_listener(StateListener& listener){ auto scope_check = m_sanitizer.check_scope(); - std::lock_guard lg(m_lock); + WriteSpinLock lg(m_listener_lock); m_state_listeners.insert(&listener); } void CameraSession::remove_state_listener(StateListener& listener){ auto scope_check = m_sanitizer.check_scope(); - std::lock_guard lg(m_lock); + WriteSpinLock lg(m_listener_lock); m_state_listeners.erase(&listener); } void CameraSession::add_frame_listener(VideoFrameListener& listener){ auto scope_check = m_sanitizer.check_scope(); - std::lock_guard lg(m_lock); + WriteSpinLock lg(m_listener_lock); m_frame_listeners.insert(&listener); } void CameraSession::remove_frame_listener(VideoFrameListener& listener){ auto scope_check = m_sanitizer.check_scope(); - std::lock_guard lg(m_lock); + WriteSpinLock lg(m_listener_lock); m_frame_listeners.erase(&listener); } @@ -102,9 +102,9 @@ CameraSession::CameraSession(Logger& logger, Resolution default_resolution) : m_logger(logger) , m_default_resolution(default_resolution) , m_resolution(default_resolution) + , m_last_image_timestamp(WallClock::min()) , m_stats_conversion("ConvertFrame", "ms", 1000, std::chrono::seconds(10)) , m_last_frame_seqnum(0) - , m_last_image_timestamp(WallClock::min()) // , m_history(GlobalSettings::instance().HISTORY_SECONDS * 1000000) { uint8_t watchdog_timeout = GlobalSettings::instance().VIDEO_PIPELINE->AUTO_RESET_SECONDS; @@ -157,15 +157,21 @@ void CameraSession::set_resolution(Resolution resolution){ m_logger.log("Resolution not supported.", COLOR_RED); return; } - for (StateListener* listener : m_state_listeners){ - listener->pre_resolution_change(resolution); + { + ReadSpinLock lg1(m_listener_lock); + for (StateListener* listener : m_state_listeners){ + listener->pre_resolution_change(resolution); + } } m_resolution = resolution; m_camera->stop(); m_camera->setCameraFormat(*iter->second); m_camera->start(); - for (StateListener* listener : m_state_listeners){ - listener->post_resolution_change(resolution); + { + ReadSpinLock lg1(m_listener_lock); + for (StateListener* listener : m_state_listeners){ + listener->post_resolution_change(resolution); + } } }); } @@ -232,13 +238,13 @@ VideoSnapshot CameraSession::snapshot(){ image = image.convertToFormat(QImage::Format_ARGB32); } + WallClock time1 = current_time(); + m_stats_conversion.report_data(m_logger, std::chrono::duration_cast(time1 - time0).count()); + m_last_image = std::move(image); m_last_image_timestamp = frame_timestamp; m_last_image_seqnum = frame_seqnum; - WallClock time1 = current_time(); - m_stats_conversion.report_data(m_logger, std::chrono::duration_cast(time1 - time0).count()); - return VideoSnapshot(m_last_image, m_last_image_timestamp); } double CameraSession::fps_source(){ @@ -271,8 +277,8 @@ void CameraSession::connect_video_sink(QVideoSink* sink){ m_fps_tracker_source.push_event(now); } // cout << now_to_filestring() << endl; - std::lock_guard lg(m_lock); + ReadSpinLock lg(m_listener_lock); if (!m_frame_listeners.empty()){ std::shared_ptr frame_ptr(new VideoFrame(now, frame)); for (VideoFrameListener* listener : m_frame_listeners){ @@ -311,13 +317,18 @@ void CameraSession::set_video_output(QGraphicsVideoItem& item){ } void CameraSession::shutdown(){ + // Must call inside state lock. + if (!m_capture_session){ return; } m_logger.log("Stopping Camera..."); - for (StateListener* listener : m_state_listeners){ - listener->pre_shutdown(); + { + ReadSpinLock lg(m_listener_lock); + for (StateListener* listener : m_state_listeners){ + listener->pre_shutdown(); + } } m_camera->stop(); m_capture_session.reset(); @@ -338,11 +349,16 @@ void CameraSession::shutdown(){ m_last_image_seqnum = m_last_frame_seqnum; } - for (StateListener* listener : m_state_listeners){ - listener->post_shutdown(); + { + ReadSpinLock lg(m_listener_lock); + for (StateListener* listener : m_state_listeners){ + listener->post_shutdown(); + } } } void CameraSession::startup(){ + // Must call inside state lock. + if (!m_device){ return; } @@ -426,8 +442,11 @@ void CameraSession::startup(){ // cout << "frame rate = " << m_camera->cameraFormat().minFrameRate() << endl; - for (StateListener* listener : m_state_listeners){ - listener->post_new_source(m_device, m_resolution); + { + ReadSpinLock lg(m_listener_lock); + for (StateListener* listener : m_state_listeners){ + listener->post_new_source(m_device, m_resolution); + } } } diff --git a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.h b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.h index 62f39de5b..4d7c904ec 100644 --- a/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.h +++ b/SerialPrograms/Source/CommonFramework/VideoPipeline/Backends/CameraWidgetQt6.5.h @@ -111,7 +111,10 @@ class CameraSession : public QObject, public PokemonAutomation::CameraSession, p Logger& m_logger; Resolution m_default_resolution; - // If you need both locks, acquire "m_lock" first. + // The general state lock. You must acquire this lock to touch anything in + // this section. + // If you need to acquire both this locks and one of the other ones, you + // must acquire this one first. mutable std::mutex m_lock; CameraInfo m_device; @@ -130,10 +133,17 @@ class CameraSession : public QObject, public PokemonAutomation::CameraSession, p EventRateTracker m_fps_tracker_source; EventRateTracker m_fps_tracker_display; + // Last Cached Image + QImage m_last_image; + WallClock m_last_image_timestamp; + uint64_t m_last_image_seqnum = 0; + PeriodicStatsReporterI32 m_stats_conversion; + private: // Frame Cache: All accesses must be under this lock. + mutable SpinLock m_frame_lock; // Last Frame @@ -141,17 +151,17 @@ class CameraSession : public QObject, public PokemonAutomation::CameraSession, p WallClock m_last_frame_timestamp; uint64_t m_last_frame_seqnum = 0; - // Last Cached Image - QImage m_last_image; - WallClock m_last_image_timestamp; - uint64_t m_last_image_seqnum = 0; private: - // Listeners + // Listeners: All accesses must be under this lock. + mutable SpinLock m_listener_lock; + std::set m_state_listeners; // std::set m_frame_ready_listeners; std::set m_frame_listeners; + +private: LifetimeSanitizer m_sanitizer; };