[libcamera-devel] [PATCH] hal: Simplify thread RPC with Object::invokeMethod()

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2019-08-12 18:02:32 +0300, Laurent Pinchart wrote:
> Replace the manual implementation of asynchronous method invocation
> through a custom message with Object::invokeMethod(). This simplifies
> the thread RPC implementation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I like the diffstat :-)

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

> ---
> 
> This patch applies on top of the "[PATCH 00/18] Object & Thread
> enhancements" series.
> 
>  src/android/camera_device.cpp |  8 +-------
>  src/android/camera_device.h   |  4 +++-
>  src/android/camera_proxy.cpp  |  8 +++-----
>  src/android/thread_rpc.cpp    | 18 +-----------------
>  src/android/thread_rpc.h      | 15 ---------------
>  5 files changed, 8 insertions(+), 45 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index e2c1f2a246c8..999c51e6ba4a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -70,14 +70,8 @@ CameraDevice::~CameraDevice()
>  /*
>   * Handle RPC request received from the associated proxy.
>   */
> -void CameraDevice::message(Message *message)
> +void CameraDevice::call(ThreadRpc *rpc)
>  {
> -	if (message->type() != ThreadRpcMessage::type())
> -		return Object::message(message);
> -
> -	ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message);
> -	ThreadRpc *rpc = rpcMessage->rpc;
> -
>  	switch (rpc->tag) {
>  	case ThreadRpc::ProcessCaptureRequest:
>  		processCaptureRequest(rpc->request);
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index ac5b95c95104..4d834ceb08a5 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -26,13 +26,15 @@
>  		return nullptr;		\
>  	} while(0);
>  
> +class ThreadRpc;
> +
>  class CameraDevice : public libcamera::Object
>  {
>  public:
>  	CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> &camera);
>  	~CameraDevice();
>  
> -	void message(libcamera::Message *message);
> +	void call(ThreadRpc *rpc);
>  
>  	int open();
>  	void close();
> diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp
> index f0cacac8025b..3eb2f9fbcffb 100644
> --- a/src/android/camera_proxy.cpp
> +++ b/src/android/camera_proxy.cpp
> @@ -9,6 +9,8 @@
>  
>  #include <system/camera_metadata.h>
>  
> +#include <libcamera/object.h>
> +
>  #include "log.h"
>  #include "message.h"
>  #include "utils.h"
> @@ -185,10 +187,6 @@ int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)
>  
>  void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)
>  {
> -	std::unique_ptr<ThreadRpcMessage> message =
> -				utils::make_unique<ThreadRpcMessage>();
> -	message->rpc = &rpcRequest;
> -
> -	cameraDevice_->postMessage(std::move(message));
> +	cameraDevice_->invokeMethod(&CameraDevice::call, &rpcRequest);
>  	rpcRequest.waitDelivery();
>  }
> diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp
> index 295a05d7c676..f57891ff56bf 100644
> --- a/src/android/thread_rpc.cpp
> +++ b/src/android/thread_rpc.cpp
> @@ -5,19 +5,11 @@
>   * thread_rpc.cpp - Inter-thread procedure call
>   */
>  
> +#include "thread.h"
>  #include "thread_rpc.h"
>  
> -#include "message.h"
> -
>  using namespace libcamera;
>  
> -libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None;
> -
> -ThreadRpcMessage::ThreadRpcMessage()
> -	: Message(type())
> -{
> -}
> -
>  void ThreadRpc::notifyReception()
>  {
>  	{
> @@ -32,11 +24,3 @@ void ThreadRpc::waitDelivery()
>  	libcamera::MutexLocker locker(mutex_);
>  	cv_.wait(locker, [&] { return delivered_; });
>  }
> -
> -Message::Type ThreadRpcMessage::type()
> -{
> -	if (ThreadRpcMessage::rpcType_ == Message::Type::None)
> -		rpcType_ = Message::registerMessageType();
> -
> -	return rpcType_;
> -}
> diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h
> index 6d8992839d0b..f577a5d9fb32 100644
> --- a/src/android/thread_rpc.h
> +++ b/src/android/thread_rpc.h
> @@ -12,9 +12,6 @@
>  
>  #include <hardware/camera3.h>
>  
> -#include "message.h"
> -#include "thread.h"
> -
>  class ThreadRpc
>  {
>  public:
> @@ -39,16 +36,4 @@ private:
>  	std::condition_variable cv_;
>  };
>  
> -class ThreadRpcMessage : public libcamera::Message
> -{
> -public:
> -	ThreadRpcMessage();
> -	ThreadRpc *rpc;
> -
> -	static Message::Type type();
> -
> -private:
> -	static libcamera::Message::Type rpcType_;
> -};
> -
>  #endif /* __ANDROID_THREAD_RPC_H__ */
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund