[libcamera-devel] [PATCH 07/18] libcamera: object: Notify objects of thread move

[Niklas Söderlund]

Hi Laurent,

On 2019-08-15 12:51:55 +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Thu, Aug 15, 2019 at 11:09:23AM +0200, Jacopo Mondi wrote:
> > On Mon, Aug 12, 2019 at 03:46:31PM +0300, Laurent Pinchart wrote:
> > > Send a synchronous message to objects just before they get moved to a
> > > new thread. This allows the object to perform any required processing.
> > > EventNotifier and Timer objects will use this mechanism to move
> > > themselves to the new thread's event disaptcher.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  include/libcamera/object.h      |  3 +++
> > >  src/libcamera/include/message.h |  1 +
> > >  src/libcamera/message.cpp       |  2 ++
> > >  src/libcamera/object.cpp        | 26 ++++++++++++++++++++++++++
> > >  4 files changed, 32 insertions(+)
> > >
> > > diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> > > index 869200a57d8c..14b939a9bd3d 100644
> > > --- a/include/libcamera/object.h
> > > +++ b/include/libcamera/object.h
> > > @@ -27,6 +27,7 @@ public:
> > >  	virtual ~Object();
> > >
> > >  	void postMessage(std::unique_ptr<Message> msg);
> > > +	void sendMessage(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)
> > > @@ -52,6 +53,8 @@ private:
> > >
> > >  	void invokeMethod(BoundMethodBase *method, void *pack);
> > >
> > > +	void notifyThreadMove();
> > > +
> > >  	void connect(SignalBase *signal);
> > >  	void disconnect(SignalBase *signal);
> > >
> > > diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> > > index 92717e316cc3..1cfde5669ede 100644
> > > --- a/src/libcamera/include/message.h
> > > +++ b/src/libcamera/include/message.h
> > > @@ -23,6 +23,7 @@ public:
> > >  	enum Type {
> > >  		None = 0,
> > >  		InvokeMessage = 1,
> > > +		ThreadMoveMessage = 2,
> > >  		UserMessage = 1000,
> > >  	};
> > >
> > > diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> > > index f6c39d40fc73..efafb655c17e 100644
> > > --- a/src/libcamera/message.cpp
> > > +++ b/src/libcamera/message.cpp
> > > @@ -47,6 +47,8 @@ std::atomic_uint Message::nextUserType_{ Message::UserMessage };
> > >   * \brief Invalid message type
> > >   * \var Message::InvokeMessage
> > >   * \brief Asynchronous method invocation across threads
> > > + * \var Message::ThreadMoveMessage
> > > + * \brief Object is being moved to a different thread
> > >   * \var Message::UserMessage
> > >   * \brief First value available for user-defined messages
> > >   */
> > > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> > > index 7d70ce21b5d0..7c68ec01f78c 100644
> > > --- a/src/libcamera/object.cpp
> > > +++ b/src/libcamera/object.cpp
> > > @@ -74,6 +74,20 @@ void Object::postMessage(std::unique_ptr<Message> msg)
> > >  	thread()->postMessage(std::move(msg), this);
> > >  }
> > >
> > > +/**
> > > + * \brief Send a message directly to the object
> > > + * \param[in] msg The message
> > > + *
> > > + * This method sends the message \a msg directly to the object, delivering it
> > > + * synchronously through the message() method in the current thread. Message
> > > + * ownership is not passed to the method, and the caller shall delete the
> > > + * message after this call.
> > > + */
> > > +void Object::sendMessage(Message *msg)
> > > +{
> > > +	message(msg);
> > > +}
> > > +
> > >  /**
> > >   * \brief Message handler for the object
> > >   * \param[in] msg The message
> > > @@ -135,6 +149,10 @@ void Object::invokeMethod(BoundMethodBase *method, void *args)
> > >   * This method moves the object from the current thread to the new \a thread.
> > >   * It shall be called from the thread in which the object currently lives,
> > >   * otherwise the behaviour is undefined.
> > > + *
> > > + * Before the object is moved, a Message::ThreadMoveMessage message is sent to
> > > + * it. The message() method can be reimplement in derived classes to be notified
> > > + * of the upcoming thread move and perform any required processing.
> > >   */
> > >  void Object::moveToThread(Thread *thread)
> > >  {
> > > @@ -143,9 +161,17 @@ void Object::moveToThread(Thread *thread)
> > >  	if (thread_ == thread)
> > >  		return;
> > >
> > > +	notifyThreadMove();
> > > +
> > >  	thread->moveObject(this);
> > >  }
> > >
> > > +void Object::notifyThreadMove()
> > > +{
> > > +	Message msg(Message::ThreadMoveMessage);
> > > +	sendMessage(&msg);
> > 
> > I missed why you can't call message(&msg) directly here.
> 
> I could do that. I wanted to provide a public function to send messages
> to objects though, and I thought I could then as well use it here. It
> makes no difference today, but maybe we'll have more code in
> sendMessage() tomorrow. We could of course then change this function.
> Should I drop sendMessage() for now ?

This is rather complex as it is, so if it's not used I think we should 
drop it until it's needed. But I don't feel strongly about it.

With or without sendMessage() removed,

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

> 
> > > +}
> > > +
> > >  void Object::connect(SignalBase *signal)
> > >  {
> > >  	signals_.push_back(signal);
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund