[libcamera-devel] [PATCH 06/11] libcamera: Add event notification infrastructure
Jacopo Mondi
jacopo at jmondi.org
Mon Jan 7 19:14:21 CET 2019
Hi Laurent,
wow, this is a lot to process... let's try...
On Sun, Jan 06, 2019 at 04:33:23AM +0200, Laurent Pinchart wrote:
> Add three new classes, EventDispatcher, EventNotifier and Timer, that
> define APIs for file descriptor event notification and timers. The
> implementation of the EventDispatcher is meant to be provided to
> libcamera by the application.
>
> The event dispatcher is integrated twith the camera manager to implement
> automatic registration of timers and events.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/camera_manager.h | 7 ++
> include/libcamera/event_dispatcher.h | 33 ++++++++
> include/libcamera/event_notifier.h | 42 ++++++++++
> include/libcamera/libcamera.h | 1 +
> include/libcamera/meson.build | 3 +
> include/libcamera/timer.h | 37 +++++++++
> src/libcamera/camera_manager.cpp | 42 +++++++++-
> src/libcamera/event_dispatcher.cpp | 102 +++++++++++++++++++++++
> src/libcamera/event_notifier.cpp | 118 +++++++++++++++++++++++++++
> src/libcamera/meson.build | 3 +
> src/libcamera/timer.cpp | 105 ++++++++++++++++++++++++
> 11 files changed, 492 insertions(+), 1 deletion(-)
> create mode 100644 include/libcamera/event_dispatcher.h
> create mode 100644 include/libcamera/event_notifier.h
> create mode 100644 include/libcamera/timer.h
> create mode 100644 src/libcamera/event_dispatcher.cpp
> create mode 100644 src/libcamera/event_notifier.cpp
> create mode 100644 src/libcamera/timer.cpp
>
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 56a3f32d8b6f..cb6a6d084a4a 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -14,6 +14,7 @@ namespace libcamera {
>
> class Camera;
> class DeviceEnumerator;
> +class EventDispatcher;
> class PipelineHandler;
>
> class CameraManager
> @@ -27,11 +28,17 @@ public:
>
> static CameraManager *instance();
>
> + void setEventDispatcher(EventDispatcher *dispatcher);
> + EventDispatcher *eventDispatcher();
> +
> private:
> CameraManager();
> + ~CameraManager();
>
> DeviceEnumerator *enumerator_;
> std::vector<PipelineHandler *> pipes_;
> +
> + EventDispatcher *dispatcher_;
> };
>
> } /* namespace libcamera */
> diff --git a/include/libcamera/event_dispatcher.h b/include/libcamera/event_dispatcher.h
> new file mode 100644
> index 000000000000..c20518c6866d
> --- /dev/null
> +++ b/include/libcamera/event_dispatcher.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_dispatcher.h - Event dispatcher
> + */
> +#ifndef __LIBCAMERA_EVENT_DISPATCHER_H__
> +#define __LIBCAMERA_EVENT_DISPATCHER_H__
> +
> +#include <vector>
> +
> +namespace libcamera {
> +
> +class EventNotifier;
> +class Timer;
Any reason why forward declaring instead of including headers? afaict
there is no risk of circular inclusion...
> +
> +class EventDispatcher
> +{
> +public:
> + virtual ~EventDispatcher();
> +
> + virtual void registerEventNotifier(EventNotifier *notifier) = 0;
> + virtual void unregisterEventNotifier(EventNotifier *notifier) = 0;
> +
> + virtual void registerTimer(Timer *timer) = 0;
> + virtual void unregisterTimer(Timer *timer) = 0;
> +
> + virtual void processEvents() = 0;
Care to clarify me how you see the processEvents() function will be integrated
in the application event loop?
The application should call into this to trigger processing of new
events, and according to the below documentation, this call is expected
to be blocking (for the poll based dispatcher, if no timer is active,
it surely is).
Do we expect application to deal with this running this in a dedicated
thread? Should we take care of that and perform asynchronous event
dispatching?
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_EVENT_DISPATCHER_H__ */
> diff --git a/include/libcamera/event_notifier.h b/include/libcamera/event_notifier.h
> new file mode 100644
> index 000000000000..278a5ff6f9e5
> --- /dev/null
> +++ b/include/libcamera/event_notifier.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_notifer.h - Event notifier
> + */
> +#ifndef __LIBCAMERA_EVENT_NOTIFIER_H__
> +#define __LIBCAMERA_EVENT_NOTIFIER_H__
> +
> +#include <libcamera/signal.h>
> +
> +namespace libcamera {
> +
> +class EventNotifier
> +{
> +public:
> + enum Type {
> + Read,
> + Write,
> + Exception,
> + };
> +
> + EventNotifier(int fd, Type type);
> + virtual ~EventNotifier();
> +
> + Type type() const { return type_; }
> + int fd() const { return fd_; }
> +
> + bool isEnabled() const { return enabled_; }
We have used the 'enabled()' syntax so far for other getters in the
library. Up to you.
> + void setEnabled(bool enable);
> +
> + Signal<EventNotifier *> activated;
> +
> +private:
> + int fd_;
> + Type type_;
> + bool enabled_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_EVENT_NOTIFIER_H__ */
> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h
> index f9556a8bce62..785babefa1c8 100644
> --- a/include/libcamera/libcamera.h
> +++ b/include/libcamera/libcamera.h
> @@ -9,5 +9,6 @@
>
> #include <libcamera/camera.h>
> #include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
>
> #endif /* __LIBCAMERA_LIBCAMERA_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 6f87689ea528..d7cb55ba4a49 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -1,8 +1,11 @@
> libcamera_api = files([
> 'camera.h',
> 'camera_manager.h',
> + 'event_dispatcher.h',
> + 'event_notifier.h',
> 'libcamera.h',
> 'signal.h',
> + 'timer.h',
> ])
>
> install_headers(libcamera_api,
> diff --git a/include/libcamera/timer.h b/include/libcamera/timer.h
> new file mode 100644
> index 000000000000..97dcc01f493d
> --- /dev/null
> +++ b/include/libcamera/timer.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * timer.h - Generic timer
> + */
> +#ifndef __LIBCAMERA_TIMER_H__
> +#define __LIBCAMERA_TIMER_H__
> +
> +#include <cstdint>
> +
> +#include <libcamera/signal.h>
> +
> +namespace libcamera {
> +
> +class Timer
> +{
> +public:
> + Timer();
> +
> + void start(unsigned int msec);
> + void stop();
> + bool isRunning() const;
> +
> + unsigned int interval() const { return interval_; }
> + uint64_t deadline() const { return deadline_; }
> +
> + Signal<Timer *> timeout;
> +
> +private:
> + unsigned int interval_;
> + uint64_t deadline_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_TIMER_H__ */
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index db2bc4b7424e..0dbf31f47039 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -6,8 +6,10 @@
> */
>
> #include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
>
> #include "device_enumerator.h"
> +#include "log.h"
> #include "pipeline_handler.h"
>
> /**
> @@ -37,10 +39,15 @@
> namespace libcamera {
>
> CameraManager::CameraManager()
> - : enumerator_(nullptr)
> + : enumerator_(nullptr), dispatcher_(nullptr)
> {
> }
>
> +CameraManager::~CameraManager()
> +{
> + delete dispatcher_;
> +}
> +
> /**
> * \brief Start the camera manager
> *
> @@ -176,4 +183,37 @@ CameraManager *CameraManager::instance()
> return &manager;
> }
>
> +/**
> + * \brief Set the event dispatcher
> + * \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.
> + *
> + * The CameraManager takes ownership of the event dispatcher and will delete it
> + * when the application terminates.
> + */
> +void CameraManager::setEventDispatcher(EventDispatcher *dispatcher)
> +{
> + if (dispatcher_) {
> + LOG(Warning) << "Event dispatcher is already set";
> + return;
> + }
> +
> + dispatcher_ = dispatcher;
> +}
> +
> +/**
> + * \fn CameraManager::eventDispatcher()
> + * \brief Retrieve the event dispatcher
> + * \return Pointer to the event dispatcher, or nullptr if no event dispatcher
> + * has been set
> + */
> +EventDispatcher *CameraManager::eventDispatcher()
> +{
> + return dispatcher_;
> +}
> +
> } /* namespace libcamera */
> diff --git a/src/libcamera/event_dispatcher.cpp b/src/libcamera/event_dispatcher.cpp
> new file mode 100644
> index 000000000000..61013f218887
> --- /dev/null
> +++ b/src/libcamera/event_dispatcher.cpp
> @@ -0,0 +1,102 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_dispatcher.cpp - Event dispatcher
> + */
> +
> +#include <libcamera/event_dispatcher.h>
> +
> +/**
> + * \file event_dispatcher.h
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class EventDispatcher
> + * \brief Interface to manage the libcamera events and timers
> + *
> + * The EventDispatcher class allows the integration of the application event
> + * loop with libcamera by abstracting how events and timers are managed and
> + * processed.
> + *
> + * To listen to events, libcamera creates EventNotifier instances and register
> + * them with the dispatcher with registerEventNotifier(). The event notifier
> + * \ref EventNotifier::activated signal is then emitted by the dispatcher
> + * whenever the event is detected.
> + *
> + * To set timers, libcamera creates Timer instances and register them with the
> + * dispatcher with registerTimer(). The timer \ref Timer::timeout signal is then
> + * emitted by the dispatcher when the timer times out.
> + */
> +
> +EventDispatcher::~EventDispatcher()
> +{
> +}
> +
> +/**
> + * \fn EventDispatcher::registerEventNotifier()
> + * \brief Register an event notifier
> + * \param notifier The event notifier to register
> + *
> + * Once the \a notifier is registered with the dispatcher, the dispatcher will
> + * emit the notifier \ref EventNotifier::activated signal whenever a
> + * corresponding event is detected on the notifier's file descriptor. The event
> + * is monitored until the notifier is unregistered with
> + * unregisterEventNotifier().
> + *
> + * Registering multiple notifiers for the same file descriptor and event type is
> + * not allowed.
> + */
> +
> +/**
> + * \fn EventDispatcher::unregisterEventNotifier()
> + * \brief Unregister an event notifier
> + * \param notifier The event notifier to unregister
> + *
> + * After this function returns the \a notifier is guaranteed not to emit the
> + * \ref EventNotifier::activated signal.
> + *
> + * If the notifier isn't registered, this function performs no operation.
> + */
> +
> +/**
> + * \fn EventDispatcher::registerTimer()
> + * \brief Register a timer
> + * \param timer The timer to register
> + *
> + * Once the \a timer is registered with the dispatcher, the dispatcher will emit
> + * the timer \ref Timer::timeout signal when the timer times out. The timer can
> + * be unregistered with unregisterTimer() before it times out, in which case the
> + * signal will not be emitted.
> + *
> + * When the \a timer times out, it is automatically unregistered by the
> + * dispatcher and can be registered back as early as from the \ref Timer::timeout
Is 'back as early as' intentional?
> + * signal handlers.
> + *
> + * Registering the same timer multiple times is not allowed.
> + */
> +
> +/**
> + * \fn EventDispatcher::unregisterTimer()
> + * \brief Unregister a timer
> + * \param timer The timer to unregister
> + *
> + * After this function returns the \a timer is guaranteed not to emit the
> + * \ref Timer::timeout signal.
> + *
> + * If the timer isn't registered, this function performs no operation.
> + */
> +
> +/**
> + * \fn EventDispatcher::processEvents()
> + * \brief Wait for and process pending events
> + *
> + * This function processes all pending events associated with registered event
> + * notifiers and timers and signals the corresponding EventNotifier and Timer
> + * objects. If no events are pending, it waits for the first event and processes
> + * it before returning.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/event_notifier.cpp b/src/libcamera/event_notifier.cpp
> new file mode 100644
> index 000000000000..edaf66725dc5
> --- /dev/null
> +++ b/src/libcamera/event_notifier.cpp
> @@ -0,0 +1,118 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * event_notifier.cpp - Event notifier
This is actually a 'file events' notifier, it's probably worth
mentioning it somewhere
> + */
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/event_notifier.h>
> +
> +/**
> + * \file event_notifier.h
> + * \brief Event notifier
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class EventNotifier
> + * \brief Notify of activity on a file descriptor
> + *
Ah, right :)
> + * The EventNotifier models a file descriptor event source that can be
> + * monitored. It is created with the file descriptor to be monitored and the
> + * type of event, and is enabled by default. It will emit the \ref activated
> + * signal whenever an event of the monitored type occurs on the file descriptor.
> + *
> + * Supported type of events are EventNotifier::Read, EventNotifier::Write and
> + * EventNotifier::Exception. The type is specified when constructed the
When the notifier is constructed
> + * notifier, and can be retrieved using the type() function. To listen to
> + * multiple event types on the same file descriptor multiple notifiers must be
> + * created.
> + *
> + * The notifier can be disabled with the setEnable() function. When the notifier
> + * is disabled it ignores events and does not emit the \ref activated signal.
> + * The notifier can then be re-enabled with the setEnable() function.
> + *
> + * Notifier events are detected and dispatched from the
> + * EventDispatcher::processEvents() function.
> + */
> +
> +/**
> + * \enum EventNotifier::Type
> + * Type of file descriptor event to listen for.
> + * \var EventNotifier::Read
> + * Data is available to be read from the file descriptor
> + * \var EventNotifier::Write
> + * Data can be written to the file descriptor
> + * \var EventNotifier::Exception
> + * An exception has occurred on the file descriptor
> + */
> +
> +/**
> + * \brief Construct an event notifier with a file descriptor and event type
> + * \param fd The file descriptor to monitor
> + * \param type The event type to monitor
> + */
> +EventNotifier::EventNotifier(int fd, Type type)
> + : fd_(fd), type_(type), enabled_(false)
> +{
> + setEnabled(true);
> +}
> +
> +EventNotifier::~EventNotifier()
> +{
> + setEnabled(false);
> +}
> +
> +/**
> + * \fn EventNotifier::type()
> + * \brief Retrive the type of the event being monitored
> + * \return The type of the event
> + */
> +
> +/**
> + * \fn EventNotifier::fd()
> + * \brief Retrive the file descriptor being monitored
> + * \return The file descriptor
> + */
> +
> +/**
> + * \fn EventNotifier::isEnabled()
> + * \brief Retrieve the notifier state
> + * \return true if the notifier is enabled, or false otherwise
> + * \sa setEnable()
> + */
> +
> +/**
> + * \brief Enable or disable the notifier
> + * \param enable true to enable the notifier, false to disable it
> + *
> + * This function enables or disables the notifier. A disabled notifier ignores
> + * events and does not emit the \ref activated signal.
> + */
> +void EventNotifier::setEnabled(bool enable)
> +{
> + if (enabled_ == enable)
> + return;
> +
> + enabled_ = enable;
> +
> + EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher();
> + if (enable)
> + dispatcher->registerEventNotifier(this);
> + else
> + dispatcher->unregisterEventNotifier(this);
> +}
> +
> +/**
> + * \var EventNotifier::activated
> + * \brief Signal emitted when the event occurs
> + *
> + * This signal is emitted when the event \ref type() occurs on the file
> + * descriptor monitored by the notifier. The notifier pointer is passed as a
> + * parameter.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 3ec86e75b57e..61fb93205b34 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -2,11 +2,14 @@ libcamera_sources = files([
> 'camera.cpp',
> 'camera_manager.cpp',
> 'device_enumerator.cpp',
> + 'event_dispatcher.cpp',
> + 'event_notifier.cpp',
> 'log.cpp',
> 'media_device.cpp',
> 'media_object.cpp',
> 'pipeline_handler.cpp',
> 'signal.cpp',
> + 'timer.cpp',
> ])
>
> libcamera_headers = files([
> diff --git a/src/libcamera/timer.cpp b/src/libcamera/timer.cpp
> new file mode 100644
> index 000000000000..57c49aa2ef36
> --- /dev/null
> +++ b/src/libcamera/timer.cpp
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * timer.cpp - Generic timer
> + */
> +
> +#include <time.h>
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file timer.h
> + * \brief Generic timer
Maybe it is just me not having the Signal/Slot thing clear enough, but
it would help mentioning that library components wanting to use timers
(and even notifiers) should instantiate an instance of this class and
connect the timeout (and activated) signals to the slots they want to be
called upon events.
If this is specified somewhere else you can ignore this. If this is
assumed to be known by how Signal/Slot works, maybe it is worth
specifying it here for people not familiar with the concept.
I'm now thinking how I would do that without Signals/Slots, and I
would expect that I would create a Timer and register a timeout
with a callback to be invoked at the timer expiration, and that's basically
it. This can of course be built with the nice Signal/Slot implementation,
but shouldn't we abstract this away to Timer (and EventNotifier) users
providing a method to register a callback with an associated timeout?
What are the advantages of having signal and slot connections exposed?
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \class Timer
> + * \brief Single-shot timer interface
> + *
> + * The Timer class models a single-shot timer that is started with start() and
> + * emits the \ref timeout signal when it times out.
> + *
> + * Once started the timer will run until it times out. It can be stopped with
> + * stop(), and once it times out or is stopped, can be started again with
> + * start().
> + */
> +
> +/**
> + * \brief Construct a timer
> + */
> +Timer::Timer()
> +{
> +}
> +
> +/**
> + * \brief Start or restart the timer with a timeout of \a msec
> + * \param msec The timer duration in milliseconds
> + *
> + * If the timer is already running it will be stopped and restarted.
> + */
> +void Timer::start(unsigned int msec)
> +{
> + struct timespec tp;
> + clock_gettime(CLOCK_MONOTONIC, &tp);
> +
> + interval_ = msec;
> + deadline_ = tp.tv_sec * 1000000000ULL + tp.tv_nsec + msec * 1000000;
> +
> + LOG(Debug) << "Starting timer " << this << " with interval " << msec
> + << ": deadline " << deadline_;
> +
> + CameraManager::instance()->eventDispatcher()->registerTimer(this);
> +}
> +
> +/**
> + * \brief Stop the timer
> + *
> + * After this function returns the timer is guaranteed not to emit the
> + * \ref timeout signal.
> + *
> + * If the timer is not running this function performs no operation.
> + */
> +void Timer::stop()
> +{
> + CameraManager::instance()->eventDispatcher()->unregisterTimer(this);
> +
> + deadline_ = 0;
> +}
> +
> +/**
> + * \brief
> + * \return true if the timer is running, false otherwise
> + */
> +bool Timer::isRunning() const
> +{
> + return deadline_ != 0;
> +}
> +
> +/**
> + * \fn Timer::interval()
> + * \brief Retrieve the timer interval
> + * \return The timer interval in milliseconds
> + */
> +
> +/**
> + * \fn Timer::deadline()
> + * \brief Retrieve the timer deadline
> + * \return The timer deadline in nanoseconds
> + */
> +
> +/**
> + * \var Timer::timeout
> + * \brief Signal emitted when the timer times out
> + *
> + * The timer pointer is passed as a parameter.
> + */
> +
> +} /* namespace libcamera */
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190107/ac07d4b0/attachment-0001.sig>
More information about the libcamera-devel
mailing list