[libcamera-devel] [PATCH v1 1/3] utils: timer: Allow Timer::start to use utils::Duration

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Tue, Mar 22, 2022 at 01:16:33PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add an overload of the Timer::start member function to accept a utils::Duration
> type for the timer duration.

I'm actually considering moving the Duration class from libcamera-base to
libcamera. The rationale is that the class picks one particular
representation of a duration that we deem suitable for the libcamera
public API. It is of limited usefulness in the base library itself, as
it's not generic. This would prevent the Timer class from using it. How
much hassle is it to use the Timer class in subsequent patches in this
series if we dropped this start() overload ?

> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/base/timer.h |  2 ++
>  src/libcamera/base/timer.cpp   | 14 ++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/include/libcamera/base/timer.h b/include/libcamera/base/timer.h
> index 09f1d3229bd5..f42a8cfd4d6d 100644
> --- a/include/libcamera/base/timer.h
> +++ b/include/libcamera/base/timer.h
> @@ -14,6 +14,7 @@
>  
>  #include <libcamera/base/object.h>
>  #include <libcamera/base/signal.h>
> +#include <libcamera/base/utils.h>
>  
>  namespace libcamera {
>  
> @@ -27,6 +28,7 @@ public:
>  
>  	void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }
>  	void start(std::chrono::milliseconds duration);
> +	void start(utils::Duration duration);
>  	void start(std::chrono::steady_clock::time_point deadline);
>  	void stop();
>  	bool isRunning() const;
> diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp
> index 187336e3a1a4..3ff526aea8ae 100644
> --- a/src/libcamera/base/timer.cpp
> +++ b/src/libcamera/base/timer.cpp
> @@ -85,6 +85,20 @@ void Timer::start(std::chrono::milliseconds duration)
>  	start(utils::clock::now() + duration);
>  }
>  
> +/**
> + * \brief Start or restart the timer with a timeout of \a duration
> + * \param[in] duration The timer duration given by \a utils::Duration
> + *
> + * If the timer is already running it will be stopped and restarted.
> + *
> + * \context This function is \threadbound.
> + */
> +void Timer::start(utils::Duration duration)
> +{
> +	auto msec = std::chrono::duration_cast<std::chrono::milliseconds>(duration);
> +	start(utils::clock::now() + msec);
> +}
> +
>  /**
>   * \brief Start or restart the timer with a \a deadline
>   * \param[in] deadline The timer deadline

-- 
Regards,

Laurent Pinchart