[libcamera-devel,04/18] libcamera: signal: Split Slot implementation to reusable classes

1782 diff mbox series

Move the Slot* classes to bound_method.{h,cpp} and rename them to
Bound*Method*. They will be reused to implement asynchronous method
invocation similar to cross-thread signal delivery.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/Doxyfile.in        |  10 +--
 include/libcamera/bound_method.h | 131 ++++++++++++++++++++++++++++
 include/libcamera/meson.build    |   1 +
 include/libcamera/object.h       |   5 +-
 include/libcamera/signal.h       | 145 ++++---------------------------
 src/libcamera/bound_method.cpp   |  33 +++++++
 src/libcamera/include/message.h  |   8 +-
 src/libcamera/meson.build        |   1 +
 src/libcamera/message.cpp        |   4 +-
 src/libcamera/object.cpp         |   2 +-
 src/libcamera/signal.cpp         |  23 -----
 11 files changed, 197 insertions(+), 166 deletions(-)
 create mode 100644 include/libcamera/bound_method.h
 create mode 100644 src/libcamera/bound_method.cpp

Hi Laurent,

On Mon, Aug 12, 2019 at 03:46:28PM +0300, Laurent Pinchart wrote:
> Move the Slot* classes to bound_method.{h,cpp} and rename them to
> Bound*Method*. They will be reused to implement asynchronous method
> invocation similar to cross-thread signal delivery.

Stupid question, but why do you need to rename to a longer
"Bound*Method*" when "Slot" was quite efficient as a name? Couldn't
you just move the Slot hierarchy definition and implementation to
their own files?

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/Doxyfile.in        |  10 +--
>  include/libcamera/bound_method.h | 131 ++++++++++++++++++++++++++++
>  include/libcamera/meson.build    |   1 +
>  include/libcamera/object.h       |   5 +-
>  include/libcamera/signal.h       | 145 ++++---------------------------
>  src/libcamera/bound_method.cpp   |  33 +++++++
>  src/libcamera/include/message.h  |   8 +-
>  src/libcamera/meson.build        |   1 +
>  src/libcamera/message.cpp        |   4 +-
>  src/libcamera/object.cpp         |   2 +-
>  src/libcamera/signal.cpp         |  23 -----
>  11 files changed, 197 insertions(+), 166 deletions(-)
>  create mode 100644 include/libcamera/bound_method.h
>  create mode 100644 src/libcamera/bound_method.cpp
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 3d94623a4b8f..a9596c2a32d8 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -865,11 +865,11 @@ EXCLUDE_PATTERNS       =
>  # Note that the wildcards are matched against the file with absolute path, so to
>  # exclude all test directories use the pattern */test/*
>
> -EXCLUDE_SYMBOLS        = libcamera::SignalBase \
> -                         libcamera::SlotArgs \
> -                         libcamera::SlotBase \
> -                         libcamera::SlotMember \
> -                         libcamera::SlotStatic \
> +EXCLUDE_SYMBOLS        = libcamera::BoundMemberMethod \
> +                         libcamera::BoundMethodArgs \
> +                         libcamera::BoundMethodBase \
> +                         libcamera::BoundStaticMethod \
> +                         libcamera::SignalBase \
>                           std::*
>
>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> new file mode 100644
> index 000000000000..38c44b923ba1
> --- /dev/null
> +++ b/include/libcamera/bound_method.h
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * bound_method.h - Method bind and invocation
> + */
> +#ifndef __LIBCAMERA_BOUND_METHOD_H__
> +#define __LIBCAMERA_BOUND_METHOD_H__
> +
> +#include <tuple>
> +#include <type_traits>
> +
> +namespace libcamera {
> +
> +class Object;
> +template<typename... Args>
> +class Signal;
> +class SignalBase;
> +
> +class BoundMethodBase
> +{
> +public:
> +	BoundMethodBase(void *obj, Object *object)
> +		: obj_(obj), object_(object) {}
> +	virtual ~BoundMethodBase() {}
> +
> +	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> +	bool match(T *obj) { return obj == obj_; }
> +	bool match(Object *object) { return object == object_; }
> +
> +	void disconnect(SignalBase *signal);
> +
> +	void activatePack(void *pack);
> +	virtual void invokePack(void *pack) = 0;
> +
> +protected:
> +	void *obj_;
> +	Object *object_;
> +};
> +
> +template<typename... Args>
> +class BoundMethodArgs : public BoundMethodBase
> +{
> +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
> +
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
> +	template<int... S>
> +	void invokePack(void *pack, sequence<S...>)
> +	{
> +		PackType *args = static_cast<PackType *>(pack);
> +		invoke(std::get<S>(*args)...);
> +		delete args;
> +	}
> +
> +public:
> +	BoundMethodArgs(void *obj, Object *object)
> +		: BoundMethodBase(obj, object) {}
> +
> +	void invokePack(void *pack) override
> +	{
> +		invokePack(pack, typename generator<sizeof...(Args)>::type());
> +	}
> +
> +	virtual void activate(Args... args) = 0;
> +	virtual void invoke(Args... args) = 0;
> +};
> +
> +template<typename T, typename... Args>
> +class BoundMemberMethod : public BoundMethodArgs<Args...>
> +{
> +public:
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
> +	BoundMemberMethod(T *obj, Object *object, void (T::*func)(Args...))
> +		: BoundMethodArgs<Args...>(obj, object), func_(func) {}
> +
> +	void activate(Args... args)
> +	{
> +		if (this->object_)
> +			BoundMethodBase::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...>;
> +	void (T::*func_)(Args...);
> +};
> +
> +template<typename... Args>
> +class BoundStaticMethod : public BoundMethodArgs<Args...>
> +{
> +public:
> +	BoundStaticMethod(void (*func)(Args...))
> +		: BoundMethodArgs<Args...>(nullptr, nullptr), func_(func) {}
> +
> +	void activate(Args... args) { (*func_)(args...); }
> +	void invoke(Args... args) {}
> +
> +private:
> +	friend class Signal<Args...>;
> +	void (*func_)(Args...);
> +};
> +
> +}; /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_BOUND_METHOD_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 920eb5fcbfd1..a8a38a9b1db7 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_api = files([
> +    'bound_method.h',
>      'buffer.h',
>      'camera.h',
>      'camera_manager.h',
> diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> index 3d08d69a9044..e3b39cf547b0 100644
> --- a/include/libcamera/object.h
> +++ b/include/libcamera/object.h
> @@ -10,13 +10,14 @@
>  #include <list>
>  #include <memory>
>
> +#include <libcamera/bound_method.h>
> +
>  namespace libcamera {
>
>  class Message;
>  template<typename... Args>
>  class Signal;
>  class SignalBase;
> -class SlotBase;
>  class Thread;
>
>  class Object
> @@ -36,7 +37,7 @@ protected:
>  private:
>  	template<typename... Args>
>  	friend class Signal;
> -	friend class SlotBase;
> +	friend class BoundMethodBase;
>  	friend class Thread;
>
>  	void connect(SignalBase *signal);
> diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> index 8f6db700cd80..3b6de30f7d35 100644
> --- a/include/libcamera/signal.h
> +++ b/include/libcamera/signal.h
> @@ -8,127 +8,14 @@
>  #define __LIBCAMERA_SIGNAL_H__
>
>  #include <list>
> -#include <tuple>
>  #include <type_traits>
>  #include <vector>
>
> +#include <libcamera/bound_method.h>
>  #include <libcamera/object.h>
>
>  namespace libcamera {
>
> -template<typename... Args>
> -class Signal;
> -class SignalBase;
> -
> -class SlotBase
> -{
> -public:
> -	SlotBase(void *obj, Object *object)
> -		: obj_(obj), object_(object) {}
> -	virtual ~SlotBase() {}
> -
> -	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> -	bool match(T *obj) { return obj == obj_; }
> -	bool match(Object *object) { return object == object_; }
> -
> -	void disconnect(SignalBase *signal);
> -
> -	void activatePack(void *pack);
> -	virtual void invokePack(void *pack) = 0;
> -
> -protected:
> -	void *obj_;
> -	Object *object_;
> -};
> -
> -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
> -
> -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> -
> -	template<int... S>
> -	void invokePack(void *pack, sequence<S...>)
> -	{
> -		PackType *args = static_cast<PackType *>(pack);
> -		invoke(std::get<S>(*args)...);
> -		delete args;
> -	}
> -
> -public:
> -	SlotArgs(void *obj, Object *object)
> -		: SlotBase(obj, object) {}
> -
> -	void invokePack(void *pack) override
> -	{
> -		invokePack(pack, typename generator<sizeof...(Args)>::type());
> -	}
> -
> -	virtual void activate(Args... args) = 0;
> -	virtual void invoke(Args... args) = 0;
> -};
> -
> -template<typename T, typename... Args>
> -class SlotMember : public SlotArgs<Args...>
> -{
> -public:
> -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> -
> -	SlotMember(T *obj, Object *object, void (T::*func)(Args...))
> -		: SlotArgs<Args...>(obj, object), func_(func) {}
> -
> -	void activate(Args... args)
> -	{
> -		if (this->object_)
> -			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...>;
> -	void (T::*func_)(Args...);
> -};
> -
> -template<typename... Args>
> -class SlotStatic : public SlotArgs<Args...>
> -{
> -public:
> -	SlotStatic(void (*func)(Args...))
> -		: SlotArgs<Args...>(nullptr, nullptr), func_(func) {}
> -
> -	void activate(Args... args) { (*func_)(args...); }
> -	void invoke(Args... args) {}
> -
> -private:
> -	friend class Signal<Args...>;
> -	void (*func_)(Args...);
> -};
> -
>  class SignalBase
>  {
>  public:
> @@ -136,7 +23,7 @@ public:
>  	void disconnect(T *obj)
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotBase *slot = *iter;
> +			BoundMethodBase *slot = *iter;
>  			if (slot->match(obj)) {
>  				iter = slots_.erase(iter);
>  				delete slot;
> @@ -148,7 +35,7 @@ public:
>
>  protected:
>  	friend class Object;
> -	std::list<SlotBase *> slots_;
> +	std::list<BoundMethodBase *> slots_;
>  };
>
>  template<typename... Args>
> @@ -158,7 +45,7 @@ public:
>  	Signal() {}
>  	~Signal()
>  	{
> -		for (SlotBase *slot : slots_) {
> +		for (BoundMethodBase *slot : slots_) {
>  			slot->disconnect(this);
>  			delete slot;
>  		}
> @@ -170,7 +57,7 @@ public:
>  	{
>  		Object *object = static_cast<Object *>(obj);
>  		object->connect(this);
> -		slots_.push_back(new SlotMember<T, Args...>(obj, object, func));
> +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, object, func));
>  	}
>
>  	template<typename T, typename std::enable_if<!std::is_base_of<Object, T>::value>::type * = nullptr>
> @@ -179,17 +66,17 @@ public:
>  #endif
>  	void connect(T *obj, void (T::*func)(Args...))
>  	{
> -		slots_.push_back(new SlotMember<T, Args...>(obj, nullptr, func));
> +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, nullptr, func));
>  	}
>
>  	void connect(void (*func)(Args...))
>  	{
> -		slots_.push_back(new SlotStatic<Args...>(func));
> +		slots_.push_back(new BoundStaticMethod<Args...>(func));
>  	}
>
>  	void disconnect()
>  	{
> -		for (SlotBase *slot : slots_)
> +		for (BoundMethodBase *slot : slots_)
>  			delete slot;
>  		slots_.clear();
>  	}
> @@ -204,15 +91,15 @@ public:
>  	void disconnect(T *obj, void (T::*func)(Args...))
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotArgs<Args...> *slot = static_cast<SlotArgs<Args...> *>(*iter);
> +			BoundMethodArgs<Args...> *slot = static_cast<BoundMethodArgs<Args...> *>(*iter);
>  			/*
>  			 * If the object matches the slot, the slot is
>  			 * guaranteed to be a member slot, so we can safely
> -			 * cast it to SlotMember<T, Args...> and access its
> +			 * cast it to BoundMemberMethod<T, Args...> and access its
>  			 * func_ member.
>  			 */
>  			if (slot->match(obj) &&
> -			    static_cast<SlotMember<T, Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundMemberMethod<T, Args...> *>(slot)->func_ == func) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> @@ -224,9 +111,9 @@ public:
>  	void disconnect(void (*func)(Args...))
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotArgs<Args...> *slot = *iter;
> +			BoundMethodArgs<Args...> *slot = *iter;
>  			if (slot->match(nullptr) &&
> -			    static_cast<SlotStatic<Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundStaticMethod<Args...> *>(slot)->func_ == func) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> @@ -241,9 +128,9 @@ public:
>  		 * Make a copy of the slots list as the slot could call the
>  		 * disconnect operation, invalidating the iterator.
>  		 */
> -		std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() };
> -		for (SlotBase *slot : slots)
> -			static_cast<SlotArgs<Args...> *>(slot)->activate(args...);
> +		std::vector<BoundMethodBase *> slots{ slots_.begin(), slots_.end() };
> +		for (BoundMethodBase *slot : slots)
> +			static_cast<BoundMethodArgs<Args...> *>(slot)->activate(args...);
>  	}
>  };
>
> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> new file mode 100644
> index 000000000000..0a2d61a6c805
> --- /dev/null
> +++ b/src/libcamera/bound_method.cpp
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * bound_method.cpp - Method bind and invocation
> + */
> +
> +#include <libcamera/bound_method.h>
> +
> +#include "message.h"
> +#include "thread.h"
> +#include "utils.h"
> +
> +namespace libcamera {
> +
> +void BoundMethodBase::disconnect(SignalBase *signal)
> +{
> +	if (object_)
> +		object_->disconnect(signal);
> +}
> +
> +void BoundMethodBase::activatePack(void *pack)
> +{
> +	if (Thread::current() == object_->thread()) {
> +		invokePack(pack);
> +	} else {
> +		std::unique_ptr<Message> msg =
> +			utils::make_unique<SignalMessage>(this, pack);
> +		object_->postMessage(std::move(msg));
> +	}
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index 416fe74b10ad..b4670c0eab76 100644
> --- a/src/libcamera/include/message.h
> +++ b/src/libcamera/include/message.h
> @@ -11,8 +11,8 @@
>
>  namespace libcamera {
>
> +class BoundMethodBase;
>  class Object;
> -class SlotBase;
>  class Thread;
>
>  class Message
> @@ -44,12 +44,12 @@ private:
>  class SignalMessage : public Message
>  {
>  public:
> -	SignalMessage(SlotBase *slot, void *pack)
> -		: Message(Message::SignalMessage), slot_(slot), pack_(pack)
> +	SignalMessage(BoundMethodBase *method, void *pack)
> +		: Message(Message::SignalMessage), method_(method), pack_(pack)
>  	{
>  	}
>
> -	SlotBase *slot_;
> +	BoundMethodBase *method_;
>  	void *pack_;
>  };
>
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 7d5d3c04fba0..c5d8f116ffc6 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_sources = files([
> +    'bound_method.cpp',
>      'buffer.cpp',
>      'camera.cpp',
>      'camera_manager.cpp',
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index d44d2a4c73a8..8d3376d8a533 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -114,12 +114,12 @@ Message::Type Message::registerMessageType()
>  /**
>   * \fn SignalMessage::SignalMessage()
>   * \brief Construct a SignalMessage
> - * \param[in] slot The slot that the signal targets
> + * \param[in] method The slot that the signal targets
>   * \param[in] pack The signal arguments
>   */
>
>  /**
> - * \var SignalMessage::slot_
> + * \var SignalMessage::method_
>   * \brief The slot that the signal targets
>   */
>
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index 61787fadac0b..0adbc203add8 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -90,7 +90,7 @@ void Object::message(Message *msg)
>  	switch (msg->type()) {
>  	case Message::SignalMessage: {
>  		SignalMessage *smsg = static_cast<SignalMessage *>(msg);
> -		smsg->slot_->invokePack(smsg->pack_);
> +		smsg->method_->invokePack(smsg->pack_);
>  		break;
>  	}
>
> diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
> index ab7dba508dfb..6ee348acc8d4 100644
> --- a/src/libcamera/signal.cpp
> +++ b/src/libcamera/signal.cpp
> @@ -7,10 +7,6 @@
>
>  #include <libcamera/signal.h>
>
> -#include "message.h"
> -#include "thread.h"
> -#include "utils.h"
> -
>  /**
>   * \file signal.h
>   * \brief Signal & slot implementation
> @@ -57,25 +53,6 @@ namespace libcamera {
>   * passed through the signal will remain valid after the signal is emitted.
>   */
>
> -void SlotBase::disconnect(SignalBase *signal)
> -{
> -	if (object_)
> -		object_->disconnect(signal);
> -}
> -
> -void SlotBase::activatePack(void *pack)
> -{
> -	Object *obj = static_cast<Object *>(object_);
> -
> -	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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

On Thu, Aug 15, 2019 at 10:47:41AM +0200, Jacopo Mondi wrote:
> On Mon, Aug 12, 2019 at 03:46:28PM +0300, Laurent Pinchart wrote:
> > Move the Slot* classes to bound_method.{h,cpp} and rename them to
> > Bound*Method*. They will be reused to implement asynchronous method
> > invocation similar to cross-thread signal delivery.
> 
> Stupid question, but why do you need to rename to a longer
> "Bound*Method*" when "Slot" was quite efficient as a name? Couldn't
> you just move the Slot hierarchy definition and implementation to
> their own files?

The name "Slot" is tied to Signal, and I wanted a better name that would
reflect the extended purpose of this class. I'm open to better
suggestions of course :-)

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  Documentation/Doxyfile.in        |  10 +--
> >  include/libcamera/bound_method.h | 131 ++++++++++++++++++++++++++++
> >  include/libcamera/meson.build    |   1 +
> >  include/libcamera/object.h       |   5 +-
> >  include/libcamera/signal.h       | 145 ++++---------------------------
> >  src/libcamera/bound_method.cpp   |  33 +++++++
> >  src/libcamera/include/message.h  |   8 +-
> >  src/libcamera/meson.build        |   1 +
> >  src/libcamera/message.cpp        |   4 +-
> >  src/libcamera/object.cpp         |   2 +-
> >  src/libcamera/signal.cpp         |  23 -----
> >  11 files changed, 197 insertions(+), 166 deletions(-)
> >  create mode 100644 include/libcamera/bound_method.h
> >  create mode 100644 src/libcamera/bound_method.cpp
> >
> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > index 3d94623a4b8f..a9596c2a32d8 100644
> > --- a/Documentation/Doxyfile.in
> > +++ b/Documentation/Doxyfile.in
> > @@ -865,11 +865,11 @@ EXCLUDE_PATTERNS       =
> >  # Note that the wildcards are matched against the file with absolute path, so to
> >  # exclude all test directories use the pattern */test/*
> >
> > -EXCLUDE_SYMBOLS        = libcamera::SignalBase \
> > -                         libcamera::SlotArgs \
> > -                         libcamera::SlotBase \
> > -                         libcamera::SlotMember \
> > -                         libcamera::SlotStatic \
> > +EXCLUDE_SYMBOLS        = libcamera::BoundMemberMethod \
> > +                         libcamera::BoundMethodArgs \
> > +                         libcamera::BoundMethodBase \
> > +                         libcamera::BoundStaticMethod \
> > +                         libcamera::SignalBase \
> >                           std::*
> >
> >  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> > diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> > new file mode 100644
> > index 000000000000..38c44b923ba1
> > --- /dev/null
> > +++ b/include/libcamera/bound_method.h
> > @@ -0,0 +1,131 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * bound_method.h - Method bind and invocation
> > + */
> > +#ifndef __LIBCAMERA_BOUND_METHOD_H__
> > +#define __LIBCAMERA_BOUND_METHOD_H__
> > +
> > +#include <tuple>
> > +#include <type_traits>
> > +
> > +namespace libcamera {
> > +
> > +class Object;
> > +template<typename... Args>
> > +class Signal;
> > +class SignalBase;
> > +
> > +class BoundMethodBase
> > +{
> > +public:
> > +	BoundMethodBase(void *obj, Object *object)
> > +		: obj_(obj), object_(object) {}
> > +	virtual ~BoundMethodBase() {}
> > +
> > +	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> > +	bool match(T *obj) { return obj == obj_; }
> > +	bool match(Object *object) { return object == object_; }
> > +
> > +	void disconnect(SignalBase *signal);
> > +
> > +	void activatePack(void *pack);
> > +	virtual void invokePack(void *pack) = 0;
> > +
> > +protected:
> > +	void *obj_;
> > +	Object *object_;
> > +};
> > +
> > +template<typename... Args>
> > +class BoundMethodArgs : public BoundMethodBase
> > +{
> > +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
> > +
> > +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> > +
> > +	template<int... S>
> > +	void invokePack(void *pack, sequence<S...>)
> > +	{
> > +		PackType *args = static_cast<PackType *>(pack);
> > +		invoke(std::get<S>(*args)...);
> > +		delete args;
> > +	}
> > +
> > +public:
> > +	BoundMethodArgs(void *obj, Object *object)
> > +		: BoundMethodBase(obj, object) {}
> > +
> > +	void invokePack(void *pack) override
> > +	{
> > +		invokePack(pack, typename generator<sizeof...(Args)>::type());
> > +	}
> > +
> > +	virtual void activate(Args... args) = 0;
> > +	virtual void invoke(Args... args) = 0;
> > +};
> > +
> > +template<typename T, typename... Args>
> > +class BoundMemberMethod : public BoundMethodArgs<Args...>
> > +{
> > +public:
> > +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> > +
> > +	BoundMemberMethod(T *obj, Object *object, void (T::*func)(Args...))
> > +		: BoundMethodArgs<Args...>(obj, object), func_(func) {}
> > +
> > +	void activate(Args... args)
> > +	{
> > +		if (this->object_)
> > +			BoundMethodBase::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...>;
> > +	void (T::*func_)(Args...);
> > +};
> > +
> > +template<typename... Args>
> > +class BoundStaticMethod : public BoundMethodArgs<Args...>
> > +{
> > +public:
> > +	BoundStaticMethod(void (*func)(Args...))
> > +		: BoundMethodArgs<Args...>(nullptr, nullptr), func_(func) {}
> > +
> > +	void activate(Args... args) { (*func_)(args...); }
> > +	void invoke(Args... args) {}
> > +
> > +private:
> > +	friend class Signal<Args...>;
> > +	void (*func_)(Args...);
> > +};
> > +
> > +}; /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_BOUND_METHOD_H__ */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index 920eb5fcbfd1..a8a38a9b1db7 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -1,4 +1,5 @@
> >  libcamera_api = files([
> > +    'bound_method.h',
> >      'buffer.h',
> >      'camera.h',
> >      'camera_manager.h',
> > diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> > index 3d08d69a9044..e3b39cf547b0 100644
> > --- a/include/libcamera/object.h
> > +++ b/include/libcamera/object.h
> > @@ -10,13 +10,14 @@
> >  #include <list>
> >  #include <memory>
> >
> > +#include <libcamera/bound_method.h>
> > +
> >  namespace libcamera {
> >
> >  class Message;
> >  template<typename... Args>
> >  class Signal;
> >  class SignalBase;
> > -class SlotBase;
> >  class Thread;
> >
> >  class Object
> > @@ -36,7 +37,7 @@ protected:
> >  private:
> >  	template<typename... Args>
> >  	friend class Signal;
> > -	friend class SlotBase;
> > +	friend class BoundMethodBase;
> >  	friend class Thread;
> >
> >  	void connect(SignalBase *signal);
> > diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> > index 8f6db700cd80..3b6de30f7d35 100644
> > --- a/include/libcamera/signal.h
> > +++ b/include/libcamera/signal.h
> > @@ -8,127 +8,14 @@
> >  #define __LIBCAMERA_SIGNAL_H__
> >
> >  #include <list>
> > -#include <tuple>
> >  #include <type_traits>
> >  #include <vector>
> >
> > +#include <libcamera/bound_method.h>
> >  #include <libcamera/object.h>
> >
> >  namespace libcamera {
> >
> > -template<typename... Args>
> > -class Signal;
> > -class SignalBase;
> > -
> > -class SlotBase
> > -{
> > -public:
> > -	SlotBase(void *obj, Object *object)
> > -		: obj_(obj), object_(object) {}
> > -	virtual ~SlotBase() {}
> > -
> > -	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> > -	bool match(T *obj) { return obj == obj_; }
> > -	bool match(Object *object) { return object == object_; }
> > -
> > -	void disconnect(SignalBase *signal);
> > -
> > -	void activatePack(void *pack);
> > -	virtual void invokePack(void *pack) = 0;
> > -
> > -protected:
> > -	void *obj_;
> > -	Object *object_;
> > -};
> > -
> > -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
> > -
> > -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> > -
> > -	template<int... S>
> > -	void invokePack(void *pack, sequence<S...>)
> > -	{
> > -		PackType *args = static_cast<PackType *>(pack);
> > -		invoke(std::get<S>(*args)...);
> > -		delete args;
> > -	}
> > -
> > -public:
> > -	SlotArgs(void *obj, Object *object)
> > -		: SlotBase(obj, object) {}
> > -
> > -	void invokePack(void *pack) override
> > -	{
> > -		invokePack(pack, typename generator<sizeof...(Args)>::type());
> > -	}
> > -
> > -	virtual void activate(Args... args) = 0;
> > -	virtual void invoke(Args... args) = 0;
> > -};
> > -
> > -template<typename T, typename... Args>
> > -class SlotMember : public SlotArgs<Args...>
> > -{
> > -public:
> > -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> > -
> > -	SlotMember(T *obj, Object *object, void (T::*func)(Args...))
> > -		: SlotArgs<Args...>(obj, object), func_(func) {}
> > -
> > -	void activate(Args... args)
> > -	{
> > -		if (this->object_)
> > -			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...>;
> > -	void (T::*func_)(Args...);
> > -};
> > -
> > -template<typename... Args>
> > -class SlotStatic : public SlotArgs<Args...>
> > -{
> > -public:
> > -	SlotStatic(void (*func)(Args...))
> > -		: SlotArgs<Args...>(nullptr, nullptr), func_(func) {}
> > -
> > -	void activate(Args... args) { (*func_)(args...); }
> > -	void invoke(Args... args) {}
> > -
> > -private:
> > -	friend class Signal<Args...>;
> > -	void (*func_)(Args...);
> > -};
> > -
> >  class SignalBase
> >  {
> >  public:
> > @@ -136,7 +23,7 @@ public:
> >  	void disconnect(T *obj)
> >  	{
> >  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> > -			SlotBase *slot = *iter;
> > +			BoundMethodBase *slot = *iter;
> >  			if (slot->match(obj)) {
> >  				iter = slots_.erase(iter);
> >  				delete slot;
> > @@ -148,7 +35,7 @@ public:
> >
> >  protected:
> >  	friend class Object;
> > -	std::list<SlotBase *> slots_;
> > +	std::list<BoundMethodBase *> slots_;
> >  };
> >
> >  template<typename... Args>
> > @@ -158,7 +45,7 @@ public:
> >  	Signal() {}
> >  	~Signal()
> >  	{
> > -		for (SlotBase *slot : slots_) {
> > +		for (BoundMethodBase *slot : slots_) {
> >  			slot->disconnect(this);
> >  			delete slot;
> >  		}
> > @@ -170,7 +57,7 @@ public:
> >  	{
> >  		Object *object = static_cast<Object *>(obj);
> >  		object->connect(this);
> > -		slots_.push_back(new SlotMember<T, Args...>(obj, object, func));
> > +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, object, func));
> >  	}
> >
> >  	template<typename T, typename std::enable_if<!std::is_base_of<Object, T>::value>::type * = nullptr>
> > @@ -179,17 +66,17 @@ public:
> >  #endif
> >  	void connect(T *obj, void (T::*func)(Args...))
> >  	{
> > -		slots_.push_back(new SlotMember<T, Args...>(obj, nullptr, func));
> > +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, nullptr, func));
> >  	}
> >
> >  	void connect(void (*func)(Args...))
> >  	{
> > -		slots_.push_back(new SlotStatic<Args...>(func));
> > +		slots_.push_back(new BoundStaticMethod<Args...>(func));
> >  	}
> >
> >  	void disconnect()
> >  	{
> > -		for (SlotBase *slot : slots_)
> > +		for (BoundMethodBase *slot : slots_)
> >  			delete slot;
> >  		slots_.clear();
> >  	}
> > @@ -204,15 +91,15 @@ public:
> >  	void disconnect(T *obj, void (T::*func)(Args...))
> >  	{
> >  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> > -			SlotArgs<Args...> *slot = static_cast<SlotArgs<Args...> *>(*iter);
> > +			BoundMethodArgs<Args...> *slot = static_cast<BoundMethodArgs<Args...> *>(*iter);
> >  			/*
> >  			 * If the object matches the slot, the slot is
> >  			 * guaranteed to be a member slot, so we can safely
> > -			 * cast it to SlotMember<T, Args...> and access its
> > +			 * cast it to BoundMemberMethod<T, Args...> and access its
> >  			 * func_ member.
> >  			 */
> >  			if (slot->match(obj) &&
> > -			    static_cast<SlotMember<T, Args...> *>(slot)->func_ == func) {
> > +			    static_cast<BoundMemberMethod<T, Args...> *>(slot)->func_ == func) {
> >  				iter = slots_.erase(iter);
> >  				delete slot;
> >  			} else {
> > @@ -224,9 +111,9 @@ public:
> >  	void disconnect(void (*func)(Args...))
> >  	{
> >  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> > -			SlotArgs<Args...> *slot = *iter;
> > +			BoundMethodArgs<Args...> *slot = *iter;
> >  			if (slot->match(nullptr) &&
> > -			    static_cast<SlotStatic<Args...> *>(slot)->func_ == func) {
> > +			    static_cast<BoundStaticMethod<Args...> *>(slot)->func_ == func) {
> >  				iter = slots_.erase(iter);
> >  				delete slot;
> >  			} else {
> > @@ -241,9 +128,9 @@ public:
> >  		 * Make a copy of the slots list as the slot could call the
> >  		 * disconnect operation, invalidating the iterator.
> >  		 */
> > -		std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() };
> > -		for (SlotBase *slot : slots)
> > -			static_cast<SlotArgs<Args...> *>(slot)->activate(args...);
> > +		std::vector<BoundMethodBase *> slots{ slots_.begin(), slots_.end() };
> > +		for (BoundMethodBase *slot : slots)
> > +			static_cast<BoundMethodArgs<Args...> *>(slot)->activate(args...);
> >  	}
> >  };
> >
> > diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> > new file mode 100644
> > index 000000000000..0a2d61a6c805
> > --- /dev/null
> > +++ b/src/libcamera/bound_method.cpp
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * bound_method.cpp - Method bind and invocation
> > + */
> > +
> > +#include <libcamera/bound_method.h>
> > +
> > +#include "message.h"
> > +#include "thread.h"
> > +#include "utils.h"
> > +
> > +namespace libcamera {
> > +
> > +void BoundMethodBase::disconnect(SignalBase *signal)
> > +{
> > +	if (object_)
> > +		object_->disconnect(signal);
> > +}
> > +
> > +void BoundMethodBase::activatePack(void *pack)
> > +{
> > +	if (Thread::current() == object_->thread()) {
> > +		invokePack(pack);
> > +	} else {
> > +		std::unique_ptr<Message> msg =
> > +			utils::make_unique<SignalMessage>(this, pack);
> > +		object_->postMessage(std::move(msg));
> > +	}
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> > index 416fe74b10ad..b4670c0eab76 100644
> > --- a/src/libcamera/include/message.h
> > +++ b/src/libcamera/include/message.h
> > @@ -11,8 +11,8 @@
> >
> >  namespace libcamera {
> >
> > +class BoundMethodBase;
> >  class Object;
> > -class SlotBase;
> >  class Thread;
> >
> >  class Message
> > @@ -44,12 +44,12 @@ private:
> >  class SignalMessage : public Message
> >  {
> >  public:
> > -	SignalMessage(SlotBase *slot, void *pack)
> > -		: Message(Message::SignalMessage), slot_(slot), pack_(pack)
> > +	SignalMessage(BoundMethodBase *method, void *pack)
> > +		: Message(Message::SignalMessage), method_(method), pack_(pack)
> >  	{
> >  	}
> >
> > -	SlotBase *slot_;
> > +	BoundMethodBase *method_;
> >  	void *pack_;
> >  };
> >
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 7d5d3c04fba0..c5d8f116ffc6 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -1,4 +1,5 @@
> >  libcamera_sources = files([
> > +    'bound_method.cpp',
> >      'buffer.cpp',
> >      'camera.cpp',
> >      'camera_manager.cpp',
> > diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> > index d44d2a4c73a8..8d3376d8a533 100644
> > --- a/src/libcamera/message.cpp
> > +++ b/src/libcamera/message.cpp
> > @@ -114,12 +114,12 @@ Message::Type Message::registerMessageType()
> >  /**
> >   * \fn SignalMessage::SignalMessage()
> >   * \brief Construct a SignalMessage
> > - * \param[in] slot The slot that the signal targets
> > + * \param[in] method The slot that the signal targets
> >   * \param[in] pack The signal arguments
> >   */
> >
> >  /**
> > - * \var SignalMessage::slot_
> > + * \var SignalMessage::method_
> >   * \brief The slot that the signal targets
> >   */
> >
> > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> > index 61787fadac0b..0adbc203add8 100644
> > --- a/src/libcamera/object.cpp
> > +++ b/src/libcamera/object.cpp
> > @@ -90,7 +90,7 @@ void Object::message(Message *msg)
> >  	switch (msg->type()) {
> >  	case Message::SignalMessage: {
> >  		SignalMessage *smsg = static_cast<SignalMessage *>(msg);
> > -		smsg->slot_->invokePack(smsg->pack_);
> > +		smsg->method_->invokePack(smsg->pack_);
> >  		break;
> >  	}
> >
> > diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
> > index ab7dba508dfb..6ee348acc8d4 100644
> > --- a/src/libcamera/signal.cpp
> > +++ b/src/libcamera/signal.cpp
> > @@ -7,10 +7,6 @@
> >
> >  #include <libcamera/signal.h>
> >
> > -#include "message.h"
> > -#include "thread.h"
> > -#include "utils.h"
> > -
> >  /**
> >   * \file signal.h
> >   * \brief Signal & slot implementation
> > @@ -57,25 +53,6 @@ namespace libcamera {
> >   * passed through the signal will remain valid after the signal is emitted.
> >   */
> >
> > -void SlotBase::disconnect(SignalBase *signal)
> > -{
> > -	if (object_)
> > -		object_->disconnect(signal);
> > -}
> > -
> > -void SlotBase::activatePack(void *pack)
> > -{
> > -	Object *obj = static_cast<Object *>(object_);
> > -
> > -	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@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

Hi Laurent,

Thanks for your work.

On 2019-08-12 15:46:28 +0300, Laurent Pinchart wrote:
> Move the Slot* classes to bound_method.{h,cpp} and rename them to
> Bound*Method*. They will be reused to implement asynchronous method
> invocation similar to cross-thread signal delivery.

I would clarify that this is *only* a move and rename patch, to ease 
review as it's a quiet big patch.

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  Documentation/Doxyfile.in        |  10 +--
>  include/libcamera/bound_method.h | 131 ++++++++++++++++++++++++++++
>  include/libcamera/meson.build    |   1 +
>  include/libcamera/object.h       |   5 +-
>  include/libcamera/signal.h       | 145 ++++---------------------------
>  src/libcamera/bound_method.cpp   |  33 +++++++
>  src/libcamera/include/message.h  |   8 +-
>  src/libcamera/meson.build        |   1 +
>  src/libcamera/message.cpp        |   4 +-
>  src/libcamera/object.cpp         |   2 +-
>  src/libcamera/signal.cpp         |  23 -----
>  11 files changed, 197 insertions(+), 166 deletions(-)
>  create mode 100644 include/libcamera/bound_method.h
>  create mode 100644 src/libcamera/bound_method.cpp
> 
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 3d94623a4b8f..a9596c2a32d8 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -865,11 +865,11 @@ EXCLUDE_PATTERNS       =
>  # Note that the wildcards are matched against the file with absolute path, so to
>  # exclude all test directories use the pattern */test/*
>  
> -EXCLUDE_SYMBOLS        = libcamera::SignalBase \
> -                         libcamera::SlotArgs \
> -                         libcamera::SlotBase \
> -                         libcamera::SlotMember \
> -                         libcamera::SlotStatic \
> +EXCLUDE_SYMBOLS        = libcamera::BoundMemberMethod \
> +                         libcamera::BoundMethodArgs \
> +                         libcamera::BoundMethodBase \
> +                         libcamera::BoundStaticMethod \
> +                         libcamera::SignalBase \
>                           std::*
>  
>  # The EXAMPLE_PATH tag can be used to specify one or more files or directories
> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> new file mode 100644
> index 000000000000..38c44b923ba1
> --- /dev/null
> +++ b/include/libcamera/bound_method.h
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * bound_method.h - Method bind and invocation
> + */
> +#ifndef __LIBCAMERA_BOUND_METHOD_H__
> +#define __LIBCAMERA_BOUND_METHOD_H__
> +
> +#include <tuple>
> +#include <type_traits>
> +
> +namespace libcamera {
> +
> +class Object;
> +template<typename... Args>
> +class Signal;
> +class SignalBase;
> +
> +class BoundMethodBase
> +{
> +public:
> +	BoundMethodBase(void *obj, Object *object)
> +		: obj_(obj), object_(object) {}
> +	virtual ~BoundMethodBase() {}
> +
> +	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> +	bool match(T *obj) { return obj == obj_; }
> +	bool match(Object *object) { return object == object_; }
> +
> +	void disconnect(SignalBase *signal);
> +
> +	void activatePack(void *pack);
> +	virtual void invokePack(void *pack) = 0;
> +
> +protected:
> +	void *obj_;
> +	Object *object_;
> +};
> +
> +template<typename... Args>
> +class BoundMethodArgs : public BoundMethodBase
> +{
> +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
> +
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
> +	template<int... S>
> +	void invokePack(void *pack, sequence<S...>)
> +	{
> +		PackType *args = static_cast<PackType *>(pack);
> +		invoke(std::get<S>(*args)...);
> +		delete args;
> +	}
> +
> +public:
> +	BoundMethodArgs(void *obj, Object *object)
> +		: BoundMethodBase(obj, object) {}
> +
> +	void invokePack(void *pack) override
> +	{
> +		invokePack(pack, typename generator<sizeof...(Args)>::type());
> +	}
> +
> +	virtual void activate(Args... args) = 0;
> +	virtual void invoke(Args... args) = 0;
> +};
> +
> +template<typename T, typename... Args>
> +class BoundMemberMethod : public BoundMethodArgs<Args...>
> +{
> +public:
> +	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> +
> +	BoundMemberMethod(T *obj, Object *object, void (T::*func)(Args...))
> +		: BoundMethodArgs<Args...>(obj, object), func_(func) {}
> +
> +	void activate(Args... args)
> +	{
> +		if (this->object_)
> +			BoundMethodBase::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...>;
> +	void (T::*func_)(Args...);
> +};
> +
> +template<typename... Args>
> +class BoundStaticMethod : public BoundMethodArgs<Args...>
> +{
> +public:
> +	BoundStaticMethod(void (*func)(Args...))
> +		: BoundMethodArgs<Args...>(nullptr, nullptr), func_(func) {}
> +
> +	void activate(Args... args) { (*func_)(args...); }
> +	void invoke(Args... args) {}
> +
> +private:
> +	friend class Signal<Args...>;
> +	void (*func_)(Args...);
> +};
> +
> +}; /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_BOUND_METHOD_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 920eb5fcbfd1..a8a38a9b1db7 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_api = files([
> +    'bound_method.h',
>      'buffer.h',
>      'camera.h',
>      'camera_manager.h',
> diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> index 3d08d69a9044..e3b39cf547b0 100644
> --- a/include/libcamera/object.h
> +++ b/include/libcamera/object.h
> @@ -10,13 +10,14 @@
>  #include <list>
>  #include <memory>
>  
> +#include <libcamera/bound_method.h>
> +
>  namespace libcamera {
>  
>  class Message;
>  template<typename... Args>
>  class Signal;
>  class SignalBase;
> -class SlotBase;
>  class Thread;
>  
>  class Object
> @@ -36,7 +37,7 @@ protected:
>  private:
>  	template<typename... Args>
>  	friend class Signal;
> -	friend class SlotBase;
> +	friend class BoundMethodBase;
>  	friend class Thread;
>  
>  	void connect(SignalBase *signal);
> diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h
> index 8f6db700cd80..3b6de30f7d35 100644
> --- a/include/libcamera/signal.h
> +++ b/include/libcamera/signal.h
> @@ -8,127 +8,14 @@
>  #define __LIBCAMERA_SIGNAL_H__
>  
>  #include <list>
> -#include <tuple>
>  #include <type_traits>
>  #include <vector>
>  
> +#include <libcamera/bound_method.h>
>  #include <libcamera/object.h>
>  
>  namespace libcamera {
>  
> -template<typename... Args>
> -class Signal;
> -class SignalBase;
> -
> -class SlotBase
> -{
> -public:
> -	SlotBase(void *obj, Object *object)
> -		: obj_(obj), object_(object) {}
> -	virtual ~SlotBase() {}
> -
> -	template<typename T, typename std::enable_if<!std::is_same<Object, T>::value>::type * = nullptr>
> -	bool match(T *obj) { return obj == obj_; }
> -	bool match(Object *object) { return object == object_; }
> -
> -	void disconnect(SignalBase *signal);
> -
> -	void activatePack(void *pack);
> -	virtual void invokePack(void *pack) = 0;
> -
> -protected:
> -	void *obj_;
> -	Object *object_;
> -};
> -
> -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
> -
> -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> -
> -	template<int... S>
> -	void invokePack(void *pack, sequence<S...>)
> -	{
> -		PackType *args = static_cast<PackType *>(pack);
> -		invoke(std::get<S>(*args)...);
> -		delete args;
> -	}
> -
> -public:
> -	SlotArgs(void *obj, Object *object)
> -		: SlotBase(obj, object) {}
> -
> -	void invokePack(void *pack) override
> -	{
> -		invokePack(pack, typename generator<sizeof...(Args)>::type());
> -	}
> -
> -	virtual void activate(Args... args) = 0;
> -	virtual void invoke(Args... args) = 0;
> -};
> -
> -template<typename T, typename... Args>
> -class SlotMember : public SlotArgs<Args...>
> -{
> -public:
> -	using PackType = std::tuple<typename std::remove_reference<Args>::type...>;
> -
> -	SlotMember(T *obj, Object *object, void (T::*func)(Args...))
> -		: SlotArgs<Args...>(obj, object), func_(func) {}
> -
> -	void activate(Args... args)
> -	{
> -		if (this->object_)
> -			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...>;
> -	void (T::*func_)(Args...);
> -};
> -
> -template<typename... Args>
> -class SlotStatic : public SlotArgs<Args...>
> -{
> -public:
> -	SlotStatic(void (*func)(Args...))
> -		: SlotArgs<Args...>(nullptr, nullptr), func_(func) {}
> -
> -	void activate(Args... args) { (*func_)(args...); }
> -	void invoke(Args... args) {}
> -
> -private:
> -	friend class Signal<Args...>;
> -	void (*func_)(Args...);
> -};
> -
>  class SignalBase
>  {
>  public:
> @@ -136,7 +23,7 @@ public:
>  	void disconnect(T *obj)
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotBase *slot = *iter;
> +			BoundMethodBase *slot = *iter;
>  			if (slot->match(obj)) {
>  				iter = slots_.erase(iter);
>  				delete slot;
> @@ -148,7 +35,7 @@ public:
>  
>  protected:
>  	friend class Object;
> -	std::list<SlotBase *> slots_;
> +	std::list<BoundMethodBase *> slots_;
>  };
>  
>  template<typename... Args>
> @@ -158,7 +45,7 @@ public:
>  	Signal() {}
>  	~Signal()
>  	{
> -		for (SlotBase *slot : slots_) {
> +		for (BoundMethodBase *slot : slots_) {
>  			slot->disconnect(this);
>  			delete slot;
>  		}
> @@ -170,7 +57,7 @@ public:
>  	{
>  		Object *object = static_cast<Object *>(obj);
>  		object->connect(this);
> -		slots_.push_back(new SlotMember<T, Args...>(obj, object, func));
> +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, object, func));
>  	}
>  
>  	template<typename T, typename std::enable_if<!std::is_base_of<Object, T>::value>::type * = nullptr>
> @@ -179,17 +66,17 @@ public:
>  #endif
>  	void connect(T *obj, void (T::*func)(Args...))
>  	{
> -		slots_.push_back(new SlotMember<T, Args...>(obj, nullptr, func));
> +		slots_.push_back(new BoundMemberMethod<T, Args...>(obj, nullptr, func));
>  	}
>  
>  	void connect(void (*func)(Args...))
>  	{
> -		slots_.push_back(new SlotStatic<Args...>(func));
> +		slots_.push_back(new BoundStaticMethod<Args...>(func));
>  	}
>  
>  	void disconnect()
>  	{
> -		for (SlotBase *slot : slots_)
> +		for (BoundMethodBase *slot : slots_)
>  			delete slot;
>  		slots_.clear();
>  	}
> @@ -204,15 +91,15 @@ public:
>  	void disconnect(T *obj, void (T::*func)(Args...))
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotArgs<Args...> *slot = static_cast<SlotArgs<Args...> *>(*iter);
> +			BoundMethodArgs<Args...> *slot = static_cast<BoundMethodArgs<Args...> *>(*iter);
>  			/*
>  			 * If the object matches the slot, the slot is
>  			 * guaranteed to be a member slot, so we can safely
> -			 * cast it to SlotMember<T, Args...> and access its
> +			 * cast it to BoundMemberMethod<T, Args...> and access its
>  			 * func_ member.
>  			 */
>  			if (slot->match(obj) &&
> -			    static_cast<SlotMember<T, Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundMemberMethod<T, Args...> *>(slot)->func_ == func) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> @@ -224,9 +111,9 @@ public:
>  	void disconnect(void (*func)(Args...))
>  	{
>  		for (auto iter = slots_.begin(); iter != slots_.end(); ) {
> -			SlotArgs<Args...> *slot = *iter;
> +			BoundMethodArgs<Args...> *slot = *iter;
>  			if (slot->match(nullptr) &&
> -			    static_cast<SlotStatic<Args...> *>(slot)->func_ == func) {
> +			    static_cast<BoundStaticMethod<Args...> *>(slot)->func_ == func) {
>  				iter = slots_.erase(iter);
>  				delete slot;
>  			} else {
> @@ -241,9 +128,9 @@ public:
>  		 * Make a copy of the slots list as the slot could call the
>  		 * disconnect operation, invalidating the iterator.
>  		 */
> -		std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() };
> -		for (SlotBase *slot : slots)
> -			static_cast<SlotArgs<Args...> *>(slot)->activate(args...);
> +		std::vector<BoundMethodBase *> slots{ slots_.begin(), slots_.end() };
> +		for (BoundMethodBase *slot : slots)
> +			static_cast<BoundMethodArgs<Args...> *>(slot)->activate(args...);
>  	}
>  };
>  
> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> new file mode 100644
> index 000000000000..0a2d61a6c805
> --- /dev/null
> +++ b/src/libcamera/bound_method.cpp
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * bound_method.cpp - Method bind and invocation
> + */
> +
> +#include <libcamera/bound_method.h>
> +
> +#include "message.h"
> +#include "thread.h"
> +#include "utils.h"
> +
> +namespace libcamera {
> +
> +void BoundMethodBase::disconnect(SignalBase *signal)
> +{
> +	if (object_)
> +		object_->disconnect(signal);
> +}
> +
> +void BoundMethodBase::activatePack(void *pack)
> +{
> +	if (Thread::current() == object_->thread()) {
> +		invokePack(pack);
> +	} else {
> +		std::unique_ptr<Message> msg =
> +			utils::make_unique<SignalMessage>(this, pack);
> +		object_->postMessage(std::move(msg));
> +	}
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index 416fe74b10ad..b4670c0eab76 100644
> --- a/src/libcamera/include/message.h
> +++ b/src/libcamera/include/message.h
> @@ -11,8 +11,8 @@
>  
>  namespace libcamera {
>  
> +class BoundMethodBase;
>  class Object;
> -class SlotBase;
>  class Thread;
>  
>  class Message
> @@ -44,12 +44,12 @@ private:
>  class SignalMessage : public Message
>  {
>  public:
> -	SignalMessage(SlotBase *slot, void *pack)
> -		: Message(Message::SignalMessage), slot_(slot), pack_(pack)
> +	SignalMessage(BoundMethodBase *method, void *pack)
> +		: Message(Message::SignalMessage), method_(method), pack_(pack)
>  	{
>  	}
>  
> -	SlotBase *slot_;
> +	BoundMethodBase *method_;
>  	void *pack_;
>  };
>  
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 7d5d3c04fba0..c5d8f116ffc6 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -1,4 +1,5 @@
>  libcamera_sources = files([
> +    'bound_method.cpp',
>      'buffer.cpp',
>      'camera.cpp',
>      'camera_manager.cpp',
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index d44d2a4c73a8..8d3376d8a533 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -114,12 +114,12 @@ Message::Type Message::registerMessageType()
>  /**
>   * \fn SignalMessage::SignalMessage()
>   * \brief Construct a SignalMessage
> - * \param[in] slot The slot that the signal targets
> + * \param[in] method The slot that the signal targets
>   * \param[in] pack The signal arguments
>   */
>  
>  /**
> - * \var SignalMessage::slot_
> + * \var SignalMessage::method_
>   * \brief The slot that the signal targets
>   */
>  
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index 61787fadac0b..0adbc203add8 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -90,7 +90,7 @@ void Object::message(Message *msg)
>  	switch (msg->type()) {
>  	case Message::SignalMessage: {
>  		SignalMessage *smsg = static_cast<SignalMessage *>(msg);
> -		smsg->slot_->invokePack(smsg->pack_);
> +		smsg->method_->invokePack(smsg->pack_);
>  		break;
>  	}
>  
> diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp
> index ab7dba508dfb..6ee348acc8d4 100644
> --- a/src/libcamera/signal.cpp
> +++ b/src/libcamera/signal.cpp
> @@ -7,10 +7,6 @@
>  
>  #include <libcamera/signal.h>
>  
> -#include "message.h"
> -#include "thread.h"
> -#include "utils.h"
> -
>  /**
>   * \file signal.h
>   * \brief Signal & slot implementation
> @@ -57,25 +53,6 @@ namespace libcamera {
>   * passed through the signal will remain valid after the signal is emitted.
>   */
>  
> -void SlotBase::disconnect(SignalBase *signal)
> -{
> -	if (object_)
> -		object_->disconnect(signal);
> -}
> -
> -void SlotBase::activatePack(void *pack)
> -{
> -	Object *obj = static_cast<Object *>(object_);
> -
> -	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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel