From db998e618aaa9f33fb1cd539dce5dce3cd02c94b Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 20 Oct 2025 18:06:03 +0300 Subject: [PATCH] libcamera: pipeline_handler: Add createIPA() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IPA proxies are created with a call to IPAManager::createIPA(), which is a static member function. This requires a complicated dance in the createIPA() function to retrieve the IPAManager instance through the camera manager, itself retrieved from the pipeline handler. Simplify the code by turning IPAManager::createIPA() into a non-static member function, and providing a wrapper in the PipelineHandler class to keep instantiation of IPA proxies easy in pipeline handlers. Signed-off-by: Laurent Pinchart Reviewed-by: Barnabás Pőcze Reviewed-by: Isaac Scott --- Documentation/guides/ipa.rst | 8 +++--- include/libcamera/internal/ipa_manager.h | 25 ++++++++----------- include/libcamera/internal/pipeline_handler.h | 11 +++++++- src/libcamera/ipa_manager.cpp | 1 + src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 +-- .../pipeline/rpi/common/pipeline_base.cpp | 3 +-- src/libcamera/pipeline/vimc/vimc.cpp | 3 +-- src/libcamera/pipeline_handler.cpp | 10 ++++++++ src/libcamera/software_isp/software_isp.cpp | 3 +-- test/ipa/ipa_interface_test.cpp | 3 +-- 12 files changed, 43 insertions(+), 33 deletions(-) diff --git a/Documentation/guides/ipa.rst b/Documentation/guides/ipa.rst index 25deadef..42993924 100644 --- a/Documentation/guides/ipa.rst +++ b/Documentation/guides/ipa.rst @@ -387,20 +387,20 @@ In the pipeline handler, we first need to construct a specialized IPA proxy. From the point of view of the pipeline hander, this is the object that is the IPA. -To do so, we invoke the IPAManager: +To do so, we call the PipelineHandler::createIPA() function: .. code-block:: C++ std::unique_ptr ipa_ = - IPAManager::createIPA(pipe_, 1, 1); + pipe_->createIPA(1, 1); The ipa::rpi namespace comes from the namespace that we defined in the mojo data definition file, in the "Namespacing" section. The name of the proxy, IPAProxyRPi, comes from the name given to the main IPA interface, IPARPiInterface, in the "The Main IPA interface" section. -The return value of IPAManager::createIPA shall be error-checked, to confirm -that the returned pointer is not a nullptr. +The return value of createIPA() shall be error-checked, to confirm that the +returned pointer is not a nullptr. After this, before initializing the IPA, slots should be connected to all of the IPA's signals, as defined in the Event IPA interface: diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index f8ce7801..ecb6ae89 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -17,15 +17,16 @@ #include #include "libcamera/internal/camera_manager.h" -#include "libcamera/internal/global_configuration.h" -#include "libcamera/internal/ipa_module.h" -#include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/pub_key.h" namespace libcamera { LOG_DECLARE_CATEGORY(IPAManager) +class GlobalConfiguration; +class IPAModule; +class PipelineHandler; + class IPAManager { public: @@ -33,23 +34,18 @@ public: ~IPAManager(); template - static std::unique_ptr createIPA(PipelineHandler *pipe, - uint32_t minVersion, - uint32_t maxVersion) + std::unique_ptr createIPA(PipelineHandler *pipe, uint32_t minVersion, + uint32_t maxVersion) { - CameraManager *cm = pipe->cameraManager(); - IPAManager *self = cm->_d()->ipaManager(); - IPAModule *m = self->module(pipe, minVersion, maxVersion); + IPAModule *m = module(pipe, minVersion, maxVersion); if (!m) return nullptr; - const GlobalConfiguration &configuration = cm->_d()->configuration(); - auto proxy = [&]() -> std::unique_ptr { - if (self->isSignatureValid(m)) - return std::make_unique(m, configuration); + if (isSignatureValid(m)) + return std::make_unique(m, configuration_); else - return std::make_unique(m, configuration); + return std::make_unique(m, configuration_); }(); if (!proxy->isValid()) { @@ -77,6 +73,7 @@ private: bool isSignatureValid(IPAModule *ipa) const; + const GlobalConfiguration &configuration_; std::vector> modules_; #if HAVE_IPA_PUBKEY diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index b4f97477..6922ce18 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -17,11 +17,13 @@ #include #include +#include "libcamera/internal/camera_manager.h" +#include "libcamera/internal/ipa_manager.h" + namespace libcamera { class Camera; class CameraConfiguration; -class CameraManager; class DeviceEnumerator; class DeviceMatch; class FrameBuffer; @@ -70,6 +72,13 @@ public: CameraManager *cameraManager() const { return manager_; } + template + std::unique_ptr createIPA(uint32_t minVersion, uint32_t maxVersion) + { + IPAManager *ipaManager = manager_->_d()->ipaManager(); + return ipaManager->createIPA(this, minVersion, maxVersion); + } + protected: void registerCamera(std::shared_ptr camera); void hotplugMediaDevice(std::shared_ptr media); diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 35171d09..1e905e8b 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -105,6 +105,7 @@ LOG_DEFINE_CATEGORY(IPAManager) * CameraManager. */ IPAManager::IPAManager(const GlobalConfiguration &configuration) + : configuration_(configuration) { #if HAVE_IPA_PUBKEY if (!pubKey_.isValid()) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index bac6f1c2..bf4c2921 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -31,7 +31,6 @@ #include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/request.h" @@ -1151,7 +1150,7 @@ int PipelineHandlerIPU3::registerCameras() int IPU3CameraData::loadIPA() { - ipa_ = IPAManager::createIPA(pipe(), 1, 1); + ipa_ = pipe()->createIPA(1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index c209b0b0..e39c8d5a 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -35,7 +35,6 @@ #include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/request.h" @@ -382,7 +381,7 @@ int MaliC55CameraData::loadIPA() if (!sensor_) return 0; - ipa_ = IPAManager::createIPA(pipe(), 1, 1); + ipa_ = pipe()->createIPA(1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 71516621..e7ddbc5a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -40,7 +40,6 @@ #include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_pipeline.h" #include "libcamera/internal/pipeline_handler.h" @@ -395,7 +394,7 @@ const PipelineHandlerRkISP1 *RkISP1CameraData::pipe() const int RkISP1CameraData::loadIPA(unsigned int hwRevision, uint32_t supportedBlocks) { - ipa_ = IPAManager::createIPA(pipe(), 1, 1); + ipa_ = pipe()->createIPA(1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index 867ecf1b..6d2072e8 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -20,7 +20,6 @@ #include #include "libcamera/internal/camera_lens.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/v4l2_subdevice.h" using namespace std::chrono_literals; @@ -1161,7 +1160,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result) { int ret; - ipa_ = IPAManager::createIPA(pipe(), 1, 1); + ipa_ = pipe()->createIPA(1, 1); if (!ipa_) return -ENOENT; diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 08b10075..e915d5c2 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -35,7 +35,6 @@ #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/request.h" @@ -488,7 +487,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) if (data->init()) return false; - data->ipa_ = IPAManager::createIPA(this, 0, 0); + data->ipa_ = createIPA(0, 0); if (!data->ipa_) { LOG(VIMC, Error) << "no matching IPA found"; return false; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5c469e5b..e7145c1d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -835,6 +835,16 @@ void PipelineHandler::disconnect() * \return The CameraManager for this pipeline handler */ +/** + * \fn PipelineHandler::createIPA() + * \brief Create an IPA proxy that matches this pipeline handler + * \param[in] minVersion Minimum acceptable version of IPA module + * \param[in] maxVersion Maximum acceptable version of IPA module + * + * \return A newly created IPA proxy, or nullptr if no matching IPA module is + * found or if the IPA proxy fails to initialize + */ + /** * \class PipelineHandlerFactoryBase * \brief Base class for pipeline handler factories diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 60228369..5b225c7c 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -23,7 +23,6 @@ #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/software_isp/debayer_params.h" #include "debayer_cpu.h" @@ -127,7 +126,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor, debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady); debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady); - ipa_ = IPAManager::createIPA(pipe, 0, 0); + ipa_ = pipe->createIPA(0, 0); if (!ipa_) { LOG(SoftwareIsp, Error) << "Creating IPA for software ISP failed"; diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index b8178366..ccff6ac1 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -22,7 +22,6 @@ #include "libcamera/internal/camera_manager.h" #include "libcamera/internal/device_enumerator.h" -#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/ipa_module.h" #include "libcamera/internal/pipeline_handler.h" @@ -99,7 +98,7 @@ protected: EventDispatcher *dispatcher = thread()->eventDispatcher(); Timer timer; - ipa_ = IPAManager::createIPA(pipe_.get(), 0, 0); + ipa_ = pipe_->createIPA(0, 0); if (!ipa_) { cerr << "Failed to create VIMC IPA interface" << endl; return TestFail;