[libcamera-devel] [PATCH 03/18] libcamera: thread: Support dispatching messages to main thread

[Jacopo Mondi]

Hi Laurent,

On Mon, Aug 12, 2019 at 03:46:27PM +0300, Laurent Pinchart wrote:
> Threads contain message queues and dispatch queue messages in their
s/dispatch queue messages/dispatch messages/ ?

> event loop, in Thread::exec(). This mechanism prevents the main thread
> from dispatching messages as it doesn't run Thread::exec().
>
> Fix this by moving message dispatching from Thread::exec() to
> EventDispatcher::processEvents().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/event_dispatcher_poll.cpp | 3 +++
>  src/libcamera/include/thread.h          | 3 ++-
>  src/libcamera/thread.cpp                | 4 +---
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/event_dispatcher_poll.cpp b/src/libcamera/event_dispatcher_poll.cpp
> index 4f15f3e3269b..281f37bdbb16 100644
> --- a/src/libcamera/event_dispatcher_poll.cpp
> +++ b/src/libcamera/event_dispatcher_poll.cpp
> @@ -19,6 +19,7 @@
>  #include <libcamera/timer.h>
>
>  #include "log.h"
> +#include "thread.h"
>
>  /**
>   * \file event_dispatcher_poll.h
> @@ -143,6 +144,8 @@ void EventDispatcherPoll::processEvents()
>  {
>  	int ret;
>
> +	Thread::current()->dispatchMessages();

I understand the reason bheing this patch, but this requires all event
dispatchers to call this in order to have messages processed.

I wonder if this should not go in EventDispatcher base class operation,
so that is executed for all subclasses automatically, and have the
base class operation call a pure virtual _processEvents.

Something like

void EventDispatcher::processEvents()
{
	Thread::current()->dispatchMessages();
        _processEvents();
}

True that right now EventDispatcher is a pure virtual base class, with
a non-pure virtual operation it might get instantiated, something I
assume we don't want at all...

> +
>  	/* Create the pollfd array. */
>  	std::vector<struct pollfd> pollfds;
>  	pollfds.reserve(notifiers_.size() + 1);
> diff --git a/src/libcamera/include/thread.h b/src/libcamera/include/thread.h
> index acae91cb6457..630abb49534f 100644
> --- a/src/libcamera/include/thread.h
> +++ b/src/libcamera/include/thread.h
> @@ -43,6 +43,8 @@ public:
>  	EventDispatcher *eventDispatcher();
>  	void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
>
> +	void dispatchMessages();
> +
>  protected:
>  	int exec();
>  	virtual void run();
> @@ -53,7 +55,6 @@ private:
>
>  	void postMessage(std::unique_ptr<Message> msg, Object *receiver);
>  	void removeMessages(Object *receiver);
> -	void dispatchMessages();
>
>  	friend class Object;
>  	friend class ThreadData;
> diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp
> index 6f86e4a90a05..24422f7b7bd0 100644
> --- a/src/libcamera/thread.cpp
> +++ b/src/libcamera/thread.cpp
> @@ -212,10 +212,8 @@ int Thread::exec()
>
>  	locker.unlock();
>
> -	while (!data_->exit_.load(std::memory_order_acquire)) {
> -		dispatchMessages();
> +	while (!data_->exit_.load(std::memory_order_acquire))
>  		dispatcher->processEvents();
> -	}
>
>  	locker.lock();
>
> --
> 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/20190815/c0dcacf2/attachment.sig>