[{"id":2797,"web_url":"https://patchwork.libcamera.org/comment/2797/","msgid":"<20191006181631.nbrh72naet3z3lvc@uno.localdomain>","date":"2019-10-06T18:16:31","subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sun, Oct 06, 2019 at 08:32:18AM +0300, Laurent Pinchart wrote:\n> The libcamera timers are single-shot timers. They are started with a\n> duration, but fire once only, not based on an interval. Remove the\n> interval concept by removing the interval() method, and rename other\n> occurences of interval to duration.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/timer.h        |  4 +---\n>  src/libcamera/timer.cpp          | 19 ++++++-------------\n>  src/qcam/qt_event_dispatcher.cpp |  7 ++++++-\n>  3 files changed, 13 insertions(+), 17 deletions(-)\n>\n> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> index 476ae45f1e53..09f426a59993 100644\n> --- a/include/libcamera/timer.h\n> +++ b/include/libcamera/timer.h\n> @@ -24,11 +24,10 @@ public:\n>  \t~Timer();\n>\n>  \tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> -\tvoid start(std::chrono::milliseconds interval);\n> +\tvoid start(std::chrono::milliseconds duration);\n>  \tvoid stop();\n>  \tbool isRunning() const;\n>\n> -\tstd::chrono::milliseconds interval() const { return interval_; }\n>  \tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n>\n>  \tSignal<Timer *> timeout;\n> @@ -40,7 +39,6 @@ private:\n>  \tvoid registerTimer();\n>  \tvoid unregisterTimer();\n>\n> -\tstd::chrono::milliseconds interval_;\n>  \tstd::chrono::steady_clock::time_point deadline_;\n>  };\n>\n> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> index b3cea3dadb49..34410bab0fb0 100644\n> --- a/src/libcamera/timer.cpp\n> +++ b/src/libcamera/timer.cpp\n> @@ -61,19 +61,18 @@ Timer::~Timer()\n>   */\n>\n>  /**\n> - * \\brief Start or restart the timer with a timeout of \\a interval\n> - * \\param[in] interval The timer duration in milliseconds\n> + * \\brief Start or restart the timer with a timeout of \\a duration\n> + * \\param[in] duration The timer duration in milliseconds\n>   *\n>   * If the timer is already running it will be stopped and restarted.\n>   */\n> -void Timer::start(std::chrono::milliseconds interval)\n> +void Timer::start(std::chrono::milliseconds duration)\n>  {\n> -\tinterval_ = interval;\n> -\tdeadline_ = utils::clock::now() + interval;\n> +\tdeadline_ = utils::clock::now() + duration;\n>\n>  \tLOG(Timer, Debug)\n> -\t\t<< \"Starting timer \" << this << \" with interval \"\n> -\t\t<< interval.count() << \": deadline \"\n> +\t\t<< \"Starting timer \" << this << \" with duration \"\n> +\t\t<< duration.count() << \": deadline \"\n>  \t\t<< utils::time_point_to_string(deadline_);\n\nShouldn't you print the duration instead of the expected deadline\nhere?\n\n>\n>  \tregisterTimer();\n> @@ -113,12 +112,6 @@ bool Timer::isRunning() const\n>  \treturn deadline_ != utils::time_point();\n>  }\n>\n> -/**\n> - * \\fn Timer::interval()\n> - * \\brief Retrieve the timer interval\n> - * \\return The timer interval in milliseconds\n> - */\n> -\n>  /**\n>   * \\fn Timer::deadline()\n>   * \\brief Retrieve the timer deadline\n> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp\n> index 994af3ead82a..9e989bef7d53 100644\n> --- a/src/qcam/qt_event_dispatcher.cpp\n> +++ b/src/qcam/qt_event_dispatcher.cpp\n> @@ -5,6 +5,7 @@\n>   * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher\n>   */\n>\n> +#include <chrono>\n>  #include <iostream>\n>\n>  #include <QAbstractEventDispatcher>\n> @@ -112,7 +113,11 @@ void QtEventDispatcher::exceptionNotifierActivated(int socket)\n>\n>  void QtEventDispatcher::registerTimer(Timer *timer)\n>  {\n> -\tint timerId = startTimer(timer->interval());\n> +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> +\tstd::chrono::steady_clock::duration duration = timer->deadline() - now;\n\nWould it make sense an helper to get this from the timer ? (the\ndeadline_ - now part). Nothing urgent though\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> +\tstd::chrono::milliseconds msec =\n> +\t\tstd::chrono::duration_cast<std::chrono::milliseconds>(duration);\n> +\tint timerId = startTimer(msec);\n>  \ttimers_[timerId] = timer;\n>  \ttimerIds_[timer] = timerId;\n>  }\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7D7D60E1E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  6 Oct 2019 20:16:00 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 343AD240009;\n\tSun,  6 Oct 2019 18:16:00 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sun, 6 Oct 2019 20:16:31 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191006181631.nbrh72naet3z3lvc@uno.localdomain>","References":"<20191006053226.8976-1-laurent.pinchart@ideasonboard.com>\n\t<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qgg4wh64gdgzygnz\"","Content-Disposition":"inline","In-Reply-To":"<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 06 Oct 2019 18:16:00 -0000"}},{"id":2798,"web_url":"https://patchwork.libcamera.org/comment/2798/","msgid":"<20191006182249.ikindvg7d5ehj6mf@uno.localdomain>","date":"2019-10-06T18:22:49","subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi again\n\nOn Sun, Oct 06, 2019 at 08:32:18AM +0300, Laurent Pinchart wrote:\n> The libcamera timers are single-shot timers. They are started with a\n> duration, but fire once only, not based on an interval. Remove the\n> interval concept by removing the interval() method, and rename other\n> occurences of interval to duration.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/timer.h        |  4 +---\n>  src/libcamera/timer.cpp          | 19 ++++++-------------\n>  src/qcam/qt_event_dispatcher.cpp |  7 ++++++-\n>  3 files changed, 13 insertions(+), 17 deletions(-)\n>\n> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> index 476ae45f1e53..09f426a59993 100644\n> --- a/include/libcamera/timer.h\n> +++ b/include/libcamera/timer.h\n> @@ -24,11 +24,10 @@ public:\n>  \t~Timer();\n>\n>  \tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> -\tvoid start(std::chrono::milliseconds interval);\n> +\tvoid start(std::chrono::milliseconds duration);\n>  \tvoid stop();\n>  \tbool isRunning() const;\n>\n> -\tstd::chrono::milliseconds interval() const { return interval_; }\n>  \tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n>\n>  \tSignal<Timer *> timeout;\n> @@ -40,7 +39,6 @@ private:\n>  \tvoid registerTimer();\n>  \tvoid unregisterTimer();\n>\n> -\tstd::chrono::milliseconds interval_;\n>  \tstd::chrono::steady_clock::time_point deadline_;\n\njust noticed, can we use the libcamera defined time related types in\nutils here ?\n\nThanks\n  j\n\n>  };\n>\n> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> index b3cea3dadb49..34410bab0fb0 100644\n> --- a/src/libcamera/timer.cpp\n> +++ b/src/libcamera/timer.cpp\n> @@ -61,19 +61,18 @@ Timer::~Timer()\n>   */\n>\n>  /**\n> - * \\brief Start or restart the timer with a timeout of \\a interval\n> - * \\param[in] interval The timer duration in milliseconds\n> + * \\brief Start or restart the timer with a timeout of \\a duration\n> + * \\param[in] duration The timer duration in milliseconds\n>   *\n>   * If the timer is already running it will be stopped and restarted.\n>   */\n> -void Timer::start(std::chrono::milliseconds interval)\n> +void Timer::start(std::chrono::milliseconds duration)\n>  {\n> -\tinterval_ = interval;\n> -\tdeadline_ = utils::clock::now() + interval;\n> +\tdeadline_ = utils::clock::now() + duration;\n>\n>  \tLOG(Timer, Debug)\n> -\t\t<< \"Starting timer \" << this << \" with interval \"\n> -\t\t<< interval.count() << \": deadline \"\n> +\t\t<< \"Starting timer \" << this << \" with duration \"\n> +\t\t<< duration.count() << \": deadline \"\n>  \t\t<< utils::time_point_to_string(deadline_);\n>\n>  \tregisterTimer();\n> @@ -113,12 +112,6 @@ bool Timer::isRunning() const\n>  \treturn deadline_ != utils::time_point();\n>  }\n>\n> -/**\n> - * \\fn Timer::interval()\n> - * \\brief Retrieve the timer interval\n> - * \\return The timer interval in milliseconds\n> - */\n> -\n>  /**\n>   * \\fn Timer::deadline()\n>   * \\brief Retrieve the timer deadline\n> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp\n> index 994af3ead82a..9e989bef7d53 100644\n> --- a/src/qcam/qt_event_dispatcher.cpp\n> +++ b/src/qcam/qt_event_dispatcher.cpp\n> @@ -5,6 +5,7 @@\n>   * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher\n>   */\n>\n> +#include <chrono>\n>  #include <iostream>\n>\n>  #include <QAbstractEventDispatcher>\n> @@ -112,7 +113,11 @@ void QtEventDispatcher::exceptionNotifierActivated(int socket)\n>\n>  void QtEventDispatcher::registerTimer(Timer *timer)\n>  {\n> -\tint timerId = startTimer(timer->interval());\n> +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> +\tstd::chrono::steady_clock::duration duration = timer->deadline() - now;\n> +\tstd::chrono::milliseconds msec =\n> +\t\tstd::chrono::duration_cast<std::chrono::milliseconds>(duration);\n> +\tint timerId = startTimer(msec);\n>  \ttimers_[timerId] = timer;\n>  \ttimerIds_[timer] = timerId;\n>  }\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD2C560E1E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  6 Oct 2019 20:21:03 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 61A37C0003;\n\tSun,  6 Oct 2019 18:21:03 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sun, 6 Oct 2019 20:22:49 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191006182249.ikindvg7d5ehj6mf@uno.localdomain>","References":"<20191006053226.8976-1-laurent.pinchart@ideasonboard.com>\n\t<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"txsvt7oen2qkl7wz\"","Content-Disposition":"inline","In-Reply-To":"<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 06 Oct 2019 18:21:04 -0000"}},{"id":2807,"web_url":"https://patchwork.libcamera.org/comment/2807/","msgid":"<20191007025318.GE4740@pendragon.ideasonboard.com>","date":"2019-10-07T02:53:18","subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Oct 06, 2019 at 08:22:49PM +0200, Jacopo Mondi wrote:\n> On Sun, Oct 06, 2019 at 08:32:18AM +0300, Laurent Pinchart wrote:\n> > The libcamera timers are single-shot timers. They are started with a\n> > duration, but fire once only, not based on an interval. Remove the\n> > interval concept by removing the interval() method, and rename other\n> > occurences of interval to duration.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/timer.h        |  4 +---\n> >  src/libcamera/timer.cpp          | 19 ++++++-------------\n> >  src/qcam/qt_event_dispatcher.cpp |  7 ++++++-\n> >  3 files changed, 13 insertions(+), 17 deletions(-)\n> >\n> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> > index 476ae45f1e53..09f426a59993 100644\n> > --- a/include/libcamera/timer.h\n> > +++ b/include/libcamera/timer.h\n> > @@ -24,11 +24,10 @@ public:\n> >  \t~Timer();\n> >\n> >  \tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> > -\tvoid start(std::chrono::milliseconds interval);\n> > +\tvoid start(std::chrono::milliseconds duration);\n> >  \tvoid stop();\n> >  \tbool isRunning() const;\n> >\n> > -\tstd::chrono::milliseconds interval() const { return interval_; }\n> >  \tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n> >\n> >  \tSignal<Timer *> timeout;\n> > @@ -40,7 +39,6 @@ private:\n> >  \tvoid registerTimer();\n> >  \tvoid unregisterTimer();\n> >\n> > -\tstd::chrono::milliseconds interval_;\n> >  \tstd::chrono::steady_clock::time_point deadline_;\n> \n> just noticed, can we use the libcamera defined time related types in\n> utils here ?\n\nNo, because utils isn't public.\n\n> >  };\n> >\n> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> > index b3cea3dadb49..34410bab0fb0 100644\n> > --- a/src/libcamera/timer.cpp\n> > +++ b/src/libcamera/timer.cpp\n> > @@ -61,19 +61,18 @@ Timer::~Timer()\n> >   */\n> >\n> >  /**\n> > - * \\brief Start or restart the timer with a timeout of \\a interval\n> > - * \\param[in] interval The timer duration in milliseconds\n> > + * \\brief Start or restart the timer with a timeout of \\a duration\n> > + * \\param[in] duration The timer duration in milliseconds\n> >   *\n> >   * If the timer is already running it will be stopped and restarted.\n> >   */\n> > -void Timer::start(std::chrono::milliseconds interval)\n> > +void Timer::start(std::chrono::milliseconds duration)\n> >  {\n> > -\tinterval_ = interval;\n> > -\tdeadline_ = utils::clock::now() + interval;\n> > +\tdeadline_ = utils::clock::now() + duration;\n> >\n> >  \tLOG(Timer, Debug)\n> > -\t\t<< \"Starting timer \" << this << \" with interval \"\n> > -\t\t<< interval.count() << \": deadline \"\n> > +\t\t<< \"Starting timer \" << this << \" with duration \"\n> > +\t\t<< duration.count() << \": deadline \"\n> >  \t\t<< utils::time_point_to_string(deadline_);\n> >\n> >  \tregisterTimer();\n> > @@ -113,12 +112,6 @@ bool Timer::isRunning() const\n> >  \treturn deadline_ != utils::time_point();\n> >  }\n> >\n> > -/**\n> > - * \\fn Timer::interval()\n> > - * \\brief Retrieve the timer interval\n> > - * \\return The timer interval in milliseconds\n> > - */\n> > -\n> >  /**\n> >   * \\fn Timer::deadline()\n> >   * \\brief Retrieve the timer deadline\n> > diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp\n> > index 994af3ead82a..9e989bef7d53 100644\n> > --- a/src/qcam/qt_event_dispatcher.cpp\n> > +++ b/src/qcam/qt_event_dispatcher.cpp\n> > @@ -5,6 +5,7 @@\n> >   * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher\n> >   */\n> >\n> > +#include <chrono>\n> >  #include <iostream>\n> >\n> >  #include <QAbstractEventDispatcher>\n> > @@ -112,7 +113,11 @@ void QtEventDispatcher::exceptionNotifierActivated(int socket)\n> >\n> >  void QtEventDispatcher::registerTimer(Timer *timer)\n> >  {\n> > -\tint timerId = startTimer(timer->interval());\n> > +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> > +\tstd::chrono::steady_clock::duration duration = timer->deadline() - now;\n> > +\tstd::chrono::milliseconds msec =\n> > +\t\tstd::chrono::duration_cast<std::chrono::milliseconds>(duration);\n> > +\tint timerId = startTimer(msec);\n> >  \ttimers_[timerId] = timer;\n> >  \ttimerIds_[timer] = timerId;\n> >  }","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A976157C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  7 Oct 2019 04:53:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(modemcable151.96-160-184.mc.videotron.ca [184.160.96.151])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D7EFDD;\n\tMon,  7 Oct 2019 04:53:21 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570416802;\n\tbh=JTUKLABVP+i1DgFPMVyuWfVQvTqz8taW4OxMtLtV8D0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DPlmAX0yi/w9THw+miwnIYpAMQveja024FWGmYIBw4p5Qa7QSsKAFX3JuwTWMJUl9\n\tcgIdHjcq92Cgdv3ve78igFVIuNv/b1b/KahfCE+/5ZEjStPsInmrOh8tlMD9Eajb86\n\tQTw3qGIyjRzdLy3F2tFZglTMO4E44zCwp9Smr/E0=","Date":"Mon, 7 Oct 2019 05:53:18 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191007025318.GE4740@pendragon.ideasonboard.com>","References":"<20191006053226.8976-1-laurent.pinchart@ideasonboard.com>\n\t<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>\n\t<20191006182249.ikindvg7d5ehj6mf@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191006182249.ikindvg7d5ehj6mf@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 07 Oct 2019 02:53:22 -0000"}},{"id":2808,"web_url":"https://patchwork.libcamera.org/comment/2808/","msgid":"<20191007025657.GF4740@pendragon.ideasonboard.com>","date":"2019-10-07T02:56:57","subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Oct 06, 2019 at 08:16:31PM +0200, Jacopo Mondi wrote:\n> On Sun, Oct 06, 2019 at 08:32:18AM +0300, Laurent Pinchart wrote:\n> > The libcamera timers are single-shot timers. They are started with a\n> > duration, but fire once only, not based on an interval. Remove the\n> > interval concept by removing the interval() method, and rename other\n> > occurences of interval to duration.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/timer.h        |  4 +---\n> >  src/libcamera/timer.cpp          | 19 ++++++-------------\n> >  src/qcam/qt_event_dispatcher.cpp |  7 ++++++-\n> >  3 files changed, 13 insertions(+), 17 deletions(-)\n> >\n> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> > index 476ae45f1e53..09f426a59993 100644\n> > --- a/include/libcamera/timer.h\n> > +++ b/include/libcamera/timer.h\n> > @@ -24,11 +24,10 @@ public:\n> >  \t~Timer();\n> >\n> >  \tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> > -\tvoid start(std::chrono::milliseconds interval);\n> > +\tvoid start(std::chrono::milliseconds duration);\n> >  \tvoid stop();\n> >  \tbool isRunning() const;\n> >\n> > -\tstd::chrono::milliseconds interval() const { return interval_; }\n> >  \tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n> >\n> >  \tSignal<Timer *> timeout;\n> > @@ -40,7 +39,6 @@ private:\n> >  \tvoid registerTimer();\n> >  \tvoid unregisterTimer();\n> >\n> > -\tstd::chrono::milliseconds interval_;\n> >  \tstd::chrono::steady_clock::time_point deadline_;\n> >  };\n> >\n> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> > index b3cea3dadb49..34410bab0fb0 100644\n> > --- a/src/libcamera/timer.cpp\n> > +++ b/src/libcamera/timer.cpp\n> > @@ -61,19 +61,18 @@ Timer::~Timer()\n> >   */\n> >\n> >  /**\n> > - * \\brief Start or restart the timer with a timeout of \\a interval\n> > - * \\param[in] interval The timer duration in milliseconds\n> > + * \\brief Start or restart the timer with a timeout of \\a duration\n> > + * \\param[in] duration The timer duration in milliseconds\n> >   *\n> >   * If the timer is already running it will be stopped and restarted.\n> >   */\n> > -void Timer::start(std::chrono::milliseconds interval)\n> > +void Timer::start(std::chrono::milliseconds duration)\n> >  {\n> > -\tinterval_ = interval;\n> > -\tdeadline_ = utils::clock::now() + interval;\n> > +\tdeadline_ = utils::clock::now() + duration;\n> >\n> >  \tLOG(Timer, Debug)\n> > -\t\t<< \"Starting timer \" << this << \" with interval \"\n> > -\t\t<< interval.count() << \": deadline \"\n> > +\t\t<< \"Starting timer \" << this << \" with duration \"\n> > +\t\t<< duration.count() << \": deadline \"\n> >  \t\t<< utils::time_point_to_string(deadline_);\n> \n> Shouldn't you print the duration instead of the expected deadline\n> here?\n\nI'm not sure to follow you, I'm printing both.\n\n> >  \tregisterTimer();\n> > @@ -113,12 +112,6 @@ bool Timer::isRunning() const\n> >  \treturn deadline_ != utils::time_point();\n> >  }\n> >\n> > -/**\n> > - * \\fn Timer::interval()\n> > - * \\brief Retrieve the timer interval\n> > - * \\return The timer interval in milliseconds\n> > - */\n> > -\n> >  /**\n> >   * \\fn Timer::deadline()\n> >   * \\brief Retrieve the timer deadline\n> > diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp\n> > index 994af3ead82a..9e989bef7d53 100644\n> > --- a/src/qcam/qt_event_dispatcher.cpp\n> > +++ b/src/qcam/qt_event_dispatcher.cpp\n> > @@ -5,6 +5,7 @@\n> >   * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher\n> >   */\n> >\n> > +#include <chrono>\n> >  #include <iostream>\n> >\n> >  #include <QAbstractEventDispatcher>\n> > @@ -112,7 +113,11 @@ void QtEventDispatcher::exceptionNotifierActivated(int socket)\n> >\n> >  void QtEventDispatcher::registerTimer(Timer *timer)\n> >  {\n> > -\tint timerId = startTimer(timer->interval());\n> > +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> > +\tstd::chrono::steady_clock::duration duration = timer->deadline() - now;\n> \n> Would it make sense an helper to get this from the timer ? (the\n> deadline_ - now part). Nothing urgent though\n\nWe could, but I'd rather expose from the Timer class only the data\nspecific to the timer, and leave the rest to the caller to figure out.\nOtherwise we could end up with lots of helpers with very few users for\neach. If some helpers tend to be very useful we can reconsider, but I\nwouldn't do so yet.\n\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> > +\tstd::chrono::milliseconds msec =\n> > +\t\tstd::chrono::duration_cast<std::chrono::milliseconds>(duration);\n> > +\tint timerId = startTimer(msec);\n> >  \ttimers_[timerId] = timer;\n> >  \ttimerIds_[timer] = timerId;\n> >  }","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 000606157C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  7 Oct 2019 04:57:00 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(modemcable151.96-160-184.mc.videotron.ca [184.160.96.151])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 27701DD;\n\tMon,  7 Oct 2019 04:57:00 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570417020;\n\tbh=rSrum1VqHwrTRUHDkKft9n4i9cjvEoMmUoKtWbJdp4w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ldq5IYIs+fz0V3VSDYMS/B/8EuDTcqNetz8BsaD9UF5ZHEE3kaF9CY9B/qZSynH+0\n\tkPfFkj2tYOqvALFmFc+pUsAyGMgoXmZpJBqlDw1YVHjdXAWeH0z9TCcPLrH9NVdRsO\n\tW+kix+UW7AFCzow9C7LSTXDBis5zbqwDO2Pz1hvk=","Date":"Mon, 7 Oct 2019 05:56:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191007025657.GF4740@pendragon.ideasonboard.com>","References":"<20191006053226.8976-1-laurent.pinchart@ideasonboard.com>\n\t<20191006053226.8976-2-laurent.pinchart@ideasonboard.com>\n\t<20191006181631.nbrh72naet3z3lvc@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20191006181631.nbrh72naet3z3lvc@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/9] libcamera: timer: Remove the\n\tinterval() method","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 07 Oct 2019 02:57:01 -0000"}}]