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

[Naushir Patuck]

Hi Laurent,


On Tue, 22 Mar 2022 at 20:46, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> 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 ?
>

As mentioned earlier, there is minimal hassle if we decide not to use
utils::Duration
in this API, so I am ok with it.

However, utils::Duration is very nice, so I was curious what would make it
unusable
here if it were in the public API? Would this affect our usage in the IPA
as well?  The
IPA does make significant use of utils::Duration, it would be painful to
stop using it
there.

Regards,
Naush


>
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220323/0aaab1cb/attachment.htm>