[{"id":244,"web_url":"https://patchwork.libcamera.org/comment/244/","msgid":"<20190108102801.ndmnamzri3kpgxjr@uno.localdomain>","date":"2019-01-08T10:28:01","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n   mostly minor stuff here as well\n\nOn Tue, Jan 08, 2019 at 01:11:47AM +0200, Laurent Pinchart wrote:\n> Provide a poll-based event dispatcher implementation as convenience for\n> applications that don't need a custom event loop. The poll-based\n> dispatcher is automatically instantiated if the application doesn't\n> provide its own dispatcher.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Changes since v1:\n>\n> - Fix ASSERT() condition check\n> - Ignore duplicate notifiers\n> - Clarify warning messages for event notifier (un)registration errors\n> ---\n>  include/libcamera/libcamera.h                 |   1 +\n>  src/libcamera/camera_manager.cpp              |  19 +-\n>  src/libcamera/event_dispatcher_poll.cpp       | 234 ++++++++++++++++++\n>  src/libcamera/include/event_dispatcher_poll.h |  50 ++++\n>  src/libcamera/meson.build                     |   2 +\n>  5 files changed, 301 insertions(+), 5 deletions(-)\n>  create mode 100644 src/libcamera/event_dispatcher_poll.cpp\n>  create mode 100644 src/libcamera/include/event_dispatcher_poll.h\n>\n> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h\n> index 785babefa1c8..2dcaeda49812 100644\n> --- a/include/libcamera/libcamera.h\n> +++ b/include/libcamera/libcamera.h\n> @@ -10,5 +10,6 @@\n>  #include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n>  #include <libcamera/event_dispatcher.h>\n> +#include <libcamera/event_dispatcher_poll.h>\n>\n>  #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index cbfd977f1e3c..348ea2fedf64 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -9,6 +9,7 @@\n>  #include <libcamera/event_dispatcher.h>\n>\n>  #include \"device_enumerator.h\"\n> +#include \"event_dispatcher_poll.h\"\n>  #include \"log.h\"\n>  #include \"pipeline_handler.h\"\n>\n> @@ -188,9 +189,10 @@ CameraManager *CameraManager::instance()\n>   * \\param dispatcher Pointer to the event dispatcher\n>   *\n>   * libcamera requires an event dispatcher to integrate event notification and\n> - * timers with the application event loop. Applications shall call this function\n> - * once and only once before the camera manager is started with start() to set\n> - * the event dispatcher.\n> + * timers with the application event loop. Applications that want to provide\n> + * their own event dispatcher shall call this function once and only once before\n> + * the camera manager is started with start(). If no event dispatcher is\n> + * provided, a default poll-based implementation will be used.\n>   *\n>   * The CameraManager takes ownership of the event dispatcher and will delete it\n>   * when the application terminates.\n> @@ -208,11 +210,18 @@ void CameraManager::setEventDispatcher(EventDispatcher *dispatcher)\n>  /**\n>   * \\fn CameraManager::eventDispatcher()\n>   * \\brief Retrieve the event dispatcher\n> - * \\return Pointer to the event dispatcher, or nullptr if no event dispatcher\n> - * has been set\n> + *\n> + * This function retrieves the event dispatcher set with setEventDispatcher().\n> + * If no dispatcher has been set, a default poll-based implementation is created\n> + * and returned.\n\nIs it worth pointing out that after this function has been called, it\nis not possible to set a custom eventDispatcher anymore?\n\n> + *\n> + * \\return Pointer to the event dispatcher\n>   */\n>  EventDispatcher *CameraManager::eventDispatcher()\n>  {\n> +\tif (!dispatcher_)\n> +\t\tdispatcher_ = new EventDispatcherPoll();\n> +\n>  \treturn dispatcher_;\n>  }\n>\n> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp\n> new file mode 100644\n> index 000000000000..0cd08b2b4cc1\n> --- /dev/null\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -0,0 +1,234 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * event_dispatcher_poll.cpp - Poll-based event dispatcher\n> + */\n> +\n> +#include <algorithm>\n> +#include <iomanip>\n> +#include <poll.h>\n> +#include <string.h>\n> +\n> +#include <libcamera/event_notifier.h>\n> +#include <libcamera/timer.h>\n> +\n> +#include \"event_dispatcher_poll.h\"\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file event_dispatcher_poll.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +static const char *notifierType(EventNotifier::Type type)\n> +{\n> +\tif (type == EventNotifier::Read)\n> +\t\treturn \"read\";\n> +\tif (type == EventNotifier::Write)\n> +\t\treturn \"write\";\n> +\tif (type == EventNotifier::Exception)\n> +\t\treturn \"exception\";\n> +\n> +\treturn \"\";\n> +}\n> +\n> +/**\n> + * \\class EventDispatcherPoll\n> + * \\brief A poll-based event dispatcher\n> + */\n> +\n\nNone of the functions here implemented is commented. Is this\nintentional? Doxygen uses comments applied on the base class so it\ndoes not actually complains...\n\n> +EventDispatcherPoll::EventDispatcherPoll()\n> +{\n> +}\n> +\n> +EventDispatcherPoll::~EventDispatcherPoll()\n> +{\n> +}\n> +\n> +void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier)\n> +{\n> +\tEventNotifierSetPoll &set = notifiers_[notifier->fd()];\n> +\tEventNotifier::Type type = notifier->type();\n> +\n> +\tif (set.notifiers[type] && set.notifiers[type] != notifier) {\n> +\t\tLOG(Warning) << \"Ignoring duplicate \" << notifierType(type)\n> +\t\t\t     << \" notifier for fd \" << notifier->fd();\n> +\t\treturn;\n> +\t}\n> +\n> +\tset.notifiers[type] = notifier;\n> +}\n> +\n> +void EventDispatcherPoll::unregisterEventNotifier(EventNotifier *notifier)\n> +{\n> +\tauto iter = notifiers_.find(notifier->fd());\n> +\tif (iter == notifiers_.end())\n> +\t\treturn;\n> +\n> +\tEventNotifierSetPoll &set = iter->second;\n> +\tEventNotifier::Type type = notifier->type();\n> +\n> +\tif (!set.notifiers[type])\n> +\t\treturn;\n> +\n> +\tif (set.notifiers[type] != notifier) {\n> +\t\tLOG(Warning) << notifierType(type) << \" notifier for fd \"\n> +\t\t\t     << notifier->fd() << \" is not registered\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tset.notifiers[type] = nullptr;\n> +\tif (!set.notifiers[0] && !set.notifiers[1] && !set.notifiers[2])\n> +\t\tnotifiers_.erase(iter);\n> +}\n> +\n> +void EventDispatcherPoll::registerTimer(Timer *timer)\n> +{\n> +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> +\t\tif ((*iter)->deadline() > timer->deadline()) {\n> +\t\t\ttimers_.insert(iter, timer);\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n> +\n> +\ttimers_.push_back(timer);\n> +}\n> +\n> +void EventDispatcherPoll::unregisterTimer(Timer *timer)\n> +{\n> +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> +\t\tif (*iter == timer) {\n> +\t\t\ttimers_.erase(iter);\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * As the timers list is ordered, we can stop as soon as we go\n> +\t\t * past the deadline.\n> +\t\t */\n> +\t\tif ((*iter)->deadline() > timer->deadline())\n> +\t\t\tbreak;\n> +\t}\n> +}\n> +\n\nThis function is probably worth being documented a bit. Just to point\nout it is blocking maybe.\n\n> +void EventDispatcherPoll::processEvents()\n> +{\n> +\tint ret;\n> +\n> +\t/* Create the pollfd array. */\n> +\tstd::vector<struct pollfd> pollfds;\n> +\tpollfds.reserve(notifiers_.size());\n> +\n> +\tfor (auto notifier : notifiers_)\n> +\t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 });\n> +\n> +\t/* Compute the timeout. */\n> +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\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> +\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> +\n> +\t\tLOG(Debug) << \"timeout \" << timeout.tv_sec << \".\"\n> +\t\t\t   << std::setfill('0') << std::setw(9)\n> +\t\t\t   << timeout.tv_nsec;\n> +\t}\n> +\n> +\t/* Wait for events and process notifers and timers. */\n> +\tret = ppoll(pollfds.data(), pollfds.size(),\n> +\t\t    nextTimer ? &timeout : nullptr, nullptr);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(Warning) << \"poll() failed with \" << strerror(-ret);\n\nWhen poll fails, are we ok processing timers?\n\n> +\t} else if (ret > 0) {\n> +\t\tprocessNotifiers(pollfds);\n> +\t}\n> +\n\nshouldn't we process timers when poll returns 0 (timeout) ? or we go\nthere to check just in case we missed a deadline?\n\n> +\tprocessTimers();\n> +}\n> +\n> +short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> +{\n> +\tshort events = 0;\n> +\n> +\tif (notifiers[EventNotifier::Read])\n> +\t\tevents |= POLLIN;\n> +\tif (notifiers[EventNotifier::Write])\n> +\t\tevents |= POLLOUT;\n> +\tif (notifiers[EventNotifier::Exception])\n> +\t\tevents |= POLLPRI;\n> +\n> +\treturn events;\n> +}\n> +\n> +void EventDispatcherPoll::processNotifiers(std::vector<struct pollfd> &pollfds)\n> +{\n> +\tstatic const struct {\n> +\t\tEventNotifier::Type type;\n> +\t\tshort events;\n> +\t} events[] = {\n> +\t\t{ EventNotifier::Read, POLLIN },\n> +\t\t{ EventNotifier::Write, POLLOUT },\n> +\t\t{ EventNotifier::Exception, POLLPRI },\n> +\t};\n> +\n> +\tfor (const struct pollfd &pfd : pollfds) {\n> +\t\tauto iter = notifiers_.find(pfd.fd);\n> +\t\tASSERT(iter != notifiers_.end());\n\nIs this fatal? I mean, this should not happen, I agree, but do we want\nto abort?\n\n> +\n> +\t\tEventNotifierSetPoll &set = iter->second;\n> +\n> +\t\tfor (const auto &event : events) {\n> +\t\t\tEventNotifier *notifier = set.notifiers[event.type];\n> +\n> +\t\t\tif (!notifier)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\t/*\n> +\t\t\t * If the file descriptor is invalid, disable the\n> +\t\t\t * notifier immediately.\n> +\t\t\t */\n> +\t\t\tif (pfd.revents & POLLNVAL) {\n> +\t\t\t\tLOG(Warning) << \"Disabling \" << notifierType(event.type)\n> +\t\t\t\t\t     << \" due to invalid file descriptor \"\n> +\t\t\t\t\t     << pfd.fd;\n> +\t\t\t\tunregisterEventNotifier(notifier);\n\nI'm not sure if POLLNVAL is exclusive (when it's set, no other bit\nfield is set in pfd.revents), but if it is not, and some other event\nsource flag is set, after unregistering, shouldn't you continue?\n\n> +\t\t\t}\n> +\n> +\t\t\tif (pfd.revents & event.events)\n> +\t\t\t\tnotifier->activated.emit(notifier);\n> +\t\t}\n> +\t}\n> +}\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> +\n> +\twhile (!timers_.empty()) {\n> +\t\tTimer *timer = timers_.front();\n> +\t\tif (timer->deadline() > now)\n> +\t\t\tbreak;\n> +\n> +\t\ttimers_.pop_front();\n> +\t\ttimer->stop();\n> +\t\ttimer->timeout.emit(timer);\n> +\t}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/include/event_dispatcher_poll.h b/src/libcamera/include/event_dispatcher_poll.h\n> new file mode 100644\n> index 000000000000..600289e455e2\n> --- /dev/null\n> +++ b/src/libcamera/include/event_dispatcher_poll.h\n> @@ -0,0 +1,50 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * event_loop.h - Camera object interface\n\nCamera object?\n\n> + */\n> +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> +\n> +#include <libcamera/event_dispatcher.h>\n> +\n> +#include <list>\n> +#include <map>\n> +#include <vector>\n> +\n> +namespace libcamera {\n> +\n> +class EventNotifier;\n> +class Timer;\n\nStill a bit skeptical about forward declarations when an include would\nsuffice, but that's up to you actually.\n\n> +\n> +class EventDispatcherPoll final : public EventDispatcher\n> +{\n> +public:\n> +\tEventDispatcherPoll();\n> +\t~EventDispatcherPoll();\n> +\n> +\tvoid registerEventNotifier(EventNotifier *notifier);\n> +\tvoid unregisterEventNotifier(EventNotifier *notifier);\n> +\n> +\tvoid registerTimer(Timer *timer);\n> +\tvoid unregisterTimer(Timer *timer);\n> +\n> +\tvoid processEvents();\n> +\n> +private:\n> +\tstruct EventNotifierSetPoll {\n> +\t\tshort events() const;\n> +\t\tEventNotifier *notifiers[3];\n\nShouldn't a value from the EventNotifier::Type be used? Like an\nadditional MaxType ? I agree this is mostly fixed though, so feel free\nto ignore the comment.\n\n> +\t};\n> +\n> +\tstd::map<int, EventNotifierSetPoll> notifiers_;\n> +\tstd::list<Timer *> timers_;\n> +\n> +\tvoid processNotifiers(std::vector<struct pollfd> &pollfds);\n> +\tvoid processTimers();\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_EVENT_DISPATCHER_POLL_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 61fb93205b34..abf9a71d4172 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -3,6 +3,7 @@ libcamera_sources = files([\n>      'camera_manager.cpp',\n>      'device_enumerator.cpp',\n>      'event_dispatcher.cpp',\n> +    'event_dispatcher_poll.cpp',\n>      'event_notifier.cpp',\n>      'log.cpp',\n>      'media_device.cpp',\n> @@ -14,6 +15,7 @@ libcamera_sources = files([\n>\n>  libcamera_headers = files([\n>      'include/device_enumerator.h',\n> +    'include/event_dispatcher_poll.h',\n>      'include/log.h',\n>      'include/media_device.h',\n>      'include/media_object.h',\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nThanks\n  j\n\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 relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6812E60B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 11:27:55 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id ED99C20000B;\n\tTue,  8 Jan 2019 10:27:54 +0000 (UTC)"],"Date":"Tue, 8 Jan 2019 11:28:01 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190108102801.ndmnamzri3kpgxjr@uno.localdomain>","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-8-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2b5nkzlfvtkwfjk7\"","Content-Disposition":"inline","In-Reply-To":"<20190107231151.23291-8-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","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":"Tue, 08 Jan 2019 10:27:55 -0000"}},{"id":246,"web_url":"https://patchwork.libcamera.org/comment/246/","msgid":"<1607976.1UoNfe8uBB@avalon>","date":"2019-01-08T10:54:57","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tuesday, 8 January 2019 12:28:01 EET Jacopo Mondi wrote:\n> Hi Laurent,\n>    mostly minor stuff here as well\n> \n> On Tue, Jan 08, 2019 at 01:11:47AM +0200, Laurent Pinchart wrote:\n> > Provide a poll-based event dispatcher implementation as convenience for\n> > applications that don't need a custom event loop. The poll-based\n> > dispatcher is automatically instantiated if the application doesn't\n> > provide its own dispatcher.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > Changes since v1:\n> > \n> > - Fix ASSERT() condition check\n> > - Ignore duplicate notifiers\n> > - Clarify warning messages for event notifier (un)registration errors\n> > ---\n> > \n> >  include/libcamera/libcamera.h                 |   1 +\n> >  src/libcamera/camera_manager.cpp              |  19 +-\n> >  src/libcamera/event_dispatcher_poll.cpp       | 234 ++++++++++++++++++\n> >  src/libcamera/include/event_dispatcher_poll.h |  50 ++++\n> >  src/libcamera/meson.build                     |   2 +\n> >  5 files changed, 301 insertions(+), 5 deletions(-)\n> >  create mode 100644 src/libcamera/event_dispatcher_poll.cpp\n> >  create mode 100644 src/libcamera/include/event_dispatcher_poll.h\n\n[snip]\n\n> > diff --git a/src/libcamera/camera_manager.cpp\n> > b/src/libcamera/camera_manager.cpp index cbfd977f1e3c..348ea2fedf64\n> > 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n\n[snip]\n\n> > @@ -208,11 +210,18 @@ void\n> > CameraManager::setEventDispatcher(EventDispatcher *dispatcher)> \n> >  /**\n> >   * \\fn CameraManager::eventDispatcher()\n> >   * \\brief Retrieve the event dispatcher\n> > - * \\return Pointer to the event dispatcher, or nullptr if no event\n> > dispatcher - * has been set\n> > + *\n> > + * This function retrieves the event dispatcher set with\n> > setEventDispatcher(). + * If no dispatcher has been set, a default\n> > poll-based implementation is created + * and returned.\n> \n> Is it worth pointing out that after this function has been called, it\n> is not possible to set a custom eventDispatcher anymore?\n\nGood point. I'll add \", and no custom event dispatcher may be installed \nanymore.\"\n\n> > + *\n> > + * \\return Pointer to the event dispatcher\n> >   */\n\n[snip]\n\n> > diff --git a/src/libcamera/event_dispatcher_poll.cpp\n> > b/src/libcamera/event_dispatcher_poll.cpp new file mode 100644\n> > index 000000000000..0cd08b2b4cc1\n> > --- /dev/null\n> > +++ b/src/libcamera/event_dispatcher_poll.cpp\n\n[snip]\n\n> > +/**\n> > + * \\class EventDispatcherPoll\n> > + * \\brief A poll-based event dispatcher\n> > + */\n> > +\n> \n> None of the functions here implemented is commented. Is this\n> intentional? Doxygen uses comments applied on the base class so it\n> does not actually complains...\n\nIt's intentional, as EventDispatcherPoll is an internal implementation that \ndoesn't create any API, neither externally not internally. There is thus not \nmuch to document from an API point of view.\n\n> > +EventDispatcherPoll::EventDispatcherPoll()\n> > +{\n> > +}\n> > +\n> > +EventDispatcherPoll::~EventDispatcherPoll()\n> > +{\n> > +}\n> > +\n> > +void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier)\n> > +{\n> > +\tEventNotifierSetPoll &set = notifiers_[notifier->fd()];\n> > +\tEventNotifier::Type type = notifier->type();\n> > +\n> > +\tif (set.notifiers[type] && set.notifiers[type] != notifier) {\n> > +\t\tLOG(Warning) << \"Ignoring duplicate \" << notifierType(type)\n> > +\t\t\t     << \" notifier for fd \" << notifier->fd();\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tset.notifiers[type] = notifier;\n> > +}\n> > +\n> > +void EventDispatcherPoll::unregisterEventNotifier(EventNotifier\n> > *notifier)\n> > +{\n> > +\tauto iter = notifiers_.find(notifier->fd());\n> > +\tif (iter == notifiers_.end())\n> > +\t\treturn;\n> > +\n> > +\tEventNotifierSetPoll &set = iter->second;\n> > +\tEventNotifier::Type type = notifier->type();\n> > +\n> > +\tif (!set.notifiers[type])\n> > +\t\treturn;\n> > +\n> > +\tif (set.notifiers[type] != notifier) {\n> > +\t\tLOG(Warning) << notifierType(type) << \" notifier for fd \"\n> > +\t\t\t     << notifier->fd() << \" is not registered\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tset.notifiers[type] = nullptr;\n> > +\tif (!set.notifiers[0] && !set.notifiers[1] && !set.notifiers[2])\n> > +\t\tnotifiers_.erase(iter);\n> > +}\n> > +\n> > +void EventDispatcherPoll::registerTimer(Timer *timer)\n> > +{\n> > +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> > +\t\tif ((*iter)->deadline() > timer->deadline()) {\n> > +\t\t\ttimers_.insert(iter, timer);\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\ttimers_.push_back(timer);\n> > +}\n> > +\n> > +void EventDispatcherPoll::unregisterTimer(Timer *timer)\n> > +{\n> > +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> > +\t\tif (*iter == timer) {\n> > +\t\t\ttimers_.erase(iter);\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\t/*\n> > +\t\t * As the timers list is ordered, we can stop as soon as we go\n> > +\t\t * past the deadline.\n> > +\t\t */\n> > +\t\tif ((*iter)->deadline() > timer->deadline())\n> > +\t\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> \n> This function is probably worth being documented a bit. Just to point\n> out it is blocking maybe.\n\nIt's not supposed to be blocking :-)\n\n> > +void EventDispatcherPoll::processEvents()\n> > +{\n> > +\tint ret;\n> > +\n> > +\t/* Create the pollfd array. */\n> > +\tstd::vector<struct pollfd> pollfds;\n> > +\tpollfds.reserve(notifiers_.size());\n> > +\n> > +\tfor (auto notifier : notifiers_)\n> > +\t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 });\n> > +\n> > +\t/* Compute the timeout. */\n> > +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\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> > +\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> > +\n> > +\t\tLOG(Debug) << \"timeout \" << timeout.tv_sec << \".\"\n> > +\t\t\t   << std::setfill('0') << std::setw(9)\n> > +\t\t\t   << timeout.tv_nsec;\n> > +\t}\n> > +\n> > +\t/* Wait for events and process notifers and timers. */\n> > +\tret = ppoll(pollfds.data(), pollfds.size(),\n> > +\t\t    nextTimer ? &timeout : nullptr, nullptr);\n> > +\tif (ret < 0) {\n> > +\t\tret = -errno;\n> > +\t\tLOG(Warning) << \"poll() failed with \" << strerror(-ret);\n> \n> When poll fails, are we ok processing timers?\n> \n> > +\t} else if (ret > 0) {\n> > +\t\tprocessNotifiers(pollfds);\n> > +\t}\n> > +\n> \n> shouldn't we process timers when poll returns 0 (timeout) ? or we go\n> there to check just in case we missed a deadline?\n\nI think processing timers is fine in that case, just in case. A poll() failure \nwill likely be fatal anyway, so it won't matter much.\n\n> > +\tprocessTimers();\n> > +}\n> > +\n> > +short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> > +{\n> > +\tshort events = 0;\n> > +\n> > +\tif (notifiers[EventNotifier::Read])\n> > +\t\tevents |= POLLIN;\n> > +\tif (notifiers[EventNotifier::Write])\n> > +\t\tevents |= POLLOUT;\n> > +\tif (notifiers[EventNotifier::Exception])\n> > +\t\tevents |= POLLPRI;\n> > +\n> > +\treturn events;\n> > +}\n> > +\n> > +void EventDispatcherPoll::processNotifiers(std::vector<struct pollfd>\n> > &pollfds)\n> > +{\n> > +\tstatic const struct {\n> > +\t\tEventNotifier::Type type;\n> > +\t\tshort events;\n> > +\t} events[] = {\n> > +\t\t{ EventNotifier::Read, POLLIN },\n> > +\t\t{ EventNotifier::Write, POLLOUT },\n> > +\t\t{ EventNotifier::Exception, POLLPRI },\n> > +\t};\n> > +\n> > +\tfor (const struct pollfd &pfd : pollfds) {\n> > +\t\tauto iter = notifiers_.find(pfd.fd);\n> > +\t\tASSERT(iter != notifiers_.end());\n> \n> Is this fatal? I mean, this should not happen, I agree, but do we want\n> to abort?\n\nIt's a serious bug in the library that would likely indicate a memory \ncorruption of some sort, or really incorrect library code. I expect the \nprogram to crash later in that case, aborting early will make it easier to \ndebug the problem.\n\n> > +\n> > +\t\tEventNotifierSetPoll &set = iter->second;\n> > +\n> > +\t\tfor (const auto &event : events) {\n> > +\t\t\tEventNotifier *notifier = set.notifiers[event.type];\n> > +\n> > +\t\t\tif (!notifier)\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\t/*\n> > +\t\t\t * If the file descriptor is invalid, disable the\n> > +\t\t\t * notifier immediately.\n> > +\t\t\t */\n> > +\t\t\tif (pfd.revents & POLLNVAL) {\n> > +\t\t\t\tLOG(Warning) << \"Disabling \" << notifierType(event.type)\n> > +\t\t\t\t\t     << \" due to invalid file descriptor \"\n> > +\t\t\t\t\t     << pfd.fd;\n> > +\t\t\t\tunregisterEventNotifier(notifier);\n> \n> I'm not sure if POLLNVAL is exclusive (when it's set, no other bit\n> field is set in pfd.revents), but if it is not, and some other event\n> source flag is set, after unregistering, shouldn't you continue?\n\nI would assume it's exclusive indeed, a continue shouldn't hurt here, I'll add \none (but it shouldn't make a big difference either, given that POLLNVAL \nshouldn't occur at all in the normal case).\n\n> > +\t\t\t}\n> > +\n> > +\t\t\tif (pfd.revents & event.events)\n> > +\t\t\t\tnotifier->activated.emit(notifier);\n> > +\t\t}\n> > +\t}\n> > +}\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> > +\n> > +\twhile (!timers_.empty()) {\n> > +\t\tTimer *timer = timers_.front();\n> > +\t\tif (timer->deadline() > now)\n> > +\t\t\tbreak;\n> > +\n> > +\t\ttimers_.pop_front();\n> > +\t\ttimer->stop();\n> > +\t\ttimer->timeout.emit(timer);\n> > +\t}\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/include/event_dispatcher_poll.h\n> > b/src/libcamera/include/event_dispatcher_poll.h new file mode 100644\n> > index 000000000000..600289e455e2\n> > --- /dev/null\n> > +++ b/src/libcamera/include/event_dispatcher_poll.h\n> > @@ -0,0 +1,50 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * event_loop.h - Camera object interface\n> \n> Camera object?\n\nCan you guess where this was copied from ? :-)\n\nI've now fixed it (as well as the file name).\n\n> > + */\n> > +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> > +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> > +\n> > +#include <libcamera/event_dispatcher.h>\n> > +\n> > +#include <list>\n> > +#include <map>\n> > +#include <vector>\n> > +\n> > +namespace libcamera {\n> > +\n> > +class EventNotifier;\n> > +class Timer;\n> \n> Still a bit skeptical about forward declarations when an include would\n> suffice, but that's up to you actually.\n\nOn the plus side, they speed up compilation and avoid potential future \ncircular inclusions. Do they cause issues ?\n\n> > +\n> > +class EventDispatcherPoll final : public EventDispatcher\n> > +{\n> > +public:\n> > +\tEventDispatcherPoll();\n> > +\t~EventDispatcherPoll();\n> > +\n> > +\tvoid registerEventNotifier(EventNotifier *notifier);\n> > +\tvoid unregisterEventNotifier(EventNotifier *notifier);\n> > +\n> > +\tvoid registerTimer(Timer *timer);\n> > +\tvoid unregisterTimer(Timer *timer);\n> > +\n> > +\tvoid processEvents();\n> > +\n> > +private:\n> > +\tstruct EventNotifierSetPoll {\n> > +\t\tshort events() const;\n> > +\t\tEventNotifier *notifiers[3];\n> \n> Shouldn't a value from the EventNotifier::Type be used? Like an\n> additional MaxType ? I agree this is mostly fixed though, so feel free\n> to ignore the comment.\n\nAs it is indeed fixed by the poll() API, which I don't expect to change any \ntime soon, I think a fixed-size array is fine.\n\n> > +\t};\n> > +\n> > +\tstd::map<int, EventNotifierSetPoll> notifiers_;\n> > +\tstd::list<Timer *> timers_;\n> > +\n> > +\tvoid processNotifiers(std::vector<struct pollfd> &pollfds);\n> > +\tvoid processTimers();\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_EVENT_DISPATCHER_POLL_H__ */\n\n[snip]","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4FBE60B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 11:53:49 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 34D69586;\n\tTue,  8 Jan 2019 11:53:49 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546944829;\n\tbh=gEnepNvyUhW2j56lIaZVI4oSYwVVAtVrMTYbHQE0GVw=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=ajwdQQZq9r8iNh/NbgoVBzc0z6ykWqLzIzBUIbLeK9YoJA9X2tnD/TwHc+S2waZWW\n\tElpTMtayljvv5aAw9plcTwjF16SP2keCjN3TaOEZwVLvcfMQbyNe2WOXCe5XsPh+ut\n\tIR9lkxecq+wSrURQP0EDw2kSJo3fV83iYKNYZDYk=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 08 Jan 2019 12:54:57 +0200","Message-ID":"<1607976.1UoNfe8uBB@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190108102801.ndmnamzri3kpgxjr@uno.localdomain>","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-8-laurent.pinchart@ideasonboard.com>\n\t<20190108102801.ndmnamzri3kpgxjr@uno.localdomain>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","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":"Tue, 08 Jan 2019 10:53:49 -0000"}},{"id":251,"web_url":"https://patchwork.libcamera.org/comment/251/","msgid":"<20190108123241.luxquqwajin7rcnr@uno.localdomain>","date":"2019-01-08T12:32:41","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Tue, Jan 08, 2019 at 12:54:57PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Tuesday, 8 January 2019 12:28:01 EET Jacopo Mondi wrote:\n> > Hi Laurent,\n> >    mostly minor stuff here as well\n> >\n> > On Tue, Jan 08, 2019 at 01:11:47AM +0200, Laurent Pinchart wrote:\n> > > Provide a poll-based event dispatcher implementation as convenience for\n> > > applications that don't need a custom event loop. The poll-based\n> > > dispatcher is automatically instantiated if the application doesn't\n> > > provide its own dispatcher.\n> > >\n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > > Changes since v1:\n> > >\n> > > - Fix ASSERT() condition check\n> > > - Ignore duplicate notifiers\n> > > - Clarify warning messages for event notifier (un)registration errors\n> > > ---\n> > >\n> > >  include/libcamera/libcamera.h                 |   1 +\n> > >  src/libcamera/camera_manager.cpp              |  19 +-\n> > >  src/libcamera/event_dispatcher_poll.cpp       | 234 ++++++++++++++++++\n> > >  src/libcamera/include/event_dispatcher_poll.h |  50 ++++\n> > >  src/libcamera/meson.build                     |   2 +\n> > >  5 files changed, 301 insertions(+), 5 deletions(-)\n> > >  create mode 100644 src/libcamera/event_dispatcher_poll.cpp\n> > >  create mode 100644 src/libcamera/include/event_dispatcher_poll.h\n>\n> [snip]\n>\n> > > diff --git a/src/libcamera/camera_manager.cpp\n> > > b/src/libcamera/camera_manager.cpp index cbfd977f1e3c..348ea2fedf64\n> > > 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n>\n> [snip]\n>\n> > > @@ -208,11 +210,18 @@ void\n> > > CameraManager::setEventDispatcher(EventDispatcher *dispatcher)>\n> > >  /**\n> > >   * \\fn CameraManager::eventDispatcher()\n> > >   * \\brief Retrieve the event dispatcher\n> > > - * \\return Pointer to the event dispatcher, or nullptr if no event\n> > > dispatcher - * has been set\n> > > + *\n> > > + * This function retrieves the event dispatcher set with\n> > > setEventDispatcher(). + * If no dispatcher has been set, a default\n> > > poll-based implementation is created + * and returned.\n> >\n> > Is it worth pointing out that after this function has been called, it\n> > is not possible to set a custom eventDispatcher anymore?\n>\n> Good point. I'll add \", and no custom event dispatcher may be installed\n> anymore.\"\n>\n> > > + *\n> > > + * \\return Pointer to the event dispatcher\n> > >   */\n>\n> [snip]\n>\n> > > diff --git a/src/libcamera/event_dispatcher_poll.cpp\n> > > b/src/libcamera/event_dispatcher_poll.cpp new file mode 100644\n> > > index 000000000000..0cd08b2b4cc1\n> > > --- /dev/null\n> > > +++ b/src/libcamera/event_dispatcher_poll.cpp\n>\n> [snip]\n>\n> > > +/**\n> > > + * \\class EventDispatcherPoll\n> > > + * \\brief A poll-based event dispatcher\n> > > + */\n> > > +\n> >\n> > None of the functions here implemented is commented. Is this\n> > intentional? Doxygen uses comments applied on the base class so it\n> > does not actually complains...\n>\n> It's intentional, as EventDispatcherPoll is an internal implementation that\n> doesn't create any API, neither externally not internally. There is thus not\n> much to document from an API point of view.\n>\n\nI see, fine with me\n\n> > > +EventDispatcherPoll::EventDispatcherPoll()\n> > > +{\n> > > +}\n> > > +\n> > > +EventDispatcherPoll::~EventDispatcherPoll()\n> > > +{\n> > > +}\n> > > +\n> > > +void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier)\n> > > +{\n> > > +\tEventNotifierSetPoll &set = notifiers_[notifier->fd()];\n> > > +\tEventNotifier::Type type = notifier->type();\n> > > +\n> > > +\tif (set.notifiers[type] && set.notifiers[type] != notifier) {\n> > > +\t\tLOG(Warning) << \"Ignoring duplicate \" << notifierType(type)\n> > > +\t\t\t     << \" notifier for fd \" << notifier->fd();\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\tset.notifiers[type] = notifier;\n> > > +}\n> > > +\n> > > +void EventDispatcherPoll::unregisterEventNotifier(EventNotifier\n> > > *notifier)\n> > > +{\n> > > +\tauto iter = notifiers_.find(notifier->fd());\n> > > +\tif (iter == notifiers_.end())\n> > > +\t\treturn;\n> > > +\n> > > +\tEventNotifierSetPoll &set = iter->second;\n> > > +\tEventNotifier::Type type = notifier->type();\n> > > +\n> > > +\tif (!set.notifiers[type])\n> > > +\t\treturn;\n> > > +\n> > > +\tif (set.notifiers[type] != notifier) {\n> > > +\t\tLOG(Warning) << notifierType(type) << \" notifier for fd \"\n> > > +\t\t\t     << notifier->fd() << \" is not registered\";\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\tset.notifiers[type] = nullptr;\n> > > +\tif (!set.notifiers[0] && !set.notifiers[1] && !set.notifiers[2])\n> > > +\t\tnotifiers_.erase(iter);\n> > > +}\n> > > +\n> > > +void EventDispatcherPoll::registerTimer(Timer *timer)\n> > > +{\n> > > +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> > > +\t\tif ((*iter)->deadline() > timer->deadline()) {\n> > > +\t\t\ttimers_.insert(iter, timer);\n> > > +\t\t\treturn;\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +\ttimers_.push_back(timer);\n> > > +}\n> > > +\n> > > +void EventDispatcherPoll::unregisterTimer(Timer *timer)\n> > > +{\n> > > +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> > > +\t\tif (*iter == timer) {\n> > > +\t\t\ttimers_.erase(iter);\n> > > +\t\t\treturn;\n> > > +\t\t}\n> > > +\n> > > +\t\t/*\n> > > +\t\t * As the timers list is ordered, we can stop as soon as we go\n> > > +\t\t * past the deadline.\n> > > +\t\t */\n> > > +\t\tif ((*iter)->deadline() > timer->deadline())\n> > > +\t\t\tbreak;\n> > > +\t}\n> > > +}\n> > > +\n> >\n> > This function is probably worth being documented a bit. Just to point\n> > out it is blocking maybe.\n>\n> It's not supposed to be blocking :-)\n>\n\nI mean processEvents() not unregisterTimer() :)\n\n> > > +void EventDispatcherPoll::processEvents()\n> > > +{\n> > > +\tint ret;\n> > > +\n> > > +\t/* Create the pollfd array. */\n> > > +\tstd::vector<struct pollfd> pollfds;\n> > > +\tpollfds.reserve(notifiers_.size());\n> > > +\n> > > +\tfor (auto notifier : notifiers_)\n> > > +\t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 });\n> > > +\n> > > +\t/* Compute the timeout. */\n> > > +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\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> > > +\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> > > +\n> > > +\t\tLOG(Debug) << \"timeout \" << timeout.tv_sec << \".\"\n> > > +\t\t\t   << std::setfill('0') << std::setw(9)\n> > > +\t\t\t   << timeout.tv_nsec;\n> > > +\t}\n> > > +\n> > > +\t/* Wait for events and process notifers and timers. */\n> > > +\tret = ppoll(pollfds.data(), pollfds.size(),\n> > > +\t\t    nextTimer ? &timeout : nullptr, nullptr);\n> > > +\tif (ret < 0) {\n> > > +\t\tret = -errno;\n> > > +\t\tLOG(Warning) << \"poll() failed with \" << strerror(-ret);\n> >\n> > When poll fails, are we ok processing timers?\n> >\n> > > +\t} else if (ret > 0) {\n> > > +\t\tprocessNotifiers(pollfds);\n> > > +\t}\n> > > +\n> >\n> > shouldn't we process timers when poll returns 0 (timeout) ? or we go\n> > there to check just in case we missed a deadline?\n>\n> I think processing timers is fine in that case, just in case. A poll() failure\n> will likely be fatal anyway, so it won't matter much.\n\nSo it is not worth aborting/returning the poll errno?\n\n>\n> > > +\tprocessTimers();\n> > > +}\n> > > +\n> > > +short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> > > +{\n> > > +\tshort events = 0;\n> > > +\n> > > +\tif (notifiers[EventNotifier::Read])\n> > > +\t\tevents |= POLLIN;\n> > > +\tif (notifiers[EventNotifier::Write])\n> > > +\t\tevents |= POLLOUT;\n> > > +\tif (notifiers[EventNotifier::Exception])\n> > > +\t\tevents |= POLLPRI;\n> > > +\n> > > +\treturn events;\n> > > +}\n> > > +\n> > > +void EventDispatcherPoll::processNotifiers(std::vector<struct pollfd>\n> > > &pollfds)\n> > > +{\n> > > +\tstatic const struct {\n> > > +\t\tEventNotifier::Type type;\n> > > +\t\tshort events;\n> > > +\t} events[] = {\n> > > +\t\t{ EventNotifier::Read, POLLIN },\n> > > +\t\t{ EventNotifier::Write, POLLOUT },\n> > > +\t\t{ EventNotifier::Exception, POLLPRI },\n> > > +\t};\n> > > +\n> > > +\tfor (const struct pollfd &pfd : pollfds) {\n> > > +\t\tauto iter = notifiers_.find(pfd.fd);\n> > > +\t\tASSERT(iter != notifiers_.end());\n> >\n> > Is this fatal? I mean, this should not happen, I agree, but do we want\n> > to abort?\n>\n> It's a serious bug in the library that would likely indicate a memory\n> corruption of some sort, or really incorrect library code. I expect the\n> program to crash later in that case, aborting early will make it easier to\n> debug the problem.\n\nA notifier can be removed between the time poll is called and poll is\nreturned. I agree it's a corner case though.\n>\n> > > +\n> > > +\t\tEventNotifierSetPoll &set = iter->second;\n> > > +\n> > > +\t\tfor (const auto &event : events) {\n> > > +\t\t\tEventNotifier *notifier = set.notifiers[event.type];\n> > > +\n> > > +\t\t\tif (!notifier)\n> > > +\t\t\t\tcontinue;\n> > > +\n> > > +\t\t\t/*\n> > > +\t\t\t * If the file descriptor is invalid, disable the\n> > > +\t\t\t * notifier immediately.\n> > > +\t\t\t */\n> > > +\t\t\tif (pfd.revents & POLLNVAL) {\n> > > +\t\t\t\tLOG(Warning) << \"Disabling \" << notifierType(event.type)\n> > > +\t\t\t\t\t     << \" due to invalid file descriptor \"\n> > > +\t\t\t\t\t     << pfd.fd;\n> > > +\t\t\t\tunregisterEventNotifier(notifier);\n> >\n> > I'm not sure if POLLNVAL is exclusive (when it's set, no other bit\n> > field is set in pfd.revents), but if it is not, and some other event\n> > source flag is set, after unregistering, shouldn't you continue?\n>\n> I would assume it's exclusive indeed, a continue shouldn't hurt here, I'll add\n> one (but it shouldn't make a big difference either, given that POLLNVAL\n> shouldn't occur at all in the normal case).\n>\n> > > +\t\t\t}\n> > > +\n> > > +\t\t\tif (pfd.revents & event.events)\n> > > +\t\t\t\tnotifier->activated.emit(notifier);\n> > > +\t\t}\n> > > +\t}\n> > > +}\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> > > +\n> > > +\twhile (!timers_.empty()) {\n> > > +\t\tTimer *timer = timers_.front();\n> > > +\t\tif (timer->deadline() > now)\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\ttimers_.pop_front();\n> > > +\t\ttimer->stop();\n> > > +\t\ttimer->timeout.emit(timer);\n> > > +\t}\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/include/event_dispatcher_poll.h\n> > > b/src/libcamera/include/event_dispatcher_poll.h new file mode 100644\n> > > index 000000000000..600289e455e2\n> > > --- /dev/null\n> > > +++ b/src/libcamera/include/event_dispatcher_poll.h\n> > > @@ -0,0 +1,50 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2019, Google Inc.\n> > > + *\n> > > + * event_loop.h - Camera object interface\n> >\n> > Camera object?\n>\n> Can you guess where this was copied from ? :-)\n>\n> I've now fixed it (as well as the file name).\n>\n> > > + */\n> > > +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> > > +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> > > +\n> > > +#include <libcamera/event_dispatcher.h>\n> > > +\n> > > +#include <list>\n> > > +#include <map>\n> > > +#include <vector>\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +class EventNotifier;\n> > > +class Timer;\n> >\n> > Still a bit skeptical about forward declarations when an include would\n> > suffice, but that's up to you actually.\n>\n> On the plus side, they speed up compilation and avoid potential future\n> circular inclusions. Do they cause issues ?\n>\n\nNot particularly, as it is unlikely we miss recompiling this file if\nthe header change as the library is built together.\n\nThanks for the multiple clarifications.\nIt is my opinion you should push the next iteration of the series.\n\nThanks\n  j\n\n\n> > > +\n> > > +class EventDispatcherPoll final : public EventDispatcher\n> > > +{\n> > > +public:\n> > > +\tEventDispatcherPoll();\n> > > +\t~EventDispatcherPoll();\n> > > +\n> > > +\tvoid registerEventNotifier(EventNotifier *notifier);\n> > > +\tvoid unregisterEventNotifier(EventNotifier *notifier);\n> > > +\n> > > +\tvoid registerTimer(Timer *timer);\n> > > +\tvoid unregisterTimer(Timer *timer);\n> > > +\n> > > +\tvoid processEvents();\n> > > +\n> > > +private:\n> > > +\tstruct EventNotifierSetPoll {\n> > > +\t\tshort events() const;\n> > > +\t\tEventNotifier *notifiers[3];\n> >\n> > Shouldn't a value from the EventNotifier::Type be used? Like an\n> > additional MaxType ? I agree this is mostly fixed though, so feel free\n> > to ignore the comment.\n>\n> As it is indeed fixed by the poll() API, which I don't expect to change any\n> time soon, I think a fixed-size array is fine.\n>\n> > > +\t};\n> > > +\n> > > +\tstd::map<int, EventNotifierSetPoll> notifiers_;\n> > > +\tstd::list<Timer *> timers_;\n> > > +\n> > > +\tvoid processNotifiers(std::vector<struct pollfd> &pollfds);\n> > > +\tvoid processTimers();\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > +\n> > > +#endif /* __LIBCAMERA_EVENT_DISPATCHER_POLL_H__ */\n>\n> [snip]\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>\n>","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BFE460B0B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 13:32:35 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D52D54000C;\n\tTue,  8 Jan 2019 12:32:34 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 8 Jan 2019 13:32:41 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190108123241.luxquqwajin7rcnr@uno.localdomain>","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<20190107231151.23291-8-laurent.pinchart@ideasonboard.com>\n\t<20190108102801.ndmnamzri3kpgxjr@uno.localdomain>\n\t<1607976.1UoNfe8uBB@avalon>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"iy6gt2jt4ay4yadn\"","Content-Disposition":"inline","In-Reply-To":"<1607976.1UoNfe8uBB@avalon>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","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":"Tue, 08 Jan 2019 12:32:35 -0000"}},{"id":254,"web_url":"https://patchwork.libcamera.org/comment/254/","msgid":"<1829657.3vXFe4VsiG@avalon>","date":"2019-01-08T14:18:38","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tuesday, 8 January 2019 14:32:41 EET Jacopo Mondi wrote:\n> On Tue, Jan 08, 2019 at 12:54:57PM +0200, Laurent Pinchart wrote:\n> > On Tuesday, 8 January 2019 12:28:01 EET Jacopo Mondi wrote:\n> >> On Tue, Jan 08, 2019 at 01:11:47AM +0200, Laurent Pinchart wrote:\n> >>> Provide a poll-based event dispatcher implementation as convenience\n> >>> for applications that don't need a custom event loop. The poll-based\n> >>> dispatcher is automatically instantiated if the application doesn't\n> >>> provide its own dispatcher.\n> >>> \n> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >>> ---\n> >>> Changes since v1:\n> >>> \n> >>> - Fix ASSERT() condition check\n> >>> - Ignore duplicate notifiers\n> >>> - Clarify warning messages for event notifier (un)registration errors\n> >>> ---\n> >>> \n> >>>  include/libcamera/libcamera.h                 |   1 +\n> >>>  src/libcamera/camera_manager.cpp              |  19 +-\n> >>>  src/libcamera/event_dispatcher_poll.cpp       | 234 ++++++++++++++++++\n> >>>  src/libcamera/include/event_dispatcher_poll.h |  50 ++++\n> >>>  src/libcamera/meson.build                     |   2 +\n> >>>  5 files changed, 301 insertions(+), 5 deletions(-)\n> >>>  create mode 100644 src/libcamera/event_dispatcher_poll.cpp\n> >>>  create mode 100644 src/libcamera/include/event_dispatcher_poll.h\n\n[snip]\n\n> >>> diff --git a/src/libcamera/event_dispatcher_poll.cpp\n> >>> b/src/libcamera/event_dispatcher_poll.cpp new file mode 100644\n> >>> index 000000000000..0cd08b2b4cc1\n> >>> --- /dev/null\n> >>> +++ b/src/libcamera/event_dispatcher_poll.cpp\n\n[snip]\n\n> >>> +void EventDispatcherPoll::unregisterTimer(Timer *timer)\n> >>> +{\n> >>> +\tfor (auto iter = timers_.begin(); iter != timers_.end(); ++iter) {\n> >>> +\t\tif (*iter == timer) {\n> >>> +\t\t\ttimers_.erase(iter);\n> >>> +\t\t\treturn;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/*\n> >>> +\t\t * As the timers list is ordered, we can stop as soon as we go\n> >>> +\t\t * past the deadline.\n> >>> +\t\t */\n> >>> +\t\tif ((*iter)->deadline() > timer->deadline())\n> >>> +\t\t\tbreak;\n> >>> +\t}\n> >>> +}\n> >>> +\n> >> \n> >> This function is probably worth being documented a bit. Just to point\n> >> out it is blocking maybe.\n> > \n> > It's not supposed to be blocking :-)\n> \n> I mean processEvents() not unregisterTimer() :)\n\nThat makes more sense indeed :-)\n\nThe EventDispatcher::processEvents() function is already documented as \nblocking, so I don't think we need to repeat this here.\n\n> >>> +void EventDispatcherPoll::processEvents()\n> >>> +{\n> >>> +\tint ret;\n> >>> +\n> >>> +\t/* Create the pollfd array. */\n> >>> +\tstd::vector<struct pollfd> pollfds;\n> >>> +\tpollfds.reserve(notifiers_.size());\n> >>> +\n> >>> +\tfor (auto notifier : notifiers_)\n> >>> +\t\tpollfds.push_back({ notifier.first, notifier.second.events(), 0 \n});\n> >>> +\n> >>> +\t/* Compute the timeout. */\n> >>> +\tTimer *nextTimer = !timers_.empty() ? timers_.front() : nullptr;\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> >>> +\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> >>> +\n> >>> +\t\tLOG(Debug) << \"timeout \" << timeout.tv_sec << \".\"\n> >>> +\t\t\t   << std::setfill('0') << std::setw(9)\n> >>> +\t\t\t   << timeout.tv_nsec;\n> >>> +\t}\n> >>> +\n> >>> +\t/* Wait for events and process notifers and timers. */\n> >>> +\tret = ppoll(pollfds.data(), pollfds.size(),\n> >>> +\t\t    nextTimer ? &timeout : nullptr, nullptr);\n> >>> +\tif (ret < 0) {\n> >>> +\t\tret = -errno;\n> >>> +\t\tLOG(Warning) << \"poll() failed with \" << strerror(-ret);\n> >> \n> >> When poll fails, are we ok processing timers?\n> >> \n> >>> +\t} else if (ret > 0) {\n> >>> +\t\tprocessNotifiers(pollfds);\n> >>> +\t}\n> >>> +\n> >> \n> >> shouldn't we process timers when poll returns 0 (timeout) ? or we go\n> >> there to check just in case we missed a deadline?\n> > \n> > I think processing timers is fine in that case, just in case. A poll()\n> > failure will likely be fatal anyway, so it won't matter much.\n> \n> So it is not worth aborting/returning the poll errno?\n\nAccording to its man page, poll() and ppoll() can return the following errors:\n\n- EFAULT The array given as argument was not contained in the calling \nprogram's address space.\n- EINTR  A signal occurred before any requested event; see signal(7).\n- EINVAL The nfds value exceeds the RLIMIT_NOFILE value.\n- ENOMEM There was no space to allocate file descriptor tables.\n\nEFAULT means we did something really wrong and I expect a crash to follow soon \nafterwards. ENOMEM is also a symptom of a really bad problem that will at best \ninvoke the OOM killer in the near future. EINVAL is more debatable, but I'm \nnot sure what we could do there. EINTR is likely the only error we really need \nto handle, and that should be handled internally. I'll post a patch on top of \nthis series (with a related test case).\n\n> >>> +\tprocessTimers();\n> >>> +}\n> >>> +\n> >>> +short EventDispatcherPoll::EventNotifierSetPoll::events() const\n> >>> +{\n> >>> +\tshort events = 0;\n> >>> +\n> >>> +\tif (notifiers[EventNotifier::Read])\n> >>> +\t\tevents |= POLLIN;\n> >>> +\tif (notifiers[EventNotifier::Write])\n> >>> +\t\tevents |= POLLOUT;\n> >>> +\tif (notifiers[EventNotifier::Exception])\n> >>> +\t\tevents |= POLLPRI;\n> >>> +\n> >>> +\treturn events;\n> >>> +}\n> >>> +\n> >>> +void EventDispatcherPoll::processNotifiers(std::vector<struct pollfd>\n> >>> &pollfds)\n> >>> +{\n> >>> +\tstatic const struct {\n> >>> +\t\tEventNotifier::Type type;\n> >>> +\t\tshort events;\n> >>> +\t} events[] = {\n> >>> +\t\t{ EventNotifier::Read, POLLIN },\n> >>> +\t\t{ EventNotifier::Write, POLLOUT },\n> >>> +\t\t{ EventNotifier::Exception, POLLPRI },\n> >>> +\t};\n> >>> +\n> >>> +\tfor (const struct pollfd &pfd : pollfds) {\n> >>> +\t\tauto iter = notifiers_.find(pfd.fd);\n> >>> +\t\tASSERT(iter != notifiers_.end());\n> >> \n> >> Is this fatal? I mean, this should not happen, I agree, but do we want\n> >> to abort?\n> > \n> > It's a serious bug in the library that would likely indicate a memory\n> > corruption of some sort, or really incorrect library code. I expect the\n> > program to crash later in that case, aborting early will make it easier to\n> > debug the problem.\n> \n> A notifier can be removed between the time poll is called and poll is\n> returned. I agree it's a corner case though.\n\nNot in a single-threaded application though. For the multi-threaded, if other \nthreads start corrupting our state, we're screwed anyway. They must not do \nthat. We'll have to document the thread-related requirements of the API when \nwe'll implement multi-thread support.\n\n> >>> +\n> >>> +\t\tEventNotifierSetPoll &set = iter->second;\n> >>> +\n> >>> +\t\tfor (const auto &event : events) {\n> >>> +\t\t\tEventNotifier *notifier = set.notifiers[event.type];\n> >>> +\n> >>> +\t\t\tif (!notifier)\n> >>> +\t\t\t\tcontinue;\n> >>> +\n> >>> +\t\t\t/*\n> >>> +\t\t\t * If the file descriptor is invalid, disable the\n> >>> +\t\t\t * notifier immediately.\n> >>> +\t\t\t */\n> >>> +\t\t\tif (pfd.revents & POLLNVAL) {\n> >>> +\t\t\t\tLOG(Warning) << \"Disabling \" << notifierType(event.type)\n> >>> +\t\t\t\t\t     << \" due to invalid file descriptor \"\n> >>> +\t\t\t\t\t     << pfd.fd;\n> >>> +\t\t\t\tunregisterEventNotifier(notifier);\n> >> \n> >> I'm not sure if POLLNVAL is exclusive (when it's set, no other bit\n> >> field is set in pfd.revents), but if it is not, and some other event\n> >> source flag is set, after unregistering, shouldn't you continue?\n> > \n> > I would assume it's exclusive indeed, a continue shouldn't hurt here, I'll\n> > add one (but it shouldn't make a big difference either, given that\n> > POLLNVAL shouldn't occur at all in the normal case).\n> > \n> >>> +\t\t\t}\n> >>> +\n> >>> +\t\t\tif (pfd.revents & event.events)\n> >>> +\t\t\t\tnotifier->activated.emit(notifier);\n> >>> +\t\t}\n> >>> +\t}\n> >>> +}\n\n[snip]\n\n> >>> diff --git a/src/libcamera/include/event_dispatcher_poll.h\n> >>> b/src/libcamera/include/event_dispatcher_poll.h new file mode 100644\n> >>> index 000000000000..600289e455e2\n> >>> --- /dev/null\n> >>> +++ b/src/libcamera/include/event_dispatcher_poll.h\n> >>> @@ -0,0 +1,50 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2019, Google Inc.\n> >>> + *\n> >>> + * event_loop.h - Camera object interface\n> >> \n> >> Camera object?\n> > \n> > Can you guess where this was copied from ? :-)\n> > \n> > I've now fixed it (as well as the file name).\n> > \n> >>> + */\n> >>> +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> >>> +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> >>> +\n> >>> +#include <libcamera/event_dispatcher.h>\n> >>> +\n> >>> +#include <list>\n> >>> +#include <map>\n> >>> +#include <vector>\n> >>> +\n> >>> +namespace libcamera {\n> >>> +\n> >>> +class EventNotifier;\n> >>> +class Timer;\n> >> \n> >> Still a bit skeptical about forward declarations when an include would\n> >> suffice, but that's up to you actually.\n> > \n> > On the plus side, they speed up compilation and avoid potential future\n> > circular inclusions. Do they cause issues ?\n> \n> Not particularly, as it is unlikely we miss recompiling this file if\n> the header change as the library is built together.\n> \n> Thanks for the multiple clarifications.\n> It is my opinion you should push the next iteration of the series.\n\nThank you.\n\n[snip]","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 73F1360B0B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Jan 2019 15:17:30 +0100 (CET)","from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02871586;\n\tTue,  8 Jan 2019 15:17:29 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546957050;\n\tbh=8GMwPgTL9xZzuiRhAAWZl9mdR5A5WSm9aqhaaUvfPsI=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=Y3HUEq6NSz683Fu51XH3qAsCXmprCAmHv0Pi4RSe7hyt8Wtv0lpeGvC1aFlzVQ1Hu\n\tLWxskq75JymFsgM2ylOvI+cPl4Mxwca4IfU+IEsCLV5IjXOGcvDN6EKqPOVqLA/saU\n\tKMHVJTGiVPE1cGW60yoOX8v8W/FR4OSXOVhcNv7s=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 08 Jan 2019 16:18:38 +0200","Message-ID":"<1829657.3vXFe4VsiG@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190108123241.luxquqwajin7rcnr@uno.localdomain>","References":"<20190107231151.23291-1-laurent.pinchart@ideasonboard.com>\n\t<1607976.1UoNfe8uBB@avalon>\n\t<20190108123241.luxquqwajin7rcnr@uno.localdomain>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: Add a poll-based\n\tevent dispatcher","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":"Tue, 08 Jan 2019 14:17:30 -0000"}}]