[libcamera-devel,8/9] libcamera: timer: Add start() method with absolute deadline
diff mbox series

Message ID 20191006053226.8976-9-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Timer fixes and enhancements
Related show

Commit Message

Laurent Pinchart Oct. 6, 2019, 5:32 a.m. UTC 
The Timer class is started using a timer duration. To help callers that
require waiting for an absolute deadline, add a start() overload that
takes a std::chrono::steady_clock::time_point value. This will be used
by IPAs.

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

Comments

Jacopo Mondi Oct. 6, 2019, 7:34 p.m. UTC | #1 
Hi Laurent

On Sun, Oct 06, 2019 at 08:32:25AM +0300, Laurent Pinchart wrote:
> The Timer class is started using a timer duration. To help callers that
> require waiting for an absolute deadline, add a start() overload that
> takes a std::chrono::steady_clock::time_point value. This will be used
> by IPAs.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/timer.h |  1 +
>  src/libcamera/timer.cpp   | 22 +++++++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> index 3540efb41b6f..34e7b8ac8e87 100644
> --- a/include/libcamera/timer.h
> +++ b/include/libcamera/timer.h
> @@ -25,6 +25,7 @@ public:
>
>  	void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }
>  	void start(std::chrono::milliseconds duration);
> +	void start(std::chrono::steady_clock::time_point deadline);
>  	void stop();
>  	bool isRunning() const;
>
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> index 8749d66c8662..705ce60b8fbd 100644
> --- a/src/libcamera/timer.cpp
> +++ b/src/libcamera/timer.cpp
> @@ -37,6 +37,11 @@ LOG_DEFINE_CATEGORY(Timer)
>   * stop(), and once it times out or is stopped, can be started again with
>   * start().
>   *
> + * The timer deadline is specified as either a duration in milliseconds or an
> + * absolute time point. If the deadline is set to the past or the current time,

'to the past of the current time' or
'to the past or to the current time' ?

otherwise
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

> + * the timer will time out immediately when execution returns to the event
> + * loop of the timer's thread.
> + *
>   * Timers run in the thread they belong to, and thus emit the \a ref timeout
>   * signal from that thread. To avoid race conditions they must not be started
>   * or stopped from a different thread, attempts to do so will be rejected and
> @@ -74,17 +79,28 @@ Timer::~Timer()
>   * the timer is already running it will be stopped and restarted.
>   */
>  void Timer::start(std::chrono::milliseconds duration)
> +{
> +	start(utils::clock::now() + duration);
> +}
> +
> +/**
> + * \brief Start or restart the timer with a \a deadline
> + * \param[in] deadline The timer deadline
> + *
> + * This method shall be called from the thread the timer is associated with. If
> + * the timer is already running it will be stopped and restarted.
> + */
> +void Timer::start(std::chrono::steady_clock::time_point deadline)
>  {
>  	if (Thread::current() != thread()) {
>  		LOG(Timer, Error) << "Timer can't be started from another thread";
>  		return;
>  	}
>
> -	deadline_ = utils::clock::now() + duration;
> +	deadline_ = deadline;
>
>  	LOG(Timer, Debug)
> -		<< "Starting timer " << this << " with duration "
> -		<< duration.count() << ": deadline "
> +		<< "Starting timer " << this << ": deadline "
>  		<< utils::time_point_to_string(deadline_);
>
>  	if (isRunning())
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Oct. 7, 2019, 3:14 a.m. UTC | #2 
Hi Jacopo,

On Sun, Oct 06, 2019 at 09:34:38PM +0200, Jacopo Mondi wrote:
> On Sun, Oct 06, 2019 at 08:32:25AM +0300, Laurent Pinchart wrote:
> > The Timer class is started using a timer duration. To help callers that
> > require waiting for an absolute deadline, add a start() overload that
> > takes a std::chrono::steady_clock::time_point value. This will be used
> > by IPAs.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/timer.h |  1 +
> >  src/libcamera/timer.cpp   | 22 +++++++++++++++++++---
> >  2 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> > index 3540efb41b6f..34e7b8ac8e87 100644
> > --- a/include/libcamera/timer.h
> > +++ b/include/libcamera/timer.h
> > @@ -25,6 +25,7 @@ public:
> >
> >  	void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }
> >  	void start(std::chrono::milliseconds duration);
> > +	void start(std::chrono::steady_clock::time_point deadline);
> >  	void stop();
> >  	bool isRunning() const;
> >
> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> > index 8749d66c8662..705ce60b8fbd 100644
> > --- a/src/libcamera/timer.cpp
> > +++ b/src/libcamera/timer.cpp
> > @@ -37,6 +37,11 @@ LOG_DEFINE_CATEGORY(Timer)
> >   * stop(), and once it times out or is stopped, can be started again with
> >   * start().
> >   *
> > + * The timer deadline is specified as either a duration in milliseconds or an
> > + * absolute time point. If the deadline is set to the past or the current time,
> 
> 'to the past of the current time' or
> 'to the past or to the current time' ?

The second option. I'll change that.

> otherwise
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > + * the timer will time out immediately when execution returns to the event
> > + * loop of the timer's thread.
> > + *
> >   * Timers run in the thread they belong to, and thus emit the \a ref timeout
> >   * signal from that thread. To avoid race conditions they must not be started
> >   * or stopped from a different thread, attempts to do so will be rejected and
> > @@ -74,17 +79,28 @@ Timer::~Timer()
> >   * the timer is already running it will be stopped and restarted.
> >   */
> >  void Timer::start(std::chrono::milliseconds duration)
> > +{
> > +	start(utils::clock::now() + duration);
> > +}
> > +
> > +/**
> > + * \brief Start or restart the timer with a \a deadline
> > + * \param[in] deadline The timer deadline
> > + *
> > + * This method shall be called from the thread the timer is associated with. If
> > + * the timer is already running it will be stopped and restarted.
> > + */
> > +void Timer::start(std::chrono::steady_clock::time_point deadline)
> >  {
> >  	if (Thread::current() != thread()) {
> >  		LOG(Timer, Error) << "Timer can't be started from another thread";
> >  		return;
> >  	}
> >
> > -	deadline_ = utils::clock::now() + duration;
> > +	deadline_ = deadline;
> >
> >  	LOG(Timer, Debug)
> > -		<< "Starting timer " << this << " with duration "
> > -		<< duration.count() << ": deadline "
> > +		<< "Starting timer " << this << ": deadline "
> >  		<< utils::time_point_to_string(deadline_);
> >
> >  	if (isRunning())

Patch
diff mbox series 

diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
index 3540efb41b6f..34e7b8ac8e87 100644
--- a/include/libcamera/timer.h
+++ b/include/libcamera/timer.h
@@ -25,6 +25,7 @@  public:
 
 	void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }
 	void start(std::chrono::milliseconds duration);
+	void start(std::chrono::steady_clock::time_point deadline);
 	void stop();
 	bool isRunning() const;
 
diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
index 8749d66c8662..705ce60b8fbd 100644
--- a/src/libcamera/timer.cpp
+++ b/src/libcamera/timer.cpp
@@ -37,6 +37,11 @@  LOG_DEFINE_CATEGORY(Timer)
  * stop(), and once it times out or is stopped, can be started again with
  * start().
  *
+ * The timer deadline is specified as either a duration in milliseconds or an
+ * absolute time point. If the deadline is set to the past or the current time,
+ * the timer will time out immediately when execution returns to the event
+ * loop of the timer's thread.
+ *
  * Timers run in the thread they belong to, and thus emit the \a ref timeout
  * signal from that thread. To avoid race conditions they must not be started
  * or stopped from a different thread, attempts to do so will be rejected and
@@ -74,17 +79,28 @@  Timer::~Timer()
  * the timer is already running it will be stopped and restarted.
  */
 void Timer::start(std::chrono::milliseconds duration)
+{
+	start(utils::clock::now() + duration);
+}
+
+/**
+ * \brief Start or restart the timer with a \a deadline
+ * \param[in] deadline The timer deadline
+ *
+ * This method shall be called from the thread the timer is associated with. If
+ * the timer is already running it will be stopped and restarted.
+ */
+void Timer::start(std::chrono::steady_clock::time_point deadline)
 {
 	if (Thread::current() != thread()) {
 		LOG(Timer, Error) << "Timer can't be started from another thread";
 		return;
 	}
 
-	deadline_ = utils::clock::now() + duration;
+	deadline_ = deadline;
 
 	LOG(Timer, Debug)
-		<< "Starting timer " << this << " with duration "
-		<< duration.count() << ": deadline "
+		<< "Starting timer " << this << ": deadline "
 		<< utils::time_point_to_string(deadline_);
 
 	if (isRunning())