[{"id":37587,"web_url":"https://patchwork.libcamera.org/comment/37587/","msgid":"<08fad687-59a7-486f-9d01-130b74d571aa@ideasonboard.com>","date":"2026-01-13T09:00:27","subject":"Re: [PATCH 04/36] libcamera: pipeline_handler: Add createIPA()\n\tfunction","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> IPA proxies are created with a call to IPAManager::createIPA(), which is\n> a static member function. This requires a complicated dance in the\n> createIPA() function to retrieve the IPAManager instance through the\n> camera manager, itself retrieved from the pipeline handler. Simplify the\n> code by turning IPAManager::createIPA() into a non-static member\n> function, and providing a wrapper in the PipelineHandler class to keep\n> instantiation of IPA proxies easy in pipeline handlers.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n\nLooks ok to me.\n\nReviewed-by: Barnabás Pőcze \n\n\n> Documentation/guides/ipa.rst | 8 +++---\n> include/libcamera/internal/ipa_manager.h | 25 ++++++++-----------\n> include/libcamera/internal/pipeline_handler.h | 11 +++++++-\n> src/libcamera/ipa_manager.cpp | 1 +\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +--\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 +--\n> .../pipeline/rpi/common/pipeline_base.cpp | 3 +--\n> src/libcamera/pipeline/vimc/vimc.cpp | 3 +--\n> src/libcamera/pipeline_handler.cpp | 10 ++++++++\n> src/libcamera/software_isp/software_isp.cpp | 3 +--\n> test/ipa/ipa_interface_test.cpp | 3 +--\n> 12 files changed, 43 insertions(+), 33 deletions(-)\n> \n> diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst\n> index 25deadefaf7c..42993924dd79 100644\n> --- a/Documentation/guides/ipa.rst\n> +++ b/Documentation/guides/ipa.rst\n> @@ -387,20 +387,20 @@ In the pipeline handler, we first need to construct a specialized IPA proxy.\n> From the point of view of the pipeline hander, this is the object that is the\n> IPA.\n> \n> -To do so, we invoke the IPAManager:\n> +To do so, we call the PipelineHandler::createIPA() function:\n> \n> .. code-block:: C++\n> \n> std::unique_ptr ipa_ =\n> - IPAManager::createIPA(pipe_, 1, 1);\n> + pipe_->createIPA(1, 1);\n> \n> The ipa::rpi namespace comes from the namespace that we defined in the mojo\n> data definition file, in the \"Namespacing\" section. The name of the proxy,\n> IPAProxyRPi, comes from the name given to the main IPA interface,\n> IPARPiInterface, in the \"The Main IPA interface\" section.\n> \n> -The return value of IPAManager::createIPA shall be error-checked, to confirm\n> -that the returned pointer is not a nullptr.\n> +The return value of createIPA() shall be error-checked, to confirm that the\n> +returned pointer is not a nullptr.\n> \n> After this, before initializing the IPA, slots should be connected to all of\n> the IPA's signals, as defined in the Event IPA interface:\n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index f8ce780169e6..ecb6ae896e0c 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -17,15 +17,16 @@\n> #include \n> \n> #include \"libcamera/internal/camera_manager.h\"\n> -#include \"libcamera/internal/global_configuration.h\"\n> -#include \"libcamera/internal/ipa_module.h\"\n> -#include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/pub_key.h\"\n> \n> namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(IPAManager)\n> \n> +class GlobalConfiguration;\n> +class IPAModule;\n> +class PipelineHandler;\n> +\n> class IPAManager\n> {\n> public:\n> @@ -33,23 +34,18 @@ public:\n> \t~IPAManager();\n> \n> \ttemplate\n> -\tstatic std::unique_ptr createIPA(PipelineHandler *pipe,\n> -\t\t\t\t\t uint32_t minVersion,\n> -\t\t\t\t\t uint32_t maxVersion)\n> +\tstd::unique_ptr createIPA(PipelineHandler *pipe, uint32_t minVersion,\n> +\t\t\t\t uint32_t maxVersion)\n> \t{\n> -\t\tCameraManager *cm = pipe->cameraManager();\n> -\t\tIPAManager *self = cm->_d()->ipaManager();\n> -\t\tIPAModule *m = self->module(pipe, minVersion, maxVersion);\n> +\t\tIPAModule *m = module(pipe, minVersion, maxVersion);\n> \t\tif (!m)\n> \t\t\treturn nullptr;\n> \n> -\t\tconst GlobalConfiguration &configuration = cm->_d()->configuration();\n> -\n> \t\tauto proxy = [&]() -> std::unique_ptr {\n> -\t\t\tif (self->isSignatureValid(m))\n> -\t\t\t\treturn std::make_unique(m, configuration);\n> +\t\t\tif (isSignatureValid(m))\n> +\t\t\t\treturn std::make_unique(m, configuration_);\n> \t\t\telse\n> -\t\t\t\treturn std::make_unique(m, configuration);\n> +\t\t\t\treturn std::make_unique(m, configuration_);\n> \t\t}();\n> \n> \t\tif (!proxy->isValid()) {\n> @@ -77,6 +73,7 @@ private:\n> \n> \tbool isSignatureValid(IPAModule *ipa) const;\n> \n> +\tconst GlobalConfiguration &configuration_;\n> \tstd::vector> modules_;\n> \n> #if HAVE_IPA_PUBKEY\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index b4f97477acec..6922ce18ec87 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -17,11 +17,13 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/camera_manager.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> +\n> namespace libcamera {\n> \n> class Camera;\n> class CameraConfiguration;\n> -class CameraManager;\n> class DeviceEnumerator;\n> class DeviceMatch;\n> class FrameBuffer;\n> @@ -70,6 +72,13 @@ public:\n> \n> \tCameraManager *cameraManager() const { return manager_; }\n> \n> +\ttemplate\n> +\tstd::unique_ptr createIPA(uint32_t minVersion, uint32_t maxVersion)\n> +\t{\n> +\t\tIPAManager *ipaManager = manager_->_d()->ipaManager();\n> +\t\treturn ipaManager->createIPA(this, minVersion, maxVersion);\n> +\t}\n> +\n> protected:\n> \tvoid registerCamera(std::shared_ptr camera);\n> \tvoid hotplugMediaDevice(std::shared_ptr media);\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 35171d097136..1e905e8b82e8 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -105,6 +105,7 @@ LOG_DEFINE_CATEGORY(IPAManager)\n> * CameraManager.\n> */\n> IPAManager::IPAManager(const GlobalConfiguration &configuration)\n> +\t: configuration_(configuration)\n> {\n> #if HAVE_IPA_PUBKEY\n> \tif (!pubKey_.isValid())\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 0190f677e679..06975b85be6b 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -31,7 +31,6 @@\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/request.h\"\n> @@ -1152,7 +1151,7 @@ int PipelineHandlerIPU3::registerCameras()\n> \n> int IPU3CameraData::loadIPA()\n> {\n> -\tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> +\tipa_ = pipe()->createIPA(1, 1);\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 77f251416f71..63450820a273 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -35,7 +35,6 @@\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/request.h\"\n> @@ -382,7 +381,7 @@ int MaliC55CameraData::loadIPA()\n> \tif (!sensor_)\n> \t\treturn 0;\n> \n> -\tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> +\tipa_ = pipe()->createIPA(1, 1);\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 320a4dc5a899..c7e3b185f89b 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -40,7 +40,6 @@\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/media_pipeline.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -389,7 +388,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const\n> \n> int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks)\n> {\n> -\tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> +\tipa_ = pipe()->createIPA(1, 1);\n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index fb8e466f6e3b..7eb197ce435c 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -20,7 +20,6 @@\n> #include \n> \n> #include \"libcamera/internal/camera_lens.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> \n> using namespace std::chrono_literals;\n> @@ -1152,7 +1151,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> {\n> \tint ret;\n> \n> -\tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> +\tipa_ = pipe()->createIPA(1, 1);\n> \n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 4a03c149a617..378928e18cc7 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -35,7 +35,6 @@\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/request.h\"\n> @@ -489,7 +488,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \tif (data->init())\n> \t\treturn false;\n> \n> -\tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n> +\tdata->ipa_ = createIPA(0, 0);\n> \tif (!data->ipa_) {\n> \t\tLOG(VIMC, Error) << \"no matching IPA found\";\n> \t\treturn false;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 5c469e5bad24..e7145c1d4815 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -835,6 +835,16 @@ void PipelineHandler::disconnect()\n> * \\return The CameraManager for this pipeline handler\n> */\n> \n> +/**\n> + * \\fn PipelineHandler::createIPA()\n> + * \\brief Create an IPA proxy that matches this pipeline handler\n> + * \\param[in] minVersion Minimum acceptable version of IPA module\n> + * \\param[in] maxVersion Maximum acceptable version of IPA module\n> + *\n> + * \\return A newly created IPA proxy, or nullptr if no matching IPA module is\n> + * found or if the IPA proxy fails to initialize\n> + */\n> +\n> /**\n> * \\class PipelineHandlerFactoryBase\n> * \\brief Base class for pipeline handler factories\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index 7ad3511db28d..aa62257340d8 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -22,7 +22,6 @@\n> #include \n> \n> #include \"libcamera/internal/framebuffer.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/software_isp/debayer_params.h\"\n> \n> #include \"debayer_cpu.h\"\n> @@ -146,7 +145,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n> \tdebayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);\n> \tdebayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);\n> \n> -\tipa_ = IPAManager::createIPA(pipe, 0, 0);\n> +\tipa_ = pipe->createIPA(0, 0);\n> \tif (!ipa_) {\n> \t\tLOG(SoftwareIsp, Error)\n> \t\t\t<< \"Creating IPA for software ISP failed\";\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index b81783664977..ccff6ac106c8 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -22,7 +22,6 @@\n> \n> #include \"libcamera/internal/camera_manager.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> -#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/ipa_module.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> @@ -99,7 +98,7 @@ protected:\n> \t\tEventDispatcher *dispatcher = thread()->eventDispatcher();\n> \t\tTimer timer;\n> \n> -\t\tipa_ = IPAManager::createIPA(pipe_.get(), 0, 0);\n> +\t\tipa_ = pipe_->createIPA(0, 0);\n> \t\tif (!ipa_) {\n> \t\t\tcerr << \"Failed to create VIMC IPA interface\" << endl;\n> \t\t\treturn TestFail;\n> --\n> Regards,\n> \n> Laurent Pinchart\n>","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 6F0CBBE08B\n\tfor ;\n\tTue, 13 Jan 2026 09:00:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93DEF61FBC;\n\tTue, 13 Jan 2026 10:00:32 +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 A957D61F84\n\tfor ;\n\tTue, 13 Jan 2026 10:00:30 +0100 (CET)","from [192.168.33.30] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 77AE1176E;\n\tTue, 13 Jan 2026 10:00:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XT5Hc0vt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768294804;\n\tbh=SL11C2yNbmXO6m2B4dWr8OEsXMeHC6bth3ExBwzOiU4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=XT5Hc0vtMegkOpQszwbv3dGaX3sOYmVsIBGOJuyRx3EUH5y2iHLhXtyTrqeIrokNX\n\tZHrGYfqifkq7E81c7xuy5/Or5SQA7/6X9ukZgxstx5DfRggkpSHd4jeA83YU/K4VkG\n\tboGYlOfEWf5HSB2Xn+oR09S1k4XAJdeW5kGpbOZk=","Message-ID":"<08fad687-59a7-486f-9d01-130b74d571aa@ideasonboard.com>","Date":"Tue, 13 Jan 2026 10:00:27 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 04/36] libcamera: pipeline_handler: Add createIPA()\n\tfunction","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20260113000808.15395-5-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260113000808.15395-5-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]