[libcamera-devel] [PATCH v7 1/2] cam: event_loop: Add timer events to event loop
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Apr 19 17:22:34 CEST 2022
Quoting Eric Curtin via libcamera-devel (2022-04-19 15:48:54)
> On Fri, 15 Apr 2022 at 17:25, Laurent Pinchart
> <laurent.pinchart at ideasonboard.com> wrote:
> >
> > Hi Eric,
> >
> > Thank you for the patch.
> >
> > On Fri, Apr 08, 2022 at 05:00:14PM +0100, Eric Curtin via libcamera-devel wrote:
> > > Extend the EventLoop class to support periodic timer events. This can be
> > > used to run tasks periodically, such as handling the event loop of SDL.
> > >
> > > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > > ---
> > > src/cam/event_loop.cpp | 37 +++++++++++++++++++++++++++++++++----
> > > src/cam/event_loop.h | 3 +++
> > > 2 files changed, 36 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> > > index e25784c0..1e1a4269 100644
> > > --- a/src/cam/event_loop.cpp
> > > +++ b/src/cam/event_loop.cpp
> > > @@ -60,6 +60,17 @@ void EventLoop::callLater(const std::function<void()> &func)
> > > event_base_once(base_, -1, EV_TIMEOUT, dispatchCallback, this, nullptr);
> > > }
> > >
> > > +int EventLoop::eventNew(const std::unique_ptr<Event> &event, const int fd, const short events)
> > > +{
> > > + event->event_ = event_new(base_, fd, events, &EventLoop::Event::dispatch, event.get());
> > > + if (!event->event_) {
> > > + std::cerr << "Failed to create event for fd " << fd << std::endl;
> >
> > This message will be a bit confusing for timer events, as it will print
> >
> > Failed to create event for fd -1
> >
> > As there's really no other code sharing in this function, how about
> > dropping it, keeping addEvent() unchanged, and called event_new()
> > directlry in addTimerEvent() with a more appropriate message (such as
> > "Failed to create timer event") ?
> >
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > void EventLoop::addEvent(int fd, EventType type,
> > > const std::function<void()> &callback)
> > > {
> > > @@ -68,10 +79,7 @@ void EventLoop::addEvent(int fd, EventType type,
> > > | (type & Write ? EV_WRITE : 0)
> > > | EV_PERSIST;
> > >
> > > - event->event_ = event_new(base_, fd, events, &EventLoop::Event::dispatch,
> > > - event.get());
> > > - if (!event->event_) {
> > > - std::cerr << "Failed to create event for fd " << fd << std::endl;
> > > + if (eventNew(event, fd, events)) {
> > > return;
> > > }
> > >
> > > @@ -84,6 +92,27 @@ void EventLoop::addEvent(int fd, EventType type,
> > > events_.push_back(std::move(event));
> > > }
> > >
> > > +void EventLoop::addTimerEvent(const suseconds_t period,
> > > + const std::function<void()> &callback)
> > > +{
> > > + std::unique_ptr<Event> event = std::make_unique<Event>(callback);
> > > + if (eventNew(event, -1, EV_PERSIST)) {
> > > + return;
> > > + }
> > > +
> > > + struct timeval tv;
> > > + tv.tv_sec = 0;
> > > + tv.tv_usec = period;
> > > +
> > > + int ret = event_add(event->event_, &tv);
> > > + if (ret < 0) {
> > > + std::cerr << "Failed to add timer event" << std::endl;
> > > + return;
> > > + }
> > > +
> > > + events_.push_back(std::move(event));
> > > +}
> > > +
> > > void EventLoop::dispatchCallback([[maybe_unused]] evutil_socket_t fd,
> > > [[maybe_unused]] short flags, void *param)
> > > {
> > > diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> > > index a4613eb2..3d3561e5 100644
> > > --- a/src/cam/event_loop.h
> > > +++ b/src/cam/event_loop.h
> > > @@ -36,6 +36,8 @@ public:
> > >
> > > void addEvent(int fd, EventType type,
> > > const std::function<void()> &handler);
> > > + void addTimerEvent(const suseconds_t period,
> >
> > This could be called addEvent() too, C++ will pick the right function.
> > The name addTimerEvent() makes the code more explicit though, which is
> > nice, but I'd then rename addEvent to addFdEvent() (in a patch before
> > this one).
>
> Funnily enough I started changing to addFdEvent, but then reverted as
> it was touching many files, and didn't want the patch to spread to so
> many files. I'll go back to doing that.
As long as the rename for addFdEvent is in it's own patch, before this
one, that's fine I think.
--
Kieran
>
> >
> > > + const std::function<void()> &handler);
> > >
> > > private:
> > > struct Event {
> > > @@ -60,4 +62,5 @@ private:
> > > static void dispatchCallback(evutil_socket_t fd, short flags,
> > > void *param);
> > > void dispatchCall();
> > > + int eventNew(const std::unique_ptr<Event> &event, const int fd, const short events);
> > > };
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
>
More information about the libcamera-devel
mailing list