| Message ID | 20190710191708.13049-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2019-07-10 22:17:05 +0300, Laurent Pinchart wrote: > Allow signals to cross thread boundaries by posting them to the > recipient through messages instead of calling the slot directly when the > recipient lives in a different thread. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/signal.h | 70 +++++++++++++++++++++++++++++---- > src/libcamera/include/message.h | 14 +++++++ > src/libcamera/message.cpp | 22 +++++++++++ > src/libcamera/object.cpp | 15 +++++++ > src/libcamera/signal.cpp | 26 ++++++++++++ > 5 files changed, 139 insertions(+), 8 deletions(-) > > diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h > index c8f3243eaf5a..04e8fe7eebe3 100644 > --- a/include/libcamera/signal.h > +++ b/include/libcamera/signal.h > @@ -8,6 +8,8 @@ > #define __LIBCAMERA_SIGNAL_H__ > > #include <list> > +#include <tuple> > +#include <type_traits> > #include <vector> > > #include <libcamera/object.h> > @@ -16,6 +18,9 @@ namespace libcamera { > > template<typename... Args> > class Signal; > +class SlotBase; > +template<typename... Args> > +class SlotArgs; These forward declarations don't seems to be needed right? With this addressed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > class SlotBase > { > @@ -27,6 +32,9 @@ public: > void *obj() { return obj_; } > bool isObject() const { return isObject_; } > > + void activatePack(void *pack); > + virtual void invokePack(void *pack) = 0; > + > protected: > void *obj_; > bool isObject_; > @@ -35,24 +43,70 @@ protected: > template<typename... Args> > class SlotArgs : public SlotBase > { > +private: > +#ifndef __DOXYGEN__ > + /* > + * This is a cheap partial implementation of std::integer_sequence<> > + * from C++14. > + */ > + template<int...> > + struct sequence { > + }; > + > + template<int N, int... S> > + struct generator : generator<N-1, N-1, S...> { > + }; > + > + template<int... S> > + struct generator<0, S...> { > + typedef sequence<S...> type; > + }; > +#endif > + > public: > + using PackType = std::tuple<typename std::remove_reference<Args>::type...>; > + > SlotArgs(void *obj, bool isObject) > : SlotBase(obj, isObject) {} > > + void invokePack(void *pack) override > + { > + invokePack(pack, typename generator<sizeof...(Args)>::type()); > + } > + > + template<int... S> > + void invokePack(void *pack, sequence<S...>) > + { > + PackType *args = static_cast<PackType *>(pack); > + invoke(std::get<S>(*args)...); > + delete args; > + } > + > + virtual void activate(Args... args) = 0; > virtual void invoke(Args... args) = 0; > - > -protected: > - friend class Signal<Args...>; > }; > > template<typename T, typename... Args> > class SlotMember : public SlotArgs<Args...> > { > public: > + using PackType = std::tuple<typename std::remove_reference<Args>::type...>; > + > SlotMember(T *obj, bool isObject, void (T::*func)(Args...)) > : SlotArgs<Args...>(obj, isObject), func_(func) {} > > - void invoke(Args... args) { (static_cast<T *>(this->obj_)->*func_)(args...); } > + void activate(Args... args) > + { > + if (this->isObject_) > + SlotBase::activatePack(new PackType{ args... }); > + else > + (static_cast<T *>(this->obj_)->*func_)(args...); > + } > + > + void invoke(Args... args) > + { > + (static_cast<T *>(this->obj_)->*func_)(args...); > + } > > private: > friend class Signal<Args...>; > @@ -66,7 +120,8 @@ public: > SlotStatic(void (*func)(Args...)) > : SlotArgs<Args...>(nullptr, false), func_(func) {} > > - void invoke(Args... args) { (*func_)(args...); } > + void activate(Args... args) { (*func_)(args...); } > + void invoke(Args... args) {} > > private: > friend class Signal<Args...>; > @@ -186,9 +241,8 @@ public: > * disconnect operation, invalidating the iterator. > */ > std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() }; > - for (SlotBase *slot : slots) { > - static_cast<SlotArgs<Args...> *>(slot)->invoke(args...); > - } > + for (SlotBase *slot : slots) > + static_cast<SlotArgs<Args...> *>(slot)->activate(args...); > } > }; > > diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h > index 97c9b80ec0e0..db17d647c280 100644 > --- a/src/libcamera/include/message.h > +++ b/src/libcamera/include/message.h > @@ -10,6 +10,7 @@ > namespace libcamera { > > class Object; > +class SlotBase; > class Thread; > > class Message > @@ -17,6 +18,7 @@ class Message > public: > enum Type { > None = 0, > + SignalMessage = 1, > }; > > Message(Type type); > @@ -32,6 +34,18 @@ private: > Object *receiver_; > }; > > +class SignalMessage : public Message > +{ > +public: > + SignalMessage(SlotBase *slot, void *pack) > + : Message(Message::SignalMessage), slot_(slot), pack_(pack) > + { > + } > + > + SlotBase *slot_; > + void *pack_; > +}; > + > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_MESSAGE_H__ */ > diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp > index 47caf44dc82d..66dd1e8bd618 100644 > --- a/src/libcamera/message.cpp > +++ b/src/libcamera/message.cpp > @@ -68,4 +68,26 @@ Message::~Message() > * \return The message receiver > */ > > +/** > + * \class SignalMessage > + * \brief A message carrying a Signal across threads > + */ > + > +/** > + * \fn SignalMessage::SignalMessage() > + * \brief Construct a SignalMessage > + * \param[in] slot The slot that the signal targets > + * \param[in] pack The signal arguments > + */ > + > +/** > + * \var SignalMessage::slot_ > + * \brief The slot that the signal targets > + */ > + > +/** > + * \var SignalMessage::pack_ > + * \brief The signal arguments > + */ > + > }; /* namespace libcamera */ > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp > index 695e6c11b3a4..1dfa159267fe 100644 > --- a/src/libcamera/object.cpp > +++ b/src/libcamera/object.cpp > @@ -10,6 +10,7 @@ > #include <libcamera/signal.h> > > #include "log.h" > +#include "message.h" > #include "thread.h" > > /** > @@ -32,6 +33,10 @@ namespace libcamera { > * This allows implementing easy message passing between threads by inheriting > * from the Object class. > * > + * Object slots connected to signals will also run in the context of the > + * object's thread, regardless of whether the signal is emitted in the same or > + * in another thread. > + * > * \sa Message, Signal, Thread > */ > > @@ -82,6 +87,16 @@ void Object::postMessage(std::unique_ptr<Message> msg) > */ > void Object::message(Message *msg) > { > + switch (msg->type()) { > + case Message::SignalMessage: { > + SignalMessage *smsg = static_cast<SignalMessage *>(msg); > + smsg->slot_->invokePack(smsg->pack_); > + break; > + } > + > + default: > + break; > + } > } > > /** > diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp > index 4cb85ecb0686..53c18535fee3 100644 > --- a/src/libcamera/signal.cpp > +++ b/src/libcamera/signal.cpp > @@ -7,6 +7,10 @@ > > #include <libcamera/signal.h> > > +#include "message.h" > +#include "thread.h" > +#include "utils.h" > + > /** > * \file signal.h > * \brief Signal & slot implementation > @@ -42,8 +46,30 @@ namespace libcamera { > * to the same slot. Duplicate connections between a signal and a slot are > * allowed and result in the slot being called multiple times for the same > * signal emission. > + * > + * When a slot belongs to an instance of the Object class, the slot is called > + * in the context of the thread that the object is bound to. If the signal is > + * emitted from the same thread, the slot will be called synchronously, before > + * Signal::emit() returns. If the signal is emitted from a different thread, > + * the slot will be called asynchronously from the object's thread's event > + * loop, after the Signal::emit() method returns, with a copy of the signal's > + * arguments. The emitter shall thus ensure that any pointer or reference > + * passed through the signal will remain valid after the signal is emitted. > */ > > +void SlotBase::activatePack(void *pack) > +{ > + Object *obj = static_cast<Object *>(obj_); > + > + if (Thread::current() == obj->thread()) { > + invokePack(pack); > + } else { > + std::unique_ptr<Message> msg = > + utils::make_unique<SignalMessage>(this, pack); > + obj->postMessage(std::move(msg)); > + } > +} > + > /** > * \fn Signal::connect(T *object, void(T::*func)(Args...)) > * \brief Connect the signal to a member function slot > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/signal.h b/include/libcamera/signal.h index c8f3243eaf5a..04e8fe7eebe3 100644 --- a/include/libcamera/signal.h +++ b/include/libcamera/signal.h @@ -8,6 +8,8 @@ #define __LIBCAMERA_SIGNAL_H__ #include <list> +#include <tuple> +#include <type_traits> #include <vector> #include <libcamera/object.h> @@ -16,6 +18,9 @@ namespace libcamera { template<typename... Args> class Signal; +class SlotBase; +template<typename... Args> +class SlotArgs; class SlotBase { @@ -27,6 +32,9 @@ public: void *obj() { return obj_; } bool isObject() const { return isObject_; } + void activatePack(void *pack); + virtual void invokePack(void *pack) = 0; + protected: void *obj_; bool isObject_; @@ -35,24 +43,70 @@ protected: template<typename... Args> class SlotArgs : public SlotBase { +private: +#ifndef __DOXYGEN__ + /* + * This is a cheap partial implementation of std::integer_sequence<> + * from C++14. + */ + template<int...> + struct sequence { + }; + + template<int N, int... S> + struct generator : generator<N-1, N-1, S...> { + }; + + template<int... S> + struct generator<0, S...> { + typedef sequence<S...> type; + }; +#endif + public: + using PackType = std::tuple<typename std::remove_reference<Args>::type...>; + SlotArgs(void *obj, bool isObject) : SlotBase(obj, isObject) {} + void invokePack(void *pack) override + { + invokePack(pack, typename generator<sizeof...(Args)>::type()); + } + + template<int... S> + void invokePack(void *pack, sequence<S...>) + { + PackType *args = static_cast<PackType *>(pack); + invoke(std::get<S>(*args)...); + delete args; + } + + virtual void activate(Args... args) = 0; virtual void invoke(Args... args) = 0; - -protected: - friend class Signal<Args...>; }; template<typename T, typename... Args> class SlotMember : public SlotArgs<Args...> { public: + using PackType = std::tuple<typename std::remove_reference<Args>::type...>; + SlotMember(T *obj, bool isObject, void (T::*func)(Args...)) : SlotArgs<Args...>(obj, isObject), func_(func) {} - void invoke(Args... args) { (static_cast<T *>(this->obj_)->*func_)(args...); } + void activate(Args... args) + { + if (this->isObject_) + SlotBase::activatePack(new PackType{ args... }); + else + (static_cast<T *>(this->obj_)->*func_)(args...); + } + + void invoke(Args... args) + { + (static_cast<T *>(this->obj_)->*func_)(args...); + } private: friend class Signal<Args...>; @@ -66,7 +120,8 @@ public: SlotStatic(void (*func)(Args...)) : SlotArgs<Args...>(nullptr, false), func_(func) {} - void invoke(Args... args) { (*func_)(args...); } + void activate(Args... args) { (*func_)(args...); } + void invoke(Args... args) {} private: friend class Signal<Args...>; @@ -186,9 +241,8 @@ public: * disconnect operation, invalidating the iterator. */ std::vector<SlotBase *> slots{ slots_.begin(), slots_.end() }; - for (SlotBase *slot : slots) { - static_cast<SlotArgs<Args...> *>(slot)->invoke(args...); - } + for (SlotBase *slot : slots) + static_cast<SlotArgs<Args...> *>(slot)->activate(args...); } }; diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h index 97c9b80ec0e0..db17d647c280 100644 --- a/src/libcamera/include/message.h +++ b/src/libcamera/include/message.h @@ -10,6 +10,7 @@ namespace libcamera { class Object; +class SlotBase; class Thread; class Message @@ -17,6 +18,7 @@ class Message public: enum Type { None = 0, + SignalMessage = 1, }; Message(Type type); @@ -32,6 +34,18 @@ private: Object *receiver_; }; +class SignalMessage : public Message +{ +public: + SignalMessage(SlotBase *slot, void *pack) + : Message(Message::SignalMessage), slot_(slot), pack_(pack) + { + } + + SlotBase *slot_; + void *pack_; +}; + } /* namespace libcamera */ #endif /* __LIBCAMERA_MESSAGE_H__ */ diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp index 47caf44dc82d..66dd1e8bd618 100644 --- a/src/libcamera/message.cpp +++ b/src/libcamera/message.cpp @@ -68,4 +68,26 @@ Message::~Message() * \return The message receiver */ +/** + * \class SignalMessage + * \brief A message carrying a Signal across threads + */ + +/** + * \fn SignalMessage::SignalMessage() + * \brief Construct a SignalMessage + * \param[in] slot The slot that the signal targets + * \param[in] pack The signal arguments + */ + +/** + * \var SignalMessage::slot_ + * \brief The slot that the signal targets + */ + +/** + * \var SignalMessage::pack_ + * \brief The signal arguments + */ + }; /* namespace libcamera */ diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp index 695e6c11b3a4..1dfa159267fe 100644 --- a/src/libcamera/object.cpp +++ b/src/libcamera/object.cpp @@ -10,6 +10,7 @@ #include <libcamera/signal.h> #include "log.h" +#include "message.h" #include "thread.h" /** @@ -32,6 +33,10 @@ namespace libcamera { * This allows implementing easy message passing between threads by inheriting * from the Object class. * + * Object slots connected to signals will also run in the context of the + * object's thread, regardless of whether the signal is emitted in the same or + * in another thread. + * * \sa Message, Signal, Thread */ @@ -82,6 +87,16 @@ void Object::postMessage(std::unique_ptr<Message> msg) */ void Object::message(Message *msg) { + switch (msg->type()) { + case Message::SignalMessage: { + SignalMessage *smsg = static_cast<SignalMessage *>(msg); + smsg->slot_->invokePack(smsg->pack_); + break; + } + + default: + break; + } } /** diff --git a/src/libcamera/signal.cpp b/src/libcamera/signal.cpp index 4cb85ecb0686..53c18535fee3 100644 --- a/src/libcamera/signal.cpp +++ b/src/libcamera/signal.cpp @@ -7,6 +7,10 @@ #include <libcamera/signal.h> +#include "message.h" +#include "thread.h" +#include "utils.h" + /** * \file signal.h * \brief Signal & slot implementation @@ -42,8 +46,30 @@ namespace libcamera { * to the same slot. Duplicate connections between a signal and a slot are * allowed and result in the slot being called multiple times for the same * signal emission. + * + * When a slot belongs to an instance of the Object class, the slot is called + * in the context of the thread that the object is bound to. If the signal is + * emitted from the same thread, the slot will be called synchronously, before + * Signal::emit() returns. If the signal is emitted from a different thread, + * the slot will be called asynchronously from the object's thread's event + * loop, after the Signal::emit() method returns, with a copy of the signal's + * arguments. The emitter shall thus ensure that any pointer or reference + * passed through the signal will remain valid after the signal is emitted. */ +void SlotBase::activatePack(void *pack) +{ + Object *obj = static_cast<Object *>(obj_); + + if (Thread::current() == obj->thread()) { + invokePack(pack); + } else { + std::unique_ptr<Message> msg = + utils::make_unique<SignalMessage>(this, pack); + obj->postMessage(std::move(msg)); + } +} + /** * \fn Signal::connect(T *object, void(T::*func)(Args...)) * \brief Connect the signal to a member function slot
Allow signals to cross thread boundaries by posting them to the recipient through messages instead of calling the slot directly when the recipient lives in a different thread. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/signal.h | 70 +++++++++++++++++++++++++++++---- src/libcamera/include/message.h | 14 +++++++ src/libcamera/message.cpp | 22 +++++++++++ src/libcamera/object.cpp | 15 +++++++ src/libcamera/signal.cpp | 26 ++++++++++++ 5 files changed, 139 insertions(+), 8 deletions(-)