[{"id":2450,"web_url":"https://patchwork.libcamera.org/comment/2450/","msgid":"<20190817154511.GA22846@wyvern>","date":"2019-08-17T15:45:11","subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-08-12 18:02:32 +0300, Laurent Pinchart wrote:\n> Replace the manual implementation of asynchronous method invocation\n> through a custom message with Object::invokeMethod(). This simplifies\n> the thread RPC implementation.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nI like the diffstat :-)\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n> \n> This patch applies on top of the \"[PATCH 00/18] Object & Thread\n> enhancements\" series.\n> \n>  src/android/camera_device.cpp |  8 +-------\n>  src/android/camera_device.h   |  4 +++-\n>  src/android/camera_proxy.cpp  |  8 +++-----\n>  src/android/thread_rpc.cpp    | 18 +-----------------\n>  src/android/thread_rpc.h      | 15 ---------------\n>  5 files changed, 8 insertions(+), 45 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index e2c1f2a246c8..999c51e6ba4a 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -70,14 +70,8 @@ CameraDevice::~CameraDevice()\n>  /*\n>   * Handle RPC request received from the associated proxy.\n>   */\n> -void CameraDevice::message(Message *message)\n> +void CameraDevice::call(ThreadRpc *rpc)\n>  {\n> -\tif (message->type() != ThreadRpcMessage::type())\n> -\t\treturn Object::message(message);\n> -\n> -\tThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message);\n> -\tThreadRpc *rpc = rpcMessage->rpc;\n> -\n>  \tswitch (rpc->tag) {\n>  \tcase ThreadRpc::ProcessCaptureRequest:\n>  \t\tprocessCaptureRequest(rpc->request);\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index ac5b95c95104..4d834ceb08a5 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -26,13 +26,15 @@\n>  \t\treturn nullptr;\t\t\\\n>  \t} while(0);\n>  \n> +class ThreadRpc;\n> +\n>  class CameraDevice : public libcamera::Object\n>  {\n>  public:\n>  \tCameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> &camera);\n>  \t~CameraDevice();\n>  \n> -\tvoid message(libcamera::Message *message);\n> +\tvoid call(ThreadRpc *rpc);\n>  \n>  \tint open();\n>  \tvoid close();\n> diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp\n> index f0cacac8025b..3eb2f9fbcffb 100644\n> --- a/src/android/camera_proxy.cpp\n> +++ b/src/android/camera_proxy.cpp\n> @@ -9,6 +9,8 @@\n>  \n>  #include <system/camera_metadata.h>\n>  \n> +#include <libcamera/object.h>\n> +\n>  #include \"log.h\"\n>  #include \"message.h\"\n>  #include \"utils.h\"\n> @@ -185,10 +187,6 @@ int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)\n>  \n>  void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)\n>  {\n> -\tstd::unique_ptr<ThreadRpcMessage> message =\n> -\t\t\t\tutils::make_unique<ThreadRpcMessage>();\n> -\tmessage->rpc = &rpcRequest;\n> -\n> -\tcameraDevice_->postMessage(std::move(message));\n> +\tcameraDevice_->invokeMethod(&CameraDevice::call, &rpcRequest);\n>  \trpcRequest.waitDelivery();\n>  }\n> diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp\n> index 295a05d7c676..f57891ff56bf 100644\n> --- a/src/android/thread_rpc.cpp\n> +++ b/src/android/thread_rpc.cpp\n> @@ -5,19 +5,11 @@\n>   * thread_rpc.cpp - Inter-thread procedure call\n>   */\n>  \n> +#include \"thread.h\"\n>  #include \"thread_rpc.h\"\n>  \n> -#include \"message.h\"\n> -\n>  using namespace libcamera;\n>  \n> -libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None;\n> -\n> -ThreadRpcMessage::ThreadRpcMessage()\n> -\t: Message(type())\n> -{\n> -}\n> -\n>  void ThreadRpc::notifyReception()\n>  {\n>  \t{\n> @@ -32,11 +24,3 @@ void ThreadRpc::waitDelivery()\n>  \tlibcamera::MutexLocker locker(mutex_);\n>  \tcv_.wait(locker, [&] { return delivered_; });\n>  }\n> -\n> -Message::Type ThreadRpcMessage::type()\n> -{\n> -\tif (ThreadRpcMessage::rpcType_ == Message::Type::None)\n> -\t\trpcType_ = Message::registerMessageType();\n> -\n> -\treturn rpcType_;\n> -}\n> diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h\n> index 6d8992839d0b..f577a5d9fb32 100644\n> --- a/src/android/thread_rpc.h\n> +++ b/src/android/thread_rpc.h\n> @@ -12,9 +12,6 @@\n>  \n>  #include <hardware/camera3.h>\n>  \n> -#include \"message.h\"\n> -#include \"thread.h\"\n> -\n>  class ThreadRpc\n>  {\n>  public:\n> @@ -39,16 +36,4 @@ private:\n>  \tstd::condition_variable cv_;\n>  };\n>  \n> -class ThreadRpcMessage : public libcamera::Message\n> -{\n> -public:\n> -\tThreadRpcMessage();\n> -\tThreadRpc *rpc;\n> -\n> -\tstatic Message::Type type();\n> -\n> -private:\n> -\tstatic libcamera::Message::Type rpcType_;\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-ed1-x543.google.com (mail-ed1-x543.google.com\n\t[IPv6:2a00:1450:4864:20::543])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C5301600F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 17 Aug 2019 17:45:13 +0200 (CEST)","by mail-ed1-x543.google.com with SMTP id h8so7600185edv.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 17 Aug 2019 08:45:13 -0700 (PDT)","from localhost ([185.224.57.161]) by smtp.gmail.com with ESMTPSA id\n\th13sm622450edw.78.2019.08.17.08.45.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 17 Aug 2019 08:45:12 -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=8q1HFJqfpjT/ySxFE5EtamufkKcPgl6a3NA2BbYGSyI=;\n\tb=QulKO9RFgcsa8E55ye6Z747lNMrh+JzHmrRo0iGG6NcfJyZQSBI/in91cNsOVh+blq\n\teqtFWjfZLB8RPReUj9I7WgthH6DRw3BQLMItWurtnDAd5uiOh9/s70eKDL1x/odm4stz\n\tIsEqA9+IDyXB6oexILsboieDc4OS+SvuW3jcY3tJ0QkCiCtUIYBE9BgA0V7tpN4efMin\n\tRD0oUuKbplT04Bi7BpSghloa1q40vTLxrvndGcmgyloj9NuPvL4MsN3I2htZP7eDg+7N\n\tKFlh6P3kpainhtKUBbjL7d80NJd3+TBaIJdYcoqT1SyAuxZK0uRY3ufedoD7TyvQxHof\n\t5JBg==","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=8q1HFJqfpjT/ySxFE5EtamufkKcPgl6a3NA2BbYGSyI=;\n\tb=NUJqP1HzbaqfkIT2KppH2yCkmX1RZR5WN7ToSOacZRtOOIzr5F1GDq9dAH00G/Zgfi\n\tBpqlaekRjexspotC3l7vssLoNqT9HAYzcnd83ZGYWH7ajz0DbaQrE2jWTrClorB+4lHv\n\tvpx6WA6SAToU6ISZH9SdKAi8DhKd23BVJpOsP82qGvmRoETg2/8oroK/qzlXpGACGUsb\n\tXog7uSlaKMN0i+CF0/9SBes3QsvS5WHBZY0V7NGyV00GHyMEGSafp35gJdkmoU+IOc8y\n\tYAZ58RSVlOVCiGRMTBltO0eNONR7Zymi8gCbmFvKLwxjEUNluM0cKADvKJMH+nIT1FQW\n\ta8FQ==","X-Gm-Message-State":"APjAAAUPOoHFB38DAYu3zp9y5O1sZg+pz2KTS+NC8VeA58NlzKF4Mf6f\n\tXwgdbM+qfAJ3NMEfGdvhUswCVEmwSAs=","X-Google-Smtp-Source":"APXvYqy3+Z7ZYVwAmRWH0t/aH3FT0y2Y9j6eBIEE7V7zpgGBgHoT0vWLdUY/pH7Kb9o475iL9Xv4bA==","X-Received":"by 2002:aa7:c35a:: with SMTP id\n\tj26mr12973474edr.270.1566056713451; \n\tSat, 17 Aug 2019 08:45:13 -0700 (PDT)","Date":"Sat, 17 Aug 2019 17:45:11 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190817154511.GA22846@wyvern>","References":"<20190812150232.1088-1-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":"<20190812150232.1088-1-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 17 Aug 2019 15:45:13 -0000"}},{"id":2486,"web_url":"https://patchwork.libcamera.org/comment/2486/","msgid":"<20190819144438.vim4l6x2qzipvlx5@uno.localdomain>","date":"2019-08-19T14:44:38","subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Aug 17, 2019 at 05:45:11PM +0200, Niklas Söderlund wrote:\n> Hi Laurent,\n>\n> Thanks for your work.\n>\n> On 2019-08-12 18:02:32 +0300, Laurent Pinchart wrote:\n> > Replace the manual implementation of asynchronous method invocation\n> > through a custom message with Object::invokeMethod(). This simplifies\n> > the thread RPC implementation.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n\nThe patch itself is good, it removes quite some stuff so it's good to\ngo in my opinion. I'm just wondering, the current CameraProxy::threadRpcCall\nimplemententation basically blocks the caller until the invoked\nmethods has not been executed on the callee. Would it make sense to\nmake out of this patter an invokeMethodsSynchronous() operation?\n\nIn any case\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n\n> I like the diffstat :-)\n>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>\n> > ---\n> >\n> > This patch applies on top of the \"[PATCH 00/18] Object & Thread\n> > enhancements\" series.\n> >\n> >  src/android/camera_device.cpp |  8 +-------\n> >  src/android/camera_device.h   |  4 +++-\n> >  src/android/camera_proxy.cpp  |  8 +++-----\n> >  src/android/thread_rpc.cpp    | 18 +-----------------\n> >  src/android/thread_rpc.h      | 15 ---------------\n> >  5 files changed, 8 insertions(+), 45 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index e2c1f2a246c8..999c51e6ba4a 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -70,14 +70,8 @@ CameraDevice::~CameraDevice()\n> >  /*\n> >   * Handle RPC request received from the associated proxy.\n> >   */\n> > -void CameraDevice::message(Message *message)\n> > +void CameraDevice::call(ThreadRpc *rpc)\n> >  {\n> > -\tif (message->type() != ThreadRpcMessage::type())\n> > -\t\treturn Object::message(message);\n> > -\n> > -\tThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message);\n> > -\tThreadRpc *rpc = rpcMessage->rpc;\n> > -\n> >  \tswitch (rpc->tag) {\n> >  \tcase ThreadRpc::ProcessCaptureRequest:\n> >  \t\tprocessCaptureRequest(rpc->request);\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index ac5b95c95104..4d834ceb08a5 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -26,13 +26,15 @@\n> >  \t\treturn nullptr;\t\t\\\n> >  \t} while(0);\n> >\n> > +class ThreadRpc;\n> > +\n> >  class CameraDevice : public libcamera::Object\n> >  {\n> >  public:\n> >  \tCameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> &camera);\n> >  \t~CameraDevice();\n> >\n> > -\tvoid message(libcamera::Message *message);\n> > +\tvoid call(ThreadRpc *rpc);\n> >\n> >  \tint open();\n> >  \tvoid close();\n> > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp\n> > index f0cacac8025b..3eb2f9fbcffb 100644\n> > --- a/src/android/camera_proxy.cpp\n> > +++ b/src/android/camera_proxy.cpp\n> > @@ -9,6 +9,8 @@\n> >\n> >  #include <system/camera_metadata.h>\n> >\n> > +#include <libcamera/object.h>\n> > +\n> >  #include \"log.h\"\n> >  #include \"message.h\"\n> >  #include \"utils.h\"\n> > @@ -185,10 +187,6 @@ int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)\n> >\n> >  void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)\n> >  {\n> > -\tstd::unique_ptr<ThreadRpcMessage> message =\n> > -\t\t\t\tutils::make_unique<ThreadRpcMessage>();\n> > -\tmessage->rpc = &rpcRequest;\n> > -\n> > -\tcameraDevice_->postMessage(std::move(message));\n> > +\tcameraDevice_->invokeMethod(&CameraDevice::call, &rpcRequest);\n> >  \trpcRequest.waitDelivery();\n> >  }\n> > diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp\n> > index 295a05d7c676..f57891ff56bf 100644\n> > --- a/src/android/thread_rpc.cpp\n> > +++ b/src/android/thread_rpc.cpp\n> > @@ -5,19 +5,11 @@\n> >   * thread_rpc.cpp - Inter-thread procedure call\n> >   */\n> >\n> > +#include \"thread.h\"\n> >  #include \"thread_rpc.h\"\n> >\n> > -#include \"message.h\"\n> > -\n> >  using namespace libcamera;\n> >\n> > -libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None;\n> > -\n> > -ThreadRpcMessage::ThreadRpcMessage()\n> > -\t: Message(type())\n> > -{\n> > -}\n> > -\n> >  void ThreadRpc::notifyReception()\n> >  {\n> >  \t{\n> > @@ -32,11 +24,3 @@ void ThreadRpc::waitDelivery()\n> >  \tlibcamera::MutexLocker locker(mutex_);\n> >  \tcv_.wait(locker, [&] { return delivered_; });\n> >  }\n> > -\n> > -Message::Type ThreadRpcMessage::type()\n> > -{\n> > -\tif (ThreadRpcMessage::rpcType_ == Message::Type::None)\n> > -\t\trpcType_ = Message::registerMessageType();\n> > -\n> > -\treturn rpcType_;\n> > -}\n> > diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h\n> > index 6d8992839d0b..f577a5d9fb32 100644\n> > --- a/src/android/thread_rpc.h\n> > +++ b/src/android/thread_rpc.h\n> > @@ -12,9 +12,6 @@\n> >\n> >  #include <hardware/camera3.h>\n> >\n> > -#include \"message.h\"\n> > -#include \"thread.h\"\n> > -\n> >  class ThreadRpc\n> >  {\n> >  public:\n> > @@ -39,16 +36,4 @@ private:\n> >  \tstd::condition_variable cv_;\n> >  };\n> >\n> > -class ThreadRpcMessage : public libcamera::Message\n> > -{\n> > -public:\n> > -\tThreadRpcMessage();\n> > -\tThreadRpc *rpc;\n> > -\n> > -\tstatic Message::Type type();\n> > -\n> > -private:\n> > -\tstatic libcamera::Message::Type rpcType_;\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\n>\n> --\n> Regards,\n> Niklas Söderlund\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87A4A60C1E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 19 Aug 2019 16:43:15 +0200 (CEST)","from uno.localdomain (unknown [87.18.63.98])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 68B5B200008;\n\tMon, 19 Aug 2019 14:43:14 +0000 (UTC)"],"Date":"Mon, 19 Aug 2019 16:44:38 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190819144438.vim4l6x2qzipvlx5@uno.localdomain>","References":"<20190812150232.1088-1-laurent.pinchart@ideasonboard.com>\n\t<20190817154511.GA22846@wyvern>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"aego2jd3an5373ii\"","Content-Disposition":"inline","In-Reply-To":"<20190817154511.GA22846@wyvern>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 19 Aug 2019 14:43:15 -0000"}},{"id":2487,"web_url":"https://patchwork.libcamera.org/comment/2487/","msgid":"<20190819150148.GG5011@pendragon.ideasonboard.com>","date":"2019-08-19T15:01:48","subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Aug 19, 2019 at 04:44:38PM +0200, Jacopo Mondi wrote:\n> On Sat, Aug 17, 2019 at 05:45:11PM +0200, Niklas Söderlund wrote:\n> > On 2019-08-12 18:02:32 +0300, Laurent Pinchart wrote:\n> >> Replace the manual implementation of asynchronous method invocation\n> >> through a custom message with Object::invokeMethod(). This simplifies\n> >> the thread RPC implementation.\n> >>\n> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> The patch itself is good, it removes quite some stuff so it's good to\n> go in my opinion. I'm just wondering, the current CameraProxy::threadRpcCall\n> implemententation basically blocks the caller until the invoked\n> methods has not been executed on the callee. Would it make sense to\n> make out of this patter an invokeMethodsSynchronous() operation?\n\nIt's on my todo list :-) I'm thinking of adding an argument to\ninvokeMethod() (as well to Signal::emit()) to select whether the method\nor slots should be called immediately (synchronously in the same\nthread), through the message queue, through the message queue with a\nsynchronous wait, or determine the behaviour automatically depending on\nthe thread of the caller and callee.\n\n> In any case\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> > I like the diffstat :-)\n> >\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >\n> >> ---\n> >>\n> >> This patch applies on top of the \"[PATCH 00/18] Object & Thread\n> >> enhancements\" series.\n> >>\n> >>  src/android/camera_device.cpp |  8 +-------\n> >>  src/android/camera_device.h   |  4 +++-\n> >>  src/android/camera_proxy.cpp  |  8 +++-----\n> >>  src/android/thread_rpc.cpp    | 18 +-----------------\n> >>  src/android/thread_rpc.h      | 15 ---------------\n> >>  5 files changed, 8 insertions(+), 45 deletions(-)\n> >>\n> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >> index e2c1f2a246c8..999c51e6ba4a 100644\n> >> --- a/src/android/camera_device.cpp\n> >> +++ b/src/android/camera_device.cpp\n> >> @@ -70,14 +70,8 @@ CameraDevice::~CameraDevice()\n> >>  /*\n> >>   * Handle RPC request received from the associated proxy.\n> >>   */\n> >> -void CameraDevice::message(Message *message)\n> >> +void CameraDevice::call(ThreadRpc *rpc)\n> >>  {\n> >> -\tif (message->type() != ThreadRpcMessage::type())\n> >> -\t\treturn Object::message(message);\n> >> -\n> >> -\tThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message);\n> >> -\tThreadRpc *rpc = rpcMessage->rpc;\n> >> -\n> >>  \tswitch (rpc->tag) {\n> >>  \tcase ThreadRpc::ProcessCaptureRequest:\n> >>  \t\tprocessCaptureRequest(rpc->request);\n> >> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> >> index ac5b95c95104..4d834ceb08a5 100644\n> >> --- a/src/android/camera_device.h\n> >> +++ b/src/android/camera_device.h\n> >> @@ -26,13 +26,15 @@\n> >>  \t\treturn nullptr;\t\t\\\n> >>  \t} while(0);\n> >>\n> >> +class ThreadRpc;\n> >> +\n> >>  class CameraDevice : public libcamera::Object\n> >>  {\n> >>  public:\n> >>  \tCameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> &camera);\n> >>  \t~CameraDevice();\n> >>\n> >> -\tvoid message(libcamera::Message *message);\n> >> +\tvoid call(ThreadRpc *rpc);\n> >>\n> >>  \tint open();\n> >>  \tvoid close();\n> >> diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp\n> >> index f0cacac8025b..3eb2f9fbcffb 100644\n> >> --- a/src/android/camera_proxy.cpp\n> >> +++ b/src/android/camera_proxy.cpp\n> >> @@ -9,6 +9,8 @@\n> >>\n> >>  #include <system/camera_metadata.h>\n> >>\n> >> +#include <libcamera/object.h>\n> >> +\n> >>  #include \"log.h\"\n> >>  #include \"message.h\"\n> >>  #include \"utils.h\"\n> >> @@ -185,10 +187,6 @@ int CameraProxy::processCaptureRequest(camera3_capture_request_t *request)\n> >>\n> >>  void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest)\n> >>  {\n> >> -\tstd::unique_ptr<ThreadRpcMessage> message =\n> >> -\t\t\t\tutils::make_unique<ThreadRpcMessage>();\n> >> -\tmessage->rpc = &rpcRequest;\n> >> -\n> >> -\tcameraDevice_->postMessage(std::move(message));\n> >> +\tcameraDevice_->invokeMethod(&CameraDevice::call, &rpcRequest);\n> >>  \trpcRequest.waitDelivery();\n> >>  }\n> >> diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp\n> >> index 295a05d7c676..f57891ff56bf 100644\n> >> --- a/src/android/thread_rpc.cpp\n> >> +++ b/src/android/thread_rpc.cpp\n> >> @@ -5,19 +5,11 @@\n> >>   * thread_rpc.cpp - Inter-thread procedure call\n> >>   */\n> >>\n> >> +#include \"thread.h\"\n> >>  #include \"thread_rpc.h\"\n> >>\n> >> -#include \"message.h\"\n> >> -\n> >>  using namespace libcamera;\n> >>\n> >> -libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None;\n> >> -\n> >> -ThreadRpcMessage::ThreadRpcMessage()\n> >> -\t: Message(type())\n> >> -{\n> >> -}\n> >> -\n> >>  void ThreadRpc::notifyReception()\n> >>  {\n> >>  \t{\n> >> @@ -32,11 +24,3 @@ void ThreadRpc::waitDelivery()\n> >>  \tlibcamera::MutexLocker locker(mutex_);\n> >>  \tcv_.wait(locker, [&] { return delivered_; });\n> >>  }\n> >> -\n> >> -Message::Type ThreadRpcMessage::type()\n> >> -{\n> >> -\tif (ThreadRpcMessage::rpcType_ == Message::Type::None)\n> >> -\t\trpcType_ = Message::registerMessageType();\n> >> -\n> >> -\treturn rpcType_;\n> >> -}\n> >> diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h\n> >> index 6d8992839d0b..f577a5d9fb32 100644\n> >> --- a/src/android/thread_rpc.h\n> >> +++ b/src/android/thread_rpc.h\n> >> @@ -12,9 +12,6 @@\n> >>\n> >>  #include <hardware/camera3.h>\n> >>\n> >> -#include \"message.h\"\n> >> -#include \"thread.h\"\n> >> -\n> >>  class ThreadRpc\n> >>  {\n> >>  public:\n> >> @@ -39,16 +36,4 @@ private:\n> >>  \tstd::condition_variable cv_;\n> >>  };\n> >>\n> >> -class ThreadRpcMessage : public libcamera::Message\n> >> -{\n> >> -public:\n> >> -\tThreadRpcMessage();\n> >> -\tThreadRpc *rpc;\n> >> -\n> >> -\tstatic Message::Type type();\n> >> -\n> >> -private:\n> >> -\tstatic libcamera::Message::Type rpcType_;\n> >> -};\n> >> -\n> >>  #endif /* __ANDROID_THREAD_RPC_H__ */","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BA0660C1E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 19 Aug 2019 17:01:54 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4502510;\n\tMon, 19 Aug 2019 17:01:53 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1566226914;\n\tbh=1TYDRxZBykGozbpPAR7iwyqGQ2VFo3u3j2pSzN1NaZY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LrO86NjQU96X6iVWEJdz/6v91JiC8HSkM3MpQxM1d5aPHODtf48ptN2Y5ZoMP9zCw\n\tJxg+JHjI9sUYUG5PQER91bm2Ub6BVDymmE4Nv3tJ4KOnDLZM0GtFXgB1Y4FljCqwab\n\tX91OF2Y4irn9NglmJbd1qJ37cAR1vmrGy4WVoheM=","Date":"Mon, 19 Aug 2019 18:01:48 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190819150148.GG5011@pendragon.ideasonboard.com>","References":"<20190812150232.1088-1-laurent.pinchart@ideasonboard.com>\n\t<20190817154511.GA22846@wyvern>\n\t<20190819144438.vim4l6x2qzipvlx5@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190819144438.vim4l6x2qzipvlx5@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] hal: Simplify thread RPC with\n\tObject::invokeMethod()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 19 Aug 2019 15:01:54 -0000"}}]