[{"id":34465,"web_url":"https://patchwork.libcamera.org/comment/34465/","msgid":"<4e3903ed-d056-4c08-92c5-e5a8926ec7e9@ideasonboard.com>","date":"2025-06-13T13:05:55","subject":"Re: [PATCH v9 03/13] config: Make GlobalConfiguration instance","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 06. 11. 16:24 keltezéssel, Milan Zamazal írta:\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\n> many places in libcamera and the configuration can be retrieved from it\n> and passed to other places if needed (it's read-only once created).\n> Using CameraManager for the purpose is still suboptimal and we use it\n> only due to lack of better options. An alternative could be Logger,\n> which is still a singleton and it's accessible from everywhere. But\n> with Logger, we have a chicken and egg problem -- GlobalConfiguration\n> should log contingent problems with the configuration when it's loaded\n> but if it is created in the logger then there are mutual infinite\n> recursive calls. Perhaps some acceptable workaround could be found,\n> which would also allow an easier logging configuration.\n> \n> If there are multiple CameraManager instances, there are also multiple\n> GlobalConfiguration instances. They may or may not contain the same\n> data, depending on whether the global configuration file in the file\n> system was changed in the meantime. This is dirty and a potential\n> source of trouble (although not expected to have much impact in\n> practice) but there is no idea how to do better if we want to avoid\n> having GlobalConfiguration singleton or dealing with the logging\n> problem.\n> \n> The configuration is stored in CameraManager privately, it's not meant\n> to be accessed by applications.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> include/libcamera/internal/camera_manager.h | 5 +++++\n> src/libcamera/camera_manager.cpp | 11 +++++++++++\n> 2 files changed, 16 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 0150ca61f..f03fe6343 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/global_configuration.h\"\n> #include \"libcamera/internal/process.h\"\n> \n> namespace libcamera {\n> @@ -38,6 +39,8 @@ 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\nIt's quite trivial, so could you inline the function?\n\n\n> +\n> \tIPAManager *ipaManager() const { return ipaManager_.get(); }\n> \n> protected:\n> @@ -66,6 +69,8 @@ private:\n> \n> \tstd::unique_ptr ipaManager_;\n> \tProcessManager processManager_;\n> +\n> +\tconst GlobalConfiguration configuration_{ GlobalConfiguration() };\n\nI think you can omit the `{ GlobalConfiguration() }` part.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index e62e7193c..7361915bf 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> @@ -254,6 +255,16 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera)\n> \to->cameraRemoved.emit(camera);\n> }\n> \n> +/**\n> + * \\brief Get global configuration bound to the camera manager\n> + *\n> + * \\return Reference to the configuration\n> + */\n> +const GlobalConfiguration &CameraManager::Private::configuration() const\n> +{\n> +\treturn configuration_;\n> +}\n> +\n> /**\n> * \\fn CameraManager::Private::ipaManager() const\n> * \\brief Retrieve the IPAManager\n> --\n> 2.49.0\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 1E3B1BDE6B\n\tfor ;\n\tFri, 13 Jun 2025 13:06:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3E0668DBE;\n\tFri, 13 Jun 2025 15:06:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8512B68DB4\n\tfor ;\n\tFri, 13 Jun 2025 15:06:01 +0200 (CEST)","from [192.168.33.21] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CE17A348;\n\tFri, 13 Jun 2025 15:05:50 +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=\"EkREznNd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749819952;\n\tbh=1Usn2SrqGdqtagcAh0PPLk1+OhysTPwbiWCpxoiA5S0=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=EkREznNdFb6A+1ePyHQ7+mozlP9/2oui18FkMTDvTG9j7ecdpIcQ7DV/A2vTLiv6+\n\tf9k38B67ptMGPrrLdYclfWSu/LMtB1MFRpHuajoM/T/x0yUI95Ts6htgljDkAbqudU\n\tJ3qoudKPOgI5iei1qbH+5MrKcPMwk2pysCGj/u8Y=","Message-ID":"<4e3903ed-d056-4c08-92c5-e5a8926ec7e9@ideasonboard.com>","Date":"Fri, 13 Jun 2025 15:05:55 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v9 03/13] config: Make GlobalConfiguration instance","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Kieran Bingham ,\n\tLaurent Pinchart ","References":"<20250611142431.33306-1-mzamazal@redhat.com>\n\t\n\t<20250611142431.33306-4-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250611142431.33306-4-mzamazal@redhat.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\" "}}]