| Message ID | 20190914005457.16926-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Sat, Sep 14, 2019 at 03:54:57AM +0300, Laurent Pinchart wrote: > Replace the clock_gettime()-based API with durations expressed as > integers with the std::chrono API. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/timer.h | 12 +++++---- > src/cam/capture.cpp | 15 +++++------ > src/cam/capture.h | 3 ++- > src/libcamera/event_dispatcher_poll.cpp | 22 ++++++----------- > src/libcamera/include/log.h | 7 ++++-- > src/libcamera/log.cpp | 20 +++------------ > src/libcamera/timer.cpp | 33 +++++++++++++++---------- > test/event-dispatcher.cpp | 13 +++++----- > test/timer.cpp | 17 +++++++------ > 9 files changed, 67 insertions(+), 75 deletions(-) > > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h > index f47b6a58404f..476ae45f1e53 100644 > --- a/include/libcamera/timer.h > +++ b/include/libcamera/timer.h > @@ -7,6 +7,7 @@ > #ifndef __LIBCAMERA_TIMER_H__ > #define __LIBCAMERA_TIMER_H__ > > +#include <chrono> > #include <cstdint> > > #include <libcamera/object.h> > @@ -22,12 +23,13 @@ public: > Timer(Object *parent = nullptr); > ~Timer(); > > - void start(unsigned int msec); > + void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); } > + void start(std::chrono::milliseconds interval); > void stop(); > bool isRunning() const; > > - unsigned int interval() const { return interval_; } > - uint64_t deadline() const { return deadline_; } > + std::chrono::milliseconds interval() const { return interval_; } > + std::chrono::steady_clock::time_point deadline() const { return deadline_; } I wonder if the type definitions you added to utils (timepoint, clock, duration) aren't better suited for this header so that applications could use them (with proper namespacing indeed). > > Signal<Timer *> timeout; > > @@ -38,8 +40,8 @@ private: > void registerTimer(); > void unregisterTimer(); > > - unsigned int interval_; > - uint64_t deadline_; > + std::chrono::milliseconds interval_; > + std::chrono::steady_clock::time_point deadline_; > }; > > } /* namespace libcamera */ > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > index df9602de4ab8..8a939c622703 100644 > --- a/src/cam/capture.cpp > +++ b/src/cam/capture.cpp > @@ -5,6 +5,7 @@ > * capture.cpp - Cam capture > */ > > +#include <chrono> > #include <climits> > #include <iomanip> > #include <iostream> > @@ -16,7 +17,7 @@ > using namespace libcamera; > > Capture::Capture(Camera *camera, CameraConfiguration *config) > - : camera_(camera), config_(config), writer_(nullptr), last_(0) > + : camera_(camera), config_(config), writer_(nullptr) > { > } > > @@ -135,17 +136,13 @@ int Capture::capture(EventLoop *loop) > > void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > { > - double fps = 0.0; > - uint64_t now; > - > if (request->status() == Request::RequestCancelled) > return; > > - struct timespec time; > - clock_gettime(CLOCK_MONOTONIC, &time); > - now = time.tv_sec * 1000 + time.tv_nsec / 1000000; > - fps = now - last_; > - fps = last_ && fps ? 1000.0 / fps : 0.0; > + std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); > + double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count(); > + fps = last_ != std::chrono::steady_clock::time_point() && fps > + ? 1000.0 / fps : 0.0; > last_ = now; > > std::stringstream info; > diff --git a/src/cam/capture.h b/src/cam/capture.h > index 1d4a25a84a51..ee0dc4211111 100644 > --- a/src/cam/capture.h > +++ b/src/cam/capture.h > @@ -7,6 +7,7 @@ > #ifndef __CAM_CAPTURE_H__ > #define __CAM_CAPTURE_H__ > > +#include <chrono> > #include <memory> > > #include <libcamera/camera.h> > @@ -35,7 +36,7 @@ private: > > std::map<libcamera::Stream *, std::string> streamName_; > BufferWriter *writer_; > - uint64_t last_; > + std::chrono::steady_clock::time_point last_; > }; > > #endif /* __CAM_CAPTURE_H__ */ > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp > index 281f37bdbb16..51ac5adf2f74 100644 > --- a/src/libcamera/event_dispatcher_poll.cpp > +++ b/src/libcamera/event_dispatcher_poll.cpp > @@ -8,6 +8,7 @@ > #include "event_dispatcher_poll.h" > > #include <algorithm> > +#include <chrono> > #include <iomanip> > #include <poll.h> > #include <stdint.h> > @@ -20,6 +21,7 @@ > > #include "log.h" > #include "thread.h" > +#include "utils.h" > > /** > * \file event_dispatcher_poll.h > @@ -206,17 +208,12 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) > struct timespec timeout; > > if (nextTimer) { > - clock_gettime(CLOCK_MONOTONIC, &timeout); > - uint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec; > + utils::time_point now = utils::clock::now(); > > - if (nextTimer->deadline() > now) { > - uint64_t delta = nextTimer->deadline() - now; > - timeout.tv_sec = delta / 1000000000ULL; > - timeout.tv_nsec = delta % 1000000000ULL; > - } else { > - timeout.tv_sec = 0; > - timeout.tv_nsec = 0; > - } > + if (nextTimer->deadline() > now) > + timeout = utils::duration_to_timespec(nextTimer->deadline() - now); > + else > + timeout = { 0, 0 }; > > LOG(Event, Debug) > << "timeout " << timeout.tv_sec << "." > @@ -295,10 +292,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol > > void EventDispatcherPoll::processTimers() > { > - struct timespec ts; > - uint64_t now; > - clock_gettime(CLOCK_MONOTONIC, &ts); > - now = ts.tv_sec * 1000000000ULL + ts.tv_nsec; > + utils::time_point now = utils::clock::now(); > > while (!timers_.empty()) { > Timer *timer = timers_.front(); > diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h > index 9b203f97e304..ee0b4069bd32 100644 > --- a/src/libcamera/include/log.h > +++ b/src/libcamera/include/log.h > @@ -7,8 +7,11 @@ > #ifndef __LIBCAMERA_LOG_H__ > #define __LIBCAMERA_LOG_H__ > > +#include <chrono> > #include <sstream> > > +#include "utils.h" > + > namespace libcamera { > > enum LogSeverity { > @@ -60,7 +63,7 @@ public: > > std::ostream &stream() { return msgStream_; } > > - const struct timespec ×tamp() const { return timestamp_; } > + const utils::time_point ×tamp() const { return timestamp_; } > LogSeverity severity() const { return severity_; } > const LogCategory &category() const { return category_; } > const std::string &fileInfo() const { return fileInfo_; } > @@ -72,7 +75,7 @@ private: > std::ostringstream msgStream_; > const LogCategory &category_; > LogSeverity severity_; > - struct timespec timestamp_; > + utils::time_point timestamp_; > std::string fileInfo_; > }; > > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > index 91f7c3ee5157..51f9f86b4c44 100644 > --- a/src/libcamera/log.cpp > +++ b/src/libcamera/log.cpp > @@ -11,7 +11,6 @@ > #include <cstdlib> > #include <ctime> > #include <fstream> > -#include <iomanip> > #include <iostream> > #include <list> > #include <string.h> > @@ -78,17 +77,6 @@ static int log_severity_to_syslog(LogSeverity severity) > } > } > > -static std::string log_timespec_to_string(const struct timespec ×tamp) > -{ > - std::ostringstream ossTimestamp; > - ossTimestamp.fill('0'); > - ossTimestamp << "[" << timestamp.tv_sec / (60 * 60) << ":" > - << std::setw(2) << (timestamp.tv_sec / 60) % 60 << ":" > - << std::setw(2) << timestamp.tv_sec % 60 << "." > - << std::setw(9) << timestamp.tv_nsec << "]"; > - return ossTimestamp.str(); > -} > - > static const char *log_severity_name(LogSeverity severity) > { > static const char *const names[] = { > @@ -216,10 +204,10 @@ void LogOutput::writeSyslog(const LogMessage &msg) > > void LogOutput::writeStream(const LogMessage &msg) > { > - std::string str = std::string(log_timespec_to_string(msg.timestamp()) + > - log_severity_name(msg.severity()) + " " + > + std::string str = "[" + utils::time_point_to_string(msg.timestamp()) + > + "]" + log_severity_name(msg.severity()) + " " + > msg.category().name() + " " + msg.fileInfo() + " " + > - msg.msg()); > + msg.msg(); > stream_->write(str.c_str(), str.size()); > stream_->flush(); > } > @@ -777,7 +765,7 @@ LogMessage::LogMessage(LogMessage &&other) > void LogMessage::init(const char *fileName, unsigned int line) > { > /* Log the timestamp, severity and file information. */ > - clock_gettime(CLOCK_MONOTONIC, ×tamp_); > + timestamp_ = utils::clock::now(); > > std::ostringstream ossFileInfo; > ossFileInfo << utils::basename(fileName) << ":" << line; > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp > index c61d77e5128f..fe47a348cff9 100644 > --- a/src/libcamera/timer.cpp > +++ b/src/libcamera/timer.cpp > @@ -7,7 +7,7 @@ > > #include <libcamera/timer.h> > > -#include <time.h> > +#include <chrono> > > #include <libcamera/camera_manager.h> > #include <libcamera/event_dispatcher.h> > @@ -15,6 +15,7 @@ > #include "log.h" > #include "message.h" > #include "thread.h" > +#include "utils.h" > > /** > * \file timer.h > @@ -42,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer) > * \param[in] parent The parent Object > */ > Timer::Timer(Object *parent) > - : Object(parent), interval_(0), deadline_(0) > + : Object(parent) > { > } > > @@ -52,22 +53,28 @@ Timer::~Timer() > } > > /** > + * \fn Timer::start(unsigned int msec) > * \brief Start or restart the timer with a timeout of \a msec > * \param[in] msec The timer duration in milliseconds > * > * If the timer is already running it will be stopped and restarted. > */ > -void Timer::start(unsigned int msec) > -{ > - struct timespec tp; > - clock_gettime(CLOCK_MONOTONIC, &tp); > > - interval_ = msec; > - deadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000ULL; > +/** > + * \brief Start or restart the timer with a timeout of \a interval > + * \param[in] interval The timer duration in milliseconds > + * > + * If the timer is already running it will be stopped and restarted. > + */ > +void Timer::start(std::chrono::milliseconds interval) > +{ > + interval_ = interval; > + deadline_ = utils::clock::now() + interval; > > LOG(Timer, Debug) > << "Starting timer " << this << " with interval " > - << msec << ": deadline " << deadline_; > + << interval.count() << ": deadline " > + << utils::time_point_to_string(deadline_); > > registerTimer(); > } > @@ -84,7 +91,7 @@ void Timer::stop() > { > unregisterTimer(); > > - deadline_ = 0; > + deadline_ = utils::time_point(); > } > > void Timer::registerTimer() > @@ -103,7 +110,7 @@ void Timer::unregisterTimer() > */ > bool Timer::isRunning() const > { > - return deadline_ != 0; > + return deadline_ != utils::time_point(); > } > > /** > @@ -115,7 +122,7 @@ bool Timer::isRunning() const > /** > * \fn Timer::deadline() > * \brief Retrieve the timer deadline > - * \return The timer deadline in nanoseconds > + * \return The timer deadline > */ > > /** > @@ -128,7 +135,7 @@ bool Timer::isRunning() const > void Timer::message(Message *msg) > { > if (msg->type() == Message::ThreadMoveMessage) { > - if (deadline_) { > + if (deadline_ != utils::time_point()) { Nit: isRunning() ? Thanks j > unregisterTimer(); > invokeMethod(&Timer::registerTimer); > } > diff --git a/test/event-dispatcher.cpp b/test/event-dispatcher.cpp > index f243ec39bc28..9f9cf17818f2 100644 > --- a/test/event-dispatcher.cpp > +++ b/test/event-dispatcher.cpp > @@ -5,6 +5,7 @@ > * event-dispatcher.cpp - Event dispatcher test > */ > > +#include <chrono> > #include <iostream> > #include <signal.h> > #include <sys/time.h> > @@ -47,8 +48,7 @@ protected: > Timer timer; > > /* Event processing interruption by signal. */ > - struct timespec start; > - clock_gettime(CLOCK_MONOTONIC, &start); > + std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); > > timer.start(1000); > > @@ -59,12 +59,11 @@ protected: > > dispatcher->processEvents(); > > - struct timespec stop; > - clock_gettime(CLOCK_MONOTONIC, &stop); > - int duration = (stop.tv_sec - start.tv_sec) * 1000; > - duration += (stop.tv_nsec - start.tv_nsec) / 1000000; > + std::chrono::steady_clock::time_point stop = std::chrono::steady_clock::now(); > + std::chrono::steady_clock::duration duration = stop - start; > + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); > > - if (abs(duration - 1000) > 50) { > + if (abs(msecs - 1000) > 50) { > cout << "Event processing restart test failed" << endl; > return TestFail; > } > diff --git a/test/timer.cpp b/test/timer.cpp > index c30709d4109a..af922cb371cd 100644 > --- a/test/timer.cpp > +++ b/test/timer.cpp > @@ -5,6 +5,7 @@ > * timer.cpp - Timer test > */ > > +#include <chrono> > #include <iostream> > > #include <libcamera/event_dispatcher.h> > @@ -28,28 +29,28 @@ public: > void start(int msec) > { > interval_ = msec; > - clock_gettime(CLOCK_MONOTONIC, &start_); > - expiration_ = { 0, 0 }; > + start_ = std::chrono::steady_clock::now(); > + expiration_ = std::chrono::steady_clock::time_point(); > > Timer::start(msec); > } > > int jitter() > { > - int duration = (expiration_.tv_sec - start_.tv_sec) * 1000; > - duration += (expiration_.tv_nsec - start_.tv_nsec) / 1000000; > - return abs(duration - interval_); > + std::chrono::steady_clock::duration duration = expiration_ - start_; > + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); > + return abs(msecs - interval_); > } > > private: > void timeoutHandler(Timer *timer) > { > - clock_gettime(CLOCK_MONOTONIC, &expiration_); > + expiration_ = std::chrono::steady_clock::now(); > } > > int interval_; > - struct timespec start_; > - struct timespec expiration_; > + std::chrono::steady_clock::time_point start_; > + std::chrono::steady_clock::time_point expiration_; > }; > > class TimerTest : public Test > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Sat, Sep 14, 2019 at 10:26:48AM +0200, Jacopo Mondi wrote: > On Sat, Sep 14, 2019 at 03:54:57AM +0300, Laurent Pinchart wrote: > > Replace the clock_gettime()-based API with durations expressed as > > integers with the std::chrono API. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/timer.h | 12 +++++---- > > src/cam/capture.cpp | 15 +++++------ > > src/cam/capture.h | 3 ++- > > src/libcamera/event_dispatcher_poll.cpp | 22 ++++++----------- > > src/libcamera/include/log.h | 7 ++++-- > > src/libcamera/log.cpp | 20 +++------------ > > src/libcamera/timer.cpp | 33 +++++++++++++++---------- > > test/event-dispatcher.cpp | 13 +++++----- > > test/timer.cpp | 17 +++++++------ > > 9 files changed, 67 insertions(+), 75 deletions(-) > > > > diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h > > index f47b6a58404f..476ae45f1e53 100644 > > --- a/include/libcamera/timer.h > > +++ b/include/libcamera/timer.h > > @@ -7,6 +7,7 @@ > > #ifndef __LIBCAMERA_TIMER_H__ > > #define __LIBCAMERA_TIMER_H__ > > > > +#include <chrono> > > #include <cstdint> > > > > #include <libcamera/object.h> > > @@ -22,12 +23,13 @@ public: > > Timer(Object *parent = nullptr); > > ~Timer(); > > > > - void start(unsigned int msec); > > + void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); } > > + void start(std::chrono::milliseconds interval); > > void stop(); > > bool isRunning() const; > > > > - unsigned int interval() const { return interval_; } > > - uint64_t deadline() const { return deadline_; } > > + std::chrono::milliseconds interval() const { return interval_; } > > + std::chrono::steady_clock::time_point deadline() const { return deadline_; } > > I wonder if the type definitions you added to utils (timepoint, clock, > duration) aren't better suited for this header so that applications could > use them (with proper namespacing indeed). I've thought about it, but I would prefer avoiding extending the public API. The only reason why the timer (and event notifier) class is exposed is to allow applications to implement custom event dispatchers. > > > > Signal<Timer *> timeout; > > > > @@ -38,8 +40,8 @@ private: > > void registerTimer(); > > void unregisterTimer(); > > > > - unsigned int interval_; > > - uint64_t deadline_; > > + std::chrono::milliseconds interval_; > > + std::chrono::steady_clock::time_point deadline_; > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > index df9602de4ab8..8a939c622703 100644 > > --- a/src/cam/capture.cpp > > +++ b/src/cam/capture.cpp > > @@ -5,6 +5,7 @@ > > * capture.cpp - Cam capture > > */ > > > > +#include <chrono> > > #include <climits> > > #include <iomanip> > > #include <iostream> > > @@ -16,7 +17,7 @@ > > using namespace libcamera; > > > > Capture::Capture(Camera *camera, CameraConfiguration *config) > > - : camera_(camera), config_(config), writer_(nullptr), last_(0) > > + : camera_(camera), config_(config), writer_(nullptr) > > { > > } > > > > @@ -135,17 +136,13 @@ int Capture::capture(EventLoop *loop) > > > > void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) > > { > > - double fps = 0.0; > > - uint64_t now; > > - > > if (request->status() == Request::RequestCancelled) > > return; > > > > - struct timespec time; > > - clock_gettime(CLOCK_MONOTONIC, &time); > > - now = time.tv_sec * 1000 + time.tv_nsec / 1000000; > > - fps = now - last_; > > - fps = last_ && fps ? 1000.0 / fps : 0.0; > > + std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); > > + double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count(); > > + fps = last_ != std::chrono::steady_clock::time_point() && fps > > + ? 1000.0 / fps : 0.0; > > last_ = now; > > > > std::stringstream info; > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > index 1d4a25a84a51..ee0dc4211111 100644 > > --- a/src/cam/capture.h > > +++ b/src/cam/capture.h > > @@ -7,6 +7,7 @@ > > #ifndef __CAM_CAPTURE_H__ > > #define __CAM_CAPTURE_H__ > > > > +#include <chrono> > > #include <memory> > > > > #include <libcamera/camera.h> > > @@ -35,7 +36,7 @@ private: > > > > std::map<libcamera::Stream *, std::string> streamName_; > > BufferWriter *writer_; > > - uint64_t last_; > > + std::chrono::steady_clock::time_point last_; > > }; > > > > #endif /* __CAM_CAPTURE_H__ */ > > diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp > > index 281f37bdbb16..51ac5adf2f74 100644 > > --- a/src/libcamera/event_dispatcher_poll.cpp > > +++ b/src/libcamera/event_dispatcher_poll.cpp > > @@ -8,6 +8,7 @@ > > #include "event_dispatcher_poll.h" > > > > #include <algorithm> > > +#include <chrono> > > #include <iomanip> > > #include <poll.h> > > #include <stdint.h> > > @@ -20,6 +21,7 @@ > > > > #include "log.h" > > #include "thread.h" > > +#include "utils.h" > > > > /** > > * \file event_dispatcher_poll.h > > @@ -206,17 +208,12 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) > > struct timespec timeout; > > > > if (nextTimer) { > > - clock_gettime(CLOCK_MONOTONIC, &timeout); > > - uint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec; > > + utils::time_point now = utils::clock::now(); > > > > - if (nextTimer->deadline() > now) { > > - uint64_t delta = nextTimer->deadline() - now; > > - timeout.tv_sec = delta / 1000000000ULL; > > - timeout.tv_nsec = delta % 1000000000ULL; > > - } else { > > - timeout.tv_sec = 0; > > - timeout.tv_nsec = 0; > > - } > > + if (nextTimer->deadline() > now) > > + timeout = utils::duration_to_timespec(nextTimer->deadline() - now); > > + else > > + timeout = { 0, 0 }; > > > > LOG(Event, Debug) > > << "timeout " << timeout.tv_sec << "." > > @@ -295,10 +292,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol > > > > void EventDispatcherPoll::processTimers() > > { > > - struct timespec ts; > > - uint64_t now; > > - clock_gettime(CLOCK_MONOTONIC, &ts); > > - now = ts.tv_sec * 1000000000ULL + ts.tv_nsec; > > + utils::time_point now = utils::clock::now(); > > > > while (!timers_.empty()) { > > Timer *timer = timers_.front(); > > diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h > > index 9b203f97e304..ee0b4069bd32 100644 > > --- a/src/libcamera/include/log.h > > +++ b/src/libcamera/include/log.h > > @@ -7,8 +7,11 @@ > > #ifndef __LIBCAMERA_LOG_H__ > > #define __LIBCAMERA_LOG_H__ > > > > +#include <chrono> > > #include <sstream> > > > > +#include "utils.h" > > + > > namespace libcamera { > > > > enum LogSeverity { > > @@ -60,7 +63,7 @@ public: > > > > std::ostream &stream() { return msgStream_; } > > > > - const struct timespec ×tamp() const { return timestamp_; } > > + const utils::time_point ×tamp() const { return timestamp_; } > > LogSeverity severity() const { return severity_; } > > const LogCategory &category() const { return category_; } > > const std::string &fileInfo() const { return fileInfo_; } > > @@ -72,7 +75,7 @@ private: > > std::ostringstream msgStream_; > > const LogCategory &category_; > > LogSeverity severity_; > > - struct timespec timestamp_; > > + utils::time_point timestamp_; > > std::string fileInfo_; > > }; > > > > diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp > > index 91f7c3ee5157..51f9f86b4c44 100644 > > --- a/src/libcamera/log.cpp > > +++ b/src/libcamera/log.cpp > > @@ -11,7 +11,6 @@ > > #include <cstdlib> > > #include <ctime> > > #include <fstream> > > -#include <iomanip> > > #include <iostream> > > #include <list> > > #include <string.h> > > @@ -78,17 +77,6 @@ static int log_severity_to_syslog(LogSeverity severity) > > } > > } > > > > -static std::string log_timespec_to_string(const struct timespec ×tamp) > > -{ > > - std::ostringstream ossTimestamp; > > - ossTimestamp.fill('0'); > > - ossTimestamp << "[" << timestamp.tv_sec / (60 * 60) << ":" > > - << std::setw(2) << (timestamp.tv_sec / 60) % 60 << ":" > > - << std::setw(2) << timestamp.tv_sec % 60 << "." > > - << std::setw(9) << timestamp.tv_nsec << "]"; > > - return ossTimestamp.str(); > > -} > > - > > static const char *log_severity_name(LogSeverity severity) > > { > > static const char *const names[] = { > > @@ -216,10 +204,10 @@ void LogOutput::writeSyslog(const LogMessage &msg) > > > > void LogOutput::writeStream(const LogMessage &msg) > > { > > - std::string str = std::string(log_timespec_to_string(msg.timestamp()) + > > - log_severity_name(msg.severity()) + " " + > > + std::string str = "[" + utils::time_point_to_string(msg.timestamp()) + > > + "]" + log_severity_name(msg.severity()) + " " + > > msg.category().name() + " " + msg.fileInfo() + " " + > > - msg.msg()); > > + msg.msg(); > > stream_->write(str.c_str(), str.size()); > > stream_->flush(); > > } > > @@ -777,7 +765,7 @@ LogMessage::LogMessage(LogMessage &&other) > > void LogMessage::init(const char *fileName, unsigned int line) > > { > > /* Log the timestamp, severity and file information. */ > > - clock_gettime(CLOCK_MONOTONIC, ×tamp_); > > + timestamp_ = utils::clock::now(); > > > > std::ostringstream ossFileInfo; > > ossFileInfo << utils::basename(fileName) << ":" << line; > > diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp > > index c61d77e5128f..fe47a348cff9 100644 > > --- a/src/libcamera/timer.cpp > > +++ b/src/libcamera/timer.cpp > > @@ -7,7 +7,7 @@ > > > > #include <libcamera/timer.h> > > > > -#include <time.h> > > +#include <chrono> > > > > #include <libcamera/camera_manager.h> > > #include <libcamera/event_dispatcher.h> > > @@ -15,6 +15,7 @@ > > #include "log.h" > > #include "message.h" > > #include "thread.h" > > +#include "utils.h" > > > > /** > > * \file timer.h > > @@ -42,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer) > > * \param[in] parent The parent Object > > */ > > Timer::Timer(Object *parent) > > - : Object(parent), interval_(0), deadline_(0) > > + : Object(parent) > > { > > } > > > > @@ -52,22 +53,28 @@ Timer::~Timer() > > } > > > > /** > > + * \fn Timer::start(unsigned int msec) > > * \brief Start or restart the timer with a timeout of \a msec > > * \param[in] msec The timer duration in milliseconds > > * > > * If the timer is already running it will be stopped and restarted. > > */ > > -void Timer::start(unsigned int msec) > > -{ > > - struct timespec tp; > > - clock_gettime(CLOCK_MONOTONIC, &tp); > > > > - interval_ = msec; > > - deadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000ULL; > > +/** > > + * \brief Start or restart the timer with a timeout of \a interval > > + * \param[in] interval The timer duration in milliseconds > > + * > > + * If the timer is already running it will be stopped and restarted. > > + */ > > +void Timer::start(std::chrono::milliseconds interval) > > +{ > > + interval_ = interval; > > + deadline_ = utils::clock::now() + interval; > > > > LOG(Timer, Debug) > > << "Starting timer " << this << " with interval " > > - << msec << ": deadline " << deadline_; > > + << interval.count() << ": deadline " > > + << utils::time_point_to_string(deadline_); > > > > registerTimer(); > > } > > @@ -84,7 +91,7 @@ void Timer::stop() > > { > > unregisterTimer(); > > > > - deadline_ = 0; > > + deadline_ = utils::time_point(); > > } > > > > void Timer::registerTimer() > > @@ -103,7 +110,7 @@ void Timer::unregisterTimer() > > */ > > bool Timer::isRunning() const > > { > > - return deadline_ != 0; > > + return deadline_ != utils::time_point(); > > } > > > > /** > > @@ -115,7 +122,7 @@ bool Timer::isRunning() const > > /** > > * \fn Timer::deadline() > > * \brief Retrieve the timer deadline > > - * \return The timer deadline in nanoseconds > > + * \return The timer deadline > > */ > > > > /** > > @@ -128,7 +135,7 @@ bool Timer::isRunning() const > > void Timer::message(Message *msg) > > { > > if (msg->type() == Message::ThreadMoveMessage) { > > - if (deadline_) { > > + if (deadline_ != utils::time_point()) { > > Nit: > isRunning() ? Good point. > > unregisterTimer(); > > invokeMethod(&Timer::registerTimer); > > } > > diff --git a/test/event-dispatcher.cpp b/test/event-dispatcher.cpp > > index f243ec39bc28..9f9cf17818f2 100644 > > --- a/test/event-dispatcher.cpp > > +++ b/test/event-dispatcher.cpp > > @@ -5,6 +5,7 @@ > > * event-dispatcher.cpp - Event dispatcher test > > */ > > > > +#include <chrono> > > #include <iostream> > > #include <signal.h> > > #include <sys/time.h> > > @@ -47,8 +48,7 @@ protected: > > Timer timer; > > > > /* Event processing interruption by signal. */ > > - struct timespec start; > > - clock_gettime(CLOCK_MONOTONIC, &start); > > + std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); > > > > timer.start(1000); > > > > @@ -59,12 +59,11 @@ protected: > > > > dispatcher->processEvents(); > > > > - struct timespec stop; > > - clock_gettime(CLOCK_MONOTONIC, &stop); > > - int duration = (stop.tv_sec - start.tv_sec) * 1000; > > - duration += (stop.tv_nsec - start.tv_nsec) / 1000000; > > + std::chrono::steady_clock::time_point stop = std::chrono::steady_clock::now(); > > + std::chrono::steady_clock::duration duration = stop - start; > > + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); > > > > - if (abs(duration - 1000) > 50) { > > + if (abs(msecs - 1000) > 50) { > > cout << "Event processing restart test failed" << endl; > > return TestFail; > > } > > diff --git a/test/timer.cpp b/test/timer.cpp > > index c30709d4109a..af922cb371cd 100644 > > --- a/test/timer.cpp > > +++ b/test/timer.cpp > > @@ -5,6 +5,7 @@ > > * timer.cpp - Timer test > > */ > > > > +#include <chrono> > > #include <iostream> > > > > #include <libcamera/event_dispatcher.h> > > @@ -28,28 +29,28 @@ public: > > void start(int msec) > > { > > interval_ = msec; > > - clock_gettime(CLOCK_MONOTONIC, &start_); > > - expiration_ = { 0, 0 }; > > + start_ = std::chrono::steady_clock::now(); > > + expiration_ = std::chrono::steady_clock::time_point(); > > > > Timer::start(msec); > > } > > > > int jitter() > > { > > - int duration = (expiration_.tv_sec - start_.tv_sec) * 1000; > > - duration += (expiration_.tv_nsec - start_.tv_nsec) / 1000000; > > - return abs(duration - interval_); > > + std::chrono::steady_clock::duration duration = expiration_ - start_; > > + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); > > + return abs(msecs - interval_); > > } > > > > private: > > void timeoutHandler(Timer *timer) > > { > > - clock_gettime(CLOCK_MONOTONIC, &expiration_); > > + expiration_ = std::chrono::steady_clock::now(); > > } > > > > int interval_; > > - struct timespec start_; > > - struct timespec expiration_; > > + std::chrono::steady_clock::time_point start_; > > + std::chrono::steady_clock::time_point expiration_; > > }; > > > > class TimerTest : public Test
diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h index f47b6a58404f..476ae45f1e53 100644 --- a/include/libcamera/timer.h +++ b/include/libcamera/timer.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_TIMER_H__ #define __LIBCAMERA_TIMER_H__ +#include <chrono> #include <cstdint> #include <libcamera/object.h> @@ -22,12 +23,13 @@ public: Timer(Object *parent = nullptr); ~Timer(); - void start(unsigned int msec); + void start(unsigned int msec) { start(std::chrono::milliseconds(msec)); } + void start(std::chrono::milliseconds interval); void stop(); bool isRunning() const; - unsigned int interval() const { return interval_; } - uint64_t deadline() const { return deadline_; } + std::chrono::milliseconds interval() const { return interval_; } + std::chrono::steady_clock::time_point deadline() const { return deadline_; } Signal<Timer *> timeout; @@ -38,8 +40,8 @@ private: void registerTimer(); void unregisterTimer(); - unsigned int interval_; - uint64_t deadline_; + std::chrono::milliseconds interval_; + std::chrono::steady_clock::time_point deadline_; }; } /* namespace libcamera */ diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index df9602de4ab8..8a939c622703 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -5,6 +5,7 @@ * capture.cpp - Cam capture */ +#include <chrono> #include <climits> #include <iomanip> #include <iostream> @@ -16,7 +17,7 @@ using namespace libcamera; Capture::Capture(Camera *camera, CameraConfiguration *config) - : camera_(camera), config_(config), writer_(nullptr), last_(0) + : camera_(camera), config_(config), writer_(nullptr) { } @@ -135,17 +136,13 @@ int Capture::capture(EventLoop *loop) void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers) { - double fps = 0.0; - uint64_t now; - if (request->status() == Request::RequestCancelled) return; - struct timespec time; - clock_gettime(CLOCK_MONOTONIC, &time); - now = time.tv_sec * 1000 + time.tv_nsec / 1000000; - fps = now - last_; - fps = last_ && fps ? 1000.0 / fps : 0.0; + std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); + double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count(); + fps = last_ != std::chrono::steady_clock::time_point() && fps + ? 1000.0 / fps : 0.0; last_ = now; std::stringstream info; diff --git a/src/cam/capture.h b/src/cam/capture.h index 1d4a25a84a51..ee0dc4211111 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -7,6 +7,7 @@ #ifndef __CAM_CAPTURE_H__ #define __CAM_CAPTURE_H__ +#include <chrono> #include <memory> #include <libcamera/camera.h> @@ -35,7 +36,7 @@ private: std::map<libcamera::Stream *, std::string> streamName_; BufferWriter *writer_; - uint64_t last_; + std::chrono::steady_clock::time_point last_; }; #endif /* __CAM_CAPTURE_H__ */ diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp index 281f37bdbb16..51ac5adf2f74 100644 --- a/src/libcamera/event_dispatcher_poll.cpp +++ b/src/libcamera/event_dispatcher_poll.cpp @@ -8,6 +8,7 @@ #include "event_dispatcher_poll.h" #include <algorithm> +#include <chrono> #include <iomanip> #include <poll.h> #include <stdint.h> @@ -20,6 +21,7 @@ #include "log.h" #include "thread.h" +#include "utils.h" /** * \file event_dispatcher_poll.h @@ -206,17 +208,12 @@ int EventDispatcherPoll::poll(std::vector<struct pollfd> *pollfds) struct timespec timeout; if (nextTimer) { - clock_gettime(CLOCK_MONOTONIC, &timeout); - uint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec; + utils::time_point now = utils::clock::now(); - if (nextTimer->deadline() > now) { - uint64_t delta = nextTimer->deadline() - now; - timeout.tv_sec = delta / 1000000000ULL; - timeout.tv_nsec = delta % 1000000000ULL; - } else { - timeout.tv_sec = 0; - timeout.tv_nsec = 0; - } + if (nextTimer->deadline() > now) + timeout = utils::duration_to_timespec(nextTimer->deadline() - now); + else + timeout = { 0, 0 }; LOG(Event, Debug) << "timeout " << timeout.tv_sec << "." @@ -295,10 +292,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector<struct pollfd> &pol void EventDispatcherPoll::processTimers() { - struct timespec ts; - uint64_t now; - clock_gettime(CLOCK_MONOTONIC, &ts); - now = ts.tv_sec * 1000000000ULL + ts.tv_nsec; + utils::time_point now = utils::clock::now(); while (!timers_.empty()) { Timer *timer = timers_.front(); diff --git a/src/libcamera/include/log.h b/src/libcamera/include/log.h index 9b203f97e304..ee0b4069bd32 100644 --- a/src/libcamera/include/log.h +++ b/src/libcamera/include/log.h @@ -7,8 +7,11 @@ #ifndef __LIBCAMERA_LOG_H__ #define __LIBCAMERA_LOG_H__ +#include <chrono> #include <sstream> +#include "utils.h" + namespace libcamera { enum LogSeverity { @@ -60,7 +63,7 @@ public: std::ostream &stream() { return msgStream_; } - const struct timespec ×tamp() const { return timestamp_; } + const utils::time_point ×tamp() const { return timestamp_; } LogSeverity severity() const { return severity_; } const LogCategory &category() const { return category_; } const std::string &fileInfo() const { return fileInfo_; } @@ -72,7 +75,7 @@ private: std::ostringstream msgStream_; const LogCategory &category_; LogSeverity severity_; - struct timespec timestamp_; + utils::time_point timestamp_; std::string fileInfo_; }; diff --git a/src/libcamera/log.cpp b/src/libcamera/log.cpp index 91f7c3ee5157..51f9f86b4c44 100644 --- a/src/libcamera/log.cpp +++ b/src/libcamera/log.cpp @@ -11,7 +11,6 @@ #include <cstdlib> #include <ctime> #include <fstream> -#include <iomanip> #include <iostream> #include <list> #include <string.h> @@ -78,17 +77,6 @@ static int log_severity_to_syslog(LogSeverity severity) } } -static std::string log_timespec_to_string(const struct timespec ×tamp) -{ - std::ostringstream ossTimestamp; - ossTimestamp.fill('0'); - ossTimestamp << "[" << timestamp.tv_sec / (60 * 60) << ":" - << std::setw(2) << (timestamp.tv_sec / 60) % 60 << ":" - << std::setw(2) << timestamp.tv_sec % 60 << "." - << std::setw(9) << timestamp.tv_nsec << "]"; - return ossTimestamp.str(); -} - static const char *log_severity_name(LogSeverity severity) { static const char *const names[] = { @@ -216,10 +204,10 @@ void LogOutput::writeSyslog(const LogMessage &msg) void LogOutput::writeStream(const LogMessage &msg) { - std::string str = std::string(log_timespec_to_string(msg.timestamp()) + - log_severity_name(msg.severity()) + " " + + std::string str = "[" + utils::time_point_to_string(msg.timestamp()) + + "]" + log_severity_name(msg.severity()) + " " + msg.category().name() + " " + msg.fileInfo() + " " + - msg.msg()); + msg.msg(); stream_->write(str.c_str(), str.size()); stream_->flush(); } @@ -777,7 +765,7 @@ LogMessage::LogMessage(LogMessage &&other) void LogMessage::init(const char *fileName, unsigned int line) { /* Log the timestamp, severity and file information. */ - clock_gettime(CLOCK_MONOTONIC, ×tamp_); + timestamp_ = utils::clock::now(); std::ostringstream ossFileInfo; ossFileInfo << utils::basename(fileName) << ":" << line; diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp index c61d77e5128f..fe47a348cff9 100644 --- a/src/libcamera/timer.cpp +++ b/src/libcamera/timer.cpp @@ -7,7 +7,7 @@ #include <libcamera/timer.h> -#include <time.h> +#include <chrono> #include <libcamera/camera_manager.h> #include <libcamera/event_dispatcher.h> @@ -15,6 +15,7 @@ #include "log.h" #include "message.h" #include "thread.h" +#include "utils.h" /** * \file timer.h @@ -42,7 +43,7 @@ LOG_DEFINE_CATEGORY(Timer) * \param[in] parent The parent Object */ Timer::Timer(Object *parent) - : Object(parent), interval_(0), deadline_(0) + : Object(parent) { } @@ -52,22 +53,28 @@ Timer::~Timer() } /** + * \fn Timer::start(unsigned int msec) * \brief Start or restart the timer with a timeout of \a msec * \param[in] msec The timer duration in milliseconds * * If the timer is already running it will be stopped and restarted. */ -void Timer::start(unsigned int msec) -{ - struct timespec tp; - clock_gettime(CLOCK_MONOTONIC, &tp); - interval_ = msec; - deadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000ULL; +/** + * \brief Start or restart the timer with a timeout of \a interval + * \param[in] interval The timer duration in milliseconds + * + * If the timer is already running it will be stopped and restarted. + */ +void Timer::start(std::chrono::milliseconds interval) +{ + interval_ = interval; + deadline_ = utils::clock::now() + interval; LOG(Timer, Debug) << "Starting timer " << this << " with interval " - << msec << ": deadline " << deadline_; + << interval.count() << ": deadline " + << utils::time_point_to_string(deadline_); registerTimer(); } @@ -84,7 +91,7 @@ void Timer::stop() { unregisterTimer(); - deadline_ = 0; + deadline_ = utils::time_point(); } void Timer::registerTimer() @@ -103,7 +110,7 @@ void Timer::unregisterTimer() */ bool Timer::isRunning() const { - return deadline_ != 0; + return deadline_ != utils::time_point(); } /** @@ -115,7 +122,7 @@ bool Timer::isRunning() const /** * \fn Timer::deadline() * \brief Retrieve the timer deadline - * \return The timer deadline in nanoseconds + * \return The timer deadline */ /** @@ -128,7 +135,7 @@ bool Timer::isRunning() const void Timer::message(Message *msg) { if (msg->type() == Message::ThreadMoveMessage) { - if (deadline_) { + if (deadline_ != utils::time_point()) { unregisterTimer(); invokeMethod(&Timer::registerTimer); } diff --git a/test/event-dispatcher.cpp b/test/event-dispatcher.cpp index f243ec39bc28..9f9cf17818f2 100644 --- a/test/event-dispatcher.cpp +++ b/test/event-dispatcher.cpp @@ -5,6 +5,7 @@ * event-dispatcher.cpp - Event dispatcher test */ +#include <chrono> #include <iostream> #include <signal.h> #include <sys/time.h> @@ -47,8 +48,7 @@ protected: Timer timer; /* Event processing interruption by signal. */ - struct timespec start; - clock_gettime(CLOCK_MONOTONIC, &start); + std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); timer.start(1000); @@ -59,12 +59,11 @@ protected: dispatcher->processEvents(); - struct timespec stop; - clock_gettime(CLOCK_MONOTONIC, &stop); - int duration = (stop.tv_sec - start.tv_sec) * 1000; - duration += (stop.tv_nsec - start.tv_nsec) / 1000000; + std::chrono::steady_clock::time_point stop = std::chrono::steady_clock::now(); + std::chrono::steady_clock::duration duration = stop - start; + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); - if (abs(duration - 1000) > 50) { + if (abs(msecs - 1000) > 50) { cout << "Event processing restart test failed" << endl; return TestFail; } diff --git a/test/timer.cpp b/test/timer.cpp index c30709d4109a..af922cb371cd 100644 --- a/test/timer.cpp +++ b/test/timer.cpp @@ -5,6 +5,7 @@ * timer.cpp - Timer test */ +#include <chrono> #include <iostream> #include <libcamera/event_dispatcher.h> @@ -28,28 +29,28 @@ public: void start(int msec) { interval_ = msec; - clock_gettime(CLOCK_MONOTONIC, &start_); - expiration_ = { 0, 0 }; + start_ = std::chrono::steady_clock::now(); + expiration_ = std::chrono::steady_clock::time_point(); Timer::start(msec); } int jitter() { - int duration = (expiration_.tv_sec - start_.tv_sec) * 1000; - duration += (expiration_.tv_nsec - start_.tv_nsec) / 1000000; - return abs(duration - interval_); + std::chrono::steady_clock::duration duration = expiration_ - start_; + int msecs = std::chrono::duration_cast<std::chrono::milliseconds>(duration).count(); + return abs(msecs - interval_); } private: void timeoutHandler(Timer *timer) { - clock_gettime(CLOCK_MONOTONIC, &expiration_); + expiration_ = std::chrono::steady_clock::now(); } int interval_; - struct timespec start_; - struct timespec expiration_; + std::chrono::steady_clock::time_point start_; + std::chrono::steady_clock::time_point expiration_; }; class TimerTest : public Test
Replace the clock_gettime()-based API with durations expressed as integers with the std::chrono API. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/timer.h | 12 +++++---- src/cam/capture.cpp | 15 +++++------ src/cam/capture.h | 3 ++- src/libcamera/event_dispatcher_poll.cpp | 22 ++++++----------- src/libcamera/include/log.h | 7 ++++-- src/libcamera/log.cpp | 20 +++------------ src/libcamera/timer.cpp | 33 +++++++++++++++---------- test/event-dispatcher.cpp | 13 +++++----- test/timer.cpp | 17 +++++++------ 9 files changed, 67 insertions(+), 75 deletions(-)