[{"id":212,"web_url":"https://patchwork.libcamera.org/comment/212/","msgid":"<20190106162530.GH11987@bigcity.dyn.berto.se>","date":"2019-01-06T16:25:30","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: Add a poll-based\n\tevent dispatcher","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-01-06 04:33:24 +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 \n> ---\n> include/libcamera/libcamera.h | 1 +\n> src/libcamera/camera_manager.cpp | 19 +-\n> src/libcamera/event_dispatcher_poll.cpp | 232 ++++++++++++++++++\n> src/libcamera/include/event_dispatcher_poll.h | 50 ++++\n> src/libcamera/meson.build | 2 +\n> 5 files changed, 299 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 \n> #include \n> #include \n> +#include \n> \n> #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 0dbf31f47039..4475ab1f9db2 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -9,6 +9,7 @@\n> #include \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> + *\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..2c64c1446772\n> --- /dev/null\n> +++ b/src/libcamera/event_dispatcher_poll.cpp\n> @@ -0,0 +1,232 @@\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 \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \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> +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) << \"Duplicate \" << notifierType(type)\n> +\t\t\t << \" notifiers for fd \" << notifier->fd();\n\nI'm not sure I what I think about this :-) Is it wise to allow the \nnotifier to be replaced with just a warning? My gut feeling tells me we \nshould warn and ignore the new notifier instead.\n\nAt least I think we should rephrase the warning message to reflect what \naction is taken. Either log that the old notifier is replaced or that \nthe new one is ignored.\n\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) << \"Duplicate \" << notifierType(type)\n> +\t\t\t << \" notifiers for fd \" << notifier->fd();\n\nI feel the log message should be rephrased, how about\n\n \"Can't unregister a not register \" << notifierType(type) << \" \n notifier for fd \" << notifier->fd()\n\n\nWith these small issues addressed\n\nReviewed-by: Niklas Söderlund \n\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> +void EventDispatcherPoll::processEvents()\n> +{\n> +\tint ret;\n> +\n> +\t/* Create the pollfd array. */\n> +\tstd::vector 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> +\t} else if (ret > 0) {\n> +\t\tprocessNotifiers(pollfds);\n> +\t}\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 &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> +\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> +\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> + */\n> +#ifndef __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> +#define __LIBCAMERA_EVENT_DISPATCHER_POLL_H__\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class EventNotifier;\n> +class Timer;\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> +\t};\n> +\n> +\tstd::map notifiers_;\n> +\tstd::list timers_;\n> +\n> +\tvoid processNotifiers(std::vector &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> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B1F360B2C\n\tfor ;\n\tSun, 6 Jan 2019 17:25:32 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id s5-v6so36119900ljd.12\n\tfor ;\n\tSun, 06 Jan 2019 08:25:32 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tj25-v6sm13325286lji.77.2019.01.06.08.25.30\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 06 Jan 2019 08:25:30 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=PHGPabOs719nKwYV3JIATjmL2VtcLg7zSFKe8Eniqb0=;\n\tb=Xi/H0tkxxaFHEKe5ztTeHNm9c4fxeTNjXkvCauJAWluROw6+rVdkwT3WZUdurx8MFv\n\t8e4SBu55kEmHP7mDs8n4nlUHG9ktYfeW6IlF+xBV4rxfqRt2moByxB549dFG2ua2CJVl\n\tYtdgwHR7RqeGe5tz7sP8OzE6cW9NIcLY164NwBk3Q9sCSbCt6RDHA/BB5bUQh0nIopHj\n\tSqj/b/CX0dP+k/OE2emsaMcCkoVqaz8LewLKs/HEbEQF/iq00L3quDxLptLNV1uJJxll\n\tDw8wGLrXAKOsAJ5Uh4wHDzY39eu8IYrHub7zjAOb+Wjsiq4Go9Pq5fa8APhMyi/Dqsf2\n\tnOtA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=PHGPabOs719nKwYV3JIATjmL2VtcLg7zSFKe8Eniqb0=;\n\tb=ftKJBZYWWpAWp+wzJyMmtkVyStZH3orTbcdOQ+e02z26AYVkPHPOVVoukD226EuDrw\n\tXN47R95TTKrdyJIKaUgZaSOpAawB3OcEcFpjSGl2TlQaX6JVPKlUnnSh6SRoaO5wmBf2\n\tEAtiGVFfggakwJJk2tAH8tuBN0W921nwBcwqFU9A+Yy0MohN6Q7zRNdwD6tqgUNBBVXX\n\tmlDRQIRWOnOegn4a7P+XQ/9WpB1YLcFUoywafON6v2MfyO3B2wdjbM3j1+TkfFJaf3nJ\n\t6QMCAyxnQuvTHwXCMpcj0qVmEWNwUesIt3iSnegJuBBKN0NP2bRzbh+zTe88ogHTYB2c\n\tu+vg==","X-Gm-Message-State":"AJcUukfl4Uq1InW9xKkVHl26AemnXcvYL8elpO1todJh88drnxcP/7Ax\n\tv7rES/g+T5wt9Ll2G9l/oKkTiKllsy8=","X-Google-Smtp-Source":"ALg8bN52PgZtZjEFUW9NqsWWjbA0aj+WA3BoQq/b3E15C9YqWILH2xtuCLaLIehD+RVNUNWoBIhicw==","X-Received":"by 2002:a2e:b00a:: with SMTP id\n\ty10-v6mr16204261ljk.109.1546791931788; \n\tSun, 06 Jan 2019 08:25:31 -0800 (PST)","Date":"Sun, 6 Jan 2019 17:25:30 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190106162530.GH11987@bigcity.dyn.berto.se>","References":"<20190106023328.10989-1-laurent.pinchart@ideasonboard.com>\n\t<20190106023328.10989-7-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190106023328.10989-7-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 06 Jan 2019 16:25:33 -0000"}},{"id":222,"web_url":"https://patchwork.libcamera.org/comment/222/","msgid":"<4762794.5dILsUiNdj@avalon>","date":"2019-01-07T12:55:56","subject":"Re: [libcamera-devel] [PATCH 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 Niklas,\n\nOn Sunday, 6 January 2019 18:25:30 EET Niklas Söderlund wrote:\n> On 2019-01-06 04:33:24 +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 \n> > ---\n> > \n> > include/libcamera/libcamera.h | 1 +\n> > src/libcamera/camera_manager.cpp | 19 +-\n> > src/libcamera/event_dispatcher_poll.cpp | 232 ++++++++++++++++++\n> > src/libcamera/include/event_dispatcher_poll.h | 50 ++++\n> > src/libcamera/meson.build | 2 +\n> > 5 files changed, 299 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..2c64c1446772\n> > --- /dev/null\n> > +++ b/src/libcamera/event_dispatcher_poll.cpp\n\n[snip]\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) << \"Duplicate \" << notifierType(type)\n> > +\t\t\t << \" notifiers for fd \" << notifier->fd();\n> \n> I'm not sure I what I think about this :-) Is it wise to allow the\n> notifier to be replaced with just a warning? My gut feeling tells me we\n> should warn and ignore the new notifier instead.\n\nI'm not sure either. It indicates a problem in either case, which can result \nin incorrect behaviour. I don't know which option would minimize the impact \n(or if we instead want to maximize the impact so that the issue will be fixed \nearlier).\n\nShould the specific behaviour we pick be documented in the EventNotifier \nclass, or be specified as undefined behaviour ?\n\n> At least I think we should rephrase the warning message to reflect what\n> action is taken. Either log that the old notifier is replaced or that\n> the new one is ignored.\n\nAgreed, I'll fix that.\n\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) << \"Duplicate \" << notifierType(type)\n> > +\t\t\t << \" notifiers for fd \" << notifier->fd();\n> \n> I feel the log message should be rephrased, how about\n> \n> \"Can't unregister a not register \" << notifierType(type) << \"\n> notifier for fd \" << notifier->fd()\n\nHow about\n\nnotifierType(type) << \" notifier for fd \" << notifier->fd() << \" not \nregistered\"\n\n?\n\n> With these small issues addressed\n> \n> Reviewed-by: Niklas Söderlund \n> \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[snip]","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBA24600CC\n\tfor ;\n\tMon, 7 Jan 2019 13:54:50 +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 4368AE22;\n\tMon, 7 Jan 2019 13:54:50 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546865690;\n\tbh=C+UygmwpsdimDcOWHkRll8q9Hr/zduVZUfb9r3nPy1Q=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=f1Y9P0+Q6U1kyVZYVhKOwumTCi0VMWhnVxmnYoc0N7sW0edksKFH7EiJkywDICUWu\n\tr3Ab45mAMzxSAklZV692N4xIciGUthRIXsK0wX2mVr7DUcZfY8nGPuxJfHYM4nsgA4\n\twYXymdqTjQccPdHfg8oa6kY0OSoiK/ySOXegWHYQ=","From":"Laurent Pinchart ","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Mon, 07 Jan 2019 14:55:56 +0200","Message-ID":"<4762794.5dILsUiNdj@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20190106162530.GH11987@bigcity.dyn.berto.se>","References":"<20190106023328.10989-1-laurent.pinchart@ideasonboard.com>\n\t<20190106023328.10989-7-laurent.pinchart@ideasonboard.com>\n\t<20190106162530.GH11987@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 07 Jan 2019 12:54:51 -0000"}}]