[libcamera-devel] [PATCH 6/6] android: Replace ThreadRPC with blocking method call
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 28 00:38:13 CET 2019
Hi Jacopo,
Thank you for the patch.
On Sun, Oct 27, 2019 at 09:33:35PM +0100, Jacopo Mondi wrote:
> Use the newly introduced InvocationTypeBlocking message type to replace
> the blocking message delivery implemented with the ThreadRPC class in the
> Android camera HAL.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
I really like this !!
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 34 ++++++------------------------
> src/android/camera_device.h | 5 +----
> src/android/camera_proxy.cpp | 21 ++++---------------
> src/android/camera_proxy.h | 3 ---
> src/android/meson.build | 1 -
> src/android/thread_rpc.cpp | 26 -----------------------
> src/android/thread_rpc.h | 39 -----------------------------------
> 7 files changed, 11 insertions(+), 118 deletions(-)
> delete mode 100644 src/android/thread_rpc.cpp
> delete mode 100644 src/android/thread_rpc.h
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index c7c9b3fd1724..897f545864a9 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -11,7 +11,6 @@
> #include "utils.h"
>
> #include "camera_metadata.h"
> -#include "thread_rpc.h"
>
> using namespace libcamera;
>
> @@ -64,25 +63,6 @@ CameraDevice::~CameraDevice()
> delete it.second;
> }
>
> -/*
> - * Handle RPC request received from the associated proxy.
> - */
> -void CameraDevice::call(ThreadRpc *rpc)
> -{
> - switch (rpc->tag) {
> - case ThreadRpc::ProcessCaptureRequest:
> - processCaptureRequest(rpc->request);
> - break;
> - case ThreadRpc::Close:
> - close();
> - break;
> - default:
> - LOG(HAL, Error) << "Unknown RPC operation: " << rpc->tag;
> - }
> -
> - rpc->notifyReception();
> -}
> -
> int CameraDevice::open()
> {
> int ret = camera_->acquire();
> @@ -698,7 +678,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> return 0;
> }
>
> -int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
> +void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
> {
> StreamConfiguration *streamConfiguration = &config_->at(0);
> Stream *stream = streamConfiguration->stream();
> @@ -706,7 +686,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> if (camera3Request->num_output_buffers != 1) {
> LOG(HAL, Error) << "Invalid number of output buffers: "
> << camera3Request->num_output_buffers;
> - return -EINVAL;
> + return;
> }
>
> /* Start the camera if that's the first request we handle. */
> @@ -714,14 +694,14 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> int ret = camera_->allocateBuffers();
> if (ret) {
> LOG(HAL, Error) << "Failed to allocate buffers";
> - return ret;
> + return;
> }
>
> ret = camera_->start();
> if (ret) {
> LOG(HAL, Error) << "Failed to start camera";
> camera_->freeBuffers();
> - return ret;
> + return;
> }
>
> running_ = true;
> @@ -769,7 +749,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> if (!buffer) {
> LOG(HAL, Error) << "Failed to create buffer";
> delete descriptor;
> - return -EINVAL;
> + return;
> }
>
> Request *request =
> @@ -782,13 +762,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> goto error;
> }
>
> - return 0;
> + return;
>
> error:
> delete request;
> delete descriptor;
> -
> - return ret;
> }
>
> void CameraDevice::requestComplete(Request *request,
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index d5d136a74f4a..2105b5b9a1e7 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -20,7 +20,6 @@
> #include "message.h"
>
> class CameraMetadata;
> -class ThreadRpc;
>
> class CameraDevice : public libcamera::Object
> {
> @@ -28,15 +27,13 @@ public:
> CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
> ~CameraDevice();
>
> - void call(ThreadRpc *rpc);
> -
> int open();
> void close();
> void setCallbacks(const camera3_callback_ops_t *callbacks);
> camera_metadata_t *getStaticMetadata();
> const camera_metadata_t *constructDefaultRequestSettings(int type);
> int configureStreams(camera3_stream_configuration_t *stream_list);
> - int processCaptureRequest(camera3_capture_request_t *request);
> + void processCaptureRequest(camera3_capture_request_t *request);
> void requestComplete(libcamera::Request *request,
> const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);
>
> diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp
> index 5f8428cfddfb..a48d8d265328 100644
> --- a/src/android/camera_proxy.cpp
> +++ b/src/android/camera_proxy.cpp
> @@ -16,7 +16,6 @@
> #include "utils.h"
>
> #include "camera_device.h"
> -#include "thread_rpc.h"
>
> using namespace libcamera;
>
> @@ -148,10 +147,8 @@ int CameraProxy::open(const hw_module_t *hardwareModule)
>
> void CameraProxy::close()
> {
> - ThreadRpc rpcRequest;
> - rpcRequest.tag = ThreadRpc::Close;
> -
> - threadRpcCall(rpcRequest);
> + cameraDevice_->invokeMethod(&CameraDevice::close,
> + InvocationTypeBlocking);
> }
>
> void CameraProxy::initialize(const camera3_callback_ops_t *callbacks)
> @@ -176,18 +173,8 @@ int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list)
>
> int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)
> {
> - ThreadRpc rpcRequest;
> - rpcRequest.tag = ThreadRpc::ProcessCaptureRequest;
> - rpcRequest.request = request;
> -
> - threadRpcCall(rpcRequest);
> + cameraDevice_->invokeMethod(&CameraDevice::processCaptureRequest,
> + InvocationTypeBlocking, request);
>
> return 0;
> }
> -
> -void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)
> -{
> - cameraDevice_->invokeMethod(&CameraDevice::call, InvocationTypeQueued,
> - &rpcRequest);
> - rpcRequest.waitDelivery();
> -}
> diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h
> index 7940eac4e376..e8cfbc9dd526 100644
> --- a/src/android/camera_proxy.h
> +++ b/src/android/camera_proxy.h
> @@ -14,7 +14,6 @@
> #include <libcamera/camera.h>
>
> class CameraDevice;
> -class ThreadRpc;
>
> class CameraProxy
> {
> @@ -35,8 +34,6 @@ public:
> camera3_device_t *camera3Device() { return &camera3Device_; }
>
> private:
> - void threadRpcCall(ThreadRpc &rpcRequest);
> -
> unsigned int id_;
> CameraDevice *cameraDevice_;
> camera3_device_t camera3Device_;
> diff --git a/src/android/meson.build b/src/android/meson.build
> index b5e4eeeb73a8..70dfcc1df27a 100644
> --- a/src/android/meson.build
> +++ b/src/android/meson.build
> @@ -4,7 +4,6 @@ android_hal_sources = files([
> 'camera_device.cpp',
> 'camera_metadata.cpp',
> 'camera_proxy.cpp',
> - 'thread_rpc.cpp'
> ])
>
> android_camera_metadata_sources = files([
> diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp
> deleted file mode 100644
> index f57891ff56bf..000000000000
> --- a/src/android/thread_rpc.cpp
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * thread_rpc.cpp - Inter-thread procedure call
> - */
> -
> -#include "thread.h"
> -#include "thread_rpc.h"
> -
> -using namespace libcamera;
> -
> -void ThreadRpc::notifyReception()
> -{
> - {
> - libcamera::MutexLocker locker(mutex_);
> - delivered_ = true;
> - }
> - cv_.notify_one();
> -}
> -
> -void ThreadRpc::waitDelivery()
> -{
> - libcamera::MutexLocker locker(mutex_);
> - cv_.wait(locker, [&] { return delivered_; });
> -}
> diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h
> deleted file mode 100644
> index f577a5d9fb32..000000000000
> --- a/src/android/thread_rpc.h
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * thread_rpc.h - Inter-thread procedure call
> - */
> -#ifndef __ANDROID_THREAD_RPC_H__
> -#define __ANDROID_THREAD_RPC_H__
> -
> -#include <condition_variable>
> -#include <mutex>
> -
> -#include <hardware/camera3.h>
> -
> -class ThreadRpc
> -{
> -public:
> - enum RpcTag {
> - ProcessCaptureRequest,
> - Close,
> - };
> -
> - ThreadRpc()
> - : delivered_(false) {}
> -
> - void notifyReception();
> - void waitDelivery();
> -
> - RpcTag tag;
> -
> - camera3_capture_request_t *request;
> -
> -private:
> - bool delivered_;
> - std::mutex mutex_;
> - std::condition_variable cv_;
> -};
> -
> -#endif /* __ANDROID_THREAD_RPC_H__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list