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

1781 diff mbox series

Threads contain message queues and dispatch queue messages in their
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@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(-)

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@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Laurent,

Thanks for your work.

On 2019-08-12 15:46:27 +0300, Laurent Pinchart wrote:
> Threads contain message queues and dispatch queue messages in their

s/queue/queued/

> 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@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  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();
> +
>  	/* 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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel