[libcamera-devel] [PATCH v2 4/9] libcamera: bound_method: Support connection types

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2019-10-28 12:49:08 +0200, Laurent Pinchart wrote:
> Support all connection types in the BoundMethodBase::activePack()
> method. To support this, add a semaphore to the InvokeMessage to signal
> delivery.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  src/libcamera/bound_method.cpp  | 30 ++++++++++++++++++++++++++++--
>  src/libcamera/include/message.h |  5 +++++
>  src/libcamera/message.cpp       | 11 +++++++++--
>  src/libcamera/object.cpp        |  8 +++++++-
>  4 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> index ab6ecd9423d1..600717363444 100644
> --- a/src/libcamera/bound_method.cpp
> +++ b/src/libcamera/bound_method.cpp
> @@ -8,6 +8,7 @@
>  #include <libcamera/bound_method.h>
>  
>  #include "message.h"
> +#include "semaphore.h"
>  #include "thread.h"
>  #include "utils.h"
>  
> @@ -49,12 +50,37 @@ namespace libcamera {
>  
>  void BoundMethodBase::activatePack(void *pack)
>  {
> -	if (Thread::current() == object_->thread()) {
> +	ConnectionType type = connectionType_;
> +	if (type == ConnectionTypeAuto) {
> +		if (Thread::current() == object_->thread())
> +			type = ConnectionTypeDirect;
> +		else
> +			type = ConnectionTypeQueued;
> +	}
> +
> +	switch (type) {
> +	case ConnectionTypeDirect:
> +	default:
>  		invokePack(pack);
> -	} else {
> +		break;
> +
> +	case ConnectionTypeQueued: {
>  		std::unique_ptr<Message> msg =
>  			utils::make_unique<InvokeMessage>(this, pack);
>  		object_->postMessage(std::move(msg));
> +		break;
> +	}
> +
> +	case ConnectionTypeBlocking: {
> +		Semaphore semaphore;
> +
> +		std::unique_ptr<Message> msg =
> +			utils::make_unique<InvokeMessage>(this, pack, &semaphore);
> +		object_->postMessage(std::move(msg));
> +
> +		semaphore.acquire();
> +		break;
> +	}
>  	}
>  }
>  
> diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h
> index 1cfde5669ede..311755cc60fe 100644
> --- a/src/libcamera/include/message.h
> +++ b/src/libcamera/include/message.h
> @@ -15,6 +15,7 @@ namespace libcamera {
>  
>  class BoundMethodBase;
>  class Object;
> +class Semaphore;
>  class Thread;
>  
>  class Message
> @@ -48,14 +49,18 @@ class InvokeMessage : public Message
>  {
>  public:
>  	InvokeMessage(BoundMethodBase *method, void *pack,
> +		      Semaphore *semaphore = nullptr,
>  		      bool deleteMethod = false);
>  	~InvokeMessage();
>  
> +	Semaphore *semaphore() const { return semaphore_; }
> +
>  	void invoke();
>  
>  private:
>  	BoundMethodBase *method_;
>  	void *pack_;
> +	Semaphore *semaphore_;
>  	bool deleteMethod_;
>  };
>  
> diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp
> index efafb655c17e..daf653221077 100644
> --- a/src/libcamera/message.cpp
> +++ b/src/libcamera/message.cpp
> @@ -119,13 +119,14 @@ Message::Type Message::registerMessageType()
>   * \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] semaphore The semaphore used to signal message delivery
>   * \param[in] deleteMethod True to delete the \a method when the message is
>   * destroyed
>   */
>  InvokeMessage::InvokeMessage(BoundMethodBase *method, void *pack,
> -			     bool deleteMethod)
> +			     Semaphore *semaphore, bool deleteMethod)
>  	: Message(Message::InvokeMessage), method_(method), pack_(pack),
> -	  deleteMethod_(deleteMethod)
> +	  semaphore_(semaphore), deleteMethod_(deleteMethod)
>  {
>  }
>  
> @@ -135,6 +136,12 @@ InvokeMessage::~InvokeMessage()
>  		delete method_;
>  }
>  
> +/**
> + * \fn InvokeMessage::semaphore()
> + * \brief Retrieve the message semaphore passed to the constructor
> + * \return The message semaphore
> + */
> +
>  /**
>   * \brief Invoke the method bound to InvokeMessage::method_ with arguments
>   * InvokeMessage::pack_
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index 98aa0af2f9b9..b0818f9aa25f 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -13,6 +13,7 @@
>  
>  #include "log.h"
>  #include "message.h"
> +#include "semaphore.h"
>  #include "thread.h"
>  #include "utils.h"
>  
> @@ -123,7 +124,12 @@ void Object::message(Message *msg)
>  	switch (msg->type()) {
>  	case Message::InvokeMessage: {
>  		InvokeMessage *iMsg = static_cast<InvokeMessage *>(msg);
> +		Semaphore *semaphore = iMsg->semaphore();
>  		iMsg->invoke();
> +
> +		if (semaphore)
> +			semaphore->release();
> +
>  		break;
>  	}
>  
> @@ -150,7 +156,7 @@ void Object::message(Message *msg)
>  void Object::invokeMethod(BoundMethodBase *method, void *args)
>  {
>  	std::unique_ptr<Message> msg =
> -		utils::make_unique<InvokeMessage>(method, args, true);
> +		utils::make_unique<InvokeMessage>(method, args, nullptr, true);
>  	postMessage(std::move(msg));
>  }
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund