[{"id":13753,"web_url":"https://patchwork.libcamera.org/comment/13753/","msgid":"<20201117160850.26umo5qgefrby546@uno.localdomain>","date":"2020-11-17T16:08:50","subject":"Re: [libcamera-devel] [PATCH v4 13/37] libcamera: IPAModule:\n\tReplace ipa_context with IPAInterface","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:43PM +0900, Paul Elder wrote:\n> With the new IPC infrastructure, we no longer need the C interface as\n> provided by struct ipa_context. Make ipaCreate_() and createInterface()\n> return IPAInterface.\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> - add documentation for IPAModule::createInterface()\n> ---\n> include/libcamera/internal/ipa_module.h | 4 ++--\n> src/libcamera/ipa_module.cpp | 10 ++++------\n> src/libcamera/meson.build | 1 -\n> 3 files changed, 6 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h\n> index c2df2476..19fc5827 100644\n> --- a/include/libcamera/internal/ipa_module.h\n> +++ b/include/libcamera/internal/ipa_module.h\n> @@ -33,7 +33,7 @@ public:\n>\n> \tbool load();\n>\n> -\tstruct ipa_context *createContext();\n> +\tIPAInterface *createInterface();\n>\n> \tbool match(PipelineHandler *pipe,\n> \t\t uint32_t minVersion, uint32_t maxVersion) const;\n> @@ -52,7 +52,7 @@ private:\n> \tbool loaded_;\n>\n> \tvoid *dlHandle_;\n> -\ttypedef struct ipa_context *(*IPAIntfFactory)();\n> +\ttypedef IPAInterface *(*IPAIntfFactory)(void);\n> \tIPAIntfFactory ipaCreate_;\n> };\n>\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index de512a7f..d5ac9820 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -439,20 +439,18 @@ bool IPAModule::load()\n> }\n>\n> /**\n> - * \\brief Instantiate an IPA context\n> + * \\brief Instantiate an IPA interface\n> *\n> * After loading the IPA module with load(), this method creates an instance of\n> - * the IPA module context. Ownership of the context is passed to the caller, and\n> - * the context shall be destroyed by calling the \\ref ipa_context_ops::destroy\n> - * \"ipa_context::ops::destroy()\" function.\n> + * the IPA module interface.\n> *\n> * Calling this function on a module that has not yet been loaded, or an\n> * invalid module (as returned by load() and isValid(), respectively) is\n> * an error.\n> *\n> - * \\return The IPA context on success, or nullptr on error\n> + * \\return The IPA interface on success, or nullptr on error\n> */\n> -struct ipa_context *IPAModule::createContext()\n> +IPAInterface *IPAModule::createInterface()\n> {\n> \tif (!valid_ || !loaded_)\n> \t\treturn nullptr;\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 9c889a4f..51925bd6 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -22,7 +22,6 @@ libcamera_sources = files([\n> 'formats.cpp',\n> 'framebuffer_allocator.cpp',\n> 'geometry.cpp',\n> - 'ipa_context_wrapper.cpp',\n> 'ipa_controls.cpp',\n> 'ipa_data_serializer.cpp',\n> 'ipa_ipc.cpp',\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 A0A5EBE082\n\tfor ;\n\tTue, 17 Nov 2020 16:08:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 29E7E6331E;\n\tTue, 17 Nov 2020 17:08:49 +0100 (CET)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FA5D6033B\n\tfor ;\n\tTue, 17 Nov 2020 17:08:48 +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 relay2-d.mail.gandi.net (Postfix) with ESMTPSA id CE08140018;\n\tTue, 17 Nov 2020 16:08:47 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Tue, 17 Nov 2020 17:08:50 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20201117160850.26umo5qgefrby546@uno.localdomain>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-14-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106103707.49660-14-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 13/37] libcamera: IPAModule:\n\tReplace ipa_context with IPAInterface","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":13819,"web_url":"https://patchwork.libcamera.org/comment/13819/","msgid":"<20201120170015.GA17570@pendragon.ideasonboard.com>","date":"2020-11-20T17:00:15","subject":"Re: [libcamera-devel] [PATCH v4 13/37] libcamera: IPAModule:\n\tReplace ipa_context with IPAInterface","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:43PM +0900, Paul Elder wrote:\n> With the new IPC infrastructure, we no longer need the C interface as\n> provided by struct ipa_context. Make ipaCreate_() and createInterface()\n> return IPAInterface.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Niklas Söderlund \n> ---\n> No change in v4\n> \n> No change in v3\n> \n> Changes in v2:\n> - add documentation for IPAModule::createInterface()\n> ---\n> include/libcamera/internal/ipa_module.h | 4 ++--\n> src/libcamera/ipa_module.cpp | 10 ++++------\n> src/libcamera/meson.build | 1 -\n> 3 files changed, 6 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h\n> index c2df2476..19fc5827 100644\n> --- a/include/libcamera/internal/ipa_module.h\n> +++ b/include/libcamera/internal/ipa_module.h\n> @@ -33,7 +33,7 @@ public:\n> \n> \tbool load();\n> \n> -\tstruct ipa_context *createContext();\n> +\tIPAInterface *createInterface();\n> \n> \tbool match(PipelineHandler *pipe,\n> \t\t uint32_t minVersion, uint32_t maxVersion) const;\n> @@ -52,7 +52,7 @@ private:\n> \tbool loaded_;\n> \n> \tvoid *dlHandle_;\n> -\ttypedef struct ipa_context *(*IPAIntfFactory)();\n> +\ttypedef IPAInterface *(*IPAIntfFactory)(void);\n> \tIPAIntfFactory ipaCreate_;\n> };\n> \n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index de512a7f..d5ac9820 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -439,20 +439,18 @@ bool IPAModule::load()\n> }\n> \n> /**\n> - * \\brief Instantiate an IPA context\n> + * \\brief Instantiate an IPA interface\n> *\n> * After loading the IPA module with load(), this method creates an instance of\n> - * the IPA module context. Ownership of the context is passed to the caller, and\n> - * the context shall be destroyed by calling the \\ref ipa_context_ops::destroy\n> - * \"ipa_context::ops::destroy()\" function.\n> + * the IPA module interface.\n> *\n> * Calling this function on a module that has not yet been loaded, or an\n> * invalid module (as returned by load() and isValid(), respectively) is\n> * an error.\n> *\n> - * \\return The IPA context on success, or nullptr on error\n> + * \\return The IPA interface on success, or nullptr on error\n> */\n> -struct ipa_context *IPAModule::createContext()\n> +IPAInterface *IPAModule::createInterface()\n\nThere are also two occurrences of createContext() in the documentation\nof IPAModule::load(), as well as occurrences of ipa_context. Could you\naddress that in this patch ?\n\n> {\n> \tif (!valid_ || !loaded_)\n> \t\treturn nullptr;\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 9c889a4f..51925bd6 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -22,7 +22,6 @@ libcamera_sources = files([\n> 'formats.cpp',\n> 'framebuffer_allocator.cpp',\n> 'geometry.cpp',\n> - 'ipa_context_wrapper.cpp',\n\nDoesn't this belong to patch 14/37 ? Alternatively you can squash 13/37\nand 14/37 together.\n\nReviewed-by: Laurent Pinchart \n\n> 'ipa_controls.cpp',\n> 'ipa_data_serializer.cpp',\n> 'ipa_ipc.cpp',","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 9AA0EBE08A\n\tfor ;\n\tFri, 20 Nov 2020 17:00:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18E31628A5;\n\tFri, 20 Nov 2020 18:00:25 +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 9A4D96220D\n\tfor ;\n\tFri, 20 Nov 2020 18:00:23 +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 15A30240;\n\tFri, 20 Nov 2020 18:00:23 +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=\"keXwGBRE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1605891623;\n\tbh=0bGQ/XJV15NlWK7h+A7THdJ6eFKeQieGEcFjSbdNddY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=keXwGBREjTisXVB+pnEobJEnAtm9bFlDKOWkROXMncYkT71aehnjh7pVvwC7g5N1P\n\t2NB2GKpjeH57NXAGPCoVksBSlgXr5iSU3mcIAP+dPKLdlyhqKNCKt3TQT1ihVhhyUU\n\tdOgSO+O/UyGinWMh8xN1dnlyj6+cR3ZfdeS6CSoc=","Date":"Fri, 20 Nov 2020 19:00:15 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20201120170015.GA17570@pendragon.ideasonboard.com>","References":"<20201106103707.49660-1-paul.elder@ideasonboard.com>\n\t<20201106103707.49660-14-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201106103707.49660-14-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 13/37] libcamera: IPAModule:\n\tReplace ipa_context with IPAInterface","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\" "}}]