[libcamera-devel] [PATCH 07/11] libcamera: Add a poll-based event dispatcher
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Jan 6 17:25:30 CET 2019
Hi Laurent,
Thanks for your work.
On 2019-01-06 04:33:24 +0200, Laurent Pinchart wrote:
> Provide a poll-based event dispatcher implementation as convenience for
> applications that don't need a custom event loop. The poll-based
> dispatcher is automatically instantiated if the application doesn't
> provide its own dispatcher.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/libcamera.h | 1 +
> src/libcamera/camera_manager.cpp | 19 +-
> src/libcamera/event_dispatcher_poll.cpp | 232 ++++++++++++++++++
> src/libcamera/include/event_dispatcher_poll.h | 50 ++++
> src/libcamera/meson.build | 2 +
> 5 files changed, 299 insertions(+), 5 deletions(-)
> create mode 100644 src/libcamera/event_dispatcher_poll.cpp
> create mode 100644 src/libcamera/include/event_dispatcher_poll.h
>
> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h
> index 785babefa1c8..2dcaeda49812 100644
> --- a/include/libcamera/libcamera.h
> +++ b/include/libcamera/libcamera.h
> @@ -10,5 +10,6 @@
> #include <libcamera/camera.h>
> #include <libcamera/camera_manager.h>
> #include <libcamera/event_dispatcher.h>
> +#include <libcamera/event_dispatcher_poll.h>
>
> #endif /* __LIBCAMERA_LIBCAMERA_H__ */
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 0dbf31f47039..4475ab1f9db2 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -9,6 +9,7 @@
> #include <libcamera/event_dispatcher.h>
>
> #include "device_enumerator.h"
> +#include "event_dispatcher_poll.h"
> #include "log.h"
> #include "pipeline_handler.h"
>
> @@ -188,9 +189,10 @@ CameraManager *CameraManager::instance()
> * \param dispatcher Pointer to the event dispatcher
> *
> * libcamera requires an event dispatcher to integrate event notification and
> - * timers with the application event loop. Applications shall call this function
> - * once and only once before the camera manager is started with start() to set
> - * the event dispatcher.
> + * timers with the application event loop. Applications that want to provide
> + * their own event dispatcher shall call this function once and only once before
> + * the camera manager is started with start(). If no event dispatcher is
> + * provided, a default poll-based implementation will be used.
> *
> * The CameraManager takes ownership of the event dispatcher and will delete it
> * when the application terminates.
> @@ -208,11 +210,18 @@ void CameraManager::setEventDispatcher(EventDispatcher *dispatcher)
> /**
> * \fn CameraManager::eventDispatcher()
> * \brief Retrieve the event dispatcher
> - * \return Pointer to the event dispatcher, or nullptr if no event dispatcher
> - * has been set
> + *
> + * This function retrieves the event dispatcher set with setEventDispatcher().
> + * If no dispatcher has been set, a default poll-based implementation is created
> + * and returned.
> + *
> + * \return Pointer to the event dispatcher
> */
> EventDispatcher *CameraManager::eventDispatcher()
> {
> + if (!dispatcher_)
> + dispatcher_ = new EventDispatcherPoll();
> +
> return dispatcher_;
> }
>
> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp
> new file mode 100644
> index 000000000000..2c64c1446772
> --- /dev/null
> +++ b/src/libcamera/event_dispatcher_poll.cpp
> @@ -0,0 +1,232 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_dispatcher_poll.cpp - Poll-based event dispatcher
> + */
> +
> +#include <algorithm>
> +#include <iomanip>
> +#include <poll.h>
> +#include <string.h>
> +
> +#include <libcamera/event_notifier.h>
> +#include <libcamera/timer.h>
> +
> +#include "event_dispatcher_poll.h"
> +#include "log.h"
> +
> +/**
> + * \file event_dispatcher_poll.h
> + */
> +
> +namespace libcamera {
> +
> +static const char *notifierType(EventNotifier::Type type)
> +{
> + if (type == EventNotifier::Read)
> + return "read";
> + if (type == EventNotifier::Write)
> + return "write";
> + if (type == EventNotifier::Exception)
> + return "exception";
> +
> + return "";
> +}
> +
> +/**
> + * \class EventDispatcherPoll
> + * \brief A poll-based event dispatcher
> + */
> +
> +EventDispatcherPoll::EventDispatcherPoll()
> +{
> +}
> +
> +EventDispatcherPoll::~EventDispatcherPoll()
> +{
> +}
> +
> +void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier)
> +{
> + EventNotifierSetPoll &set = notifiers_[notifier->fd()];
> + EventNotifier::Type type = notifier->type();
> +
> + if (set.notifiers[type] && set.notifiers[type] != notifier)
> + LOG(Warning) << "Duplicate " << notifierType(type)
> + << " notifiers for fd " << notifier->fd();
I'm not sure I what I think about this :-) Is it wise to allow the
notifier to be replaced with just a warning? My gut feeling tells me we
should warn and ignore the new notifier instead.
At least I think we should rephrase the warning message to reflect what
action is taken. Either log that the old notifier is replaced or that
the new one is ignored.
> +
> + set.notifiers[type] = notifier;
> +}
> +
> +void EventDispatcherPoll::unregisterEventNotifier(EventNotifier *notifier)
> +{
> + auto iter = notifiers_.find(notifier->fd());
> + if (iter == notifiers_.end())
> + return;
> +
> + EventNotifierSetPoll &set = iter->second;
> + EventNotifier::Type type = notifier->type();
> +
> + if (!set.notifiers[type])
> + return;
> +
> + if (set.notifiers[type] != notifier) {
> + LOG(Warning) << "Duplicate " << notifierType(type)
> + << " notifiers for fd " << notifier->fd();
I feel the log message should be rephrased, how about
"Can't unregister a not register " << notifierType(type) << "
notifier for fd " << notifier->fd()
With these small issues addressed
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> + return;
> + }
> +
> + set.notifiers[type] = nullptr;
> + if (!set.notifiers[0] && !set.notifiers[1] && !set.notifiers[2])
> + notifiers_.erase(iter);
> +}
> +
> +void EventDispatcherPoll::registerTimer(Timer *timer)
> +{
> + for (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {
> + if ((*iter)->deadline() > timer->deadline()) {
> + timers_.insert(iter, timer);
> + return;
> + }
> + }
> +
> + timers_.push_back(timer);
> +}
> +
> +void EventDispatcherPoll::unregisterTimer(Timer *timer)
> +{
> + for (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {
> + if (*iter == timer) {
> + timers_.erase(iter);
> + return;
> + }
> +
> + /*
> + * As the timers list is ordered, we can stop as soon as we go
> + * past the deadline.
> + */
> + if ((*iter)->deadline() > timer->deadline())
> + break;
> + }
> +}
> +
> +void EventDispatcherPoll::processEvents()
> +{
> + int ret;
> +
> + /* Create the pollfd array. */
> + std::vector<struct pollfd> pollfds;
> + pollfds.reserve(notifiers_.size());
> +
> + for (auto notifier : notifiers_)
> + pollfds.push_back({ notifier.first, notifier.second.events(), 0 });
> +
> + /* Compute the timeout. */
> + Timer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;
> + struct timespec timeout;
> +
> + if (nextTimer) {
> + clock_gettime(CLOCK_MONOTONIC, &timeout);
> + uint64_t now = timeout.tv_sec * 1000000000ULL + timeout.tv_nsec;
> +
> + 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;
> + }
> +
> + LOG(Debug) << "timeout " << timeout.tv_sec << "."
> + << std::setfill('0') << std::setw(9)
> + << timeout.tv_nsec;
> + }
> +
> + /* Wait for events and process notifers and timers. */
> + ret = ppoll(pollfds.data(), pollfds.size(),
> + nextTimer ? &timeout : nullptr, nullptr);
> + if (ret < 0) {
> + ret = -errno;
> + LOG(Warning) << "poll() failed with " << strerror(-ret);
> + } else if (ret > 0) {
> + processNotifiers(pollfds);
> + }
> +
> + processTimers();
> +}
> +
> +short EventDispatcherPoll::EventNotifierSetPoll::events() const
> +{
> + short events = 0;
> +
> + if (notifiers[EventNotifier::Read])
> + events |= POLLIN;
> + if (notifiers[EventNotifier::Write])
> + events |= POLLOUT;
> + if (notifiers[EventNotifier::Exception])
> + events |= POLLPRI;
> +
> + return events;
> +}
> +
> +void EventDispatcherPoll::processNotifiers(std::vector<struct pollfd> &pollfds)
> +{
> + static const struct {
> + EventNotifier::Type type;
> + short events;
> + } events[] = {
> + { EventNotifier::Read, POLLIN },
> + { EventNotifier::Write, POLLOUT },
> + { EventNotifier::Exception, POLLPRI },
> + };
> +
> + for (const struct pollfd &pfd : pollfds) {
> + auto iter = notifiers_.find(pfd.fd);
> + ASSERT(iter == notifiers_.end());
> +
> + EventNotifierSetPoll &set = iter->second;
> +
> + for (const auto &event : events) {
> + EventNotifier *notifier = set.notifiers[event.type];
> +
> + if (!notifier)
> + continue;
> +
> + /*
> + * If the file descriptor is invalid, disable the
> + * notifier immediately.
> + */
> + if (pfd.revents & POLLNVAL) {
> + LOG(Warning) << "Disabling " << notifierType(event.type)
> + << " due to invalid file descriptor "
> + << pfd.fd;
> + unregisterEventNotifier(notifier);
> + }
> +
> + if (pfd.revents & event.events)
> + notifier->activated.emit(notifier);
> + }
> + }
> +}
> +
> +void EventDispatcherPoll::processTimers()
> +{
> + struct timespec ts;
> + uint64_t now;
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + now = ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> +
> + while (!timers_.empty()) {
> + Timer *timer = timers_.front();
> + if (timer->deadline() > now)
> + break;
> +
> + timers_.pop_front();
> + timer->stop();
> + timer->timeout.emit(timer);
> + }
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h
> new file mode 100644
> index 000000000000..600289e455e2
> --- /dev/null
> +++ b/src/libcamera/include/event_dispatcher_poll.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_loop.h - Camera object interface
> + */
> +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
> +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__
> +
> +#include <libcamera/event_dispatcher.h>
> +
> +#include <list>
> +#include <map>
> +#include <vector>
> +
> +namespace libcamera {
> +
> +class EventNotifier;
> +class Timer;
> +
> +class EventDispatcherPoll final : public EventDispatcher
> +{
> +public:
> + EventDispatcherPoll();
> + ~EventDispatcherPoll();
> +
> + void registerEventNotifier(EventNotifier *notifier);
> + void unregisterEventNotifier(EventNotifier *notifier);
> +
> + void registerTimer(Timer *timer);
> + void unregisterTimer(Timer *timer);
> +
> + void processEvents();
> +
> +private:
> + struct EventNotifierSetPoll {
> + short events() const;
> + EventNotifier *notifiers[3];
> + };
> +
> + std::map<int, EventNotifierSetPoll> notifiers_;
> + std::list<Timer *> timers_;
> +
> + void processNotifiers(std::vector<struct pollfd> &pollfds);
> + void processTimers();
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_EVENT_DISPATCHER_POLL_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 61fb93205b34..abf9a71d4172 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -3,6 +3,7 @@ libcamera_sources = files([
> 'camera_manager.cpp',
> 'device_enumerator.cpp',
> 'event_dispatcher.cpp',
> + 'event_dispatcher_poll.cpp',
> 'event_notifier.cpp',
> 'log.cpp',
> 'media_device.cpp',
> @@ -14,6 +15,7 @@ libcamera_sources = files([
>
> libcamera_headers = files([
> 'include/device_enumerator.h',
> + 'include/event_dispatcher_poll.h',
> 'include/log.h',
> 'include/media_device.h',
> 'include/media_object.h',
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list