[{"id":2991,"web_url":"https://patchwork.libcamera.org/comment/2991/","msgid":"<20191029233137.GQ20198@bigcity.dyn.berto.se>","date":"2019-10-29T23:31:37","subject":"Re: [libcamera-devel] [PATCH v2 9/9] android: Replace ThreadRPC\n\twith blocking method call","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch.\n\nOn 2019-10-28 12:49:13 +0200, Laurent Pinchart wrote:\n> From: Jacopo Mondi \n> \n> Use the newly introduced InvocationTypeBlocking message type to replace\n> the blocking message delivery implemented with the ThreadRPC class in the\n> Android camera HAL.\n> \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/android/camera_device.cpp | 34 ++++++------------------------\n> src/android/camera_device.h | 5 +----\n> src/android/camera_proxy.cpp | 21 ++++---------------\n> src/android/camera_proxy.h | 3 ---\n> src/android/meson.build | 1 -\n> src/android/thread_rpc.cpp | 26 -----------------------\n> src/android/thread_rpc.h | 39 -----------------------------------\n> 7 files changed, 11 insertions(+), 118 deletions(-)\n> delete mode 100644 src/android/thread_rpc.cpp\n> delete mode 100644 src/android/thread_rpc.h\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index c7c9b3fd1724..897f545864a9 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -11,7 +11,6 @@\n> #include \"utils.h\"\n> \n> #include \"camera_metadata.h\"\n> -#include \"thread_rpc.h\"\n> \n> using namespace libcamera;\n> \n> @@ -64,25 +63,6 @@ CameraDevice::~CameraDevice()\n> \t\tdelete it.second;\n> }\n> \n> -/*\n> - * Handle RPC request received from the associated proxy.\n> - */\n> -void CameraDevice::call(ThreadRpc *rpc)\n> -{\n> -\tswitch (rpc->tag) {\n> -\tcase ThreadRpc::ProcessCaptureRequest:\n> -\t\tprocessCaptureRequest(rpc->request);\n> -\t\tbreak;\n> -\tcase ThreadRpc::Close:\n> -\t\tclose();\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tLOG(HAL, Error) << \"Unknown RPC operation: \" << rpc->tag;\n> -\t}\n> -\n> -\trpc->notifyReception();\n> -}\n> -\n> int CameraDevice::open()\n> {\n> \tint ret = camera_->acquire();\n> @@ -698,7 +678,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \treturn 0;\n> }\n> \n> -int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n> +void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n> {\n> \tStreamConfiguration *streamConfiguration = &config_->at(0);\n> \tStream *stream = streamConfiguration->stream();\n> @@ -706,7 +686,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \tif (camera3Request->num_output_buffers != 1) {\n> \t\tLOG(HAL, Error) << \"Invalid number of output buffers: \"\n> \t\t\t\t<< camera3Request->num_output_buffers;\n> -\t\treturn -EINVAL;\n> +\t\treturn;\n> \t}\n> \n> \t/* Start the camera if that's the first request we handle. */\n> @@ -714,14 +694,14 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\tint ret = camera_->allocateBuffers();\n> \t\tif (ret) {\n> \t\t\tLOG(HAL, Error) << \"Failed to allocate buffers\";\n> -\t\t\treturn ret;\n> +\t\t\treturn;\n> \t\t}\n> \n> \t\tret = camera_->start();\n> \t\tif (ret) {\n> \t\t\tLOG(HAL, Error) << \"Failed to start camera\";\n> \t\t\tcamera_->freeBuffers();\n> -\t\t\treturn ret;\n> +\t\t\treturn;\n> \t\t}\n> \n> \t\trunning_ = true;\n> @@ -769,7 +749,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \tif (!buffer) {\n> \t\tLOG(HAL, Error) << \"Failed to create buffer\";\n> \t\tdelete descriptor;\n> -\t\treturn -EINVAL;\n> +\t\treturn;\n> \t}\n> \n> \tRequest *request =\n> @@ -782,13 +762,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\tgoto error;\n> \t}\n> \n> -\treturn 0;\n> +\treturn;\n> \n> error:\n> \tdelete request;\n> \tdelete descriptor;\n> -\n> -\treturn ret;\n> }\n> \n> void CameraDevice::requestComplete(Request *request,\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index d5d136a74f4a..2105b5b9a1e7 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -20,7 +20,6 @@\n> #include \"message.h\"\n> \n> class CameraMetadata;\n> -class ThreadRpc;\n> \n> class CameraDevice : public libcamera::Object\n> {\n> @@ -28,15 +27,13 @@ public:\n> \tCameraDevice(unsigned int id, const std::shared_ptr &camera);\n> \t~CameraDevice();\n> \n> -\tvoid call(ThreadRpc *rpc);\n> -\n> \tint open();\n> \tvoid close();\n> \tvoid setCallbacks(const camera3_callback_ops_t *callbacks);\n> \tcamera_metadata_t *getStaticMetadata();\n> \tconst camera_metadata_t *constructDefaultRequestSettings(int type);\n> \tint configureStreams(camera3_stream_configuration_t *stream_list);\n> -\tint processCaptureRequest(camera3_capture_request_t *request);\n> +\tvoid processCaptureRequest(camera3_capture_request_t *request);\n> \tvoid requestComplete(libcamera::Request *request,\n> \t\t\t const std::map &buffers);\n> \n> diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp\n> index 43e1e1c3af53..3964b5665e2f 100644\n> --- a/src/android/camera_proxy.cpp\n> +++ b/src/android/camera_proxy.cpp\n> @@ -16,7 +16,6 @@\n> #include \"utils.h\"\n> \n> #include \"camera_device.h\"\n> -#include \"thread_rpc.h\"\n> \n> using namespace libcamera;\n> \n> @@ -148,10 +147,8 @@ int CameraProxy::open(const hw_module_t *hardwareModule)\n> \n> void CameraProxy::close()\n> {\n> -\tThreadRpc rpcRequest;\n> -\trpcRequest.tag = ThreadRpc::Close;\n> -\n> -\tthreadRpcCall(rpcRequest);\n> +\tcameraDevice_->invokeMethod(&CameraDevice::close,\n> +\t\t\t\t ConnectionTypeBlocking);\n> }\n> \n> void CameraProxy::initialize(const camera3_callback_ops_t *callbacks)\n> @@ -176,18 +173,8 @@ int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list)\n> \n> int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)\n> {\n> -\tThreadRpc rpcRequest;\n> -\trpcRequest.tag = ThreadRpc::ProcessCaptureRequest;\n> -\trpcRequest.request = request;\n> -\n> -\tthreadRpcCall(rpcRequest);\n> +\tcameraDevice_->invokeMethod(&CameraDevice::processCaptureRequest,\n> +\t\t\t\t ConnectionTypeBlocking, request);\n> \n> \treturn 0;\n> }\n> -\n> -void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)\n> -{\n> -\tcameraDevice_->invokeMethod(&CameraDevice::call, ConnectionTypeQueued,\n> -\t\t\t\t &rpcRequest);\n> -\trpcRequest.waitDelivery();\n> -}\n> diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h\n> index 7940eac4e376..e8cfbc9dd526 100644\n> --- a/src/android/camera_proxy.h\n> +++ b/src/android/camera_proxy.h\n> @@ -14,7 +14,6 @@\n> #include \n> \n> class CameraDevice;\n> -class ThreadRpc;\n> \n> class CameraProxy\n> {\n> @@ -35,8 +34,6 @@ public:\n> \tcamera3_device_t *camera3Device() { return &camera3Device_; }\n> \n> private:\n> -\tvoid threadRpcCall(ThreadRpc &rpcRequest);\n> -\n> \tunsigned int id_;\n> \tCameraDevice *cameraDevice_;\n> \tcamera3_device_t camera3Device_;\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index b5e4eeeb73a8..70dfcc1df27a 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -4,7 +4,6 @@ android_hal_sources = files([\n> 'camera_device.cpp',\n> 'camera_metadata.cpp',\n> 'camera_proxy.cpp',\n> - 'thread_rpc.cpp'\n> ])\n> \n> android_camera_metadata_sources = files([\n> diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp\n> deleted file mode 100644\n> index f57891ff56bf..000000000000\n> --- a/src/android/thread_rpc.cpp\n> +++ /dev/null\n> @@ -1,26 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * thread_rpc.cpp - Inter-thread procedure call\n> - */\n> -\n> -#include \"thread.h\"\n> -#include \"thread_rpc.h\"\n> -\n> -using namespace libcamera;\n> -\n> -void ThreadRpc::notifyReception()\n> -{\n> -\t{\n> -\t\tlibcamera::MutexLocker locker(mutex_);\n> -\t\tdelivered_ = true;\n> -\t}\n> -\tcv_.notify_one();\n> -}\n> -\n> -void ThreadRpc::waitDelivery()\n> -{\n> -\tlibcamera::MutexLocker locker(mutex_);\n> -\tcv_.wait(locker, [&] { return delivered_; });\n> -}\n> diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h\n> deleted file mode 100644\n> index f577a5d9fb32..000000000000\n> --- a/src/android/thread_rpc.h\n> +++ /dev/null\n> @@ -1,39 +0,0 @@\n> -/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> -/*\n> - * Copyright (C) 2019, Google Inc.\n> - *\n> - * thread_rpc.h - Inter-thread procedure call\n> - */\n> -#ifndef __ANDROID_THREAD_RPC_H__\n> -#define __ANDROID_THREAD_RPC_H__\n> -\n> -#include \n> -#include \n> -\n> -#include \n> -\n> -class ThreadRpc\n> -{\n> -public:\n> -\tenum RpcTag {\n> -\t\tProcessCaptureRequest,\n> -\t\tClose,\n> -\t};\n> -\n> -\tThreadRpc()\n> -\t\t: delivered_(false) {}\n> -\n> -\tvoid notifyReception();\n> -\tvoid waitDelivery();\n> -\n> -\tRpcTag tag;\n> -\n> -\tcamera3_capture_request_t *request;\n> -\n> -private:\n> -\tbool delivered_;\n> -\tstd::mutex mutex_;\n> -\tstd::condition_variable cv_;\n> -};\n> -\n> -#endif /* __ANDROID_THREAD_RPC_H__ */\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D86460180\n\tfor ;\n\tWed, 30 Oct 2019 00:31:38 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id y127so105942lfc.0\n\tfor ;\n\tTue, 29 Oct 2019 16:31:38 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tg5sm125513ljk.22.2019.10.29.16.31.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 29 Oct 2019 16:31:37 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=5vPlwER+f2HHa7aVn36JC2IzHGtRgxtLc9kExORVJu8=;\n\tb=WEdZL9j/DO5zAQLG8dHn9u5ojCADpSr2d+zFBssWk+M2JxGLR4uZ0jPl9GQ2+ETfv2\n\tV0NP3COGPdJVWY23oVWWc+raO8nshCa3ubMtcIuS/ppmu+8nW6uIyt5twyiSY0YpCGoY\n\tN11Yk3g9OKQae65Mr+YxDJouZJ03PwIVwY1FAtx9CalblJ9iyIi2NnO7TdKK1Nc3B90A\n\tgAlNv3xYfQRiQfmhC0BbK0VdNBz7DVZ6AzjgLar8MJ+sa1KreXBcmLxh/DdcBHyz2eQO\n\t0FYcXagnW5y0UueyhwjP+9VETs9aXiWsaTiGcn+Ps+hwSq8Ah+4QJ1MUSBrmORhCxmWy\n\ty2uQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=5vPlwER+f2HHa7aVn36JC2IzHGtRgxtLc9kExORVJu8=;\n\tb=jaZhKcFmuv0zT1RcsI9ExGvBgWV5DAE9VLkNalfp0YncYJgAbzFTC+bV7MMqR0qRFb\n\txBp9paPoh1wHQIRh5UI0iNMwxHAwNYwwts+Wj2f2cEqAXH07Kc3/6airAAJLz53jdbK/\n\t3enrQ7KKpDSsbqes8bp9JDmdPYVuM7XK8qrtP795a3VivHE6f8RiGyh/AIm9QyxIAZmv\n\tsAp3aC3W31GQaEsPWBhOk+eysKjLm9Jj0cUdddZNhA2x31jOcIypGx/1GuCgkUUdH3a0\n\t8kSkmhKr3NvoWHBrKzZjw1RO3UNa2MA0PwU6DfnYK9ITCDn8wSotnN/yZiTITx3pOnT1\n\th1aQ==","X-Gm-Message-State":"APjAAAUmrGaaf0aGUWJ2iCle58qs9SnRPrJvcOzDUxlkB49WUcVe9Emc\n\tchoZc+a1CjZmL1ANn2/DIRxL4HFxj0c=","X-Google-Smtp-Source":"APXvYqwcxDqFa+IaTsP1QzyUuPkRM5RV7t/0qL9sJySMVxnUeq6KP90w4YhcfkUqiEdUHacCJ+WOCg==","X-Received":"by 2002:ac2:4a72:: with SMTP id\n\tq18mr4151453lfp.184.1572391898004; \n\tTue, 29 Oct 2019 16:31:38 -0700 (PDT)","Date":"Wed, 30 Oct 2019 00:31:37 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191029233137.GQ20198@bigcity.dyn.berto.se>","References":"<20191028104913.14985-1-laurent.pinchart@ideasonboard.com>\n\t<20191028104913.14985-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191028104913.14985-10-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 9/9] android: Replace ThreadRPC\n\twith blocking method call","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 29 Oct 2019 23:31:38 -0000"}}]