[libcamera-devel] [PATCH 15/18] libcamera: object: Create parent-child relationships

[Laurent Pinchart]

Hi Jacopo,

On Thu, Aug 15, 2019 at 11:56:47AM +0200, Jacopo Mondi wrote:
> On Mon, Aug 12, 2019 at 03:46:39PM +0300, Laurent Pinchart wrote:
> > Add a parent Object to Object instances, and track the parent-children
> > relationships. Children are bound to the same thread as their parent,
> > and moving an Object to a thread automatically moves all its children.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/object.h     |  8 ++++-
> >  src/libcamera/include/thread.h |  2 ++
> >  src/libcamera/object.cpp       | 65 ++++++++++++++++++++++++++++------
> >  src/libcamera/thread.cpp       | 12 ++++++-
> >  4 files changed, 74 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> > index 14b939a9bd3d..4a0a272b875a 100644
> > --- a/include/libcamera/object.h
> > +++ b/include/libcamera/object.h
> > @@ -9,6 +9,7 @@
> >
> >  #include <list>
> >  #include <memory>
> > +#include <vector>
> >
> >  #include <libcamera/bound_method.h>
> >
> > @@ -23,7 +24,7 @@ class Thread;
> >  class Object
> >  {
> >  public:
> > -	Object();
> > +	Object(Object *parent = nullptr);
> >  	virtual ~Object();
> >
> >  	void postMessage(std::unique_ptr<Message> msg);
> > @@ -42,6 +43,8 @@ public:
> >  	Thread *thread() const { return thread_; }
> >  	void moveToThread(Thread *thread);
> >
> > +	Object *parent() const { return parent_; }
> 
> maybe const Object * ?

There's a need to call non-const function of the parent, so we need a
non-const pointer. I could add two versions of this method, const Object
*parent() const and Object *parent(), but I don't think that's needed.
The parent isn't part of the child, so modifying it doesn't modify the
state of the child.

> > +
> >  protected:
> >  	virtual void message(Message *msg);
> >
> > @@ -58,6 +61,9 @@ private:
> >  	void connect(SignalBase *signal);
> >  	void disconnect(SignalBase *signal);
> >
> > +	Object *parent_;
> > +	std::vector<Object *> children_;
> > +
> >  	Thread *thread_;
> >  	std::list<SignalBase *> signals_;
> >  	unsigned int pendingMessages_;
> > diff --git a/src/libcamera/include/thread.h b/src/libcamera/include/thread.h
> > index 630abb49534f..37edd4f5138b 100644
> > --- a/src/libcamera/include/thread.h
> > +++ b/src/libcamera/include/thread.h
> > @@ -61,6 +61,8 @@ private:
> >  	friend class ThreadMain;
> >
> >  	void moveObject(Object *object);
> > +	void moveObject(Object *object, ThreadData *currentData,
> > +			ThreadData *targetData);
> >
> >  	std::thread thread_;
> >  	ThreadData *data_;
> > diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> > index 7c68ec01f78c..57cd6fefe20f 100644
> > --- a/src/libcamera/object.cpp
> > +++ b/src/libcamera/object.cpp
> > @@ -7,6 +7,8 @@
> >
> >  #include <libcamera/object.h>
> >
> > +#include <algorithm>
> > +
> >  #include <libcamera/signal.h>
> >
> >  #include "log.h"
> > @@ -21,6 +23,8 @@
> >
> >  namespace libcamera {
> >
> > +LOG_DEFINE_CATEGORY(Object)
> > +
> >  /**
> >   * \class Object
> >   * \brief Base object to support automatic signal disconnection
> > @@ -29,10 +33,11 @@ namespace libcamera {
> >   * slots. By inheriting from Object, an object is automatically disconnected
> >   * from all connected signals when it gets destroyed.
> >   *
> > - * Object instances are bound to the thread in which they're created. When a
> > - * message is posted to an object, its handler will run in the object's thread.
> > - * This allows implementing easy message passing between threads by inheriting
> > - * from the Object class.
> > + * Object instances are bound to the thread of their parent, or the thread in
> > + * which they're created when they have no parent. When a message is posted to
> > + * an object, its handler will run in the object's thread. 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
> > @@ -41,10 +46,20 @@ namespace libcamera {
> >   * \sa Message, Signal, Thread
> >   */
> >
> > -Object::Object()
> > -	: pendingMessages_(0)
> > +/**
> > + * \brief Construct an Object instance
> > + * \param[in] parent The object parent
> > + *
> > + * The new Object instance is bound to the thread of its \a parent, or to the
> > + * current thread if the \a parent is nullptr.
> > + */
> > +Object::Object(Object *parent)
> > +	: parent_(parent), pendingMessages_(0)
> >  {
> > -	thread_ = Thread::current();
> > +	thread_ = parent ? parent->thread() : Thread::current();
> > +
> > +	if (parent)
> > +		parent->children_.push_back(this);
> >  }
> >
> >  Object::~Object()
> > @@ -54,6 +69,16 @@ Object::~Object()
> >
> >  	if (pendingMessages_)
> >  		thread()->removeMessages(this);
> > +
> > +	if (parent_) {
> > +		auto it = std::find(parent_->children_.begin(),
> > +				    parent_->children_.end(), this);
> > +		ASSERT(it != parent_->children_.end());
> > +		parent_->children_.erase(it);
> > +	}
> > +
> > +	for (auto child : children_)
> > +		child->parent_ = nullptr;
> >  }
> >
> >  /**
> > @@ -143,16 +168,19 @@ void Object::invokeMethod(BoundMethodBase *method, void *args)
> >   */
> >
> >  /**
> > - * \brief Move the object to a different thread
> > + * \brief Move the object and all its children to a different thread
> >   * \param[in] thread The target thread
> >   *
> > - * 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.
> > + * This method moves the object and all its children 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.
> > + *
> > + * Moving an object that has a parent is not allowed, and causes undefined
> > + * behaviour.
> >   */
> >  void Object::moveToThread(Thread *thread)
> >  {
> > @@ -161,6 +189,12 @@ void Object::moveToThread(Thread *thread)
> >  	if (thread_ == thread)
> >  		return;
> >
> > +	if (parent_) {
> > +		LOG(Object, Error)
> > +			<< "Moving object to thread with a parent is not permitted";
> > +		return;
> > +	}
> > +
> >  	notifyThreadMove();
> >
> >  	thread->moveObject(this);
> > @@ -170,8 +204,17 @@ void Object::notifyThreadMove()
> >  {
> >  	Message msg(Message::ThreadMoveMessage);
> >  	sendMessage(&msg);
> > +
> > +	for (auto child : children_)
> > +		child->notifyThreadMove();
> >  }
> >
> > +/**
> > + * \fn Object::parent()
> > + * \brief Retrieve the object's parent
> > + * \return The object's parent
> > + */
> > +
> >  void Object::connect(SignalBase *signal)
> >  {
> >  	signals_.push_back(signal);
> > diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp
> > index 24422f7b7bd0..872ad1bd9d69 100644
> > --- a/src/libcamera/thread.cpp
> > +++ b/src/libcamera/thread.cpp
> > @@ -448,7 +448,7 @@ void Thread::dispatchMessages()
> >  }
> >
> >  /**
> > - * \brief Move an \a object to the thread
> > + * \brief Move an \a object and all its children to the thread
> >   * \param[in] object The object
> >   */
> >  void Thread::moveObject(Object *object)
> > @@ -460,6 +460,12 @@ void Thread::moveObject(Object *object)
> >  	MutexLocker lockerTo(targetData->mutex_, std::defer_lock);
> >  	std::lock(lockerFrom, lockerTo);
> >
> > +	moveObject(object, currentData, targetData);
> > +}
> > +
> > +void Thread::moveObject(Object *object, ThreadData *currentData,
> > +			ThreadData *targetData)
> > +{
> >  	/* Move pending messages to the message queue of the new thread. */
> >  	if (object->pendingMessages_) {
> >  		unsigned int movedMessages = 0;
> > @@ -483,6 +489,10 @@ void Thread::moveObject(Object *object)
> >  	}
> >
> >  	object->thread_ = this;
> > +
> > +	/* Move all children. */
> > +	for (auto child : object->children_)
> > +		moveObject(child, currentData, targetData);
> >  }
> >
> >  }; /* namespace libcamera */
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel



-- 
Regards,

Laurent Pinchart