[libcamera-devel] [PATCH 3/6] libcamera: signal: Support cross-thread signals

Niklas Söderlund niklas.soderlund at ragnatech.se
Thu Jul 11 07:39:49 CEST 2019


Hi Laurent,

Thanks for your patch.

On 2019-07-10 22:17:05 +0300, Laurent Pinchart wrote:
> Allow signals to cross thread boundaries by posting them to the
> recipient through messages instead of calling the slot directly when the
> recipient lives in a different thread.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/signal.h      | 70 +++++++++++++++++++++++++++++----
>  src/libcamera/include/message.h | 14 +++++++
>  src/libcamera/message.cpp       | 22 +++++++++++
>  src/libcamera/object.cpp        | 15 +++++++
>  src/libcamera/signal.cpp        | 26 ++++++++++++
>  5 files changed, 139 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> index c8f3243eaf5a..04e8fe7eebe3 100644
> --- a/include/libcamera/signal.h
> +++ b/include/libcamera/signal.h
> @@ -8,6 +8,8 @@
>  #define __LIBCAMERA_SIGNAL_H__
>  
>  #include <list>
> +#include <tuple>
> +#include <type_traits>
>  #include <vector>
>  
>  #include <libcamera/object.h>
> @@ -16,6 +18,9 @@ namespace libcamera {
>  
>  template<typename... Args>
>  class Signal;
> +class SlotBase;
> +template<typename... Args>
> +class SlotArgs;

These forward declarations  don't seems to be needed right?

With this addressed,

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

>  
>  class SlotBase
>  {
> @@ -27,6 +32,9 @@ public:
>  	void *obj() { return obj_; }
>  	bool isObject() const { return isObject_; }
>  
> +	void activatePack(void *pack);
> +	virtual void invokePack(void *pack) = 0;
> +
>  protected:
>  	void *obj_;
>  	bool isObject_;
> @@ -35,24 +43,70 @@ protected:
>  template<typename... Args>
>  class SlotArgs : public SlotBase
>  {
> +private:
> +#ifndef __DOXYGEN__
> +	/*
> +	 * This is a cheap partial implementation of std::integer_sequence<>
> +	 * from C++14.
> +	 */
> +	template<int...>
> +	struct sequence {
> +	};
> +
> +	template<int N, int... S>
> +	struct generator : generator<N-1, N-1, S...> {
> +	};
> +
> +	template<int... S>
> +	struct generator<0, S...> {
> +		typedef sequence<S...> type;
> +	};
> +#endif
> +
>  public:
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
>  	SlotArgs(void *obj, bool isObject)
>  		: SlotBase(obj, isObject) {}
>  
> +	void invokePack(void *pack) override
> +	{
> +		invokePack(pack, typename generator<sizeof...(Args)>::type());
> +	}
> +
> +	template<int... S>
> +	void invokePack(void *pack, sequence<S...>)
> +	{
> +		PackType *args = static_cast<PackType *>(pack);
> +		invoke(std::get<S>(*args)...);
> +		delete args;
> +	}
> +
> +	virtual void activate(Args... args) = 0;
>  	virtual void invoke(Args... args) = 0;
> -
> -protected:
> -	friend class Signal<Args...>;
>  };
>  
>  template<typename T, typename... Args>
>  class SlotMember : public SlotArgs<Args...>
>  {
>  public:
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
>  	SlotMember(T *obj, bool isObject, void (T::*func)(Args...))
>  		: SlotArgs<Args...>(obj, isObject), func_(func) {}
>  
> -	void invoke(Args... args) { (static_cast<T *>(this->obj_)->*func_)(args...); }
> +	void activate(Args... args)
> +	{
> +		if (this->isObject_)
> +			SlotBase::activatePack(new PackType{ args... });
> +		else
> +			(static_cast<T *>(this->obj_)->*func_)(args...);
> +	}
> +
> +	void invoke(Args... args)
> +	{
> +		(static_cast<T *>(this->obj_)->*func_)(args...);
> +	}
>  
>  private:
>  	friend class Signal<Args...>;
> @@ -66,7 +120,8 @@ public:
>  	SlotStatic(void (*func)(Args...))
>  		: SlotArgs<Args...>(nullptr, false), func_(func) {}
>  
> -	void invoke(Args... args) { (*func_)(args...); }
> +	void activate(Args... args) { (*func_)(args...); }
> +	void invoke(Args... args) {}
>  
>  private:
>  	friend class Signal<Args...>;
> @@ -186,9 +241,8 @@ public:
>  		 * disconnect operation, invalidating the iterator.
>  		 */
>  		std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() };
> -		for (SlotBase *slot : slots) {
> -			static_cast<SlotArgs<Args...> *>(slot)->invoke(args...);
> -		}
> +		for (SlotBase *slot : slots)
> +			static_cast<SlotArgs<Args...> *>(slot)->activate(args...);
>  	}
>  };
>  
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index 97c9b80ec0e0..db17d647c280 100644
> --- a/src/libcamera/include/message.h
> +++ b/src/libcamera/include/message.h
> @@ -10,6 +10,7 @@
>  namespace libcamera {
>  
>  class Object;
> +class SlotBase;
>  class Thread;
>  
>  class Message
> @@ -17,6 +18,7 @@ class Message
>  public:
>  	enum Type {
>  		None = 0,
> +		SignalMessage = 1,
>  	};
>  
>  	Message(Type type);
> @@ -32,6 +34,18 @@ private:
>  	Object *receiver_;
>  };
>  
> +class SignalMessage : public Message
> +{
> +public:
> +	SignalMessage(SlotBase *slot, void *pack)
> +		: Message(Message::SignalMessage), slot_(slot), pack_(pack)
> +	{
> +	}
> +
> +	SlotBase *slot_;
> +	void *pack_;
> +};
> +
>  } /* namespace libcamera */
>  
>  #endif /* __LIBCAMERA_MESSAGE_H__ */
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index 47caf44dc82d..66dd1e8bd618 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -68,4 +68,26 @@ Message::~Message()
>   * \return The message receiver
>   */
>  
> +/**
> + * \class SignalMessage
> + * \brief A message carrying a Signal across threads
> + */
> +
> +/**
> + * \fn SignalMessage::SignalMessage()
> + * \brief Construct a SignalMessage
> + * \param[in] slot The slot that the signal targets
> + * \param[in] pack The signal arguments
> + */
> +
> +/**
> + * \var SignalMessage::slot_
> + * \brief The slot that the signal targets
> + */
> +
> +/**
> + * \var SignalMessage::pack_
> + * \brief The signal arguments
> + */
> +
>  }; /* namespace libcamera */
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index 695e6c11b3a4..1dfa159267fe 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -10,6 +10,7 @@
>  #include <libcamera/signal.h>
>  
>  #include "log.h"
> +#include "message.h"
>  #include "thread.h"
>  
>  /**
> @@ -32,6 +33,10 @@ namespace libcamera {
>   * This allows implementing easy message passing between threads by inheriting
>   * from the Object class.
>   *
> + * Object slots connected to signals will also run in the context of the
> + * object's thread, regardless of whether the signal is emitted in the same or
> + * in another thread.
> + *
>   * \sa Message, Signal, Thread
>   */
>  
> @@ -82,6 +87,16 @@ void Object::postMessage(std::unique_ptr<Message> msg)
>   */
>  void Object::message(Message *msg)
>  {
> +	switch (msg->type()) {
> +	case Message::SignalMessage: {
> +		SignalMessage *smsg = static_cast<SignalMessage *>(msg);
> +		smsg->slot_->invokePack(smsg->pack_);
> +		break;
> +	}
> +
> +	default:
> +		break;
> +	}
>  }
>  
>  /**
> diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
> index 4cb85ecb0686..53c18535fee3 100644
> --- a/src/libcamera/signal.cpp
> +++ b/src/libcamera/signal.cpp
> @@ -7,6 +7,10 @@
>  
>  #include <libcamera/signal.h>
>  
> +#include "message.h"
> +#include "thread.h"
> +#include "utils.h"
> +
>  /**
>   * \file signal.h
>   * \brief Signal & slot implementation
> @@ -42,8 +46,30 @@ namespace libcamera {
>   * to the same slot. Duplicate connections between a signal and a slot are
>   * allowed and result in the slot being called multiple times for the same
>   * signal emission.
> + *
> + * When a slot belongs to an instance of the Object class, the slot is called
> + * in the context of the thread that the object is bound to. If the signal is
> + * emitted from the same thread, the slot will be called synchronously, before
> + * Signal::emit() returns. If the signal is emitted from a different thread,
> + * the slot will be called asynchronously from the object's thread's event
> + * loop, after the Signal::emit() method returns, with a copy of the signal's
> + * arguments. The emitter shall thus ensure that any pointer or reference
> + * passed through the signal will remain valid after the signal is emitted.
>   */
>  
> +void SlotBase::activatePack(void *pack)
> +{
> +	Object *obj = static_cast<Object *>(obj_);
> +
> +	if (Thread::current() == obj->thread()) {
> +		invokePack(pack);
> +	} else {
> +		std::unique_ptr<Message> msg =
> +			utils::make_unique<SignalMessage>(this, pack);
> +		obj->postMessage(std::move(msg));
> +	}
> +}
> +
>  /**
>   * \fn Signal::connect(T *object, void(T::*func)(Args...))
>   * \brief Connect the signal to a member function slot
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list