[libcamera-devel] [PATCH 01/21] qcam: Remove custom event dispatcher
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Mar 23 15:33:45 CET 2020
Hi Laurent,
On 23/03/2020 14:21, Laurent Pinchart wrote:
> The qcam application installs a custom event dispatcher based on the Qt
> event loop. As the camera manager now creates an internal thread, it
> doesn't use that event dispatcher of the application thread at all.
>
> Furthermore, the custom event dispatcher is buggy, as it doesn't
> dispatch messages posted to the main thread's event loop. This isn't an
> issue as no messages are posted there in the first place, but would
> cause incorrect behaviour if we were to use that feature (for instance
> to deliver signals from the camera manager thread to the application
> thread).
(for topics such as hotplug notifications ...)
> Fixing the event dispatcher requires a change in the libcamera public
> API, as there's currently no way to dispatch messages using the public
> API (Thread::dispatchMessages() is not exposed). This isn't worth it at
> the moment, so just remove the custom event dispatcher. If qcam later
> needs the libcamera request and buffer completion signals to be
> delivered in the application thread, it will need to handle that
> internally, using Qt's cross-thread signal delivery.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Code removal is always good right? :-)
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/qcam/main.cpp | 3 -
> src/qcam/meson.build | 1 -
> src/qcam/qt_event_dispatcher.cpp | 152 -------------------------------
> src/qcam/qt_event_dispatcher.h | 62 -------------
> 4 files changed, 218 deletions(-)
> delete mode 100644 src/qcam/qt_event_dispatcher.cpp
> delete mode 100644 src/qcam/qt_event_dispatcher.h
>
> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> index a7ff5c52663b..297453914ae9 100644
> --- a/src/qcam/main.cpp
> +++ b/src/qcam/main.cpp
> @@ -15,7 +15,6 @@
>
> #include "main_window.h"
> #include "../cam/options.h"
> -#include "qt_event_dispatcher.h"
>
> void signalHandler(int signal)
> {
> @@ -62,9 +61,7 @@ int main(int argc, char **argv)
> sa.sa_handler = &signalHandler;
> sigaction(SIGINT, &sa, nullptr);
>
> - std::unique_ptr<EventDispatcher> dispatcher(new QtEventDispatcher());
> CameraManager *cm = new CameraManager();
> - cm->setEventDispatcher(std::move(dispatcher));
>
> ret = cm->start();
> if (ret) {
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 5150631b55c8..214bfb12aabb 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -3,7 +3,6 @@ qcam_sources = files([
> 'main.cpp',
> 'main_window.cpp',
> '../cam/options.cpp',
> - 'qt_event_dispatcher.cpp',
> 'viewfinder.cpp',
> ])
>
> diff --git a/src/qcam/qt_event_dispatcher.cpp b/src/qcam/qt_event_dispatcher.cpp
> deleted file mode 100644
> index 2780c9123ac3..000000000000
> --- a/src/qcam/qt_event_dispatcher.cpp
> +++ /dev/null
> @@ -1,152 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * qt_event_dispatcher.cpp - qcam - Qt-based event dispatcher
> - */
> -
> -#include <chrono>
> -#include <iostream>
> -
> -#include <QAbstractEventDispatcher>
> -#include <QCoreApplication>
> -#include <QSocketNotifier>
> -#include <QTimerEvent>
> -
> -#include <libcamera/event_notifier.h>
> -#include <libcamera/timer.h>
> -
> -#include "qt_event_dispatcher.h"
> -
> -using namespace libcamera;
> -
> -QtEventDispatcher::QtEventDispatcher()
> -{
> -}
> -
> -QtEventDispatcher::~QtEventDispatcher()
> -{
> - for (auto &it : notifiers_) {
> - NotifierSet &set = it.second;
> - delete set.read.qnotifier;
> - delete set.write.qnotifier;
> - delete set.exception.qnotifier;
> - }
> -}
> -
> -void QtEventDispatcher::registerEventNotifier(EventNotifier *notifier)
> -{
> - NotifierSet &set = notifiers_[notifier->fd()];
> - QSocketNotifier::Type qtype;
> - void (QtEventDispatcher::*method)(int);
> - NotifierPair *pair;
> -
> - switch (notifier->type()) {
> - case EventNotifier::Read:
> - default:
> - qtype = QSocketNotifier::Read;
> - method = &QtEventDispatcher::readNotifierActivated;
> - pair = &set.read;
> - break;
> -
> - case EventNotifier::Write:
> - qtype = QSocketNotifier::Write;
> - method = &QtEventDispatcher::writeNotifierActivated;
> - pair = &set.write;
> - break;
> -
> - case EventNotifier::Exception:
> - qtype = QSocketNotifier::Exception;
> - method = &QtEventDispatcher::exceptionNotifierActivated;
> - pair = &set.exception;
> - break;
> - }
> -
> - QSocketNotifier *qnotifier = new QSocketNotifier(notifier->fd(), qtype);
> - connect(qnotifier, &QSocketNotifier::activated, this, method);
> - pair->notifier = notifier;
> - pair->qnotifier = qnotifier;
> -}
> -
> -void QtEventDispatcher::unregisterEventNotifier(EventNotifier *notifier)
> -{
> - NotifierSet &set = notifiers_[notifier->fd()];
> - NotifierPair *pair;
> -
> - switch (notifier->type()) {
> - case EventNotifier::Read:
> - default:
> - pair = &set.read;
> - break;
> -
> - case EventNotifier::Write:
> - pair = &set.write;
> - break;
> -
> - case EventNotifier::Exception:
> - pair = &set.exception;
> - break;
> - }
> -
> - delete pair->qnotifier;
> - pair->qnotifier = nullptr;
> - pair->notifier = nullptr;
> -}
> -
> -void QtEventDispatcher::readNotifierActivated(int socket)
> -{
> - EventNotifier *notifier = notifiers_[socket].read.notifier;
> - notifier->activated.emit(notifier);
> -}
> -
> -void QtEventDispatcher::writeNotifierActivated(int socket)
> -{
> - EventNotifier *notifier = notifiers_[socket].write.notifier;
> - notifier->activated.emit(notifier);
> -}
> -
> -void QtEventDispatcher::exceptionNotifierActivated(int socket)
> -{
> - EventNotifier *notifier = notifiers_[socket].exception.notifier;
> - notifier->activated.emit(notifier);
> -}
> -
> -void QtEventDispatcher::registerTimer(Timer *timer)
> -{
> - std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
> - std::chrono::steady_clock::duration duration = timer->deadline() - now;
> - std::chrono::milliseconds msec =
> - std::chrono::duration_cast<std::chrono::milliseconds>(duration);
> - int timerId = startTimer(msec.count());
> - timers_[timerId] = timer;
> - timerIds_[timer] = timerId;
> -}
> -
> -void QtEventDispatcher::unregisterTimer(Timer *timer)
> -{
> - auto it = timerIds_.find(timer);
> - if (it == timerIds_.end())
> - return;
> -
> - timers_.erase(it->second);
> - killTimer(it->second);
> - timerIds_.erase(it);
> -}
> -
> -void QtEventDispatcher::timerEvent(QTimerEvent *event)
> -{
> - Timer *timer = timers_[event->timerId()];
> - timer->stop();
> - timer->timeout.emit(timer);
> -}
> -
> -void QtEventDispatcher::processEvents()
> -{
> - std::cout << "QtEventDispatcher::processEvents() should not be called"
> - << std::endl;
> -}
> -
> -void QtEventDispatcher::interrupt()
> -{
> - QCoreApplication::eventDispatcher()->interrupt();
> -}
> diff --git a/src/qcam/qt_event_dispatcher.h b/src/qcam/qt_event_dispatcher.h
> deleted file mode 100644
> index b0f123e52d06..000000000000
> --- a/src/qcam/qt_event_dispatcher.h
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * qt_event_dispatcher.h - qcam - Qt-based event dispatcher
> - */
> -#ifndef __QCAM_QT_EVENT_DISPATCHER_H__
> -#define __QCAM_QT_EVENT_DISPATCHER_H__
> -
> -#include <map>
> -
> -#include <libcamera/event_dispatcher.h>
> -
> -using namespace libcamera;
> -
> -class QSocketNotifier;
> -
> -class QtEventDispatcher final : public EventDispatcher, public QObject
> -{
> -public:
> - QtEventDispatcher();
> - ~QtEventDispatcher();
> -
> - void registerEventNotifier(EventNotifier *notifier);
> - void unregisterEventNotifier(EventNotifier *notifier);
> -
> - void registerTimer(Timer *timer);
> - void unregisterTimer(Timer *timer);
> -
> - void processEvents();
> -
> - void interrupt();
> -
> -protected:
> - void timerEvent(QTimerEvent *event);
> -
> -private:
> - void readNotifierActivated(int socket);
> - void writeNotifierActivated(int socket);
> - void exceptionNotifierActivated(int socket);
> -
> - struct NotifierPair {
> - NotifierPair()
> - : notifier(nullptr), qnotifier(nullptr)
> - {
> - }
> - EventNotifier *notifier;
> - QSocketNotifier *qnotifier;
> - };
> -
> - struct NotifierSet {
> - NotifierPair read;
> - NotifierPair write;
> - NotifierPair exception;
> - };
> -
> - std::map<int, NotifierSet> notifiers_;
> - std::map<int, Timer *> timers_;
> - std::map<Timer *, int> timerIds_;
> -};
> -
> -#endif /* __QCAM_QT_EVENT_DISPATCHER_H__ */
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list