[libcamera-devel] [PATCH 04/18] libcamera: signal: Split Slot implementation to reusable classes
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Aug 15 11:49:07 CEST 2019
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 at 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 at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list