[libcamera-devel] [PATCH 04/18] libcamera: signal: Split Slot implementation to reusable classes
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sat Aug 17 16:21:07 CEST 2019
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 at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list