[PATCH v4 00/15] Add global configuration file

[Milan Zamazal]

Milan Zamazal <mzamazal at redhat.com> writes:

> Kieran Bingham <kieran.bingham at ideasonboard.com> writes:
>
>> Quoting Milan Zamazal (2024-09-25 10:58:32)
>>> 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.
>>> 
>>> 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.  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.
>>> Environment variables, if set, still take precedence.
>>> 
>>> 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.
>>
>> It looks like this breaks on the unit tests:
>>
>>  - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/64126236
>>
>> And I re-ran incase it was spurious, but that looks like it's going to
>> fail too:
>>
>>  - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/64131685
>>
>> Could you check/take a look please?
>
> I can reproduce it locally.  It happens in the patch #4 "Look up logging
> levels in the configuration file" where the configuration starts being
> used.  And apparently only if the already loaded configuration is
> actually queried.  The errors don't make sense to me, indicating a
> possible memory or thread problem.  I'll continue investigating it
> later.

I haven't identified the exact cause but I reworked storing the
configuration and its initialization to be a bit more robust, which also
fixed the test.

>> --
>> Kieran
>>
>>
>>> 
>>> 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 (15):
>>>   config: Introduce global runtime configuration
>>>   yaml: Include stdlib.h instead of cstdlib
>>>   config: Move global configuration to base
>>>   config: Look up logging levels in the configuration file
>>>   config: Add configuration retrieval helpers
>>>   config: Look up log file in configuration file
>>>   config: Look up log color configuration in configuration file
>>>   config: Look up rpi paths in configuration file
>>>   config: Look up IPA configurables in configuration file
>>>   config: Look up RkISP1 tuning file 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
>>> 
>>>  Documentation/documentation-contents.rst      |   2 +-
>>>  Documentation/index.rst                       |   4 +-
>>>  Documentation/meson.build                     |   2 +-
>>>  ...ariables.rst => runtime_configuration.rst} | 119 ++++++++--
>>>  .../libcamera/internal/global_configuration.h |  67 ++++++
>>>  include/libcamera/internal/meson.build        |   1 +
>>>  src/libcamera/base/global_configuration.cpp   | 219 ++++++++++++++++++
>>>  src/libcamera/base/log.cpp                    |  24 +-
>>>  src/libcamera/base/meson.build                |  15 ++
>>>  src/libcamera/{ => base}/yaml_parser.cpp      |   2 +-
>>>  src/libcamera/camera_manager.cpp              |  19 +-
>>>  src/libcamera/ipa_manager.cpp                 |  23 +-
>>>  src/libcamera/ipa_proxy.cpp                   |  16 +-
>>>  src/libcamera/meson.build                     |  14 --
>>>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  20 +-
>>>  .../pipeline/rpi/common/pipeline_base.cpp     |  18 +-
>>>  src/libcamera/pipeline/simple/simple.cpp      |  12 +
>>>  src/libcamera/process.cpp                     |  11 +-
>>>  src/libcamera/software_isp/TODO               |  36 ---
>>>  src/libcamera/software_isp/debayer_cpu.cpp    |  30 ++-
>>>  src/libcamera/software_isp/debayer_cpu.h      |   7 +-
>>>  21 files changed, 549 insertions(+), 112 deletions(-)
>>>  rename Documentation/{environment_variables.rst => runtime_configuration.rst} (61%)
>>>  create mode 100644 include/libcamera/internal/global_configuration.h
>>>  create mode 100644 src/libcamera/base/global_configuration.cpp
>>>  rename src/libcamera/{ => base}/yaml_parser.cpp (99%)
>>> 
>>> -- 
>>> 2.44.1
>>>