From 1c3e1baa6e6cdae153645ffded11936eff2a0bf8 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 13 Oct 2025 23:51:09 +0300 Subject: [PATCH] libcamera: Pass CameraManager around instead of GlobalConfiguration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GlobalConfiguration is explicitly passed around through constructors of various objects that need access to the configuration. This ad-hoc solution works for the specific use cases it was meant to support, but isn't very generic. We have a top-level object in libcamera, the CameraManager, that also needs to be accessed from various locations and is passed to object constructors. Standardize on passing the CameraManager everywhere, and access the global configuration through it. Signed-off-by: Laurent Pinchart Reviewed-by: Barnabás Pőcze Reviewed-by: Isaac Scott --- include/libcamera/internal/ipa_manager.h | 10 +++++----- include/libcamera/internal/ipa_proxy.h | 4 ++-- .../libcamera/internal/software_isp/benchmark.h | 6 +++--- .../internal/software_isp/swstats_cpu.h | 6 +++--- src/libcamera/camera_manager.cpp | 3 ++- src/libcamera/ipa_manager.cpp | 7 +++++-- src/libcamera/ipa_proxy.cpp | 16 +++++++++++----- src/libcamera/software_isp/benchmark.cpp | 7 ++++++- src/libcamera/software_isp/debayer.cpp | 14 ++++++-------- src/libcamera/software_isp/debayer.h | 4 ++-- src/libcamera/software_isp/debayer_cpu.cpp | 7 ++++--- src/libcamera/software_isp/debayer_cpu.h | 4 ++-- src/libcamera/software_isp/debayer_egl.cpp | 7 +++---- src/libcamera/software_isp/debayer_egl.h | 4 +++- src/libcamera/software_isp/software_isp.cpp | 9 +++++---- src/libcamera/software_isp/swstats_cpu.cpp | 8 ++++---- test/ipa/ipa_interface_test.cpp | 5 +---- .../module_ipa_proxy.cpp.tmpl | 8 ++++---- .../libcamera_templates/module_ipa_proxy.h.tmpl | 4 ++-- 19 files changed, 73 insertions(+), 60 deletions(-) diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index ecb6ae89..aaa3ca37 100644 --- a/include/libcamera/internal/ipa_manager.h +++ b/include/libcamera/internal/ipa_manager.h @@ -16,13 +16,13 @@ #include #include -#include "libcamera/internal/camera_manager.h" #include "libcamera/internal/pub_key.h" namespace libcamera { LOG_DECLARE_CATEGORY(IPAManager) +class CameraManager; class GlobalConfiguration; class IPAModule; class PipelineHandler; @@ -30,7 +30,7 @@ class PipelineHandler; class IPAManager { public: - IPAManager(const GlobalConfiguration &configuration); + IPAManager(const CameraManager &cm); ~IPAManager(); template @@ -43,9 +43,9 @@ public: auto proxy = [&]() -> std::unique_ptr { if (isSignatureValid(m)) - return std::make_unique(m, configuration_); + return std::make_unique(m, cm_); else - return std::make_unique(m, configuration_); + return std::make_unique(m, cm_); }(); if (!proxy->isValid()) { @@ -73,7 +73,7 @@ private: bool isSignatureValid(IPAModule *ipa) const; - const GlobalConfiguration &configuration_; + const CameraManager &cm_; std::vector> modules_; #if HAVE_IPA_PUBKEY diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h index f1865d67..72397842 100644 --- a/include/libcamera/internal/ipa_proxy.h +++ b/include/libcamera/internal/ipa_proxy.h @@ -13,7 +13,7 @@ #include -#include "libcamera/internal/global_configuration.h" +#include "libcamera/internal/camera_manager.h" namespace libcamera { @@ -28,7 +28,7 @@ public: ProxyRunning, }; - IPAProxy(IPAModule *ipam, const GlobalConfiguration &configuration); + IPAProxy(IPAModule *ipam, const CameraManager &cm); ~IPAProxy(); bool isValid() const { return valid_; } diff --git a/include/libcamera/internal/software_isp/benchmark.h b/include/libcamera/internal/software_isp/benchmark.h index 5526abc5..cc7f9d71 100644 --- a/include/libcamera/internal/software_isp/benchmark.h +++ b/include/libcamera/internal/software_isp/benchmark.h @@ -13,15 +13,15 @@ #include #include #include -#include -#include "libcamera/internal/global_configuration.h" namespace libcamera { +class CameraManager; + class Benchmark { public: - Benchmark(const GlobalConfiguration &configuration, const std::string &name); + Benchmark(const CameraManager &cm, const std::string &name); ~Benchmark(); void startFrame(void); diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h index feee92f9..802370bd 100644 --- a/include/libcamera/internal/software_isp/swstats_cpu.h +++ b/include/libcamera/internal/software_isp/swstats_cpu.h @@ -20,7 +20,6 @@ #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/framebuffer.h" -#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/shared_mem_object.h" #include "libcamera/internal/software_isp/swisp_stats.h" @@ -28,14 +27,15 @@ namespace libcamera { -class PixelFormat; +class CameraManager; class MappedFrameBuffer; +class PixelFormat; struct StreamConfiguration; class SwStatsCpu { public: - SwStatsCpu(const GlobalConfiguration &configuration); + SwStatsCpu(const CameraManager &cm); ~SwStatsCpu() = default; /* diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 5762c210..f774bd84 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -93,7 +93,8 @@ void CameraManager::Private::run() int CameraManager::Private::init() { - ipaManager_ = std::make_unique(configuration()); + CameraManager *const o = LIBCAMERA_O_PTR(); + ipaManager_ = std::make_unique(*o); enumerator_ = DeviceEnumerator::create(); if (!enumerator_ || enumerator_->enumerate()) diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index 1e905e8b..dd1f483b 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -100,13 +100,16 @@ LOG_DEFINE_CATEGORY(IPAManager) /** * \brief Construct an IPAManager instance + * \param[in] cm The camera manager * * The IPAManager class is meant to only be instantiated once, by the * CameraManager. */ -IPAManager::IPAManager(const GlobalConfiguration &configuration) - : configuration_(configuration) +IPAManager::IPAManager(const CameraManager &cm) + : cm_(cm) { + const GlobalConfiguration &configuration = cm._d()->configuration(); + #if HAVE_IPA_PUBKEY if (!pubKey_.isValid()) LOG(IPAManager, Warning) << "Public key not valid"; diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp index a3ccfa60..6c8780a0 100644 --- a/src/libcamera/ipa_proxy.cpp +++ b/src/libcamera/ipa_proxy.cpp @@ -117,13 +117,19 @@ std::string ipaConfigurationFile(const std::string &ipaName, const std::string & /** * \brief Construct an IPAProxy instance * \param[in] ipam The IPA module - * \param[in] configuration The global configuration + * \param[in] cm The camera manager */ -IPAProxy::IPAProxy(IPAModule *ipam, const GlobalConfiguration &configuration) - : valid_(false), state_(ProxyStopped), ipam_(ipam), - configPaths_(configuration.envListOption("LIBCAMERA_IPA_CONFIG_PATH", { "ipa", "config_paths" }).value_or(std::vector())), - execPaths_(configuration.envListOption("LIBCAMERA_IPA_PROXY_PATH", { "ipa", "proxy_paths" }).value_or(std::vector())) +IPAProxy::IPAProxy(IPAModule *ipam, const CameraManager &cm) + : valid_(false), state_(ProxyStopped), ipam_(ipam) { + const GlobalConfiguration &configuration = cm._d()->configuration(); + + configPaths_ = configuration.envListOption("LIBCAMERA_IPA_CONFIG_PATH", + { "ipa", "config_paths" }) + .value_or(std::vector()); + execPaths_ = configuration.envListOption("LIBCAMERA_IPA_PROXY_PATH", + { "ipa", "proxy_paths" }) + .value_or(std::vector()); } IPAProxy::~IPAProxy() diff --git a/src/libcamera/software_isp/benchmark.cpp b/src/libcamera/software_isp/benchmark.cpp index 36c49770..96f0ae00 100644 --- a/src/libcamera/software_isp/benchmark.cpp +++ b/src/libcamera/software_isp/benchmark.cpp @@ -12,6 +12,9 @@ #include +#include "libcamera/internal/camera_manager.h" +#include "libcamera/internal/global_configuration.h" + namespace libcamera { LOG_DEFINE_CATEGORY(Benchmark) @@ -26,9 +29,11 @@ LOG_DEFINE_CATEGORY(Benchmark) /** * \brief Constructs a Benchmark object */ -Benchmark::Benchmark(const GlobalConfiguration &configuration, const std::string &name) +Benchmark::Benchmark(const CameraManager &cm, const std::string &name) : name_(name) { + const GlobalConfiguration &configuration = cm._d()->configuration(); + skipBeforeMeasure_ = configuration.option( { "software_isp", "measure", "skip" }) .value_or(skipBeforeMeasure_); diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp index a6bceb58..2d7abfb8 100644 --- a/src/libcamera/software_isp/debayer.cpp +++ b/src/libcamera/software_isp/debayer.cpp @@ -18,12 +18,6 @@ namespace libcamera { * \brief Struct to hold the debayer parameters. */ -/** - * \fn Debayer::Debayer(const GlobalConfiguration &configuration) - * \brief Construct a Debayer object - * \param[in] configuration Global configuration reference - */ - /** * \var DebayerParams::gains * \brief Colour channel gains @@ -58,8 +52,12 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Debayer) -Debayer::Debayer(const GlobalConfiguration &configuration) - : bench_(configuration, "Debayer") +/** + * \brief Construct a Debayer object + * \param[in] cm The camera manager + */ +Debayer::Debayer(const CameraManager &cm) + : bench_(cm, "Debayer") { } diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h index ea1ec6dc..a2a17ec1 100644 --- a/src/libcamera/software_isp/debayer.h +++ b/src/libcamera/software_isp/debayer.h @@ -22,12 +22,12 @@ #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/dma_buf_allocator.h" -#include "libcamera/internal/global_configuration.h" #include "libcamera/internal/software_isp/benchmark.h" #include "libcamera/internal/software_isp/debayer_params.h" namespace libcamera { +class CameraManager; class FrameBuffer; LOG_DECLARE_CATEGORY(Debayer) @@ -35,7 +35,7 @@ LOG_DECLARE_CATEGORY(Debayer) class Debayer : public Object { public: - Debayer(const GlobalConfiguration &configuration); + Debayer(const CameraManager &cm); virtual ~Debayer() = 0; virtual int configure(const StreamConfiguration &inputCfg, diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index dd0fff87..d9f5b326 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -89,10 +89,10 @@ DebayerCpuThread::DebayerCpuThread(DebayerCpu *debayer, unsigned int threadIndex /** * \brief Constructs a DebayerCpu object * \param[in] stats Pointer to the stats object to use - * \param[in] configuration The global configuration + * \param[in] cm The camera manager */ -DebayerCpu::DebayerCpu(std::unique_ptr stats, const GlobalConfiguration &configuration) - : Debayer(configuration), stats_(std::move(stats)) +DebayerCpu::DebayerCpu(std::unique_ptr stats, const CameraManager &cm) + : Debayer(cm), stats_(std::move(stats)) { /* * Reading from uncached buffers may be very slow. @@ -105,6 +105,7 @@ DebayerCpu::DebayerCpu(std::unique_ptr stats, const GlobalConfigurat * \todo Make memcpy automatic based on runtime detection of platform * capabilities. */ + const GlobalConfiguration &configuration = cm._d()->configuration(); bool enableInputMemcpy = configuration.option({ "software_isp", "copy_input_buffer" }).value_or(true); diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index 39a0ea6e..8c58775d 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -19,7 +19,7 @@ #include #include "libcamera/internal/bayer_format.h" -#include "libcamera/internal/global_configuration.h" +#include "libcamera/internal/camera_manager.h" #include "libcamera/internal/software_isp/debayer_params.h" #include "libcamera/internal/software_isp/swstats_cpu.h" @@ -31,7 +31,7 @@ class DebayerCpuThread; class DebayerCpu : public Debayer { public: - DebayerCpu(std::unique_ptr stats, const GlobalConfiguration &configuration); + DebayerCpu(std::unique_ptr stats, const CameraManager &cm); ~DebayerCpu(); int configure(const StreamConfiguration &inputCfg, diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp index 2ad258bc..8f0c229f 100644 --- a/src/libcamera/software_isp/debayer_egl.cpp +++ b/src/libcamera/software_isp/debayer_egl.cpp @@ -29,13 +29,12 @@ namespace libcamera { */ /** - * \fn DebayerEGL::DebayerEGL(std::unique_ptr stats, const GlobalConfiguration &configuration) * \brief Construct a DebayerEGL object * \param[in] stats Statistics processing object - * \param[in] configuration Global configuration reference + * \param[in] cm The camera manager */ -DebayerEGL::DebayerEGL(std::unique_ptr stats, const GlobalConfiguration &configuration) - : Debayer(configuration), stats_(std::move(stats)) +DebayerEGL::DebayerEGL(std::unique_ptr stats, const CameraManager &cm) + : Debayer(cm), stats_(std::move(stats)) { } diff --git a/src/libcamera/software_isp/debayer_egl.h b/src/libcamera/software_isp/debayer_egl.h index 644f6604..fcd281f4 100644 --- a/src/libcamera/software_isp/debayer_egl.h +++ b/src/libcamera/software_isp/debayer_egl.h @@ -35,10 +35,12 @@ namespace libcamera { #define DEBAYER_EGL_MIN_SIMPLE_RGB_GAIN_TEXTURE_UNITS 4 #define DEBAYER_OPENGL_COORDS 4 +class CameraManager; + class DebayerEGL : public Debayer { public: - DebayerEGL(std::unique_ptr stats, const GlobalConfiguration &configuration); + DebayerEGL(std::unique_ptr stats, const CameraManager &cm); ~DebayerEGL(); int configure(const StreamConfiguration &inputCfg, diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 5b225c7c..cd0e9d06 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -95,9 +95,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor, return; } - const GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration(); + const CameraManager &cm = *pipe->cameraManager(); - auto stats = std::make_unique(configuration); + auto stats = std::make_unique(cm); if (!stats->isValid()) { LOG(SoftwareIsp, Error) << "Failed to create SwStatsCpu object"; return; @@ -105,6 +105,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor, stats->statsReady.connect(this, &SoftwareIsp::statsReady); #if HAVE_DEBAYER_EGL + const GlobalConfiguration &configuration = cm._d()->configuration(); std::optional softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" }); if (softISPMode) { if (softISPMode != "gpu" && softISPMode != "cpu") { @@ -117,11 +118,11 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor, } if (!softISPMode || softISPMode == "gpu") - debayer_ = std::make_unique(std::move(stats), configuration); + debayer_ = std::make_unique(std::move(stats), cm); #endif if (!debayer_) - debayer_ = std::make_unique(std::move(stats), configuration); + debayer_ = std::make_unique(std::move(stats), cm); debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady); debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady); diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index 3595c9e6..5366e019 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -36,9 +36,9 @@ namespace libcamera { */ /** - * \fn SwStatsCpu::SwStatsCpu(const GlobalConfiguration &configuration) + * \fn SwStatsCpu::SwStatsCpu(const CameraManager &cm) * \brief Construct a SwStatsCpu object - * \param[in] configuration Global configuration reference + * \param[in] cm The camera manager * * Creates a SwStatsCpu object and initialises shared memory for statistics * exchange. @@ -159,8 +159,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(SwStatsCpu) -SwStatsCpu::SwStatsCpu(const GlobalConfiguration &configuration) - : sharedStats_("softIsp_stats"), bench_(configuration, "CPU stats") +SwStatsCpu::SwStatsCpu(const CameraManager &cm) + : sharedStats_("softIsp_stats"), bench_(cm, "CPU stats") { if (!sharedStats_) LOG(SwStatsCpu, Error) diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index c1fe2267..271c4e2c 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -47,7 +47,6 @@ public: notifier_.reset(); ipa_.reset(); ipaManager_.reset(); - config_.reset(); cameraManager_.reset(); } @@ -90,8 +89,7 @@ protected: notifier_->activated.connect(this, &IPAInterfaceTest::readTrace); /* Create the IPA manager. */ - config_ = std::make_unique(); - ipaManager_ = std::make_unique(*config_); + ipaManager_ = std::make_unique(*cameraManager_); return TestPass; } @@ -169,7 +167,6 @@ private: std::shared_ptr pipe_; std::unique_ptr ipa_; std::unique_ptr cameraManager_; - std::unique_ptr config_; std::unique_ptr ipaManager_; enum ipa::vimc::IPAOperationCode trace_; std::unique_ptr notifier_; diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index e6e19b30..0d0a1614 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -45,8 +45,8 @@ namespace {{ns}} { {% endfor %} {%- endif %} -{{proxy_name}}Threaded::{{proxy_name}}Threaded(IPAModule *ipam, const GlobalConfiguration &configuration) - : {{proxy_name}}(ipam, configuration), thread_("{{proxy_name}}") +{{proxy_name}}Threaded::{{proxy_name}}Threaded(IPAModule *ipam, const CameraManager &cm) + : {{proxy_name}}(ipam, cm), thread_("{{proxy_name}}") { LOG(IPAProxy, Debug) << "initializing {{module_name}} proxy in thread: loading IPA from " @@ -127,8 +127,8 @@ namespace {{ns}} { /* ========================================================================== */ -{{proxy_name}}Isolated::{{proxy_name}}Isolated(IPAModule *ipam, const GlobalConfiguration &configuration) - : {{proxy_name}}(ipam, configuration), +{{proxy_name}}Isolated::{{proxy_name}}Isolated(IPAModule *ipam, const CameraManager &cm) + : {{proxy_name}}(ipam, cm), controlSerializer_(ControlSerializer::Role::Proxy), seq_(0) { LOG(IPAProxy, Debug) diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ef280ca4..d48b90dc 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -50,7 +50,7 @@ protected: class {{proxy_name}}Threaded : public {{proxy_name}} { public: - {{proxy_name}}Threaded(IPAModule *ipam, const GlobalConfiguration &configuration); + {{proxy_name}}Threaded(IPAModule *ipam, const CameraManager &cm); ~{{proxy_name}}Threaded(); {% for method in interface_main.methods %} @@ -112,7 +112,7 @@ private: class {{proxy_name}}Isolated : public {{proxy_name}} { public: - {{proxy_name}}Isolated(IPAModule *ipam, const GlobalConfiguration &configuration); + {{proxy_name}}Isolated(IPAModule *ipam, const CameraManager &cm); ~{{proxy_name}}Isolated(); {% for method in interface_main.methods %}