[{"id":2988,"web_url":"https://patchwork.libcamera.org/comment/2988/","msgid":"<20191029144424.GN20198@bigcity.dyn.berto.se>","date":"2019-10-29T14:44:24","subject":"Re: [libcamera-devel] [PATCH v2 6/9] libcamera: object: Use bound\n\tmethod activePack() for 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-10-28 12:49:10 +0200, Laurent Pinchart wrote:\n> The BoundMethodBase::activatePack() and the internal\n> Object::invokeMethod() are duplicate implementation of the same\n> mechanism. Use the former to replace the latter.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/bound_method.h | 4 ++--\n>  include/libcamera/object.h       | 8 ++++----\n>  src/libcamera/bound_method.cpp   | 6 +++---\n>  src/libcamera/object.cpp         | 7 -------\n>  4 files changed, 9 insertions(+), 16 deletions(-)\n> \n> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h\n> index 06c5a3b12305..bdeb5469cda3 100644\n> --- a/include/libcamera/bound_method.h\n> +++ b/include/libcamera/bound_method.h\n> @@ -37,7 +37,7 @@ public:\n>  \tObject *object() const { return object_; }\n>  \tConnectionType connectionType() const { return connectionType_; }\n>  \n> -\tvoid activatePack(void *pack);\n> +\tvoid activatePack(void *pack, bool deleteMethod);\n>  \tvirtual void invokePack(void *pack) = 0;\n>  \n>  protected:\n> @@ -109,7 +109,7 @@ public:\n>  \tvoid activate(Args... args)\n>  \t{\n>  \t\tif (this->object_)\n> -\t\t\tBoundMethodBase::activatePack(new PackType{ args... });\n> +\t\t\tBoundMethodBase::activatePack(new PackType{ args... }, false);\n>  \t\telse\n>  \t\t\t(static_cast<T *>(this->obj_)->*func_)(args...);\n>  \t}\n> diff --git a/include/libcamera/object.h b/include/libcamera/object.h\n> index 3308330af857..603a229288c2 100644\n> --- a/include/libcamera/object.h\n> +++ b/include/libcamera/object.h\n> @@ -33,10 +33,12 @@ public:\n>  \tvoid invokeMethod(void (T::*func)(Args...), Args... args)\n>  \t{\n>  \t\tT *obj = static_cast<T *>(this);\n> -\t\tBoundMethodBase *method = new BoundMemberMethod<T, Args...>(obj, this, func);\n> +\t\tBoundMethodBase *method =\n> +\t\t\tnew BoundMemberMethod<T, Args...>(obj, this, func,\n> +\t\t\t\t\t\t\t  ConnectionTypeQueued);\n>  \t\tvoid *pack = new typename BoundMemberMethod<T, Args...>::PackType{ args... };\n>  \n> -\t\tinvokeMethod(method, pack);\n> +\t\tmethod->activatePack(pack, true);\n>  \t}\n>  \n>  \tThread *thread() const { return thread_; }\n> @@ -53,8 +55,6 @@ private:\n>  \tfriend class BoundMethodBase;\n>  \tfriend class Thread;\n>  \n> -\tvoid invokeMethod(BoundMethodBase *method, void *pack);\n> -\n>  \tvoid notifyThreadMove();\n>  \n>  \tvoid connect(SignalBase *signal);\n> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp\n> index 600717363444..4c0cd415a3f1 100644\n> --- a/src/libcamera/bound_method.cpp\n> +++ b/src/libcamera/bound_method.cpp\n> @@ -48,7 +48,7 @@ namespace libcamera {\n>   * deadlock will occur.\n>   */\n>  \n> -void BoundMethodBase::activatePack(void *pack)\n> +void BoundMethodBase::activatePack(void *pack, bool deleteMethod)\n>  {\n>  \tConnectionType type = connectionType_;\n>  \tif (type == ConnectionTypeAuto) {\n> @@ -66,7 +66,7 @@ void BoundMethodBase::activatePack(void *pack)\n>  \n>  \tcase ConnectionTypeQueued: {\n>  \t\tstd::unique_ptr<Message> msg =\n> -\t\t\tutils::make_unique<InvokeMessage>(this, pack);\n> +\t\t\tutils::make_unique<InvokeMessage>(this, pack, nullptr, deleteMethod);\n>  \t\tobject_->postMessage(std::move(msg));\n>  \t\tbreak;\n>  \t}\n> @@ -75,7 +75,7 @@ void BoundMethodBase::activatePack(void *pack)\n>  \t\tSemaphore semaphore;\n>  \n>  \t\tstd::unique_ptr<Message> msg =\n> -\t\t\tutils::make_unique<InvokeMessage>(this, pack, &semaphore);\n> +\t\t\tutils::make_unique<InvokeMessage>(this, pack, &semaphore, deleteMethod);\n>  \t\tobject_->postMessage(std::move(msg));\n>  \n>  \t\tsemaphore.acquire();\n> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp\n> index b0818f9aa25f..509b2ebac537 100644\n> --- a/src/libcamera/object.cpp\n> +++ b/src/libcamera/object.cpp\n> @@ -153,13 +153,6 @@ void Object::message(Message *msg)\n>   * remains valid until the method is invoked.\n>   */\n>  \n> -void Object::invokeMethod(BoundMethodBase *method, void *args)\n> -{\n> -\tstd::unique_ptr<Message> msg =\n> -\t\tutils::make_unique<InvokeMessage>(method, args, nullptr, true);\n> -\tpostMessage(std::move(msg));\n> -}\n> -\n>  /**\n>   * \\fn Object::thread()\n>   * \\brief Retrieve the thread the object is bound to\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-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F6036017E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Oct 2019 15:44:26 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id s4so14682051ljj.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Oct 2019 07:44:26 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\td9sm10342252lfj.81.2019.10.29.07.44.24\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 29 Oct 2019 07:44:24 -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=NicWE9kQ6vzecz7xlJuOKI9TCGGTAUNcWIadgS1dF5Y=;\n\tb=h5Rt2Ml1ZGKFNoO8WRlj21FasNFpMsJTN/FL2ReNfZ4jcEhpJZqFZOELFV2TiJnvhh\n\tnbbkD5e0NJ0sP4gxq0MXIB6D+PIxltEEOyxFcsM8zE6dHawDwBInk80wtbciXg8E6wdI\n\tb9+0uMywBgUoDQ8tduAAEDvo88LpdNHRgN1NSjKS+c5TUT05xXFsdEFXqmUbQ5b1sHrZ\n\tY205cgi474cuHEJD/MbNd/XfktjXJqlRy87wsA+nUToXo+tIF65sDh4fLDXDBaIgHHHa\n\tGHLGRjV3OxXPJRtFrbt/N+CZMIks4L2Tdl1YmBa44K7xfUqTo6fPyxqf43TO31b3qZQt\n\twVkA==","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=NicWE9kQ6vzecz7xlJuOKI9TCGGTAUNcWIadgS1dF5Y=;\n\tb=IDrrZzYGVSlkiNKtOgszkLbxevxjG8n9nGv3ofCWYIUJlEKkiu2wyfSFQdnyBomIRI\n\tm1XwDJj8zso4Gaaf5gqI1EKvR6sqeFYp2EZMswm43Lt5qWTyqn9f3FylbQ/XQ1hCbjxm\n\tji77y2eJh3SqQsX9x7kLhwlUwz8v+ytslPla+HyjllOA76uYhKojS19wDvhfvJKH2mMq\n\ttKhqjtGxcAUuQXmDUgJyB+teVeVDQIxA4xi6iSBQeL3EJU7pErFI1kWVPCPVADvhUdyA\n\tAP8Nci8Umt/J8IZVqTBBVZgByWWKYirY8jfZXltWLBIijnofYkMDB7LMqFmTBSKiuFd0\n\tOYVw==","X-Gm-Message-State":"APjAAAWjs3SbPmLe1XrMiFCe/TvsUaHSEZj4Ybojx3tDTjUt3nPJ3Zcs\n\tlHZdAjAzAd9QpbcFZCioZlvegw==","X-Google-Smtp-Source":"APXvYqyDqwYgyKIbjClYFU3h5bEdfoowBhA0JzC7fzQ4x/WGL+wOwOc+S/l19e6YxUuTUgB3Gjf6zQ==","X-Received":"by 2002:a05:651c:28a:: with SMTP id\n\tb10mr2933956ljo.193.1572360265446; \n\tTue, 29 Oct 2019 07:44:25 -0700 (PDT)","Date":"Tue, 29 Oct 2019 15:44:24 +0100","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":"<20191029144424.GN20198@bigcity.dyn.berto.se>","References":"<20191028104913.14985-1-laurent.pinchart@ideasonboard.com>\n\t<20191028104913.14985-7-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-7-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v2 6/9] libcamera: object: Use bound\n\tmethod activePack() for invokeMethod()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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":"Tue, 29 Oct 2019 14:44:26 -0000"}}]