[{"id":35929,"web_url":"https://patchwork.libcamera.org/comment/35929/","msgid":"<20250919170334.GB8038@pendragon.ideasonboard.com>","date":"2025-09-19T17:03:34","subject":"Re: [PATCH v18 02/12] libcamera: camera_manager: Construct\n\tGlobalConfiguration instance","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Sep 12, 2025 at 04:29:03PM +0200, Milan Zamazal wrote:\n> Global configuration is accessed via a GlobalConfiguration instance.\n> The instance is conceptually a singleton, but singletons are not welcome\n> in libcamera so we must store the (preferably single) instance\n> somewhere.\n> \n> This patch creates a GlobalConfiguration instance in CameraManager and\n> defines the corresponding access method. CameraManager is typically\n> instantiated only once or a few times, it is accessible in many places\n> in libcamera and the configuration can be retrieved from it and passed\n> to other places if needed (it's read-only once created). Using\n> CameraManager for the purpose is still suboptimal and we use it only due\n> to lack of better options.\n\nI think it's actually optimal :-) But let's not dwell on that, the\nresult looks good to me, no need to update the commit message.\n\n> An alternative could be Logger, which is\n> still a singleton and it's accessible from everywhere. But with Logger,\n> we have a chicken and egg problem -- GlobalConfiguration should log\n> contingent problems with the configuration when it's loaded but if it is\n> created in the logger then there are mutual infinite recursive calls.\n> One possible way to deal with this is to look at the environment\n> variables only during logging initialisation and apply the logging\n> configuration when a CameraManager is constructed. Considering there\n> are intentions to remove the Logger singleton, let's omit logging\n> configuration for now.\n> \n> If there are multiple CameraManager instances, there are also multiple\n> GlobalConfiguration instances, each CameraManager instance is meant to\n> be fully independent, including configuration. They may or may not\n> contain the same data, depending on whether the global configuration\n> file in the file system was changed in the meantime.\n> \n> The configuration is stored in the private CameraManager. It's\n> accessible within libcamera (via CameraManager) but it's not meant to be\n> accessed by applications.\n> \n> Reviewed-by: Paul Elder \n> Signed-off-by: Milan Zamazal \n> ---\n> include/libcamera/internal/camera_manager.h | 12 ++++++++++--\n> src/libcamera/camera_manager.cpp | 8 ++++++++\n> 2 files changed, 18 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 5dfbe1f65..b8b2966b5 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -7,8 +7,6 @@\n> \n> #pragma once\n> \n> -#include \n> -\n\nThis was done on purpose, to ensure that camera_manager.h is\nself-contained. I'll restore this when applying the patch.\n\nReviewed-by: Laurent Pinchart \n\n> #include \n> #include \n> #include \n> @@ -18,6 +16,9 @@\n> #include \n> #include \n> \n> +#include \n> +\n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/process.h\"\n> \n> namespace libcamera {\n> @@ -38,6 +39,11 @@ public:\n> \tvoid addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> \tvoid removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> \n> +\tconst GlobalConfiguration &configuration() const\n> +\t{\n> +\t\treturn configuration_;\n> +\t}\n> +\n> \tIPAManager *ipaManager() const { return ipaManager_.get(); }\n> \n> protected:\n> @@ -65,6 +71,8 @@ private:\n> \tstd::unique_ptr enumerator_;\n> \n> \tstd::unique_ptr ipaManager_;\n> +\n> +\tconst GlobalConfiguration configuration_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index f81794bfd..dca3d9a83 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -15,6 +15,7 @@\n> \n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> @@ -258,6 +259,13 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera)\n> \to->cameraRemoved.emit(camera);\n> }\n> \n> +/**\n> + * \\fn const GlobalConfiguration &CameraManager::Private::configuration() const\n> + * \\brief Get global configuration bound to the camera manager\n> + *\n> + * \\return Reference to the configuration\n> + */\n> +\n> /**\n> * \\fn CameraManager::Private::ipaManager() const\n> * \\brief Retrieve the IPAManager","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 D1E9ABE173\n\tfor ;\n\tFri, 19 Sep 2025 17:04:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C37116B597;\n\tFri, 19 Sep 2025 19:04:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9B1362C35\n\tfor ;\n\tFri, 19 Sep 2025 19:04:04 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1920C842;\n\tFri, 19 Sep 2025 19:02:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"t6s/fBJp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758301364;\n\tbh=FDbKx6/6UbnBGhohZo54X0pHX3JeoswlL0PY/yk3GnU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t6s/fBJpbD0IlkdEgvUkijFgarO0pXS7PvLuAPbF1FPg71IE8B4wGs/ptCdCxGZAT\n\tS+azn5+mAf8rn4fJFglykuu0kMDbl/+mTTFzhVAu+hjFWmbqp20RBiqzU6tEuJ7BC5\n\tT2btwEZ5xpbd9XT4JBOC1XKplOE5jQMIhW64wgGE=","Date":"Fri, 19 Sep 2025 20:03:34 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v18 02/12] libcamera: camera_manager: Construct\n\tGlobalConfiguration instance","Message-ID":"<20250919170334.GB8038@pendragon.ideasonboard.com>","References":"<20250912142915.53949-1-mzamazal@redhat.com>\n\t<20250912142915.53949-3-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250912142915.53949-3-mzamazal@redhat.com>","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\" "}}]