[{"id":13773,"web_url":"https://patchwork.libcamera.org/comment/13773/","msgid":"<20201118131122.klzq7z6ggsblxzpo@uno.localdomain>","date":"2020-11-18T13:11:22","subject":"Re: [libcamera-devel] [PATCH v4 16/37] libcamera: IPAProxy: Add\n\tisolate parameter to create()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Fri, Nov 06, 2020 at 07:36:46PM +0900, Paul Elder wrote:\n> Since IPAProxy implementations now always encapsulate IPA modules, add a\n> parameter to create() to signal if the proxy should isolate the IPA or not.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Niklas Söderlund \n>\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> No change in v4\n>\n> No change in v3\n>\n> Changes in v2:\n> - document isolate argument\n> ---\n> include/libcamera/internal/ipa_proxy.h | 22 +++++++++++-----------\n> src/libcamera/ipa_proxy.cpp | 4 +++-\n> 2 files changed, 14 insertions(+), 12 deletions(-)\n>\n> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n> index 59a5b841..195d2cb4 100644\n> --- a/include/libcamera/internal/ipa_proxy.h\n> +++ b/include/libcamera/internal/ipa_proxy.h\n> @@ -42,7 +42,7 @@ public:\n> \tIPAProxyFactory(const char *name);\n> \tvirtual ~IPAProxyFactory() = default;\n>\n> -\tvirtual std::unique_ptr create(IPAModule *ipam) = 0;\n> +\tvirtual std::unique_ptr create(IPAModule *ipam, bool isolate) = 0;\n>\n> \tconst std::string &name() const { return name_; }\n>\n> @@ -53,16 +53,16 @@ private:\n> \tstd::string name_;\n> };\n>\n> -#define REGISTER_IPA_PROXY(proxy)\t\t\t\\\n> -class proxy##Factory final : public IPAProxyFactory\t\\\n> -{\t\t\t\t\t\t\t\\\n> -public:\t\t\t\t\t\t\t\\\n> -\tproxy##Factory() : IPAProxyFactory(#proxy) {}\t\\\n> -\tstd::unique_ptr create(IPAModule *ipam)\t\\\n> -\t{\t\t\t\t\t\t\\\n> -\t\treturn std::make_unique(ipam);\t\\\n> -\t}\t\t\t\t\t\t\\\n> -};\t\t\t\t\t\t\t\\\n> +#define REGISTER_IPA_PROXY(proxy)\t\t\t\t\t\\\n> +class proxy##Factory final : public IPAProxyFactory\t\t\t\\\n> +{\t\t\t\t\t\t\t\t\t\\\n> +public:\t\t\t\t\t\t\t\t\t\\\n> +\tproxy##Factory() : IPAProxyFactory(#proxy) {}\t\t\t\\\n> +\tstd::unique_ptr create(IPAModule *ipam, bool isolate)\t\\\n> +\t{\t\t\t\t\t\t\t\t\\\n> +\t\treturn std::make_unique(ipam, isolate);\t\t\\\n> +\t}\t\t\t\t\t\t\t\t\\\n> +};\t\t\t\t\t\t\t\t\t\\\n> static proxy##Factory global_##proxy##Factory;\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 23be24ad..ee5e6f3e 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -250,10 +250,12 @@ IPAProxyFactory::IPAProxyFactory(const char *name)\n> * \\fn IPAProxyFactory::create()\n> * \\brief Create an instance of the IPAProxy corresponding to the factory\n> * \\param[in] ipam The IPA module\n> + * \\param[in] isolate Flag to isolate the IPA module\n> *\n> * This virtual function is implemented by the REGISTER_IPA_PROXY() macro.\n> * It creates a IPAProxy instance that isolates an IPA interface designated\n> - * by the IPA module \\a ipam.\n> + * by the IPA module \\a ipam. If \\a isolate is true, then the IPA module will\n> + * be isolated.\n> *\n> * \\return A pointer to a newly constructed instance of the IPAProxy subclass\n> * corresponding to the factory\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0E1E6BE08A\n\tfor ;\n\tWed, 18 Nov 2020 13:11:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63D496156B;\n\tWed, 18 Nov 2020 14:11:20 +0100 (CET)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29C5861565\n\tfor ;\n\tWed, 18 Nov 2020 14:11:19 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 9AECAE0003;\n\tWed, 18 Nov 2020 13:11:18 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 18 Nov 2020 14:11:22 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20201118131122.klzq7z6ggsblxzpo@uno.localdomain>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-17-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106103707.49660-17-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 16/37] libcamera: IPAProxy: Add\n\tisolate parameter to create()","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13879,"web_url":"https://patchwork.libcamera.org/comment/13879/","msgid":"<20201125145406.GB26606@pendragon.ideasonboard.com>","date":"2020-11-25T14:54:06","subject":"Re: [libcamera-devel] [PATCH v4 16/37] libcamera: IPAProxy: Add\n\tisolate parameter to create()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Nov 06, 2020 at 07:36:46PM +0900, Paul Elder wrote:\n> Since IPAProxy implementations now always encapsulate IPA modules, add a\n> parameter to create() to signal if the proxy should isolate the IPA or not.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Niklas Söderlund \n> \n> ---\n> No change in v4\n> \n> No change in v3\n> \n> Changes in v2:\n> - document isolate argument\n> ---\n> include/libcamera/internal/ipa_proxy.h | 22 +++++++++++-----------\n> src/libcamera/ipa_proxy.cpp | 4 +++-\n> 2 files changed, 14 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\n> index 59a5b841..195d2cb4 100644\n> --- a/include/libcamera/internal/ipa_proxy.h\n> +++ b/include/libcamera/internal/ipa_proxy.h\n> @@ -42,7 +42,7 @@ public:\n> \tIPAProxyFactory(const char *name);\n> \tvirtual ~IPAProxyFactory() = default;\n> \n> -\tvirtual std::unique_ptr create(IPAModule *ipam) = 0;\n> +\tvirtual std::unique_ptr create(IPAModule *ipam, bool isolate) = 0;\n> \n> \tconst std::string &name() const { return name_; }\n> \n> @@ -53,16 +53,16 @@ private:\n> \tstd::string name_;\n> };\n> \n> -#define REGISTER_IPA_PROXY(proxy)\t\t\t\\\n> -class proxy##Factory final : public IPAProxyFactory\t\\\n> -{\t\t\t\t\t\t\t\\\n> -public:\t\t\t\t\t\t\t\\\n> -\tproxy##Factory() : IPAProxyFactory(#proxy) {}\t\\\n> -\tstd::unique_ptr create(IPAModule *ipam)\t\\\n> -\t{\t\t\t\t\t\t\\\n> -\t\treturn std::make_unique(ipam);\t\\\n> -\t}\t\t\t\t\t\t\\\n> -};\t\t\t\t\t\t\t\\\n> +#define REGISTER_IPA_PROXY(proxy)\t\t\t\t\t\\\n> +class proxy##Factory final : public IPAProxyFactory\t\t\t\\\n> +{\t\t\t\t\t\t\t\t\t\\\n> +public:\t\t\t\t\t\t\t\t\t\\\n> +\tproxy##Factory() : IPAProxyFactory(#proxy) {}\t\t\t\\\n> +\tstd::unique_ptr create(IPAModule *ipam, bool isolate)\t\\\n> +\t{\t\t\t\t\t\t\t\t\\\n> +\t\treturn std::make_unique(ipam, isolate);\t\t\\\n> +\t}\t\t\t\t\t\t\t\t\\\n> +};\t\t\t\t\t\t\t\t\t\\\n> static proxy##Factory global_##proxy##Factory;\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 23be24ad..ee5e6f3e 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -250,10 +250,12 @@ IPAProxyFactory::IPAProxyFactory(const char *name)\n> * \\fn IPAProxyFactory::create()\n> * \\brief Create an instance of the IPAProxy corresponding to the factory\n> * \\param[in] ipam The IPA module\n> + * \\param[in] isolate Flag to isolate the IPA module\n> *\n> * This virtual function is implemented by the REGISTER_IPA_PROXY() macro.\n> * It creates a IPAProxy instance that isolates an IPA interface designated\n> - * by the IPA module \\a ipam.\n> + * by the IPA module \\a ipam. If \\a isolate is true, then the IPA module will\n> + * be isolated.\n\nReading the whole paragraph, isolation is a bit confusing. How about the following ?\n\n * This virtual function is implemented by the REGISTER_IPA_PROXY() macro.\n * It creates a IPAProxy instance that encapsulates the IPA interface provided\n * by the IPA module \\a ipam. If \\a isolate is true, the proxy isolates the IPA\n * interface in a separate process. Otherwise it runs it in a thread.\n\nReviewed-by: Laurent Pinchart \n\n> *\n> * \\return A pointer to a newly constructed instance of the IPAProxy subclass\n> * corresponding to the factory","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B6BC4BE08A\n\tfor ;\n\tWed, 25 Nov 2020 14:54:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3204263412;\n\tWed, 25 Nov 2020 15:54:16 +0100 (CET)","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 5F14763402\n\tfor ;\n\tWed, 25 Nov 2020 15:54:15 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33407292;\n\tWed, 25 Nov 2020 15:54:14 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ECi33vf2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606316054;\n\tbh=N+dj6f1aDsQ+vlvbe5YkrYI84ZYykKxxq7bExQ4uVtg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ECi33vf2+mYEvxBl0VkbMs8DIk2EUSpGzs734XOUq8KXtuCnNzEJ3TTdfgeJ4KKpl\n\tny9BsIW4QURAjwozk8cJ2ob1VQkDODpnv1u2t+xxcyI4pUONl/LPHyo0cdQtJ44/8f\n\tOJGogFBk9z7A6s8PpnlWu6FE87GCQeXWkaz5k6As=","Date":"Wed, 25 Nov 2020 16:54:06 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20201125145406.GB26606@pendragon.ideasonboard.com>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-17-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106103707.49660-17-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 16/37] libcamera: IPAProxy: Add\n\tisolate parameter to create()","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]