[RFC,v2,00/22] libcamera: Add `MetadataList`

23868 mbox series

The main purpose of this patch series is to introduce a mechanism for pipeline handlers
by which they can report metadata items before a request completes. That part is based
on https://patchwork.libcamera.org/cover/22226/, which itself was created in response
to https://patchwork.libcamera.org/cover/22111/. Most of the former patch series is
integrated here.

In contrast, this patch series introduces a new type `MetadataList` that is used for
storing the metadata items, instead of a `ControlList`. This is needed to solve the
inherent synchronization issues caused by the fact that user callbacks (e.g. `requestComplete`)
are invoked synchronously. Specifically, the newly introduced `metadataAvailable` signal
should provide a way for users to defer the processing of the completed metadata items
to a separate thread (as blocking the internal libcamera thread is undesired). Using
a `ControlList` would not be ideal.

This patch series also adds `Documentation/design/metadata-list.rst`. I believe reading
that (first) provides useful information to better understand the reasons and design
of the implemented mechanism. I tried to put most of the information in the commit
messages or actual documentation as this cover letter will be lost to time.

### TODOs

There are some TODOs in the code at the moment, those still has to be discussed.

The old `ControlList` metadata list is not removed yet for testing,
but everything uses the new metadata list in `Request::metadata2()`.

The pipeline handlers guide is also not updated yet.


### user interface

The `MetadataListPlan` is now exposed in `Camera::metadata()` for applications.
(E.g. for pipewire to be able to convert libcamera metadata and send it alongside
the buffers.)


### rpi

No special handling is needed anymore, after https://patchwork.libcamera.org/cover/23474/
has been merged.


### debug metadata

The current approach is to automatically inject debug metadata controls into
a camera's metadata plan. Dynamically sized controls are added with a hard-coded
size of 32.


---

# v1 (https://patchwork.libcamera.org/cover/23481/):

### TODOs

There are some TODOs in the code at the moment, those still has to be discussed.

The old `ControlList` metadata list is not removed yet for testing,
but everything uses the new metadata list in `Request::metadata2()`.

The pipeline handlers guide is also not updated yet.


### user interface

An important question remaining to be answered is what exactly should be exposed to user applications
and how. For example, should `MetadataListPLan` be accessible? There are arguments to be made for that:
for example, if an application has to know beforehand which metadata items are reported in order to
allocate memory for them (e.g. pipewire). I see this as very similar to the `ControlInfoMap` of the camera.

Similarly, e.g. `MetadataList::iterator` has an inline `operator++`. I don't think going through a shared
library for an operator increment is ideal, but an argument could be made that doing so enables
the structure to be changed. I believe it is more likely than not that it will have to be changed
at least initially. Since libcamera does not promise any stability at the moment, I think this is
not a great concern at the moment.


### rpi

The rpi related changes conflict with https://patchwork.libcamera.org/cover/23474/
some things can be simplified after that is accepted.


### testing

I was only able to test the uvc, rkisp1, rpi/vc4, vimc, virtual pipeline handlers.
The rest compile and all, but I am not entirely sure everything is in the right
place (especially in the `simple` pipeline handler).


### debug metadata

One could argue, that this implementation is based on one of the simplest ideas considered,
but that naturally has some drawbacks. One such issues arises when considering "debug metadata".
The current implementation requires the set of all possibly reported metadata items to be known.
This means that using debug metadata will become a lot less convenient, as the new metadata has
to be added to a `MetadataListPlan` object. As opposed to the current situation where metadata
items can be just set, and `gen-debug-controls.py` will automagically generate them.
A couple ideas to address this situation:

  (1) do nothing
        * by far the simplest
        * inconvenient for debug metadata
  (2) decouple "debug metadata" from "normal metadata"
        * no early reporting (probably not too important)
        * can keep using `ControlList`
        * how to expose and to which (all?) applications?
        * needs some, possibly quite a bit of, modifications in the IPA interface
  (3) different `MetadataList` implementation
        * I have a prototype for another implementation
          - somewhat more complex
          - needs to allocate on demand
          - provides the essentially same interface and guarantees
          - can accommodate any number of metadata items just like a `ControlList`
            - no `MetadataListPlan` needed


### alternative implementation

There is a prototype for an alternative implementation that implements basically the
same interface, but is a bit more complicated. Comparison of the here proposed ("contiguous")
metadata list, and the more complex ("chunked") metadata list is as follows:

                                         cont.                               chunk.
==============================+===================================+==========================+
complexity                    | simpler                           | more complex             |
memory allocation             | single one when constructed       | on demand                |
contiguous<1>                 | yes                               | no                       |
backing data structure<2>     | sorted array                      | chaining hash table<3>   |
iterator step                 | constant time                     | constant time            |
wasted space                  | <4>                               | <5>                      |

<1>: The reason why this is highlighted is that initially I expected that if
     a serialized version of the `ControlList` is ever introduced, then it should
     use the same format as this metadata list. This would allow the currently completed
     items in the metadata list to be accessed through a hypothetical non-owning
     `ControlListView` type referring to the storage of the metadata list. Such a
     `ControlList` would most likely use a contiguous chunk of memory, so having
     contiguous storage helps compatibility. Nonetheless, I am becoming less sure
     that is is actually useful; and with sufficient complexity the non-contiguous
     version can also be supported.

<2>: for lookup, wrt. the numeric identifier of the control

<3>: fixed number buckets, making it resizable is not trivial

<4>: Space for all metadata items is allocated, so if there are metadata items with large space requirements
     (e.g. `rpi::PispStatsOutput` - 23200 bytes) that are seldom reported, then that space is essentially wasted.

<5>: (I think) bounded by (items / 2 * chunk_size), but with the right parameters
     all "usually" reported metadata can fit in one chunk.


Barnabás Pőcze (16):
  libcamera: controls: Strings are arrays
  libcamera: controls: Add `ControlValueView`
  libcamera: base: Add file for C++20 polyfills
  libcamera: base: cxx20: Add `type_identity{,_t}`
  libcamera: base: cxx20: Add `has_single_bit()`
  libcamera: base: Add alignment utility functions
  libcamera: Add `MetadataList`
  Documentation: design: Document `MetadataList`
  libcamera: ipa_data_serializer: Support `MetadataListPlan`
  libcamera: camera: Store `MetadataListPlan` in `Camera::Private`
  libcamera: request: Store `MetadataList`
  [DNI] apps: cam: Print `MetadataListPlan` of camera
  [DNI] apps: cam: Print `MetadataList` of `Request` as well
  libcamera: pipeline_handler: Inject "debug" metadata
  libcamera: pipeline: Fill `MetadataListPlan` of cameras
  libcamera: request: Swap the two metadata lists

Jacopo Mondi (6):
  libcamera: camera: Introduce metadataAvailable signal
  guides: application: Document Camera::metadataAvailable
  libcamera: pipeline_handler: Add metadataAvailable() function
  guides: pipeline_handler: Document PipelineHandler::metadataAvailable
  [DNI] apps: cam: Use Camera::metadataAvailable signal
  libcamera: pipeline: Use `metadataAvailable()`

 Documentation/design/metadata-list.rst        | 263 +++++++++
 .../guides/application-developer.rst          |   8 +
 Documentation/guides/pipeline-handler.rst     |  26 +-
 Documentation/index.rst                       |   1 +
 Documentation/meson.build                     |   1 +
 include/libcamera/base/details/align.h        |  78 +++
 include/libcamera/base/details/cxx20.h        |  25 +
 include/libcamera/base/meson.build            |   8 +
 include/libcamera/camera.h                    |   2 +
 include/libcamera/controls.h                  |  70 ++-
 include/libcamera/internal/camera.h           |   2 +
 include/libcamera/internal/controls.h         |  39 ++
 include/libcamera/internal/pipeline_handler.h |  49 ++
 .../internal/software_isp/software_isp.h      |   3 +-
 include/libcamera/ipa/core.mojom              |   1 +
 include/libcamera/ipa/ipu3.mojom              |   3 +-
 include/libcamera/ipa/mali-c55.mojom          |   3 +-
 include/libcamera/ipa/raspberrypi.mojom       |   1 +
 include/libcamera/ipa/rkisp1.mojom            |   3 +-
 include/libcamera/ipa/soft.mojom              |   3 +-
 include/libcamera/meson.build                 |   2 +
 include/libcamera/metadata_list.h             | 547 ++++++++++++++++++
 include/libcamera/metadata_list_plan.h        | 130 +++++
 include/libcamera/request.h                   |   9 +-
 src/android/camera_device.cpp                 |   2 +-
 src/apps/cam/camera_session.cpp               |  38 +-
 src/apps/cam/file_sink.cpp                    |   2 +-
 src/apps/cam/file_sink.h                      |   4 +-
 src/apps/common/dng_writer.cpp                |   2 +-
 src/apps/common/dng_writer.h                  |   4 +-
 src/apps/qcam/main_window.cpp                 |   2 +-
 src/apps/qcam/main_window.h                   |   3 +-
 src/gstreamer/gstlibcamera-controls.cpp.in    |   4 +-
 src/ipa/ipu3/algorithms/agc.cpp               |   4 +
 src/ipa/ipu3/algorithms/awb.cpp               |  12 +
 src/ipa/ipu3/algorithms/awb.h                 |   1 +
 src/ipa/ipu3/ipa_context.cpp                  |   3 +
 src/ipa/ipu3/ipa_context.h                    |   3 +
 src/ipa/ipu3/ipu3.cpp                         |   8 +-
 src/ipa/mali-c55/algorithms/agc.cpp           |   5 +
 src/ipa/mali-c55/algorithms/awb.cpp           |   7 +
 src/ipa/mali-c55/algorithms/awb.h             |   1 +
 src/ipa/mali-c55/algorithms/blc.cpp           |   2 +
 src/ipa/mali-c55/ipa_context.h                |   3 +
 src/ipa/mali-c55/mali-c55.cpp                 |   6 +-
 src/ipa/rkisp1/algorithms/agc.cpp             |  10 +
 src/ipa/rkisp1/algorithms/awb.cpp             |   4 +
 src/ipa/rkisp1/algorithms/blc.cpp             |   2 +
 src/ipa/rkisp1/algorithms/ccm.cpp             |   2 +
 src/ipa/rkisp1/algorithms/goc.cpp             |   2 +
 src/ipa/rkisp1/algorithms/lux.cpp             |   8 +-
 src/ipa/rkisp1/ipa_context.h                  |   1 +
 src/ipa/rkisp1/rkisp1.cpp                     |   8 +-
 src/ipa/rpi/common/ipa_base.cpp               |  34 ++
 src/ipa/rpi/pisp/pisp.cpp                     |   5 +-
 src/ipa/rpi/vc4/vc4.cpp                       |   4 +-
 src/ipa/simple/algorithms/agc.cpp             |   8 +
 src/ipa/simple/algorithms/agc.h               |   1 +
 src/ipa/simple/algorithms/awb.cpp             |   8 +
 src/ipa/simple/algorithms/awb.h               |   1 +
 src/ipa/simple/algorithms/blc.cpp             |   3 +
 src/ipa/simple/algorithms/ccm.cpp             |   3 +
 src/ipa/simple/algorithms/lut.cpp             |   3 +
 src/ipa/simple/ipa_context.h                  |   2 +
 src/ipa/simple/soft_simple.cpp                |   8 +-
 src/libcamera/camera.cpp                      |  75 +++
 src/libcamera/controls.cpp                    | 275 ++++++---
 src/libcamera/ipa_data_serializer.cpp         |  91 +++
 src/libcamera/meson.build                     |   1 +
 src/libcamera/metadata_list.cpp               | 344 +++++++++++
 src/libcamera/pipeline/imx8-isi/imx8-isi.cpp  |   7 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  21 +-
 src/libcamera/pipeline/mali-c55/mali-c55.cpp  |   4 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  11 +-
 .../pipeline/rpi/common/pipeline_base.cpp     |  37 +-
 src/libcamera/pipeline/simple/simple.cpp      |   6 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |   5 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |   5 +-
 .../pipeline/virtual/config_parser.cpp        |   3 +
 src/libcamera/pipeline/virtual/virtual.cpp    |   2 +-
 src/libcamera/pipeline_handler.cpp            | 129 +++++
 src/libcamera/request.cpp                     |   8 +-
 src/libcamera/software_isp/software_isp.cpp   |   6 +-
 src/py/libcamera/py_helpers.cpp               |   4 +-
 src/py/libcamera/py_helpers.h                 |   2 +-
 src/py/libcamera/py_main.cpp                  |   2 +-
 test/controls/meson.build                     |   1 +
 test/controls/metadata_list.cpp               | 170 ++++++
 .../core_ipa_interface.h.tmpl                 |   1 +
 89 files changed, 2549 insertions(+), 171 deletions(-)
 create mode 100644 Documentation/design/metadata-list.rst
 create mode 100644 include/libcamera/base/details/align.h
 create mode 100644 include/libcamera/base/details/cxx20.h
 create mode 100644 include/libcamera/internal/controls.h
 create mode 100644 include/libcamera/metadata_list.h
 create mode 100644 include/libcamera/metadata_list_plan.h
 create mode 100644 src/libcamera/metadata_list.cpp
 create mode 100644 test/controls/metadata_list.cpp

--
2.50.1