[v2,00/42] libcamera: Global configuration file improvements

26440 mbox series

Hello,

This series started small with a few improvements to the global
configuration file handling, fixing some issues I pointed out during
reviews. It then grew over time, and while I have ideas for more
improvements, adding more patches will only slow down review.

The series starts with a few drive-by cleanups in patches 01/42 to
07/42. Those could be merged separately, but some of them would then end
up conflicting with the rest of the series, so I've included them here.

The rework of the configuration file starts in patch 08/42 with
improvements to the YamlParser and YamlObject classes, until patch 19/42
that renames YamlObject to ValueNode to make the class usable outside of
YAML parsing use cases. Patches 20/42 to 26/42 continues with extensions
to the ValueNode class with new functions that are used by subsequent
patches. The next four patches finishes the ValueNode rework with unit
tests for the new class in 27/42, and simplification of the YamlParser
unit tests in 28/42 to 30/42.

After that, the focus shifs to the global configuration. Patches 31/42
to 36/42 rework the GlobalConfiguration class, with the most notable
change in 31/42 that passes a CameraManager around to access the
GlobalConfiguration. Patches 37/42 and 38/42 continue with centralizing
configuration override through environment variables. This simplifies
usage of the GlobalConfiguration class through libcamera, at the cost of
splitting information about environment variables from the place where
the corresponding configuration setting is used. I believe this is an
acceptable drawback, as now that we have a configuration file, new
environment variables should be introduced very seldom. An alternative
that I may explore at some point is an API that would allow components
within libcamera to declare the options they support, in which case
environment variables would be declared using the same API. This will be
a bigger rework that I don't plan to explore for the time being.

Patch 39/42 is a small documentation improvement, before the last three
patches that break backward compatibility. They are conceptually
independent from each other, 42/42 currently depends on 41/42, but it
could be rebased if 41/42 is deemed undesirable.

On top of this series I would like to extend the CameraManager class to
support overridding options, allowing applications to configure
libcamera programmatically. This could take the form of a ValueNode.
Another possible improvement would be to restructure the configuration
file to provide per-camera configuration in addition to per-pipeline
handler options. If anyone is interested in giving this a try, let's
discuss it.


Laurent Pinchart (42):
  libcamera: Drop unneeded usage of this pointer
  libcamera: request: Move all private member variables to Private class
  libcamera: Replace plain pointers with std::unique<>
  libcamera: pipeline_handler: Add createIPA() function
  test: ipa: ipa_interface: Use IPAManager::createIPA()
  test: ipa: ipa_interface: Replace FIFO with pipe
  libcamera: camera_manager: Move IPAManager creation to start() time
  libcamera: yaml_parser: Use std::make_unique<>
  libcamera: yaml_parser: Rename Container to ValueContainer
  libcamera: yaml_parser: Rename Getter to Accessor
  libcamera: yaml_parser: Replace getList() with get() specializations
  libcamera: yaml_parser: Add function to set a YamlObject value
  libcamera: yaml_parser: Add functions to add children
  libcamera: yaml_parser: Un-friend YamlParserContext from YamlObject
  libcamera: yaml_parser: Move Size handling to geometry.cpp
  libcamera: yaml_parser: Drop unneeded \fn Doxygen commands
  libcamera: yaml_parser: Split YamlObject from YamlParser
  libcamera: yaml_object: Miscellaneous documentation improvements
  libcamera: Rename YamlObject to ValueNode
  libcamera: value_node: Add constructor with value
  libcamera: value_node: Rework templates to prepare for mutable views
  libcamera: value_node: Add mutable adapters
  libcamera: value_node: Add mutable children accessors
  libcamera: value_node: Support adding nested children in one operation
  libcamera: value_node: Support looking up descendant node by path
  libcamera: value_node: Add functions to erase child nodes
  test: Add ValueNode unit test
  test: yaml-parser: Simplify test
  test: yaml-parser: Standardize on explicitly qualifying std namespace
  test: yaml-parser: Replace "object" with "node"
  libcamera: Pass CameraManager around instead of GlobalConfiguration
  libcamera: global_configuration: Reorder functions
  libcamera: global_configuration: Add missing include and comment
  libcamera: global_configuration: Rename yamlConfiguration_
  libcamera: global_configuration: Rename Configuration to Option
  libcamera: global_configuration: Drop Option type
  libcamera: global_configuration: Populate empty configuration
  libcamera: global_configuration: Override options with environment
    variables
  Documentation: Rename runtime configuration title
  libcamera: software_isp: Rename "measure" option to "benchmark"
  pipeline: simple: Rename supported_devices configuration option to
    devices
  pipeline: simple: Turn devices configuration option into dictionary

 .../guides/application-developer.rst          |   5 +-
 Documentation/guides/ipa.rst                  |   8 +-
 Documentation/runtime_configuration.rst       |  24 +-
 .../internal/converter/converter_dw100.h      |   2 +-
 .../internal/device_enumerator_udev.h         |   2 +-
 .../libcamera/internal/global_configuration.h |  22 +-
 include/libcamera/internal/ipa_manager.h      |  29 +-
 include/libcamera/internal/ipa_proxy.h        |   4 +-
 include/libcamera/internal/ipc_unixsocket.h   |   3 +-
 include/libcamera/internal/matrix.h           |  10 +-
 include/libcamera/internal/meson.build        |   1 +
 include/libcamera/internal/pipeline_handler.h |  11 +-
 include/libcamera/internal/request.h          |  12 +-
 .../libcamera/internal/shared_mem_object.h    |   4 +-
 .../internal/software_isp/benchmark.h         |   6 +-
 .../internal/software_isp/swstats_cpu.h       |   6 +-
 include/libcamera/internal/v4l2_device.h      |   2 +-
 include/libcamera/internal/v4l2_videodevice.h |  12 +-
 include/libcamera/internal/value_node.h       | 276 +++++++
 include/libcamera/internal/vector.h           |  10 +-
 include/libcamera/internal/yaml_parser.h      | 227 +-----
 include/libcamera/ipa/vimc.mojom              |   3 +-
 include/libcamera/request.h                   |  15 +-
 src/android/camera_hal_config.cpp             |  16 +-
 src/apps/qcam/main.cpp                        |   5 +-
 src/apps/qcam/main_window.cpp                 |  11 +-
 src/apps/qcam/main_window.h                   |   2 +-
 src/ipa/ipu3/algorithms/agc.cpp               |   4 +-
 src/ipa/ipu3/algorithms/agc.h                 |   2 +-
 src/ipa/ipu3/ipu3.cpp                         |   2 +-
 src/ipa/libipa/agc_mean_luminance.cpp         |  20 +-
 src/ipa/libipa/agc_mean_luminance.h           |  12 +-
 src/ipa/libipa/algorithm.cpp                  |   2 +-
 src/ipa/libipa/algorithm.h                    |   4 +-
 src/ipa/libipa/awb.cpp                        |   6 +-
 src/ipa/libipa/awb.h                          |   6 +-
 src/ipa/libipa/awb_bayes.cpp                  |   8 +-
 src/ipa/libipa/awb_bayes.h                    |   6 +-
 src/ipa/libipa/awb_grey.cpp                   |   2 +-
 src/ipa/libipa/awb_grey.h                     |   2 +-
 src/ipa/libipa/interpolator.cpp               |   2 +-
 src/ipa/libipa/interpolator.h                 |   4 +-
 src/ipa/libipa/lsc_polynomial.h               |   8 +-
 src/ipa/libipa/lux.cpp                        |   6 +-
 src/ipa/libipa/lux.h                          |   4 +-
 src/ipa/libipa/module.cpp                     |   2 +-
 src/ipa/libipa/module.h                       |   6 +-
 src/ipa/libipa/pwl.cpp                        |   2 +-
 src/ipa/mali-c55/algorithms/agc.cpp           |   2 +-
 src/ipa/mali-c55/algorithms/agc.h             |   2 +-
 src/ipa/mali-c55/algorithms/blc.cpp           |   4 +-
 src/ipa/mali-c55/algorithms/blc.h             |   2 +-
 src/ipa/mali-c55/algorithms/lsc.cpp           |  12 +-
 src/ipa/mali-c55/algorithms/lsc.h             |   2 +-
 src/ipa/mali-c55/mali-c55.cpp                 |   2 +-
 src/ipa/rkisp1/algorithms/agc.cpp             |  12 +-
 src/ipa/rkisp1/algorithms/agc.h               |   4 +-
 src/ipa/rkisp1/algorithms/awb.cpp             |   2 +-
 src/ipa/rkisp1/algorithms/awb.h               |   2 +-
 src/ipa/rkisp1/algorithms/blc.cpp             |   4 +-
 src/ipa/rkisp1/algorithms/blc.h               |   2 +-
 src/ipa/rkisp1/algorithms/ccm.cpp             |   4 +-
 src/ipa/rkisp1/algorithms/ccm.h               |   4 +-
 src/ipa/rkisp1/algorithms/cproc.cpp           |   2 +-
 src/ipa/rkisp1/algorithms/cproc.h             |   2 +-
 src/ipa/rkisp1/algorithms/dpcc.cpp            |  22 +-
 src/ipa/rkisp1/algorithms/dpcc.h              |   2 +-
 src/ipa/rkisp1/algorithms/dpf.cpp             |  14 +-
 src/ipa/rkisp1/algorithms/dpf.h               |   2 +-
 src/ipa/rkisp1/algorithms/filter.cpp          |   2 +-
 src/ipa/rkisp1/algorithms/filter.h            |   2 +-
 src/ipa/rkisp1/algorithms/goc.cpp             |   4 +-
 src/ipa/rkisp1/algorithms/goc.h               |   2 +-
 src/ipa/rkisp1/algorithms/gsl.cpp             |  14 +-
 src/ipa/rkisp1/algorithms/gsl.h               |   2 +-
 src/ipa/rkisp1/algorithms/lsc.cpp             |  24 +-
 src/ipa/rkisp1/algorithms/lsc.h               |   2 +-
 src/ipa/rkisp1/algorithms/lux.cpp             |   2 +-
 src/ipa/rkisp1/algorithms/lux.h               |   2 +-
 src/ipa/rkisp1/algorithms/wdr.cpp             |   4 +-
 src/ipa/rkisp1/algorithms/wdr.h               |   2 +-
 src/ipa/rkisp1/rkisp1.cpp                     |   2 +-
 src/ipa/rpi/controller/algorithm.cpp          |   2 +-
 src/ipa/rpi/controller/algorithm.h            |   4 +-
 src/ipa/rpi/controller/controller.cpp         |   4 +-
 src/ipa/rpi/controller/controller.h           |   4 +-
 src/ipa/rpi/controller/rpi/af.cpp             |  10 +-
 src/ipa/rpi/controller/rpi/af.h               |   8 +-
 src/ipa/rpi/controller/rpi/agc.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/agc.h              |   2 +-
 src/ipa/rpi/controller/rpi/agc_channel.cpp    |  28 +-
 src/ipa/rpi/controller/rpi/agc_channel.h      |  12 +-
 src/ipa/rpi/controller/rpi/alsc.cpp           |  10 +-
 src/ipa/rpi/controller/rpi/alsc.h             |   2 +-
 src/ipa/rpi/controller/rpi/awb.cpp            |   6 +-
 src/ipa/rpi/controller/rpi/awb.h              |   4 +-
 src/ipa/rpi/controller/rpi/awb_bayes.cpp      |  12 +-
 src/ipa/rpi/controller/rpi/black_level.cpp    |   2 +-
 src/ipa/rpi/controller/rpi/black_level.h      |   2 +-
 src/ipa/rpi/controller/rpi/cac.cpp            |   4 +-
 src/ipa/rpi/controller/rpi/cac.h              |   2 +-
 src/ipa/rpi/controller/rpi/ccm.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/ccm.h              |   2 +-
 src/ipa/rpi/controller/rpi/contrast.cpp       |   2 +-
 src/ipa/rpi/controller/rpi/contrast.h         |   2 +-
 src/ipa/rpi/controller/rpi/decompand.cpp      |   2 +-
 src/ipa/rpi/controller/rpi/decompand.h        |   2 +-
 src/ipa/rpi/controller/rpi/denoise.cpp        |   4 +-
 src/ipa/rpi/controller/rpi/denoise.h          |   4 +-
 src/ipa/rpi/controller/rpi/dpc.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/dpc.h              |   2 +-
 src/ipa/rpi/controller/rpi/geq.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/geq.h              |   2 +-
 src/ipa/rpi/controller/rpi/hdr.cpp            |  12 +-
 src/ipa/rpi/controller/rpi/hdr.h              |   4 +-
 src/ipa/rpi/controller/rpi/lux.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/lux.h              |   2 +-
 src/ipa/rpi/controller/rpi/noise.cpp          |   2 +-
 src/ipa/rpi/controller/rpi/noise.h            |   2 +-
 src/ipa/rpi/controller/rpi/saturation.cpp     |   2 +-
 src/ipa/rpi/controller/rpi/saturation.h       |   2 +-
 src/ipa/rpi/controller/rpi/sdn.cpp            |   2 +-
 src/ipa/rpi/controller/rpi/sdn.h              |   2 +-
 src/ipa/rpi/controller/rpi/sharpen.cpp        |   2 +-
 src/ipa/rpi/controller/rpi/sharpen.h          |   2 +-
 src/ipa/rpi/controller/rpi/tonemap.cpp        |   2 +-
 src/ipa/rpi/controller/rpi/tonemap.h          |   2 +-
 src/ipa/simple/algorithms/adjust.cpp          |   2 +-
 src/ipa/simple/algorithms/adjust.h            |   2 +-
 src/ipa/simple/algorithms/blc.cpp             |   2 +-
 src/ipa/simple/algorithms/blc.h               |   2 +-
 src/ipa/simple/algorithms/ccm.cpp             |   2 +-
 src/ipa/simple/algorithms/ccm.h               |   2 +-
 src/ipa/simple/soft_simple.cpp                |   2 +-
 src/ipa/vimc/vimc.cpp                         |  33 +-
 src/libcamera/camera_manager.cpp              |   8 +-
 src/libcamera/converter/converter_dw100.cpp   |   4 +-
 src/libcamera/device_enumerator_udev.cpp      |   6 +-
 src/libcamera/geometry.cpp                    |  29 +
 src/libcamera/global_configuration.cpp        | 319 +++++----
 src/libcamera/ipa_manager.cpp                 |  15 +-
 src/libcamera/ipa_proxy.cpp                   |  14 +-
 src/libcamera/ipc_unixsocket.cpp              |   7 +-
 src/libcamera/matrix.cpp                      |   2 +-
 src/libcamera/meson.build                     |   1 +
 src/libcamera/pipeline/ipu3/ipu3.cpp          |   3 +-
 src/libcamera/pipeline/mali-c55/mali-c55.cpp  |   3 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   5 +-
 .../pipeline/rpi/common/pipeline_base.cpp     |   8 +-
 .../pipeline/rpi/common/pipeline_base.h       |   4 +-
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      |   6 +-
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   6 +-
 src/libcamera/pipeline/simple/simple.cpp      |  21 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |   5 +-
 src/libcamera/pipeline/virtual/README.md      |   2 +-
 .../pipeline/virtual/config_parser.cpp        |  20 +-
 .../pipeline/virtual/config_parser.h          |  12 +-
 src/libcamera/pipeline/virtual/virtual.cpp    |   2 +-
 src/libcamera/pipeline_handler.cpp            |  10 +
 src/libcamera/request.cpp                     | 104 +--
 src/libcamera/shared_mem_object.cpp           |   8 +-
 src/libcamera/software_isp/benchmark.cpp      |  17 +-
 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   |  16 +-
 src/libcamera/software_isp/swstats_cpu.cpp    |   8 +-
 src/libcamera/v4l2_device.cpp                 |   5 +-
 src/libcamera/v4l2_videodevice.cpp            |  22 +-
 src/libcamera/value_node.cpp                  | 675 ++++++++++++++++++
 src/libcamera/vector.cpp                      |   2 +-
 src/libcamera/yaml_parser.cpp                 | 482 ++-----------
 src/v4l2/v4l2_camera.cpp                      |   7 +-
 src/v4l2/v4l2_camera.h                        |   2 +-
 src/v4l2/v4l2_compat_manager.cpp              |  18 +-
 src/v4l2/v4l2_compat_manager.h                |   2 +-
 test/ipa/ipa_interface_test.cpp               |  57 +-
 test/meson.build                              |   1 +
 test/value-node.cpp                           | 565 +++++++++++++++
 test/yaml-parser.cpp                          | 522 ++------------
 .../module_ipa_proxy.cpp.tmpl                 |   8 +-
 .../module_ipa_proxy.h.tmpl                   |   4 +-
 185 files changed, 2465 insertions(+), 1853 deletions(-)
 create mode 100644 include/libcamera/internal/value_node.h
 create mode 100644 src/libcamera/value_node.cpp
 create mode 100644 test/value-node.cpp


base-commit: 4b6c47bd6675c428c19ea76370f0301c24f23bf1