From patchwork Mon Jul 21 10:46:00 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 23868 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 4FC5EC3237 for ; Mon, 21 Jul 2025 10:46:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EC8A668FDC; Mon, 21 Jul 2025 12:46:27 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="G4zYffLE"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5FF5568FB1 for ; Mon, 21 Jul 2025 12:46:26 +0200 (CEST) Received: from pb-laptop.local (185.221.140.39.nat.pool.zt.hu [185.221.140.39]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 785E17939 for ; Mon, 21 Jul 2025 12:45:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1753094749; bh=PW2P7jRbz1n7FSAi+jeLySDUQMvhBA32NVNnIMfkDuU=; h=From:To:Subject:Date:From; b=G4zYffLEN79Yk2YjB2JWWGJSF6JS6sj0SBMNeqMLY61C2a6+QWKCBEkbcuyr40S/k JEOeOtfX5NkJWrKuvHwty3EVr5By3gUWyaLfaP8N1Bs0OfRpguH4JbM5danZCMtP3s Cez5/Qm6cnVQk0eGEKv0xe06LG6DsaF8VAIb0Rks= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v2 00/22] libcamera: Add `MetadataList` Date: Mon, 21 Jul 2025 12:46:00 +0200 Message-ID: <20250721104622.1550908-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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