Message ID | 20250711201232.129264-1-mzamazal@redhat.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi Milan, I'm not sure where - but it seems this is breaking CI on the virtual or vimc cameras ? 6/80 libcamera:camera / camera_reconfigure FAIL 0.97s (exit status 255 or signal 127 SIGinvalid) https://gitlab.freedesktop.org/camera/libcamera/-/jobs/80404217 Could you run the unit tests locally and/or check the CI results please? -- Kieran Quoting Milan Zamazal (2025-07-11 21:12:19) > This patch series introduces global configuration file for libcamera, to > provide runtime configuration means other than environment variables. > Instead of, or in addition to, the growing list of configuration > environment variables, the whole configuration can be specified in a > single configuration file. This is both simpler and more flexible. > > This is not a replacement for specific configuration files already > present in libcamera, with the exception of rpi configuration file (see > “Look up rpi configuration in the configuration file” patch). > > The patches implement what is needed to introduce a configuration file > that can handle the current environment variables and software ISP > TODOs. They demonstrate how to deal with the key points that must be > considered. Logging is omitted from the configuration file for > technical reasons. See commit messages for more details. > > The configuration file is a YAML file. It is looked up in user’s home > directory or, if not present, in system-wide libcamera directories. > Both the directories and file name of the configuration file can be > overridden using newly introduced environment variables. > > The current configuration environment variables are still supported and > take precedence, if defined, over the corresponding entries in the YAML > file. > > This patch series is not exhaustive, there can be future enhancements, > most notably configuration file validation to avoid confusions caused by > typos etc. > > Not everything has been tested because some of the patches are related > to specific hardware. > > Changes in v13: > - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is > loaded. > - Standard user configuration directory has no longer precedence over > LIBCAMERA_CONFIG_DIR. > - Improvements to the rpi code suggested by Barnabás. > - Documentation fixes suggested by Barnabás. > - Make sure yamlConfiguration_ is initialized. > > Changes in v12: > - The commit message of "Make GlobalConfiguration instance" updated > according to the recent discussions. > - Use initializer_list rather than a string with separators for > GlobalConfiguration option path arguments. > - char *const qualifiers dropped. > - rpi can have separate configurations for different targets. > - GlobalConfiguration::option template is not type limited anymore. > - utils::join used instead of manual string concatenation. > - envListOption and listOption return std::optional now. > - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped. > - Isolation configuration stored to IPAManager to avoid passing the > whole configuraton to isSignatureValid. > - Similarly to paths in IPAProxy. > - `delimiter' argument added to GlobalConfiguration::envListOption and > used for LIBCAMERA_PIPELINES_MATCH_LIST processing. > - Configuration no longer looked up in /etc/libcamera and additionally > looked up in LIBCAMERA_DATA_DIR. > - Cosmetic changes suggested by Barnabás. > > Changes in v11: > - global_configuration.cpp moved out of base and yaml_parser.cpp left in > its original place. > - Configuration initialized as empty if there is no configuration file, > preventing a segfault on later access. > - Cosmetic changes suggested by Laurent. > - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly. > - Failing to read an existent configuration file is an error, rather > than warning, now. (It should be also fatal, but how to propagate it?) > - Slash is used instead of dot to separate configuration names. > - rpi configuration read is in the global configuration directly (unless > overridden by LIBCAMERA_RPI_CONFIG_FILE) now. > > Changes in v10: > - Minor improvements suggested by Barnabás. > > Changes in v9: > - The configuration instance is now stored in CameraManager and accessed > from there rather than being a separate singleton wrapped by global > accessors. This solves the ugly problem of delayed initialization but > I don’t like anything else about it. I played a bit with the idea of > attaching it to Logger instead, see the commit message of patch 03 for > some discussion, but stayed with the CameraManager proposal > eventually. > - I’ve given up on the logging configuration now when the configuration > is stored in CameraManager and removed the corresponding patches. I > think it’s possible to add it later, but for now, it’s already > complicated enough. > - Not much tested, let’s see first if the current implementation gets in > an acceptable direction. > > Changes in v8: > - Rebased on current master. > - Anniversary edition: 400 days since v1 posted. ;-) > > Changes in v7: > - Rebased on current master. > - Tuning file path configuration updated for recent changes. > A significant change is that the tuning file configuration is no > longer sensor dependent as there is apparently no access to the sensor > info in the IPA proxy. > - Minor improvements of some commit messages. > > Changes in v6: > - Rebased on master. > - File names from file header descriptions removed. > - Indentation fix in moved code as requested by checkstyle.py. > - Unneeded const_cast's removed. > - Using GlobalConfiguration namespace rather than a class. > - Path configuration options are defined as sequences in YAML. > - A patch introducing LIBCAMERA_CONFIG_NAME added. > - A patch introducing LIBCAMERA_CONFIG_DIR added. > - Miscellaneous minor code changes suggested by Barnabás. > > Changes in v5: > - A pointer is used to store the global configuration singleton rather > than a static variable. This makes the things more robust and fixes > the problem with re-entrancy on logging and a failing > camera_reconfigure test. > - In relation to the change above, a new initialization method > GlobalConfiguration::initialize() was introduced that replaces the > initialization calls in CameraManager and IPAManager. > - Logging YAML errors when reading the configuration was also fixed. > - Global configuration is placed to base directly, without an > intermediate patch. > - An `optional' value comparison simplified. > - A temporary typo in a comment fixed. > - Unused stdint.h include removed. > > Changes in v4: > - Rebased on current master. > - Configuration option for LIBCAMERA_IPA_PROXY_PATH added. > - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp. > > Changes in v3: > - Added a configuration item for the newly introduced > LIBCAMERA_PIPELINES_MATCH_LIST variable. > - A minor indentation fix. > - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch. > - Tuning files are looked up now by particular cameras attached rather than > being specified for the whole pipeline. > - Helpers use std::string& instead of char* for confPath arguments. > - Protection against returning YamlObject::empty as a regular value (the > problem has been probably exposed by addition of > LIBCAMERA_PIPELINES_MATCH_LIST). > > Changes in v2: > - Rebased on master. > - Various cleanups, documentation improvements and minor fixes. > - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush). > - Two more patches for software ISP configuration added. > > Milan Zamazal (12): > config: Introduce global runtime configuration > config: Make GlobalConfiguration instance > config: Add configuration retrieval helpers > config: Look up rpi configuration in the configuration file > config: Look up IPA configurables in configuration file > config: Look up pipelines match list in configuration file > config: Allow enabling software ISP in runtime > config: Add global configuration file documentation > libcamera: software_isp: Make input buffer copying configurable > libcamera: software_isp: Make measurement configurable > config: Make configuration file configurable > config: Make configuration directories configurable > > Documentation/documentation-contents.rst | 2 +- > Documentation/index.rst | 2 +- > Documentation/meson.build | 2 +- > ...ariables.rst => runtime_configuration.rst} | 129 ++++++++- > include/libcamera/internal/camera_manager.h | 8 + > .../libcamera/internal/global_configuration.h | 62 +++++ > include/libcamera/internal/ipa_manager.h | 7 +- > include/libcamera/internal/ipa_proxy.h | 8 +- > include/libcamera/internal/meson.build | 1 + > src/libcamera/camera_manager.cpp | 20 +- > src/libcamera/global_configuration.cpp | 259 ++++++++++++++++++ > src/libcamera/ipa_manager.cpp | 39 ++- > src/libcamera/ipa_proxy.cpp | 51 ++-- > src/libcamera/meson.build | 1 + > .../pipeline/rpi/common/pipeline_base.cpp | 62 +++-- > .../pipeline/rpi/common/pipeline_base.h | 3 +- > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +- > src/libcamera/pipeline/simple/simple.cpp | 13 + > src/libcamera/software_isp/TODO | 36 --- > src/libcamera/software_isp/debayer_cpu.cpp | 32 ++- > src/libcamera/software_isp/debayer_cpu.h | 10 +- > src/libcamera/software_isp/software_isp.cpp | 3 +- > .../module_ipa_proxy.cpp.tmpl | 4 +- > .../module_ipa_proxy.h.tmpl | 2 +- > 25 files changed, 633 insertions(+), 175 deletions(-) > rename Documentation/{environment_variables.rst => runtime_configuration.rst} (59%) > create mode 100644 include/libcamera/internal/global_configuration.h > create mode 100644 src/libcamera/global_configuration.cpp > > -- > 2.50.1 >
Hi 2025. 07. 14. 12:02 keltezéssel, Kieran Bingham írta: > Hi Milan, > > I'm not sure where - but it seems this is breaking CI on the virtual or > vimc cameras ? > > 6/80 libcamera:camera / camera_reconfigure FAIL > 0.97s (exit status 255 or signal 127 SIGinvalid) > > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/80404217 > > Could you run the unit tests locally and/or check the CI results please? Now the `LIBCAMERA_IPA_FORCE_ISOLATION` env var is checked during construction, so it has to be set before a `CameraManager` is constructed. diff --git a/test/libtest/camera_test.cpp b/test/libtest/camera_test.cpp index fe13d6acf..6fb44989d 100644 --- a/test/libtest/camera_test.cpp +++ b/test/libtest/camera_test.cpp @@ -15,11 +15,11 @@ using namespace std; CameraTest::CameraTest(const char *name, bool isolate) { - cm_ = new CameraManager(); - if (isolate) setenv("LIBCAMERA_IPA_FORCE_ISOLATION", "1", 1); + cm_ = new CameraManager(); + if (cm_->start()) { cerr << "Failed to start camera manager" << endl; status_ = TestFail; Regards, Barnabás Pőcze > > -- > Kieran > > > Quoting Milan Zamazal (2025-07-11 21:12:19) >> This patch series introduces global configuration file for libcamera, to >> provide runtime configuration means other than environment variables. >> Instead of, or in addition to, the growing list of configuration >> environment variables, the whole configuration can be specified in a >> single configuration file. This is both simpler and more flexible. >> >> This is not a replacement for specific configuration files already >> present in libcamera, with the exception of rpi configuration file (see >> “Look up rpi configuration in the configuration file” patch). >> >> The patches implement what is needed to introduce a configuration file >> that can handle the current environment variables and software ISP >> TODOs. They demonstrate how to deal with the key points that must be >> considered. Logging is omitted from the configuration file for >> technical reasons. See commit messages for more details. >> >> The configuration file is a YAML file. It is looked up in user’s home >> directory or, if not present, in system-wide libcamera directories. >> Both the directories and file name of the configuration file can be >> overridden using newly introduced environment variables. >> >> The current configuration environment variables are still supported and >> take precedence, if defined, over the corresponding entries in the YAML >> file. >> >> This patch series is not exhaustive, there can be future enhancements, >> most notably configuration file validation to avoid confusions caused by >> typos etc. >> >> Not everything has been tested because some of the patches are related >> to specific hardware. >> >> Changes in v13: >> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is >> loaded. >> - Standard user configuration directory has no longer precedence over >> LIBCAMERA_CONFIG_DIR. >> - Improvements to the rpi code suggested by Barnabás. >> - Documentation fixes suggested by Barnabás. >> - Make sure yamlConfiguration_ is initialized. >> >> Changes in v12: >> - The commit message of "Make GlobalConfiguration instance" updated >> according to the recent discussions. >> - Use initializer_list rather than a string with separators for >> GlobalConfiguration option path arguments. >> - char *const qualifiers dropped. >> - rpi can have separate configurations for different targets. >> - GlobalConfiguration::option template is not type limited anymore. >> - utils::join used instead of manual string concatenation. >> - envListOption and listOption return std::optional now. >> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped. >> - Isolation configuration stored to IPAManager to avoid passing the >> whole configuraton to isSignatureValid. >> - Similarly to paths in IPAProxy. >> - `delimiter' argument added to GlobalConfiguration::envListOption and >> used for LIBCAMERA_PIPELINES_MATCH_LIST processing. >> - Configuration no longer looked up in /etc/libcamera and additionally >> looked up in LIBCAMERA_DATA_DIR. >> - Cosmetic changes suggested by Barnabás. >> >> Changes in v11: >> - global_configuration.cpp moved out of base and yaml_parser.cpp left in >> its original place. >> - Configuration initialized as empty if there is no configuration file, >> preventing a segfault on later access. >> - Cosmetic changes suggested by Laurent. >> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly. >> - Failing to read an existent configuration file is an error, rather >> than warning, now. (It should be also fatal, but how to propagate it?) >> - Slash is used instead of dot to separate configuration names. >> - rpi configuration read is in the global configuration directly (unless >> overridden by LIBCAMERA_RPI_CONFIG_FILE) now. >> >> Changes in v10: >> - Minor improvements suggested by Barnabás. >> >> Changes in v9: >> - The configuration instance is now stored in CameraManager and accessed >> from there rather than being a separate singleton wrapped by global >> accessors. This solves the ugly problem of delayed initialization but >> I don’t like anything else about it. I played a bit with the idea of >> attaching it to Logger instead, see the commit message of patch 03 for >> some discussion, but stayed with the CameraManager proposal >> eventually. >> - I’ve given up on the logging configuration now when the configuration >> is stored in CameraManager and removed the corresponding patches. I >> think it’s possible to add it later, but for now, it’s already >> complicated enough. >> - Not much tested, let’s see first if the current implementation gets in >> an acceptable direction. >> >> Changes in v8: >> - Rebased on current master. >> - Anniversary edition: 400 days since v1 posted. ;-) >> >> Changes in v7: >> - Rebased on current master. >> - Tuning file path configuration updated for recent changes. >> A significant change is that the tuning file configuration is no >> longer sensor dependent as there is apparently no access to the sensor >> info in the IPA proxy. >> - Minor improvements of some commit messages. >> >> Changes in v6: >> - Rebased on master. >> - File names from file header descriptions removed. >> - Indentation fix in moved code as requested by checkstyle.py. >> - Unneeded const_cast's removed. >> - Using GlobalConfiguration namespace rather than a class. >> - Path configuration options are defined as sequences in YAML. >> - A patch introducing LIBCAMERA_CONFIG_NAME added. >> - A patch introducing LIBCAMERA_CONFIG_DIR added. >> - Miscellaneous minor code changes suggested by Barnabás. >> >> Changes in v5: >> - A pointer is used to store the global configuration singleton rather >> than a static variable. This makes the things more robust and fixes >> the problem with re-entrancy on logging and a failing >> camera_reconfigure test. >> - In relation to the change above, a new initialization method >> GlobalConfiguration::initialize() was introduced that replaces the >> initialization calls in CameraManager and IPAManager. >> - Logging YAML errors when reading the configuration was also fixed. >> - Global configuration is placed to base directly, without an >> intermediate patch. >> - An `optional' value comparison simplified. >> - A temporary typo in a comment fixed. >> - Unused stdint.h include removed. >> >> Changes in v4: >> - Rebased on current master. >> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added. >> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp. >> >> Changes in v3: >> - Added a configuration item for the newly introduced >> LIBCAMERA_PIPELINES_MATCH_LIST variable. >> - A minor indentation fix. >> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch. >> - Tuning files are looked up now by particular cameras attached rather than >> being specified for the whole pipeline. >> - Helpers use std::string& instead of char* for confPath arguments. >> - Protection against returning YamlObject::empty as a regular value (the >> problem has been probably exposed by addition of >> LIBCAMERA_PIPELINES_MATCH_LIST). >> >> Changes in v2: >> - Rebased on master. >> - Various cleanups, documentation improvements and minor fixes. >> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush). >> - Two more patches for software ISP configuration added. >> >> Milan Zamazal (12): >> config: Introduce global runtime configuration >> config: Make GlobalConfiguration instance >> config: Add configuration retrieval helpers >> config: Look up rpi configuration in the configuration file >> config: Look up IPA configurables in configuration file >> config: Look up pipelines match list in configuration file >> config: Allow enabling software ISP in runtime >> config: Add global configuration file documentation >> libcamera: software_isp: Make input buffer copying configurable >> libcamera: software_isp: Make measurement configurable >> config: Make configuration file configurable >> config: Make configuration directories configurable >> >> Documentation/documentation-contents.rst | 2 +- >> Documentation/index.rst | 2 +- >> Documentation/meson.build | 2 +- >> ...ariables.rst => runtime_configuration.rst} | 129 ++++++++- >> include/libcamera/internal/camera_manager.h | 8 + >> .../libcamera/internal/global_configuration.h | 62 +++++ >> include/libcamera/internal/ipa_manager.h | 7 +- >> include/libcamera/internal/ipa_proxy.h | 8 +- >> include/libcamera/internal/meson.build | 1 + >> src/libcamera/camera_manager.cpp | 20 +- >> src/libcamera/global_configuration.cpp | 259 ++++++++++++++++++ >> src/libcamera/ipa_manager.cpp | 39 ++- >> src/libcamera/ipa_proxy.cpp | 51 ++-- >> src/libcamera/meson.build | 1 + >> .../pipeline/rpi/common/pipeline_base.cpp | 62 +++-- >> .../pipeline/rpi/common/pipeline_base.h | 3 +- >> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +- >> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +- >> src/libcamera/pipeline/simple/simple.cpp | 13 + >> src/libcamera/software_isp/TODO | 36 --- >> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++- >> src/libcamera/software_isp/debayer_cpu.h | 10 +- >> src/libcamera/software_isp/software_isp.cpp | 3 +- >> .../module_ipa_proxy.cpp.tmpl | 4 +- >> .../module_ipa_proxy.h.tmpl | 2 +- >> 25 files changed, 633 insertions(+), 175 deletions(-) >> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (59%) >> create mode 100644 include/libcamera/internal/global_configuration.h >> create mode 100644 src/libcamera/global_configuration.cpp >> >> -- >> 2.50.1 >>
On Mon, Jul 14, 2025 at 12:46:59PM +0200, Barnabás Pőcze wrote: > 2025. 07. 14. 12:02 keltezéssel, Kieran Bingham írta: > > Hi Milan, > > > > I'm not sure where - but it seems this is breaking CI on the virtual or > > vimc cameras ? > > > > 6/80 libcamera:camera / camera_reconfigure FAIL > > 0.97s (exit status 255 or signal 127 SIGinvalid) > > > > > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/80404217 > > > > Could you run the unit tests locally and/or check the CI results please? > > Now the `LIBCAMERA_IPA_FORCE_ISOLATION` env var is checked during construction, so > it has to be set before a `CameraManager` is constructed. > > diff --git a/test/libtest/camera_test.cpp b/test/libtest/camera_test.cpp > index fe13d6acf..6fb44989d 100644 > --- a/test/libtest/camera_test.cpp > +++ b/test/libtest/camera_test.cpp > @@ -15,11 +15,11 @@ using namespace std; > > CameraTest::CameraTest(const char *name, bool isolate) > { > - cm_ = new CameraManager(); > - > if (isolate) > setenv("LIBCAMERA_IPA_FORCE_ISOLATION", "1", 1); > > + cm_ = new CameraManager(); > + > if (cm_->start()) { > cerr << "Failed to start camera manager" << endl; > status_ = TestFail; > That looks like a worthy change on its own. Could you please send a patch ? > > Quoting Milan Zamazal (2025-07-11 21:12:19) > >> This patch series introduces global configuration file for libcamera, to > >> provide runtime configuration means other than environment variables. > >> Instead of, or in addition to, the growing list of configuration > >> environment variables, the whole configuration can be specified in a > >> single configuration file. This is both simpler and more flexible. > >> > >> This is not a replacement for specific configuration files already > >> present in libcamera, with the exception of rpi configuration file (see > >> “Look up rpi configuration in the configuration file” patch). > >> > >> The patches implement what is needed to introduce a configuration file > >> that can handle the current environment variables and software ISP > >> TODOs. They demonstrate how to deal with the key points that must be > >> considered. Logging is omitted from the configuration file for > >> technical reasons. See commit messages for more details. > >> > >> The configuration file is a YAML file. It is looked up in user’s home > >> directory or, if not present, in system-wide libcamera directories. > >> Both the directories and file name of the configuration file can be > >> overridden using newly introduced environment variables. > >> > >> The current configuration environment variables are still supported and > >> take precedence, if defined, over the corresponding entries in the YAML > >> file. > >> > >> This patch series is not exhaustive, there can be future enhancements, > >> most notably configuration file validation to avoid confusions caused by > >> typos etc. > >> > >> Not everything has been tested because some of the patches are related > >> to specific hardware. > >> > >> Changes in v13: > >> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is > >> loaded. > >> - Standard user configuration directory has no longer precedence over > >> LIBCAMERA_CONFIG_DIR. > >> - Improvements to the rpi code suggested by Barnabás. > >> - Documentation fixes suggested by Barnabás. > >> - Make sure yamlConfiguration_ is initialized. > >> > >> Changes in v12: > >> - The commit message of "Make GlobalConfiguration instance" updated > >> according to the recent discussions. > >> - Use initializer_list rather than a string with separators for > >> GlobalConfiguration option path arguments. > >> - char *const qualifiers dropped. > >> - rpi can have separate configurations for different targets. > >> - GlobalConfiguration::option template is not type limited anymore. > >> - utils::join used instead of manual string concatenation. > >> - envListOption and listOption return std::optional now. > >> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped. > >> - Isolation configuration stored to IPAManager to avoid passing the > >> whole configuraton to isSignatureValid. > >> - Similarly to paths in IPAProxy. > >> - `delimiter' argument added to GlobalConfiguration::envListOption and > >> used for LIBCAMERA_PIPELINES_MATCH_LIST processing. > >> - Configuration no longer looked up in /etc/libcamera and additionally > >> looked up in LIBCAMERA_DATA_DIR. > >> - Cosmetic changes suggested by Barnabás. > >> > >> Changes in v11: > >> - global_configuration.cpp moved out of base and yaml_parser.cpp left in > >> its original place. > >> - Configuration initialized as empty if there is no configuration file, > >> preventing a segfault on later access. > >> - Cosmetic changes suggested by Laurent. > >> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly. > >> - Failing to read an existent configuration file is an error, rather > >> than warning, now. (It should be also fatal, but how to propagate it?) > >> - Slash is used instead of dot to separate configuration names. > >> - rpi configuration read is in the global configuration directly (unless > >> overridden by LIBCAMERA_RPI_CONFIG_FILE) now. > >> > >> Changes in v10: > >> - Minor improvements suggested by Barnabás. > >> > >> Changes in v9: > >> - The configuration instance is now stored in CameraManager and accessed > >> from there rather than being a separate singleton wrapped by global > >> accessors. This solves the ugly problem of delayed initialization but > >> I don’t like anything else about it. I played a bit with the idea of > >> attaching it to Logger instead, see the commit message of patch 03 for > >> some discussion, but stayed with the CameraManager proposal > >> eventually. > >> - I’ve given up on the logging configuration now when the configuration > >> is stored in CameraManager and removed the corresponding patches. I > >> think it’s possible to add it later, but for now, it’s already > >> complicated enough. > >> - Not much tested, let’s see first if the current implementation gets in > >> an acceptable direction. > >> > >> Changes in v8: > >> - Rebased on current master. > >> - Anniversary edition: 400 days since v1 posted. ;-) > >> > >> Changes in v7: > >> - Rebased on current master. > >> - Tuning file path configuration updated for recent changes. > >> A significant change is that the tuning file configuration is no > >> longer sensor dependent as there is apparently no access to the sensor > >> info in the IPA proxy. > >> - Minor improvements of some commit messages. > >> > >> Changes in v6: > >> - Rebased on master. > >> - File names from file header descriptions removed. > >> - Indentation fix in moved code as requested by checkstyle.py. > >> - Unneeded const_cast's removed. > >> - Using GlobalConfiguration namespace rather than a class. > >> - Path configuration options are defined as sequences in YAML. > >> - A patch introducing LIBCAMERA_CONFIG_NAME added. > >> - A patch introducing LIBCAMERA_CONFIG_DIR added. > >> - Miscellaneous minor code changes suggested by Barnabás. > >> > >> Changes in v5: > >> - A pointer is used to store the global configuration singleton rather > >> than a static variable. This makes the things more robust and fixes > >> the problem with re-entrancy on logging and a failing > >> camera_reconfigure test. > >> - In relation to the change above, a new initialization method > >> GlobalConfiguration::initialize() was introduced that replaces the > >> initialization calls in CameraManager and IPAManager. > >> - Logging YAML errors when reading the configuration was also fixed. > >> - Global configuration is placed to base directly, without an > >> intermediate patch. > >> - An `optional' value comparison simplified. > >> - A temporary typo in a comment fixed. > >> - Unused stdint.h include removed. > >> > >> Changes in v4: > >> - Rebased on current master. > >> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added. > >> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp. > >> > >> Changes in v3: > >> - Added a configuration item for the newly introduced > >> LIBCAMERA_PIPELINES_MATCH_LIST variable. > >> - A minor indentation fix. > >> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch. > >> - Tuning files are looked up now by particular cameras attached rather than > >> being specified for the whole pipeline. > >> - Helpers use std::string& instead of char* for confPath arguments. > >> - Protection against returning YamlObject::empty as a regular value (the > >> problem has been probably exposed by addition of > >> LIBCAMERA_PIPELINES_MATCH_LIST). > >> > >> Changes in v2: > >> - Rebased on master. > >> - Various cleanups, documentation improvements and minor fixes. > >> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush). > >> - Two more patches for software ISP configuration added. > >> > >> Milan Zamazal (12): > >> config: Introduce global runtime configuration > >> config: Make GlobalConfiguration instance > >> config: Add configuration retrieval helpers > >> config: Look up rpi configuration in the configuration file > >> config: Look up IPA configurables in configuration file > >> config: Look up pipelines match list in configuration file > >> config: Allow enabling software ISP in runtime > >> config: Add global configuration file documentation > >> libcamera: software_isp: Make input buffer copying configurable > >> libcamera: software_isp: Make measurement configurable > >> config: Make configuration file configurable > >> config: Make configuration directories configurable > >> > >> Documentation/documentation-contents.rst | 2 +- > >> Documentation/index.rst | 2 +- > >> Documentation/meson.build | 2 +- > >> ...ariables.rst => runtime_configuration.rst} | 129 ++++++++- > >> include/libcamera/internal/camera_manager.h | 8 + > >> .../libcamera/internal/global_configuration.h | 62 +++++ > >> include/libcamera/internal/ipa_manager.h | 7 +- > >> include/libcamera/internal/ipa_proxy.h | 8 +- > >> include/libcamera/internal/meson.build | 1 + > >> src/libcamera/camera_manager.cpp | 20 +- > >> src/libcamera/global_configuration.cpp | 259 ++++++++++++++++++ > >> src/libcamera/ipa_manager.cpp | 39 ++- > >> src/libcamera/ipa_proxy.cpp | 51 ++-- > >> src/libcamera/meson.build | 1 + > >> .../pipeline/rpi/common/pipeline_base.cpp | 62 +++-- > >> .../pipeline/rpi/common/pipeline_base.h | 3 +- > >> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +- > >> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +- > >> src/libcamera/pipeline/simple/simple.cpp | 13 + > >> src/libcamera/software_isp/TODO | 36 --- > >> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++- > >> src/libcamera/software_isp/debayer_cpu.h | 10 +- > >> src/libcamera/software_isp/software_isp.cpp | 3 +- > >> .../module_ipa_proxy.cpp.tmpl | 4 +- > >> .../module_ipa_proxy.h.tmpl | 2 +- > >> 25 files changed, 633 insertions(+), 175 deletions(-) > >> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (59%) > >> create mode 100644 include/libcamera/internal/global_configuration.h > >> create mode 100644 src/libcamera/global_configuration.cpp
Hi Barnabás, thank you for the fix, it passes now: https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1470485 Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > Hi > > 2025. 07. 14. 12:02 keltezéssel, Kieran Bingham írta: >> Hi Milan, >> I'm not sure where - but it seems this is breaking CI on the virtual or >> vimc cameras ? >> 6/80 libcamera:camera / camera_reconfigure FAIL >> 0.97s (exit status 255 or signal 127 SIGinvalid) >> https://gitlab.freedesktop.org/camera/libcamera/-/jobs/80404217 >> Could you run the unit tests locally and/or check the CI results please? > > Now the `LIBCAMERA_IPA_FORCE_ISOLATION` env var is checked during construction, so > it has to be set before a `CameraManager` is constructed. > > diff --git a/test/libtest/camera_test.cpp b/test/libtest/camera_test.cpp > index fe13d6acf..6fb44989d 100644 > --- a/test/libtest/camera_test.cpp > +++ b/test/libtest/camera_test.cpp > @@ -15,11 +15,11 @@ using namespace std; > CameraTest::CameraTest(const char *name, bool isolate) > { > - cm_ = new CameraManager(); > - > if (isolate) > setenv("LIBCAMERA_IPA_FORCE_ISOLATION", "1", 1); > + cm_ = new CameraManager(); > + > if (cm_->start()) { > cerr << "Failed to start camera manager" << endl; > status_ = TestFail; > > > Regards, > Barnabás Pőcze > >> -- >> Kieran >> Quoting Milan Zamazal (2025-07-11 21:12:19) >>> This patch series introduces global configuration file for libcamera, to >>> provide runtime configuration means other than environment variables. >>> Instead of, or in addition to, the growing list of configuration >>> environment variables, the whole configuration can be specified in a >>> single configuration file. This is both simpler and more flexible. >>> >>> This is not a replacement for specific configuration files already >>> present in libcamera, with the exception of rpi configuration file (see >>> “Look up rpi configuration in the configuration file” patch). >>> >>> The patches implement what is needed to introduce a configuration file >>> that can handle the current environment variables and software ISP >>> TODOs. They demonstrate how to deal with the key points that must be >>> considered. Logging is omitted from the configuration file for >>> technical reasons. See commit messages for more details. >>> >>> The configuration file is a YAML file. It is looked up in user’s home >>> directory or, if not present, in system-wide libcamera directories. >>> Both the directories and file name of the configuration file can be >>> overridden using newly introduced environment variables. >>> >>> The current configuration environment variables are still supported and >>> take precedence, if defined, over the corresponding entries in the YAML >>> file. >>> >>> This patch series is not exhaustive, there can be future enhancements, >>> most notably configuration file validation to avoid confusions caused by >>> typos etc. >>> >>> Not everything has been tested because some of the patches are related >>> to specific hardware. >>> >>> Changes in v13: >>> - If LIBCAMERA_CONFIG_NAME is defined but empty, no configuration is >>> loaded. >>> - Standard user configuration directory has no longer precedence over >>> LIBCAMERA_CONFIG_DIR. >>> - Improvements to the rpi code suggested by Barnabás. >>> - Documentation fixes suggested by Barnabás. >>> - Make sure yamlConfiguration_ is initialized. >>> >>> Changes in v12: >>> - The commit message of "Make GlobalConfiguration instance" updated >>> according to the recent discussions. >>> - Use initializer_list rather than a string with separators for >>> GlobalConfiguration option path arguments. >>> - char *const qualifiers dropped. >>> - rpi can have separate configurations for different targets. >>> - GlobalConfiguration::option template is not type limited anymore. >>> - utils::join used instead of manual string concatenation. >>> - envListOption and listOption return std::optional now. >>> - Configuration corresponding to LIBCAMERA_*_TUNING_FILE dropped. >>> - Isolation configuration stored to IPAManager to avoid passing the >>> whole configuraton to isSignatureValid. >>> - Similarly to paths in IPAProxy. >>> - `delimiter' argument added to GlobalConfiguration::envListOption and >>> used for LIBCAMERA_PIPELINES_MATCH_LIST processing. >>> - Configuration no longer looked up in /etc/libcamera and additionally >>> looked up in LIBCAMERA_DATA_DIR. >>> - Cosmetic changes suggested by Barnabás. >>> >>> Changes in v11: >>> - global_configuration.cpp moved out of base and yaml_parser.cpp left in >>> its original place. >>> - Configuration initialized as empty if there is no configuration file, >>> preventing a segfault on later access. >>> - Cosmetic changes suggested by Laurent. >>> - GlobalConfiguration::get() removed, *yamlConfiguration_ used directly. >>> - Failing to read an existent configuration file is an error, rather >>> than warning, now. (It should be also fatal, but how to propagate it?) >>> - Slash is used instead of dot to separate configuration names. >>> - rpi configuration read is in the global configuration directly (unless >>> overridden by LIBCAMERA_RPI_CONFIG_FILE) now. >>> >>> Changes in v10: >>> - Minor improvements suggested by Barnabás. >>> >>> Changes in v9: >>> - The configuration instance is now stored in CameraManager and accessed >>> from there rather than being a separate singleton wrapped by global >>> accessors. This solves the ugly problem of delayed initialization but >>> I don’t like anything else about it. I played a bit with the idea of >>> attaching it to Logger instead, see the commit message of patch 03 for >>> some discussion, but stayed with the CameraManager proposal >>> eventually. >>> - I’ve given up on the logging configuration now when the configuration >>> is stored in CameraManager and removed the corresponding patches. I >>> think it’s possible to add it later, but for now, it’s already >>> complicated enough. >>> - Not much tested, let’s see first if the current implementation gets in >>> an acceptable direction. >>> >>> Changes in v8: >>> - Rebased on current master. >>> - Anniversary edition: 400 days since v1 posted. ;-) >>> >>> Changes in v7: >>> - Rebased on current master. >>> - Tuning file path configuration updated for recent changes. >>> A significant change is that the tuning file configuration is no >>> longer sensor dependent as there is apparently no access to the sensor >>> info in the IPA proxy. >>> - Minor improvements of some commit messages. >>> >>> Changes in v6: >>> - Rebased on master. >>> - File names from file header descriptions removed. >>> - Indentation fix in moved code as requested by checkstyle.py. >>> - Unneeded const_cast's removed. >>> - Using GlobalConfiguration namespace rather than a class. >>> - Path configuration options are defined as sequences in YAML. >>> - A patch introducing LIBCAMERA_CONFIG_NAME added. >>> - A patch introducing LIBCAMERA_CONFIG_DIR added. >>> - Miscellaneous minor code changes suggested by Barnabás. >>> >>> Changes in v5: >>> - A pointer is used to store the global configuration singleton rather >>> than a static variable. This makes the things more robust and fixes >>> the problem with re-entrancy on logging and a failing >>> camera_reconfigure test. >>> - In relation to the change above, a new initialization method >>> GlobalConfiguration::initialize() was introduced that replaces the >>> initialization calls in CameraManager and IPAManager. >>> - Logging YAML errors when reading the configuration was also fixed. >>> - Global configuration is placed to base directly, without an >>> intermediate patch. >>> - An `optional' value comparison simplified. >>> - A temporary typo in a comment fixed. >>> - Unused stdint.h include removed. >>> >>> Changes in v4: >>> - Rebased on current master. >>> - Configuration option for LIBCAMERA_IPA_PROXY_PATH added. >>> - Added a patch to include stdlib.h instead of cstdlib in yaml_parser.cpp. >>> >>> Changes in v3: >>> - Added a configuration item for the newly introduced >>> LIBCAMERA_PIPELINES_MATCH_LIST variable. >>> - A minor indentation fix. >>> - Fixed `pipelines.' x `pipeline.' configuration item naming mismatch. >>> - Tuning files are looked up now by particular cameras attached rather than >>> being specified for the whole pipeline. >>> - Helpers use std::string& instead of char* for confPath arguments. >>> - Protection against returning YamlObject::empty as a regular value (the >>> problem has been probably exposed by addition of >>> LIBCAMERA_PIPELINES_MATCH_LIST). >>> >>> Changes in v2: >>> - Rebased on master. >>> - Various cleanups, documentation improvements and minor fixes. >>> - Configuration option for LIBCAMERA_RPI_TUNING_FILE added (Naush). >>> - Two more patches for software ISP configuration added. >>> >>> Milan Zamazal (12): >>> config: Introduce global runtime configuration >>> config: Make GlobalConfiguration instance >>> config: Add configuration retrieval helpers >>> config: Look up rpi configuration in the configuration file >>> config: Look up IPA configurables in configuration file >>> config: Look up pipelines match list in configuration file >>> config: Allow enabling software ISP in runtime >>> config: Add global configuration file documentation >>> libcamera: software_isp: Make input buffer copying configurable >>> libcamera: software_isp: Make measurement configurable >>> config: Make configuration file configurable >>> config: Make configuration directories configurable >>> >>> Documentation/documentation-contents.rst | 2 +- >>> Documentation/index.rst | 2 +- >>> Documentation/meson.build | 2 +- >>> ...ariables.rst => runtime_configuration.rst} | 129 ++++++++- >>> include/libcamera/internal/camera_manager.h | 8 + >>> .../libcamera/internal/global_configuration.h | 62 +++++ >>> include/libcamera/internal/ipa_manager.h | 7 +- >>> include/libcamera/internal/ipa_proxy.h | 8 +- >>> include/libcamera/internal/meson.build | 1 + >>> src/libcamera/camera_manager.cpp | 20 +- >>> src/libcamera/global_configuration.cpp | 259 ++++++++++++++++++ >>> src/libcamera/ipa_manager.cpp | 39 ++- >>> src/libcamera/ipa_proxy.cpp | 51 ++-- >>> src/libcamera/meson.build | 1 + >>> .../pipeline/rpi/common/pipeline_base.cpp | 62 +++-- >>> .../pipeline/rpi/common/pipeline_base.h | 3 +- >>> src/libcamera/pipeline/rpi/pisp/pisp.cpp | 26 +- >>> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 26 +- >>> src/libcamera/pipeline/simple/simple.cpp | 13 + >>> src/libcamera/software_isp/TODO | 36 --- >>> src/libcamera/software_isp/debayer_cpu.cpp | 32 ++- >>> src/libcamera/software_isp/debayer_cpu.h | 10 +- >>> src/libcamera/software_isp/software_isp.cpp | 3 +- >>> .../module_ipa_proxy.cpp.tmpl | 4 +- >>> .../module_ipa_proxy.h.tmpl | 2 +- >>> 25 files changed, 633 insertions(+), 175 deletions(-) >>> rename Documentation/{environment_variables.rst => runtime_configuration.rst} (59%) >>> create mode 100644 include/libcamera/internal/global_configuration.h >>> create mode 100644 src/libcamera/global_configuration.cpp >>> >>> -- 2.50.1 >>>