From 6d42cafeadf657e036f3ea36db39f06a90b09758 Mon Sep 17 00:00:00 2001 From: Marcin Wojciechowski Date: Mon, 14 Sep 2026 14:51:10 +0200 Subject: [PATCH 1/5] timer impl change --- common/CMakeLists.txt | 5 ++++ common/include/Timer.h | 6 ++-- common/source/Timer.cpp | 61 ++++++++++++++++++----------------------- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 58fd49fa0..25fca5312 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -22,6 +22,9 @@ set( CMAKE_CXX_STANDARD 17 ) set( CMAKE_CXX_STANDARD_REQUIRED ON ) include( CheckCXXCompilerFlag ) +find_package( PkgConfig REQUIRED ) +pkg_check_modules( GStreamerApp REQUIRED IMPORTED_TARGET gstreamer-app-1.0 gstreamer-pbutils-1.0 gstreamer-audio-1.0) + add_subdirectory(public) add_library( @@ -49,6 +52,7 @@ target_include_directories( PRIVATE include + ${GStreamerApp_INCLUDE_DIRS} ) target_link_libraries ( @@ -56,4 +60,5 @@ target_link_libraries ( PRIVATE RialtoLogging + ${GStreamerApp_LIBRARIES} ) diff --git a/common/include/Timer.h b/common/include/Timer.h index 7393c9a61..c4cef4f7c 100644 --- a/common/include/Timer.h +++ b/common/include/Timer.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -61,11 +62,8 @@ class Timer : public ITimer private: std::atomic m_active; - std::chrono::milliseconds m_timeout; std::function m_callback; - mutable std::mutex m_mutex; - std::thread m_thread; - std::condition_variable m_cv; + guint m_timerId; }; } // namespace firebolt::rialto::common diff --git a/common/source/Timer.cpp b/common/source/Timer.cpp index 8469a4737..1bc4d153d 100644 --- a/common/source/Timer.cpp +++ b/common/source/Timer.cpp @@ -52,32 +52,37 @@ std::unique_ptr TimerFactory::createTimer(const std::chrono::millisecond } Timer::Timer(const std::chrono::milliseconds &timeout, const std::function &callback, TimerType timerType) - : m_active{true}, m_timeout{timeout}, m_callback{callback} + : m_active{true}, m_callback{callback} { - m_thread = std::thread( - [this, timerType]() - { - do + if (timerType == TimerType::PERIODIC) + { + m_timerId = g_timeout_add( + static_cast(timeout.count()), + [](gpointer data) -> gboolean { - bool shouldExecuteCallback = false; + Timer *timer = static_cast(data); + if (timer->m_active && timer->m_callback) { - std::unique_lock lock{m_mutex}; - if (!m_cv.wait_for(lock, m_timeout, [this]() { return !m_active; })) - { - if (m_active && m_callback) - { - shouldExecuteCallback = true; - } - } + timer->m_callback(); } - - if (shouldExecuteCallback) + return timer->m_active ? TRUE : FALSE; + }, + this); + } + else + { + m_timerId = g_timeout_add_once( + static_cast(timeout.count()), + [](gpointer data) + { + Timer *timer = static_cast(data); + if (timer->m_active && timer->m_callback) { - m_callback(); + timer->m_callback(); } - } while (timerType == TimerType::PERIODIC && m_active); - m_active = false; - }); + }, + this); + } } Timer::~Timer() @@ -88,21 +93,7 @@ Timer::~Timer() void Timer::cancel() { m_active = false; - m_cv.notify_one(); - - if (std::this_thread::get_id() == m_thread.get_id()) - { - if (m_thread.joinable()) - { - m_thread.detach(); - } - return; - } - - if (m_thread.joinable()) - { - m_thread.join(); - } + g_source_remove(m_timerId); } bool Timer::isActive() const From 7a508157eec0503cbbd6abe4280aace77a68cd86 Mon Sep 17 00:00:00 2001 From: Marcin Wojciechowski Date: Tue, 15 Sep 2026 10:51:36 +0200 Subject: [PATCH 2/5] timer loop thread added --- common/include/Timer.h | 7 ++----- common/source/Timer.cpp | 42 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/common/include/Timer.h b/common/include/Timer.h index c4cef4f7c..747d6e6d9 100644 --- a/common/include/Timer.h +++ b/common/include/Timer.h @@ -23,10 +23,7 @@ #include "ITimer.h" #include -#include -#include -#include -#include +#include #include namespace firebolt::rialto::common @@ -63,7 +60,7 @@ class Timer : public ITimer private: std::atomic m_active; std::function m_callback; - guint m_timerId; + std::atomic m_timerId; }; } // namespace firebolt::rialto::common diff --git a/common/source/Timer.cpp b/common/source/Timer.cpp index 1bc4d153d..550c7c7a4 100644 --- a/common/source/Timer.cpp +++ b/common/source/Timer.cpp @@ -19,6 +19,40 @@ #include "Timer.h" #include "RialtoCommonLogging.h" +#include + +namespace +{ +class CommonTimerLoop +{ +public: + static CommonTimerLoop &instance() + { + static CommonTimerLoop instance; + return instance; + } + +private: + CommonTimerLoop() + { + m_loop = g_main_loop_new(nullptr, FALSE); + m_thread = std::thread([this]() { g_main_loop_run(m_loop); }); + } + + ~CommonTimerLoop() + { + g_main_loop_quit(m_loop); + if (m_thread.joinable()) + { + m_thread.join(); + } + g_main_loop_unref(m_loop); + } + + GMainLoop *m_loop; + std::thread m_thread; +}; +} // namespace namespace firebolt::rialto::common { @@ -54,6 +88,7 @@ std::unique_ptr TimerFactory::createTimer(const std::chrono::millisecond Timer::Timer(const std::chrono::milliseconds &timeout, const std::function &callback, TimerType timerType) : m_active{true}, m_callback{callback} { + CommonTimerLoop::instance(); if (timerType == TimerType::PERIODIC) { m_timerId = g_timeout_add( @@ -79,6 +114,7 @@ Timer::Timer(const std::chrono::milliseconds &timeout, const std::functionm_active && timer->m_callback) { timer->m_callback(); + timer->m_timerId = 0; } }, this); @@ -93,7 +129,11 @@ Timer::~Timer() void Timer::cancel() { m_active = false; - g_source_remove(m_timerId); + if (m_timerId != 0) + { + g_source_remove(m_timerId); + m_timerId = 0; + } } bool Timer::isActive() const From 746f40ccbb7e60922032c8ee53f9e754ccf7411e Mon Sep 17 00:00:00 2001 From: Marcin Wojciechowski Date: Wed, 23 Sep 2026 08:53:02 +0200 Subject: [PATCH 3/5] cpplint fix --- common/include/Timer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/common/include/Timer.h b/common/include/Timer.h index 747d6e6d9..cf263555f 100644 --- a/common/include/Timer.h +++ b/common/include/Timer.h @@ -24,6 +24,7 @@ #include #include +#include #include namespace firebolt::rialto::common From 595c2347103face43269299774263d0fe80d1280 Mon Sep 17 00:00:00 2001 From: Marcin Wojciechowski Date: Wed, 23 Sep 2026 14:40:00 +0200 Subject: [PATCH 4/5] fix --- common/source/Timer.cpp | 42 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/common/source/Timer.cpp b/common/source/Timer.cpp index 550c7c7a4..dc801b7c4 100644 --- a/common/source/Timer.cpp +++ b/common/source/Timer.cpp @@ -19,7 +19,9 @@ #include "Timer.h" #include "RialtoCommonLogging.h" +#include #include +#include namespace { @@ -32,6 +34,24 @@ class CommonTimerLoop return instance; } + void storeTimerId(guint timerId) + { + std::lock_guard lock(m_mutex); + m_activeTimers.insert(timerId); + } + + void removeTimerId(guint timerId) + { + std::lock_guard lock(m_mutex); + m_activeTimers.erase(timerId); + } + + bool isTimerActive(guint timerId) + { + std::lock_guard lock(m_mutex); + return 0 != timerId && m_activeTimers.find(timerId) != m_activeTimers.end(); + } + private: CommonTimerLoop() { @@ -51,6 +71,8 @@ class CommonTimerLoop GMainLoop *m_loop; std::thread m_thread; + std::mutex m_mutex; + std::unordered_set m_activeTimers; }; } // namespace @@ -88,7 +110,7 @@ std::unique_ptr TimerFactory::createTimer(const std::chrono::millisecond Timer::Timer(const std::chrono::milliseconds &timeout, const std::function &callback, TimerType timerType) : m_active{true}, m_callback{callback} { - CommonTimerLoop::instance(); + CommonTimerLoop &commonTimerLoop = CommonTimerLoop::instance(); if (timerType == TimerType::PERIODIC) { m_timerId = g_timeout_add( @@ -98,9 +120,13 @@ Timer::Timer(const std::chrono::milliseconds &timeout, const std::function(data); if (timer->m_active && timer->m_callback) { - timer->m_callback(); + // We need to call the callback copy here, because the timer instance may be deleted during the callback execution + guint idCopy = timer->m_timerId; + std::function cbCopy = timer->m_callback; + cbCopy(); + return CommonTimerLoop::instance().isTimerActive(idCopy) ? TRUE : FALSE; } - return timer->m_active ? TRUE : FALSE; + return FALSE; }, this); } @@ -113,12 +139,19 @@ Timer::Timer(const std::chrono::milliseconds &timeout, const std::function(data); if (timer->m_active && timer->m_callback) { - timer->m_callback(); + // We need to call the callback copy here, because the timer instance may be deleted during the callback execution + std::function cbCopy = timer->m_callback; + CommonTimerLoop::instance().removeTimerId(timer->m_timerId); timer->m_timerId = 0; + cbCopy(); } }, this); } + if (m_timerId != 0) + { + commonTimerLoop.storeTimerId(m_timerId); + } } Timer::~Timer() @@ -131,6 +164,7 @@ void Timer::cancel() m_active = false; if (m_timerId != 0) { + CommonTimerLoop::instance().removeTimerId(m_timerId); g_source_remove(m_timerId); m_timerId = 0; } From 35496d588e33e52c1d629df0d7b625ade1bdbdc2 Mon Sep 17 00:00:00 2001 From: Marcin Wojciechowski Date: Thu, 24 Sep 2026 08:41:02 +0200 Subject: [PATCH 5/5] better solution --- common/include/Timer.h | 4 +--- common/source/Timer.cpp | 51 ++++++++++++++++------------------------- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/common/include/Timer.h b/common/include/Timer.h index cf263555f..2d010e3a4 100644 --- a/common/include/Timer.h +++ b/common/include/Timer.h @@ -59,9 +59,7 @@ class Timer : public ITimer bool isActive() const override; private: - std::atomic m_active; - std::function m_callback; - std::atomic m_timerId; + std::atomic m_timerId{0}; }; } // namespace firebolt::rialto::common diff --git a/common/source/Timer.cpp b/common/source/Timer.cpp index dc801b7c4..0735ed2ae 100644 --- a/common/source/Timer.cpp +++ b/common/source/Timer.cpp @@ -21,7 +21,7 @@ #include "RialtoCommonLogging.h" #include #include -#include +#include namespace { @@ -34,22 +34,23 @@ class CommonTimerLoop return instance; } - void storeTimerId(guint timerId) + void storeTimerCallback(const firebolt::rialto::common::Timer *timer, const std::function &callback) { std::lock_guard lock(m_mutex); - m_activeTimers.insert(timerId); + m_activeTimers[timer] = callback; } - void removeTimerId(guint timerId) + void removeTimerCallback(const firebolt::rialto::common::Timer *timer) { std::lock_guard lock(m_mutex); - m_activeTimers.erase(timerId); + m_activeTimers.erase(timer); } - bool isTimerActive(guint timerId) + std::function getTimerCallback(const firebolt::rialto::common::Timer *timer) { std::lock_guard lock(m_mutex); - return 0 != timerId && m_activeTimers.find(timerId) != m_activeTimers.end(); + auto it = m_activeTimers.find(timer); + return it != m_activeTimers.end() ? it->second : nullptr; } private: @@ -72,7 +73,7 @@ class CommonTimerLoop GMainLoop *m_loop; std::thread m_thread; std::mutex m_mutex; - std::unordered_set m_activeTimers; + std::unordered_map> m_activeTimers; }; } // namespace @@ -108,23 +109,19 @@ std::unique_ptr TimerFactory::createTimer(const std::chrono::millisecond } Timer::Timer(const std::chrono::milliseconds &timeout, const std::function &callback, TimerType timerType) - : m_active{true}, m_callback{callback} { - CommonTimerLoop &commonTimerLoop = CommonTimerLoop::instance(); + CommonTimerLoop::instance().storeTimerCallback(this, callback); if (timerType == TimerType::PERIODIC) { m_timerId = g_timeout_add( static_cast(timeout.count()), [](gpointer data) -> gboolean { - Timer *timer = static_cast(data); - if (timer->m_active && timer->m_callback) + auto callback = CommonTimerLoop::instance().getTimerCallback(static_cast(data)); + if (callback) { - // We need to call the callback copy here, because the timer instance may be deleted during the callback execution - guint idCopy = timer->m_timerId; - std::function cbCopy = timer->m_callback; - cbCopy(); - return CommonTimerLoop::instance().isTimerActive(idCopy) ? TRUE : FALSE; + callback(); + return CommonTimerLoop::instance().getTimerCallback(static_cast(data)) ? TRUE : FALSE; } return FALSE; }, @@ -136,22 +133,15 @@ Timer::Timer(const std::chrono::milliseconds &timeout, const std::function(timeout.count()), [](gpointer data) { - Timer *timer = static_cast(data); - if (timer->m_active && timer->m_callback) + auto callback = CommonTimerLoop::instance().getTimerCallback(static_cast(data)); + if (callback) { - // We need to call the callback copy here, because the timer instance may be deleted during the callback execution - std::function cbCopy = timer->m_callback; - CommonTimerLoop::instance().removeTimerId(timer->m_timerId); - timer->m_timerId = 0; - cbCopy(); + callback(); + CommonTimerLoop::instance().removeTimerCallback(static_cast(data)); } }, this); } - if (m_timerId != 0) - { - commonTimerLoop.storeTimerId(m_timerId); - } } Timer::~Timer() @@ -161,10 +151,9 @@ Timer::~Timer() void Timer::cancel() { - m_active = false; + CommonTimerLoop::instance().removeTimerCallback(this); if (m_timerId != 0) { - CommonTimerLoop::instance().removeTimerId(m_timerId); g_source_remove(m_timerId); m_timerId = 0; } @@ -172,6 +161,6 @@ void Timer::cancel() bool Timer::isActive() const { - return m_active; + return CommonTimerLoop::instance().getTimerCallback(this) != nullptr; } } // namespace firebolt::rialto::common