<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 22 Mar 2022 at 20:46, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Tue, Mar 22, 2022 at 01:16:33PM +0000, Naushir Patuck via libcamera-devel wrote:<br>
> Add an overload of the Timer::start member function to accept a utils::Duration<br>
> type for the timer duration.<br>
<br>
I'm actually considering moving the Duration class from libcamera-base to<br>
libcamera. The rationale is that the class picks one particular<br>
representation of a duration that we deem suitable for the libcamera<br>
public API. It is of limited usefulness in the base library itself, as<br>
it's not generic. This would prevent the Timer class from using it. How<br>
much hassle is it to use the Timer class in subsequent patches in this<br>
series if we dropped this start() overload ?<br></blockquote><div><br></div><div>As mentioned earlier, there is minimal hassle if we decide not to use utils::Duration</div><div>in this API, so I am ok with it.</div><div><br></div><div>However, utils::Duration is very nice, so I was curious what would make it unusable</div><div>here if it were in the public API? Would this affect our usage in the IPA as well?  The</div><div>IPA does make significant use of utils::Duration, it would be painful to stop using it</div><div>there.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
>  include/libcamera/base/timer.h |  2 ++<br>
>  src/libcamera/base/timer.cpp   | 14 ++++++++++++++<br>
>  2 files changed, 16 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/base/timer.h b/include/libcamera/base/timer.h<br>
> index 09f1d3229bd5..f42a8cfd4d6d 100644<br>
> --- a/include/libcamera/base/timer.h<br>
> +++ b/include/libcamera/base/timer.h<br>
> @@ -14,6 +14,7 @@<br>
>  <br>
>  #include <libcamera/base/object.h><br>
>  #include <libcamera/base/signal.h><br>
> +#include <libcamera/base/utils.h><br>
>  <br>
>  namespace libcamera {<br>
>  <br>
> @@ -27,6 +28,7 @@ public:<br>
>  <br>
>       void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }<br>
>       void start(std::chrono::milliseconds duration);<br>
> +     void start(utils::Duration duration);<br>
>       void start(std::chrono::steady_clock::time_point deadline);<br>
>       void stop();<br>
>       bool isRunning() const;<br>
> diff --git a/src/libcamera/base/timer.cpp b/src/libcamera/base/timer.cpp<br>
> index 187336e3a1a4..3ff526aea8ae 100644<br>
> --- a/src/libcamera/base/timer.cpp<br>
> +++ b/src/libcamera/base/timer.cpp<br>
> @@ -85,6 +85,20 @@ void Timer::start(std::chrono::milliseconds duration)<br>
>       start(utils::clock::now() + duration);<br>
>  }<br>
>  <br>
> +/**<br>
> + * \brief Start or restart the timer with a timeout of \a duration<br>
> + * \param[in] duration The timer duration given by \a utils::Duration<br>
> + *<br>
> + * If the timer is already running it will be stopped and restarted.<br>
> + *<br>
> + * \context This function is \threadbound.<br>
> + */<br>
> +void Timer::start(utils::Duration duration)<br>
> +{<br>
> +     auto msec = std::chrono::duration_cast<std::chrono::milliseconds>(duration);<br>
> +     start(utils::clock::now() + msec);<br>
> +}<br>
> +<br>
>  /**<br>
>   * \brief Start or restart the timer with a \a deadline<br>
>   * \param[in] deadline The timer deadline<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>