[{"id":2654,"web_url":"https://patchwork.libcamera.org/comment/2654/","msgid":"<20190914082648.zzvcntdgqpmmr32n@uno.localdomain>","date":"2019-09-14T08:26:48","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: Switch to the\n\tstd::chrono API","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Sep 14, 2019 at 03:54:57AM +0300, Laurent Pinchart wrote:\n> Replace the clock_gettime()-based API with durations expressed as\n> integers with the std::chrono API.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/timer.h               | 12 +++++----\n>  src/cam/capture.cpp                     | 15 +++++------\n>  src/cam/capture.h                       |  3 ++-\n>  src/libcamera/event_dispatcher_poll.cpp | 22 ++++++-----------\n>  src/libcamera/include/log.h             |  7 ++++--\n>  src/libcamera/log.cpp                   | 20 +++------------\n>  src/libcamera/timer.cpp                 | 33 +++++++++++++++----------\n>  test/event-dispatcher.cpp               | 13 +++++-----\n>  test/timer.cpp                          | 17 +++++++------\n>  9 files changed, 67 insertions(+), 75 deletions(-)\n>\n> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> index f47b6a58404f..476ae45f1e53 100644\n> --- a/include/libcamera/timer.h\n> +++ b/include/libcamera/timer.h\n> @@ -7,6 +7,7 @@\n>  #ifndef __LIBCAMERA_TIMER_H__\n>  #define __LIBCAMERA_TIMER_H__\n>\n> +#include <chrono>\n>  #include <cstdint>\n>\n>  #include <libcamera/object.h>\n> @@ -22,12 +23,13 @@ public:\n>  \tTimer(Object *parent = nullptr);\n>  \t~Timer();\n>\n> -\tvoid start(unsigned int msec);\n> +\tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> +\tvoid start(std::chrono::milliseconds interval);\n>  \tvoid stop();\n>  \tbool isRunning() const;\n>\n> -\tunsigned int interval() const { return interval_; }\n> -\tuint64_t deadline() const { return deadline_; }\n> +\tstd::chrono::milliseconds interval() const { return interval_; }\n> +\tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n\nI wonder if the type definitions you added to utils (timepoint, clock,\nduration) aren't better suited for this header so that applications could\nuse them (with proper namespacing indeed).\n\n>\n>  \tSignal<Timer *> timeout;\n>\n> @@ -38,8 +40,8 @@ private:\n>  \tvoid registerTimer();\n>  \tvoid unregisterTimer();\n>\n> -\tunsigned int interval_;\n> -\tuint64_t deadline_;\n> +\tstd::chrono::milliseconds interval_;\n> +\tstd::chrono::steady_clock::time_point deadline_;\n>  };\n>\n>  } /* namespace libcamera */\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index df9602de4ab8..8a939c622703 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -5,6 +5,7 @@\n>   * capture.cpp - Cam capture\n>   */\n>\n> +#include <chrono>\n>  #include <climits>\n>  #include <iomanip>\n>  #include <iostream>\n> @@ -16,7 +17,7 @@\n>  using namespace libcamera;\n>\n>  Capture::Capture(Camera *camera, CameraConfiguration *config)\n> -\t: camera_(camera), config_(config), writer_(nullptr), last_(0)\n> +\t: camera_(camera), config_(config), writer_(nullptr)\n>  {\n>  }\n>\n> @@ -135,17 +136,13 @@ int Capture::capture(EventLoop *loop)\n>\n>  void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n>  {\n> -\tdouble fps = 0.0;\n> -\tuint64_t now;\n> -\n>  \tif (request->status() == Request::RequestCancelled)\n>  \t\treturn;\n>\n> -\tstruct timespec time;\n> -\tclock_gettime(CLOCK_MONOTONIC, &time);\n> -\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> -\tfps = now - last_;\n> -\tfps = last_ && fps ? 1000.0 / fps : 0.0;\n> +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> +\tdouble fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count();\n> +\tfps = last_ != std::chrono::steady_clock::time_point() && fps\n> +\t    ? 1000.0 / fps : 0.0;\n>  \tlast_ = now;\n>\n>  \tstd::stringstream info;\n> diff --git a/src/cam/capture.h b/src/cam/capture.h\n> index 1d4a25a84a51..ee0dc4211111 100644\n> --- a/src/cam/capture.h\n> +++ b/src/cam/capture.h\n> @@ -7,6 +7,7 @@\n>  #ifndef __CAM_CAPTURE_H__\n>  #define __CAM_CAPTURE_H__\n>\n> +#include <chrono>\n>  #include <memory>\n>\n>  #include <libcamera/camera.h>\n> @@ -35,7 +36,7 @@ private:\n>\n>  \tstd::map<libcamera::Stream *, std::string> streamName_;\n>  \tBufferWriter *writer_;\n> -\tuint64_t last_;\n> +\tstd::chrono::steady_clock::time_point last_;\n>  };\n>\n>  #endif /* __CAM_CAPTURE_H__ */\n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> index 281f37bdbb16..51ac5adf2f74 100644\n> --- a/src/libcamera/event_dispatcher_poll.cpp\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -8,6 +8,7 @@\n>  #include \"event_dispatcher_poll.h\"\n>\n>  #include <algorithm>\n> +#include <chrono>\n>  #include <iomanip>\n>  #include <poll.h>\n>  #include <stdint.h>\n> @@ -20,6 +21,7 @@\n>\n>  #include \"log.h\"\n>  #include \"thread.h\"\n> +#include \"utils.h\"\n>\n>  /**\n>   * \\file event_dispatcher_poll.h\n> @@ -206,17 +208,12 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds)\n>  \tstruct timespec timeout;\n>\n>  \tif (nextTimer) {\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> -\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> +\t\tutils::time_point now = utils::clock::now();\n>\n> -\t\tif (nextTimer->deadline() > now) {\n> -\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> -\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> -\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> -\t\t} else {\n> -\t\t\ttimeout.tv_sec = 0;\n> -\t\t\ttimeout.tv_nsec = 0;\n> -\t\t}\n> +\t\tif (nextTimer->deadline() > now)\n> +\t\t\ttimeout = utils::duration_to_timespec(nextTimer->deadline() - now);\n> +\t\telse\n> +\t\t\ttimeout = { 0, 0 };\n>\n>  \t\tLOG(Event, Debug)\n>  \t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> @@ -295,10 +292,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n>\n>  void EventDispatcherPoll::processTimers()\n>  {\n> -\tstruct timespec ts;\n> -\tuint64_t now;\n> -\tclock_gettime(CLOCK_MONOTONIC, &ts);\n> -\tnow = ts.tv_sec * 1000000000ULL + ts.tv_nsec;\n> +\tutils::time_point now = utils::clock::now();\n>\n>  \twhile (!timers_.empty()) {\n>  \t\tTimer *timer = timers_.front();\n> diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h\n> index 9b203f97e304..ee0b4069bd32 100644\n> --- a/src/libcamera/include/log.h\n> +++ b/src/libcamera/include/log.h\n> @@ -7,8 +7,11 @@\n>  #ifndef __LIBCAMERA_LOG_H__\n>  #define __LIBCAMERA_LOG_H__\n>\n> +#include <chrono>\n>  #include <sstream>\n>\n> +#include \"utils.h\"\n> +\n>  namespace libcamera {\n>\n>  enum LogSeverity {\n> @@ -60,7 +63,7 @@ public:\n>\n>  \tstd::ostream &stream() { return msgStream_; }\n>\n> -\tconst struct timespec &timestamp() const { return timestamp_; }\n> +\tconst utils::time_point &timestamp() const { return timestamp_; }\n>  \tLogSeverity severity() const { return severity_; }\n>  \tconst LogCategory &category() const { return category_; }\n>  \tconst std::string &fileInfo() const { return fileInfo_; }\n> @@ -72,7 +75,7 @@ private:\n>  \tstd::ostringstream msgStream_;\n>  \tconst LogCategory &category_;\n>  \tLogSeverity severity_;\n> -\tstruct timespec timestamp_;\n> +\tutils::time_point timestamp_;\n>  \tstd::string fileInfo_;\n>  };\n>\n> diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> index 91f7c3ee5157..51f9f86b4c44 100644\n> --- a/src/libcamera/log.cpp\n> +++ b/src/libcamera/log.cpp\n> @@ -11,7 +11,6 @@\n>  #include <cstdlib>\n>  #include <ctime>\n>  #include <fstream>\n> -#include <iomanip>\n>  #include <iostream>\n>  #include <list>\n>  #include <string.h>\n> @@ -78,17 +77,6 @@ static int log_severity_to_syslog(LogSeverity severity)\n>  \t}\n>  }\n>\n> -static std::string log_timespec_to_string(const struct timespec &timestamp)\n> -{\n> -\tstd::ostringstream ossTimestamp;\n> -\tossTimestamp.fill('0');\n> -\tossTimestamp << \"[\" << timestamp.tv_sec / (60 * 60) << \":\"\n> -\t\t     << std::setw(2) << (timestamp.tv_sec / 60) % 60 << \":\"\n> -\t\t     << std::setw(2) << timestamp.tv_sec % 60 << \".\"\n> -\t\t     << std::setw(9) << timestamp.tv_nsec << \"]\";\n> -\treturn ossTimestamp.str();\n> -}\n> -\n>  static const char *log_severity_name(LogSeverity severity)\n>  {\n>  \tstatic const char *const names[] = {\n> @@ -216,10 +204,10 @@ void LogOutput::writeSyslog(const LogMessage &msg)\n>\n>  void LogOutput::writeStream(const LogMessage &msg)\n>  {\n> -\tstd::string str = std::string(log_timespec_to_string(msg.timestamp()) +\n> -\t      \t\t  log_severity_name(msg.severity()) + \" \" +\n> +\tstd::string str = \"[\" + utils::time_point_to_string(msg.timestamp()) +\n> +\t\t\t  \"]\" + log_severity_name(msg.severity()) + \" \" +\n>  \t\t\t  msg.category().name() + \" \" + msg.fileInfo() + \" \" +\n> -\t\t\t  msg.msg());\n> +\t\t\t  msg.msg();\n>  \tstream_->write(str.c_str(), str.size());\n>  \tstream_->flush();\n>  }\n> @@ -777,7 +765,7 @@ LogMessage::LogMessage(LogMessage &&other)\n>  void LogMessage::init(const char *fileName, unsigned int line)\n>  {\n>  \t/* Log the timestamp, severity and file information. */\n> -\tclock_gettime(CLOCK_MONOTONIC, &timestamp_);\n> +\ttimestamp_ = utils::clock::now();\n>\n>  \tstd::ostringstream ossFileInfo;\n>  \tossFileInfo << utils::basename(fileName) << \":\" << line;\n> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> index c61d77e5128f..fe47a348cff9 100644\n> --- a/src/libcamera/timer.cpp\n> +++ b/src/libcamera/timer.cpp\n> @@ -7,7 +7,7 @@\n>\n>  #include <libcamera/timer.h>\n>\n> -#include <time.h>\n> +#include <chrono>\n>\n>  #include <libcamera/camera_manager.h>\n>  #include <libcamera/event_dispatcher.h>\n> @@ -15,6 +15,7 @@\n>  #include \"log.h\"\n>  #include \"message.h\"\n>  #include \"thread.h\"\n> +#include \"utils.h\"\n>\n>  /**\n>   * \\file timer.h\n> @@ -42,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer)\n>   * \\param[in] parent The parent Object\n>   */\n>  Timer::Timer(Object *parent)\n> -\t: Object(parent), interval_(0), deadline_(0)\n> +\t: Object(parent)\n>  {\n>  }\n>\n> @@ -52,22 +53,28 @@ Timer::~Timer()\n>  }\n>\n>  /**\n> + * \\fn Timer::start(unsigned int msec)\n>   * \\brief Start or restart the timer with a timeout of \\a msec\n>   * \\param[in] msec The timer duration in milliseconds\n>   *\n>   * If the timer is already running it will be stopped and restarted.\n>   */\n> -void Timer::start(unsigned int msec)\n> -{\n> -\tstruct timespec tp;\n> -\tclock_gettime(CLOCK_MONOTONIC, &tp);\n>\n> -\tinterval_ = msec;\n> -\tdeadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000ULL;\n> +/**\n> + * \\brief Start or restart the timer with a timeout of \\a interval\n> + * \\param[in] interval 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> +{\n> +\tinterval_ = interval;\n> +\tdeadline_ = utils::clock::now() + interval;\n>\n>  \tLOG(Timer, Debug)\n>  \t\t<< \"Starting timer \" << this << \" with interval \"\n> -\t\t<< msec << \": deadline \" << deadline_;\n> +\t\t<< interval.count() << \": deadline \"\n> +\t\t<< utils::time_point_to_string(deadline_);\n>\n>  \tregisterTimer();\n>  }\n> @@ -84,7 +91,7 @@ void Timer::stop()\n>  {\n>  \tunregisterTimer();\n>\n> -\tdeadline_ = 0;\n> +\tdeadline_ = utils::time_point();\n>  }\n>\n>  void Timer::registerTimer()\n> @@ -103,7 +110,7 @@ void Timer::unregisterTimer()\n>   */\n>  bool Timer::isRunning() const\n>  {\n> -\treturn deadline_ != 0;\n> +\treturn deadline_ != utils::time_point();\n>  }\n>\n>  /**\n> @@ -115,7 +122,7 @@ bool Timer::isRunning() const\n>  /**\n>   * \\fn Timer::deadline()\n>   * \\brief Retrieve the timer deadline\n> - * \\return The timer deadline in nanoseconds\n> + * \\return The timer deadline\n>   */\n>\n>  /**\n> @@ -128,7 +135,7 @@ bool Timer::isRunning() const\n>  void Timer::message(Message *msg)\n>  {\n>  \tif (msg->type() == Message::ThreadMoveMessage) {\n> -\t\tif (deadline_) {\n> +\t\tif (deadline_ != utils::time_point()) {\n\nNit:\nisRunning() ?\n\nThanks\n   j\n\n>  \t\t\tunregisterTimer();\n>  \t\t\tinvokeMethod(&Timer::registerTimer);\n>  \t\t}\n> diff --git a/test/event-dispatcher.cpp b/test/event-dispatcher.cpp\n> index f243ec39bc28..9f9cf17818f2 100644\n> --- a/test/event-dispatcher.cpp\n> +++ b/test/event-dispatcher.cpp\n> @@ -5,6 +5,7 @@\n>   * event-dispatcher.cpp - Event dispatcher test\n>   */\n>\n> +#include <chrono>\n>  #include <iostream>\n>  #include <signal.h>\n>  #include <sys/time.h>\n> @@ -47,8 +48,7 @@ protected:\n>  \t\tTimer timer;\n>\n>  \t\t/* Event processing interruption by signal. */\n> -\t\tstruct timespec start;\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &start);\n> +\t\tstd::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();\n>\n>  \t\ttimer.start(1000);\n>\n> @@ -59,12 +59,11 @@ protected:\n>\n>  \t\tdispatcher->processEvents();\n>\n> -\t\tstruct timespec stop;\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &stop);\n> -\t\tint duration = (stop.tv_sec - start.tv_sec) * 1000;\n> -\t\tduration += (stop.tv_nsec - start.tv_nsec) / 1000000;\n> +\t\tstd::chrono::steady_clock::time_point stop = std::chrono::steady_clock::now();\n> +\t\tstd::chrono::steady_clock::duration duration = stop - start;\n> +\t\tint msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();\n>\n> -\t\tif (abs(duration - 1000) > 50) {\n> +\t\tif (abs(msecs - 1000) > 50) {\n>  \t\t\tcout << \"Event processing restart test failed\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> diff --git a/test/timer.cpp b/test/timer.cpp\n> index c30709d4109a..af922cb371cd 100644\n> --- a/test/timer.cpp\n> +++ b/test/timer.cpp\n> @@ -5,6 +5,7 @@\n>   * timer.cpp - Timer test\n>   */\n>\n> +#include <chrono>\n>  #include <iostream>\n>\n>  #include <libcamera/event_dispatcher.h>\n> @@ -28,28 +29,28 @@ public:\n>  \tvoid start(int msec)\n>  \t{\n>  \t\tinterval_ = msec;\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &start_);\n> -\t\texpiration_ = { 0, 0 };\n> +\t\tstart_ = std::chrono::steady_clock::now();\n> +\t\texpiration_ = std::chrono::steady_clock::time_point();\n>\n>  \t\tTimer::start(msec);\n>  \t}\n>\n>  \tint jitter()\n>  \t{\n> -\t\tint duration = (expiration_.tv_sec - start_.tv_sec) * 1000;\n> -\t\tduration += (expiration_.tv_nsec - start_.tv_nsec) / 1000000;\n> -\t\treturn abs(duration - interval_);\n> +\t\tstd::chrono::steady_clock::duration duration = expiration_ - start_;\n> +\t\tint msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();\n> +\t\treturn abs(msecs - interval_);\n>  \t}\n>\n>  private:\n>  \tvoid timeoutHandler(Timer *timer)\n>  \t{\n> -\t\tclock_gettime(CLOCK_MONOTONIC, &expiration_);\n> +\t\texpiration_ = std::chrono::steady_clock::now();\n>  \t}\n>\n>  \tint interval_;\n> -\tstruct timespec start_;\n> -\tstruct timespec expiration_;\n> +\tstd::chrono::steady_clock::time_point start_;\n> +\tstd::chrono::steady_clock::time_point expiration_;\n>  };\n>\n>  class TimerTest : public Test\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 relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FC3D60BB6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 14 Sep 2019 10:25:17 +0200 (CEST)","from uno.localdomain (bl10-204-24.dsl.telepac.pt [85.243.204.24])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 196C560006;\n\tSat, 14 Sep 2019 08:25:15 +0000 (UTC)"],"X-Originating-IP":"85.243.204.24","Date":"Sat, 14 Sep 2019 10:26:48 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190914082648.zzvcntdgqpmmr32n@uno.localdomain>","References":"<20190914005457.16926-1-laurent.pinchart@ideasonboard.com>\n\t<20190914005457.16926-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"uavhlgevttfdfwbk\"","Content-Disposition":"inline","In-Reply-To":"<20190914005457.16926-2-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: Switch to the\n\tstd::chrono API","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Sat, 14 Sep 2019 08:25:17 -0000"}},{"id":2655,"web_url":"https://patchwork.libcamera.org/comment/2655/","msgid":"<20190914110417.GA4734@pendragon.ideasonboard.com>","date":"2019-09-14T11:04:17","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: Switch to the\n\tstd::chrono API","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sat, Sep 14, 2019 at 10:26:48AM +0200, Jacopo Mondi wrote:\n> On Sat, Sep 14, 2019 at 03:54:57AM +0300, Laurent Pinchart wrote:\n> > Replace the clock_gettime()-based API with durations expressed as\n> > integers with the std::chrono API.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/timer.h               | 12 +++++----\n> >  src/cam/capture.cpp                     | 15 +++++------\n> >  src/cam/capture.h                       |  3 ++-\n> >  src/libcamera/event_dispatcher_poll.cpp | 22 ++++++-----------\n> >  src/libcamera/include/log.h             |  7 ++++--\n> >  src/libcamera/log.cpp                   | 20 +++------------\n> >  src/libcamera/timer.cpp                 | 33 +++++++++++++++----------\n> >  test/event-dispatcher.cpp               | 13 +++++-----\n> >  test/timer.cpp                          | 17 +++++++------\n> >  9 files changed, 67 insertions(+), 75 deletions(-)\n> >\n> > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h\n> > index f47b6a58404f..476ae45f1e53 100644\n> > --- a/include/libcamera/timer.h\n> > +++ b/include/libcamera/timer.h\n> > @@ -7,6 +7,7 @@\n> >  #ifndef __LIBCAMERA_TIMER_H__\n> >  #define __LIBCAMERA_TIMER_H__\n> >\n> > +#include <chrono>\n> >  #include <cstdint>\n> >\n> >  #include <libcamera/object.h>\n> > @@ -22,12 +23,13 @@ public:\n> >  \tTimer(Object *parent = nullptr);\n> >  \t~Timer();\n> >\n> > -\tvoid start(unsigned int msec);\n> > +\tvoid start(unsigned int msec) { start(std::chrono::milliseconds(msec)); }\n> > +\tvoid start(std::chrono::milliseconds interval);\n> >  \tvoid stop();\n> >  \tbool isRunning() const;\n> >\n> > -\tunsigned int interval() const { return interval_; }\n> > -\tuint64_t deadline() const { return deadline_; }\n> > +\tstd::chrono::milliseconds interval() const { return interval_; }\n> > +\tstd::chrono::steady_clock::time_point deadline() const { return deadline_; }\n> \n> I wonder if the type definitions you added to utils (timepoint, clock,\n> duration) aren't better suited for this header so that applications could\n> use them (with proper namespacing indeed).\n\nI've thought about it, but I would prefer avoiding extending the public\nAPI. The only reason why the timer (and event notifier) class is exposed\nis to allow applications to implement custom event dispatchers.\n\n> >\n> >  \tSignal<Timer *> timeout;\n> >\n> > @@ -38,8 +40,8 @@ private:\n> >  \tvoid registerTimer();\n> >  \tvoid unregisterTimer();\n> >\n> > -\tunsigned int interval_;\n> > -\tuint64_t deadline_;\n> > +\tstd::chrono::milliseconds interval_;\n> > +\tstd::chrono::steady_clock::time_point deadline_;\n> >  };\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> > index df9602de4ab8..8a939c622703 100644\n> > --- a/src/cam/capture.cpp\n> > +++ b/src/cam/capture.cpp\n> > @@ -5,6 +5,7 @@\n> >   * capture.cpp - Cam capture\n> >   */\n> >\n> > +#include <chrono>\n> >  #include <climits>\n> >  #include <iomanip>\n> >  #include <iostream>\n> > @@ -16,7 +17,7 @@\n> >  using namespace libcamera;\n> >\n> >  Capture::Capture(Camera *camera, CameraConfiguration *config)\n> > -\t: camera_(camera), config_(config), writer_(nullptr), last_(0)\n> > +\t: camera_(camera), config_(config), writer_(nullptr)\n> >  {\n> >  }\n> >\n> > @@ -135,17 +136,13 @@ int Capture::capture(EventLoop *loop)\n> >\n> >  void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> >  {\n> > -\tdouble fps = 0.0;\n> > -\tuint64_t now;\n> > -\n> >  \tif (request->status() == Request::RequestCancelled)\n> >  \t\treturn;\n> >\n> > -\tstruct timespec time;\n> > -\tclock_gettime(CLOCK_MONOTONIC, &time);\n> > -\tnow = time.tv_sec * 1000 + time.tv_nsec / 1000000;\n> > -\tfps = now - last_;\n> > -\tfps = last_ && fps ? 1000.0 / fps : 0.0;\n> > +\tstd::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();\n> > +\tdouble fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count();\n> > +\tfps = last_ != std::chrono::steady_clock::time_point() && fps\n> > +\t    ? 1000.0 / fps : 0.0;\n> >  \tlast_ = now;\n> >\n> >  \tstd::stringstream info;\n> > diff --git a/src/cam/capture.h b/src/cam/capture.h\n> > index 1d4a25a84a51..ee0dc4211111 100644\n> > --- a/src/cam/capture.h\n> > +++ b/src/cam/capture.h\n> > @@ -7,6 +7,7 @@\n> >  #ifndef __CAM_CAPTURE_H__\n> >  #define __CAM_CAPTURE_H__\n> >\n> > +#include <chrono>\n> >  #include <memory>\n> >\n> >  #include <libcamera/camera.h>\n> > @@ -35,7 +36,7 @@ private:\n> >\n> >  \tstd::map<libcamera::Stream *, std::string> streamName_;\n> >  \tBufferWriter *writer_;\n> > -\tuint64_t last_;\n> > +\tstd::chrono::steady_clock::time_point last_;\n> >  };\n> >\n> >  #endif /* __CAM_CAPTURE_H__ */\n> > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> > index 281f37bdbb16..51ac5adf2f74 100644\n> > --- a/src/libcamera/event_dispatcher_poll.cpp\n> > +++ b/src/libcamera/event_dispatcher_poll.cpp\n> > @@ -8,6 +8,7 @@\n> >  #include \"event_dispatcher_poll.h\"\n> >\n> >  #include <algorithm>\n> > +#include <chrono>\n> >  #include <iomanip>\n> >  #include <poll.h>\n> >  #include <stdint.h>\n> > @@ -20,6 +21,7 @@\n> >\n> >  #include \"log.h\"\n> >  #include \"thread.h\"\n> > +#include \"utils.h\"\n> >\n> >  /**\n> >   * \\file event_dispatcher_poll.h\n> > @@ -206,17 +208,12 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds)\n> >  \tstruct timespec timeout;\n> >\n> >  \tif (nextTimer) {\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &timeout);\n> > -\t\tuint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;\n> > +\t\tutils::time_point now = utils::clock::now();\n> >\n> > -\t\tif (nextTimer->deadline() > now) {\n> > -\t\t\tuint64_t delta = nextTimer->deadline() - now;\n> > -\t\t\ttimeout.tv_sec = delta / 1000000000ULL;\n> > -\t\t\ttimeout.tv_nsec = delta % 1000000000ULL;\n> > -\t\t} else {\n> > -\t\t\ttimeout.tv_sec = 0;\n> > -\t\t\ttimeout.tv_nsec = 0;\n> > -\t\t}\n> > +\t\tif (nextTimer->deadline() > now)\n> > +\t\t\ttimeout = utils::duration_to_timespec(nextTimer->deadline() - now);\n> > +\t\telse\n> > +\t\t\ttimeout = { 0, 0 };\n> >\n> >  \t\tLOG(Event, Debug)\n> >  \t\t\t<< \"timeout \" << timeout.tv_sec << \".\"\n> > @@ -295,10 +292,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol\n> >\n> >  void EventDispatcherPoll::processTimers()\n> >  {\n> > -\tstruct timespec ts;\n> > -\tuint64_t now;\n> > -\tclock_gettime(CLOCK_MONOTONIC, &ts);\n> > -\tnow = ts.tv_sec * 1000000000ULL + ts.tv_nsec;\n> > +\tutils::time_point now = utils::clock::now();\n> >\n> >  \twhile (!timers_.empty()) {\n> >  \t\tTimer *timer = timers_.front();\n> > diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h\n> > index 9b203f97e304..ee0b4069bd32 100644\n> > --- a/src/libcamera/include/log.h\n> > +++ b/src/libcamera/include/log.h\n> > @@ -7,8 +7,11 @@\n> >  #ifndef __LIBCAMERA_LOG_H__\n> >  #define __LIBCAMERA_LOG_H__\n> >\n> > +#include <chrono>\n> >  #include <sstream>\n> >\n> > +#include \"utils.h\"\n> > +\n> >  namespace libcamera {\n> >\n> >  enum LogSeverity {\n> > @@ -60,7 +63,7 @@ public:\n> >\n> >  \tstd::ostream &stream() { return msgStream_; }\n> >\n> > -\tconst struct timespec &timestamp() const { return timestamp_; }\n> > +\tconst utils::time_point &timestamp() const { return timestamp_; }\n> >  \tLogSeverity severity() const { return severity_; }\n> >  \tconst LogCategory &category() const { return category_; }\n> >  \tconst std::string &fileInfo() const { return fileInfo_; }\n> > @@ -72,7 +75,7 @@ private:\n> >  \tstd::ostringstream msgStream_;\n> >  \tconst LogCategory &category_;\n> >  \tLogSeverity severity_;\n> > -\tstruct timespec timestamp_;\n> > +\tutils::time_point timestamp_;\n> >  \tstd::string fileInfo_;\n> >  };\n> >\n> > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp\n> > index 91f7c3ee5157..51f9f86b4c44 100644\n> > --- a/src/libcamera/log.cpp\n> > +++ b/src/libcamera/log.cpp\n> > @@ -11,7 +11,6 @@\n> >  #include <cstdlib>\n> >  #include <ctime>\n> >  #include <fstream>\n> > -#include <iomanip>\n> >  #include <iostream>\n> >  #include <list>\n> >  #include <string.h>\n> > @@ -78,17 +77,6 @@ static int log_severity_to_syslog(LogSeverity severity)\n> >  \t}\n> >  }\n> >\n> > -static std::string log_timespec_to_string(const struct timespec &timestamp)\n> > -{\n> > -\tstd::ostringstream ossTimestamp;\n> > -\tossTimestamp.fill('0');\n> > -\tossTimestamp << \"[\" << timestamp.tv_sec / (60 * 60) << \":\"\n> > -\t\t     << std::setw(2) << (timestamp.tv_sec / 60) % 60 << \":\"\n> > -\t\t     << std::setw(2) << timestamp.tv_sec % 60 << \".\"\n> > -\t\t     << std::setw(9) << timestamp.tv_nsec << \"]\";\n> > -\treturn ossTimestamp.str();\n> > -}\n> > -\n> >  static const char *log_severity_name(LogSeverity severity)\n> >  {\n> >  \tstatic const char *const names[] = {\n> > @@ -216,10 +204,10 @@ void LogOutput::writeSyslog(const LogMessage &msg)\n> >\n> >  void LogOutput::writeStream(const LogMessage &msg)\n> >  {\n> > -\tstd::string str = std::string(log_timespec_to_string(msg.timestamp()) +\n> > -\t      \t\t  log_severity_name(msg.severity()) + \" \" +\n> > +\tstd::string str = \"[\" + utils::time_point_to_string(msg.timestamp()) +\n> > +\t\t\t  \"]\" + log_severity_name(msg.severity()) + \" \" +\n> >  \t\t\t  msg.category().name() + \" \" + msg.fileInfo() + \" \" +\n> > -\t\t\t  msg.msg());\n> > +\t\t\t  msg.msg();\n> >  \tstream_->write(str.c_str(), str.size());\n> >  \tstream_->flush();\n> >  }\n> > @@ -777,7 +765,7 @@ LogMessage::LogMessage(LogMessage &&other)\n> >  void LogMessage::init(const char *fileName, unsigned int line)\n> >  {\n> >  \t/* Log the timestamp, severity and file information. */\n> > -\tclock_gettime(CLOCK_MONOTONIC, &timestamp_);\n> > +\ttimestamp_ = utils::clock::now();\n> >\n> >  \tstd::ostringstream ossFileInfo;\n> >  \tossFileInfo << utils::basename(fileName) << \":\" << line;\n> > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp\n> > index c61d77e5128f..fe47a348cff9 100644\n> > --- a/src/libcamera/timer.cpp\n> > +++ b/src/libcamera/timer.cpp\n> > @@ -7,7 +7,7 @@\n> >\n> >  #include <libcamera/timer.h>\n> >\n> > -#include <time.h>\n> > +#include <chrono>\n> >\n> >  #include <libcamera/camera_manager.h>\n> >  #include <libcamera/event_dispatcher.h>\n> > @@ -15,6 +15,7 @@\n> >  #include \"log.h\"\n> >  #include \"message.h\"\n> >  #include \"thread.h\"\n> > +#include \"utils.h\"\n> >\n> >  /**\n> >   * \\file timer.h\n> > @@ -42,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer)\n> >   * \\param[in] parent The parent Object\n> >   */\n> >  Timer::Timer(Object *parent)\n> > -\t: Object(parent), interval_(0), deadline_(0)\n> > +\t: Object(parent)\n> >  {\n> >  }\n> >\n> > @@ -52,22 +53,28 @@ Timer::~Timer()\n> >  }\n> >\n> >  /**\n> > + * \\fn Timer::start(unsigned int msec)\n> >   * \\brief Start or restart the timer with a timeout of \\a msec\n> >   * \\param[in] msec The timer duration in milliseconds\n> >   *\n> >   * If the timer is already running it will be stopped and restarted.\n> >   */\n> > -void Timer::start(unsigned int msec)\n> > -{\n> > -\tstruct timespec tp;\n> > -\tclock_gettime(CLOCK_MONOTONIC, &tp);\n> >\n> > -\tinterval_ = msec;\n> > -\tdeadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000ULL;\n> > +/**\n> > + * \\brief Start or restart the timer with a timeout of \\a interval\n> > + * \\param[in] interval 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> > +{\n> > +\tinterval_ = interval;\n> > +\tdeadline_ = utils::clock::now() + interval;\n> >\n> >  \tLOG(Timer, Debug)\n> >  \t\t<< \"Starting timer \" << this << \" with interval \"\n> > -\t\t<< msec << \": deadline \" << deadline_;\n> > +\t\t<< interval.count() << \": deadline \"\n> > +\t\t<< utils::time_point_to_string(deadline_);\n> >\n> >  \tregisterTimer();\n> >  }\n> > @@ -84,7 +91,7 @@ void Timer::stop()\n> >  {\n> >  \tunregisterTimer();\n> >\n> > -\tdeadline_ = 0;\n> > +\tdeadline_ = utils::time_point();\n> >  }\n> >\n> >  void Timer::registerTimer()\n> > @@ -103,7 +110,7 @@ void Timer::unregisterTimer()\n> >   */\n> >  bool Timer::isRunning() const\n> >  {\n> > -\treturn deadline_ != 0;\n> > +\treturn deadline_ != utils::time_point();\n> >  }\n> >\n> >  /**\n> > @@ -115,7 +122,7 @@ bool Timer::isRunning() const\n> >  /**\n> >   * \\fn Timer::deadline()\n> >   * \\brief Retrieve the timer deadline\n> > - * \\return The timer deadline in nanoseconds\n> > + * \\return The timer deadline\n> >   */\n> >\n> >  /**\n> > @@ -128,7 +135,7 @@ bool Timer::isRunning() const\n> >  void Timer::message(Message *msg)\n> >  {\n> >  \tif (msg->type() == Message::ThreadMoveMessage) {\n> > -\t\tif (deadline_) {\n> > +\t\tif (deadline_ != utils::time_point()) {\n> \n> Nit:\n> isRunning() ?\n\nGood point.\n\n> >  \t\t\tunregisterTimer();\n> >  \t\t\tinvokeMethod(&Timer::registerTimer);\n> >  \t\t}\n> > diff --git a/test/event-dispatcher.cpp b/test/event-dispatcher.cpp\n> > index f243ec39bc28..9f9cf17818f2 100644\n> > --- a/test/event-dispatcher.cpp\n> > +++ b/test/event-dispatcher.cpp\n> > @@ -5,6 +5,7 @@\n> >   * event-dispatcher.cpp - Event dispatcher test\n> >   */\n> >\n> > +#include <chrono>\n> >  #include <iostream>\n> >  #include <signal.h>\n> >  #include <sys/time.h>\n> > @@ -47,8 +48,7 @@ protected:\n> >  \t\tTimer timer;\n> >\n> >  \t\t/* Event processing interruption by signal. */\n> > -\t\tstruct timespec start;\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &start);\n> > +\t\tstd::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();\n> >\n> >  \t\ttimer.start(1000);\n> >\n> > @@ -59,12 +59,11 @@ protected:\n> >\n> >  \t\tdispatcher->processEvents();\n> >\n> > -\t\tstruct timespec stop;\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &stop);\n> > -\t\tint duration = (stop.tv_sec - start.tv_sec) * 1000;\n> > -\t\tduration += (stop.tv_nsec - start.tv_nsec) / 1000000;\n> > +\t\tstd::chrono::steady_clock::time_point stop = std::chrono::steady_clock::now();\n> > +\t\tstd::chrono::steady_clock::duration duration = stop - start;\n> > +\t\tint msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();\n> >\n> > -\t\tif (abs(duration - 1000) > 50) {\n> > +\t\tif (abs(msecs - 1000) > 50) {\n> >  \t\t\tcout << \"Event processing restart test failed\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > diff --git a/test/timer.cpp b/test/timer.cpp\n> > index c30709d4109a..af922cb371cd 100644\n> > --- a/test/timer.cpp\n> > +++ b/test/timer.cpp\n> > @@ -5,6 +5,7 @@\n> >   * timer.cpp - Timer test\n> >   */\n> >\n> > +#include <chrono>\n> >  #include <iostream>\n> >\n> >  #include <libcamera/event_dispatcher.h>\n> > @@ -28,28 +29,28 @@ public:\n> >  \tvoid start(int msec)\n> >  \t{\n> >  \t\tinterval_ = msec;\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &start_);\n> > -\t\texpiration_ = { 0, 0 };\n> > +\t\tstart_ = std::chrono::steady_clock::now();\n> > +\t\texpiration_ = std::chrono::steady_clock::time_point();\n> >\n> >  \t\tTimer::start(msec);\n> >  \t}\n> >\n> >  \tint jitter()\n> >  \t{\n> > -\t\tint duration = (expiration_.tv_sec - start_.tv_sec) * 1000;\n> > -\t\tduration += (expiration_.tv_nsec - start_.tv_nsec) / 1000000;\n> > -\t\treturn abs(duration - interval_);\n> > +\t\tstd::chrono::steady_clock::duration duration = expiration_ - start_;\n> > +\t\tint msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();\n> > +\t\treturn abs(msecs - interval_);\n> >  \t}\n> >\n> >  private:\n> >  \tvoid timeoutHandler(Timer *timer)\n> >  \t{\n> > -\t\tclock_gettime(CLOCK_MONOTONIC, &expiration_);\n> > +\t\texpiration_ = std::chrono::steady_clock::now();\n> >  \t}\n> >\n> >  \tint interval_;\n> > -\tstruct timespec start_;\n> > -\tstruct timespec expiration_;\n> > +\tstd::chrono::steady_clock::time_point start_;\n> > +\tstd::chrono::steady_clock::time_point expiration_;\n> >  };\n> >\n> >  class TimerTest : public Test","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 1F10260BB0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 14 Sep 2019 13:04:26 +0200 (CEST)","from pendragon.ideasonboard.com (bl10-204-24.dsl.telepac.pt\n\t[85.243.204.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F0FDF23F;\n\tSat, 14 Sep 2019 13:04:24 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1568459065;\n\tbh=0UAnn2XuTO5+hEUG2oxVZpYf9mHEGloUHp9VQFoy1Vs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rvgJRF7C7ree1sfhjkig8jsg68k5Tn2auiHURFORZU77CHGoUebPRKL0OezC/aYGL\n\tyNkFa4p3Xb8aoxNyRHeEZ9uMi36D48NWqXhI8NHJzfg/Y0N3RvkW0FTslhOpI2Bvfg\n\tGlDZNk/RYvSuV4/QA6BIDFg4TlA0yp5Nv18JJLK4=","Date":"Sat, 14 Sep 2019 14:04:17 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190914110417.GA4734@pendragon.ideasonboard.com>","References":"<20190914005457.16926-1-laurent.pinchart@ideasonboard.com>\n\t<20190914005457.16926-2-laurent.pinchart@ideasonboard.com>\n\t<20190914082648.zzvcntdgqpmmr32n@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190914082648.zzvcntdgqpmmr32n@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: Switch to the\n\tstd::chrono API","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Sat, 14 Sep 2019 11:04:26 -0000"}}]