[libcamera-devel,12/18] libcamera: timer: Bind timers to threads

Message ID 20190812124642.24287-13-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Object & Thread enhancements
Related show

Commit Message

Laurent Pinchart Aug. 12, 2019, 12:46 p.m. UTC
The Timer instances are registered with the event dispatcher instance of
the CameraManager. This makes it impossible to use timers in other
threads.

Fix this by inheriting from Object, which allows binding instances to a
thread, and register them with the event dispatcher for the thread they
are bound to.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/timer.h | 11 ++++++++++-
 src/libcamera/timer.cpp   | 28 ++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 3 deletions(-)

Comments

Niklas Söderlund Aug. 17, 2019, 3:01 p.m. UTC | #1
Hi Laurent,

Thanks for your patch.

On 2019-08-12 15:46:36 +0300, Laurent Pinchart wrote:
> The Timer instances are registered with the event dispatcher instance of
> the CameraManager. This makes it impossible to use timers in other
> threads.
> 
> Fix this by inheriting from Object, which allows binding instances to a
> thread, and register them with the event dispatcher for the thread they
> are bound to.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/libcamera/timer.h | 11 ++++++++++-
>  src/libcamera/timer.cpp   | 28 ++++++++++++++++++++++++++--
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> index f082339b1fed..853808e07da8 100644
> --- a/include/libcamera/timer.h
> +++ b/include/libcamera/timer.h
> @@ -9,11 +9,14 @@
>  
>  #include <cstdint>
>  
> +#include <libcamera/object.h>
>  #include <libcamera/signal.h>
>  
>  namespace libcamera {
>  
> -class Timer
> +class Message;
> +
> +class Timer : public Object
>  {
>  public:
>  	Timer();
> @@ -28,7 +31,13 @@ public:
>  
>  	Signal<Timer *> timeout;
>  
> +protected:
> +	void message(Message *msg) override;
> +
>  private:
> +	void registerTimer();
> +	void unregisterTimer();
> +
>  	unsigned int interval_;
>  	uint64_t deadline_;
>  };
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> index 0dcb4e767be3..21c0a444cdce 100644
> --- a/src/libcamera/timer.cpp
> +++ b/src/libcamera/timer.cpp
> @@ -13,6 +13,8 @@
>  #include <libcamera/event_dispatcher.h>
>  
>  #include "log.h"
> +#include "message.h"
> +#include "thread.h"
>  
>  /**
>   * \file timer.h
> @@ -66,7 +68,7 @@ void Timer::start(unsigned int msec)
>  		<< "Starting timer " << this << " with interval "
>  		<< msec << ": deadline " << deadline_;
>  
> -	CameraManager::instance()->eventDispatcher()->registerTimer(this);
> +	registerTimer();
>  }
>  
>  /**
> @@ -79,11 +81,21 @@ void Timer::start(unsigned int msec)
>   */
>  void Timer::stop()
>  {
> -	CameraManager::instance()->eventDispatcher()->unregisterTimer(this);
> +	unregisterTimer();
>  
>  	deadline_ = 0;
>  }
>  
> +void Timer::registerTimer()
> +{
> +	thread()->eventDispatcher()->registerTimer(this);
> +}
> +
> +void Timer::unregisterTimer()
> +{
> +	thread()->eventDispatcher()->unregisterTimer(this);
> +}
> +
>  /**
>   * \brief Check if the timer is running
>   * \return True if the timer is running, false otherwise
> @@ -112,4 +124,16 @@ bool Timer::isRunning() const
>   * The timer pointer is passed as a parameter.
>   */
>  
> +void Timer::message(Message *msg)
> +{
> +	if (msg->type() == Message::ThreadMoveMessage) {
> +		if (deadline_) {
> +			unregisterTimer();
> +			invokeMethod(this, &Timer::registerTimer);
> +		}
> +	}
> +
> +	Object::message(msg);
> +}
> +
>  } /* namespace libcamera */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
index f082339b1fed..853808e07da8 100644
--- a/include/libcamera/timer.h
+++ b/include/libcamera/timer.h
@@ -9,11 +9,14 @@ 
 
 #include <cstdint>
 
+#include <libcamera/object.h>
 #include <libcamera/signal.h>
 
 namespace libcamera {
 
-class Timer
+class Message;
+
+class Timer : public Object
 {
 public:
 	Timer();
@@ -28,7 +31,13 @@  public:
 
 	Signal<Timer *> timeout;
 
+protected:
+	void message(Message *msg) override;
+
 private:
+	void registerTimer();
+	void unregisterTimer();
+
 	unsigned int interval_;
 	uint64_t deadline_;
 };
diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
index 0dcb4e767be3..21c0a444cdce 100644
--- a/src/libcamera/timer.cpp
+++ b/src/libcamera/timer.cpp
@@ -13,6 +13,8 @@ 
 #include <libcamera/event_dispatcher.h>
 
 #include "log.h"
+#include "message.h"
+#include "thread.h"
 
 /**
  * \file timer.h
@@ -66,7 +68,7 @@  void Timer::start(unsigned int msec)
 		<< "Starting timer " << this << " with interval "
 		<< msec << ": deadline " << deadline_;
 
-	CameraManager::instance()->eventDispatcher()->registerTimer(this);
+	registerTimer();
 }
 
 /**
@@ -79,11 +81,21 @@  void Timer::start(unsigned int msec)
  */
 void Timer::stop()
 {
-	CameraManager::instance()->eventDispatcher()->unregisterTimer(this);
+	unregisterTimer();
 
 	deadline_ = 0;
 }
 
+void Timer::registerTimer()
+{
+	thread()->eventDispatcher()->registerTimer(this);
+}
+
+void Timer::unregisterTimer()
+{
+	thread()->eventDispatcher()->unregisterTimer(this);
+}
+
 /**
  * \brief Check if the timer is running
  * \return True if the timer is running, false otherwise
@@ -112,4 +124,16 @@  bool Timer::isRunning() const
  * The timer pointer is passed as a parameter.
  */
 
+void Timer::message(Message *msg)
+{
+	if (msg->type() == Message::ThreadMoveMessage) {
+		if (deadline_) {
+			unregisterTimer();
+			invokeMethod(this, &Timer::registerTimer);
+		}
+	}
+
+	Object::message(msg);
+}
+
 } /* namespace libcamera */