[libcamera-devel] [PATCH 06/18] libcamera: object: Add an asynchronous method invocation method
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sat Aug 17 17:18:43 CEST 2019
Hi Laurent,
On 2019-08-17 16:33:10 +0200, Niklas Söderlund wrote:
> Hi Laurent,
>
> Thanks for your work.
>
> On 2019-08-12 15:46:30 +0300, Laurent Pinchart wrote:
> > Add a helper invokeMethod() to the Object class that allows asynchrnous
> > invocation of any method of an Object instance. Asynchronous invocation
> > occurs when control returns to the event dispatcher of the target
> > object's thread, in the context of that thread.
> >
> > To support this, generalise the SignalMessage implementation to support
> > automatic deletion of the associated BoundMethod, and rename the message
> > to InvokeMessage to reflect the more generic purpose.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> I think this patch looks OK but there seems to be some code that should
> be moved here from a later patch and I would like to review the final
> result before I add my tag. This is tricky enough as it is then copy
> pasting code in local meat memory ;-)
As clarified on IRC, no change is expected in this patch,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > ---
> > include/libcamera/object.h | 12 +++++++++
> > src/libcamera/bound_method.cpp | 2 +-
> > src/libcamera/include/message.h | 17 +++++++-----
> > src/libcamera/message.cpp | 48 ++++++++++++++++++++++++---------
> > src/libcamera/object.cpp | 29 +++++++++++++++++---
> > 5 files changed, 86 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> > index e3b39cf547b0..869200a57d8c 100644
> > --- a/include/libcamera/object.h
> > +++ b/include/libcamera/object.h
> > @@ -28,6 +28,16 @@ public:
> >
> > void postMessage(std::unique_ptr<Message> msg);
> >
> > + template<typename T, typename... Args, typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr>
> > + void invokeMethod(void (T::*func)(Args...), Args... args)
> > + {
> > + T *obj = static_cast<T *>(this);
> > + BoundMethodBase *method = new BoundMemberMethod<T, Args...>(obj, this, func);
> > + void *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... };
> > +
> > + invokeMethod(method, pack);
> > + }
> > +
> > Thread *thread() const { return thread_; }
> > void moveToThread(Thread *thread);
> >
> > @@ -40,6 +50,8 @@ private:
> > friend class BoundMethodBase;
> > friend class Thread;
> >
> > + void invokeMethod(BoundMethodBase *method, void *pack);
> > +
> > void connect(SignalBase *signal);
> > void disconnect(SignalBase *signal);
> >
> > diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> > index 23b8f4122283..d89f84c03f4d 100644
> > --- a/src/libcamera/bound_method.cpp
> > +++ b/src/libcamera/bound_method.cpp
> > @@ -19,7 +19,7 @@ void BoundMethodBase::activatePack(void *pack)
> > invokePack(pack);
> > } else {
> > std::unique_ptr<Message> msg =
> > - utils::make_unique<SignalMessage>(this, pack);
> > + utils::make_unique<InvokeMessage>(this, pack);
> > object_->postMessage(std::move(msg));
> > }
> > }
> > diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> > index b4670c0eab76..92717e316cc3 100644
> > --- a/src/libcamera/include/message.h
> > +++ b/src/libcamera/include/message.h
> > @@ -9,6 +9,8 @@
> >
> > #include <atomic>
> >
> > +#include <libcamera/bound_method.h>
> > +
> > namespace libcamera {
> >
> > class BoundMethodBase;
> > @@ -20,7 +22,7 @@ class Message
> > public:
> > enum Type {
> > None = 0,
> > - SignalMessage = 1,
> > + InvokeMessage = 1,
> > UserMessage = 1000,
> > };
> >
> > @@ -41,16 +43,19 @@ private:
> > static std::atomic_uint nextUserType_;
> > };
> >
> > -class SignalMessage : public Message
> > +class InvokeMessage : public Message
> > {
> > public:
> > - SignalMessage(BoundMethodBase *method, void *pack)
> > - : Message(Message::SignalMessage), method_(method), pack_(pack)
> > - {
> > - }
> > + InvokeMessage(BoundMethodBase *method, void *pack,
> > + bool deleteMethod = false);
> > + ~InvokeMessage();
> >
> > + void invoke();
> > +
> > +private:
> > BoundMethodBase *method_;
> > void *pack_;
> > + bool deleteMethod_;
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> > index 8d3376d8a533..f6c39d40fc73 100644
> > --- a/src/libcamera/message.cpp
> > +++ b/src/libcamera/message.cpp
> > @@ -7,6 +7,8 @@
> >
> > #include "message.h"
> >
> > +#include <libcamera/signal.h>
> > +
> > #include "log.h"
> >
> > /**
> > @@ -43,8 +45,8 @@ std::atomic_uint Message::nextUserType_{ Message::UserMessage };
> > * \brief The message type
> > * \var Message::None
> > * \brief Invalid message type
> > - * \var Message::SignalMessage
> > - * \brief Asynchronous signal delivery across threads
> > + * \var Message::InvokeMessage
> > + * \brief Asynchronous method invocation across threads
> > * \var Message::UserMessage
> > * \brief First value available for user-defined messages
> > */
> > @@ -107,25 +109,47 @@ Message::Type Message::registerMessageType()
> > }
> >
> > /**
> > - * \class SignalMessage
> > - * \brief A message carrying a Signal across threads
> > + * \class InvokeMessage
> > + * \brief A message carrying a method invocation across threads
> > */
> >
> > /**
> > - * \fn SignalMessage::SignalMessage()
> > - * \brief Construct a SignalMessage
> > - * \param[in] method The slot that the signal targets
> > - * \param[in] pack The signal arguments
> > + * \brief Construct an InvokeMessage for method invocation on an Object
> > + * \param[in] method The bound method
> > + * \param[in] pack The packed method arguments
> > + * \param[in] deleteMethod True to delete the \a method when the message is
> > + * destroyed
> > */
> > +InvokeMessage::InvokeMessage(BoundMethodBase *method, void *pack,
> > + bool deleteMethod)
> > + : Message(Message::InvokeMessage), method_(method), pack_(pack),
> > + deleteMethod_(deleteMethod)
> > +{
> > +}
> > +
> > +InvokeMessage::~InvokeMessage()
> > +{
> > + if (deleteMethod_)
> > + delete method_;
> > +}
> > +
> > +/**
> > + * \brief Invoke the method bound to InvokeMessage::method_ with arguments
> > + * InvokeMessage::pack_
> > + */
> > +void InvokeMessage::invoke()
> > +{
> > + method_->invokePack(pack_);
> > +}
> >
> > /**
> > - * \var SignalMessage::method_
> > - * \brief The slot that the signal targets
> > + * \var InvokeMessage::method_
> > + * \brief The method to be invoked
> > */
> >
> > /**
> > - * \var SignalMessage::pack_
> > - * \brief The signal arguments
> > + * \var InvokeMessage::pack_
> > + * \brief The packed method invocation arguments
> > */
> >
> > }; /* namespace libcamera */
> > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> > index 0adbc203add8..7d70ce21b5d0 100644
> > --- a/src/libcamera/object.cpp
> > +++ b/src/libcamera/object.cpp
> > @@ -12,6 +12,7 @@
> > #include "log.h"
> > #include "message.h"
> > #include "thread.h"
> > +#include "utils.h"
> >
> > /**
> > * \file object.h
> > @@ -88,9 +89,9 @@ 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->method_->invokePack(smsg->pack_);
> > + case Message::InvokeMessage: {
> > + InvokeMessage *iMsg = static_cast<InvokeMessage *>(msg);
> > + iMsg->invoke();
> > break;
> > }
> >
> > @@ -99,6 +100,28 @@ void Object::message(Message *msg)
> > }
> > }
> >
> > +/**
> > + * \fn void Object::invokeMethod(void (T::*func)(Args...), Args... args)
> > + * \brief Invoke a method asynchronously on an Object instance
> > + * \param[in] func The object method to invoke
> > + * \param[in] args The method arguments
> > + *
> > + * This method invokes the member method \a func when control returns to the
> > + * event loop of the object's thread. The method is executed in the object's
> > + * thread with arguments \a args.
> > + *
> > + * Arguments \a args passed by value or reference are copied, while pointers
> > + * are passed untouched. The caller shall ensure that any pointer argument
> > + * remains valid until the method is invoked.
> > + */
> > +
> > +void Object::invokeMethod(BoundMethodBase *method, void *args)
> > +{
> > + std::unique_ptr<Message> msg =
> > + utils::make_unique<InvokeMessage>(method, args, true);
> > + postMessage(std::move(msg));
> > +}
> > +
> > /**
> > * \fn Object::thread()
> > * \brief Retrieve the thread the object is bound to
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list