From patchwork Fri Jun 6 16:41:33 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: 23481 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 1E11BC31E9 for ; Fri, 6 Jun 2025 16:42:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CD12A68DC3; Fri, 6 Jun 2025 18:42:16 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LnT3ZeLv"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3244B68DB0 for ; Fri, 6 Jun 2025 18:42:15 +0200 (CEST) Received: from pb-laptop.local (185.182.215.79.nat.pool.zt.hu [185.182.215.79]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 995376DC for ; Fri, 6 Jun 2025 18:42:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1749228130; bh=W/QxqwnwZ0RP8W+o7Yp6e8iKeMUYbDa3ZSE562Nr7uU=; h=From:To:Subject:Date:From; b=LnT3ZeLvMONeiaTavMPYzt9CvU/XXr3D1SRVHtIMDxXjLXOJw0pJ79YyeU+/heS8h XPmlXVgEk5w6n20mFS2o02cdfrsfhH+RSYJw3Ps1PTwFSg7f4q6Zm1d9ExFl258NeO Kq7nUkjGScc3edXdZ6EGRsJ3ViAMu04j85Yz5/jQ= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v1 00/23] libcamera: Add `MetadataList` Date: Fri, 6 Jun 2025 18:41:33 +0200 Message-ID: <20250606164156.1442682-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 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 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 (17): 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 `MetadataList` of `Request` as well libcamera: pipeline: Fill `MetadataListPlan` of cameras libcamera: pipeline: rpi: Queue metadata until completion libcamera: pipeline: rpi: Use `metadataAvailable()` py: Use `Request::metadata2()` treewide: Use `Request::metadata2()` 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 | 234 +++++++ .../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 | 72 ++ include/libcamera/base/details/cxx20.h | 25 + include/libcamera/base/meson.build | 1 + include/libcamera/camera.h | 1 + include/libcamera/controls.h | 71 +- include/libcamera/internal/camera.h | 3 + include/libcamera/internal/pipeline_handler.h | 21 + .../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 | 2 +- include/libcamera/ipa/soft.mojom | 3 +- include/libcamera/meson.build | 2 + include/libcamera/metadata_list.h | 619 ++++++++++++++++++ include/libcamera/metadata_list_plan.h | 109 +++ include/libcamera/request.h | 5 + src/android/camera_device.cpp | 4 +- src/apps/cam/camera_session.cpp | 19 + src/apps/cam/file_sink.cpp | 4 +- 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 | 4 +- src/apps/qcam/main_window.h | 4 +- src/gstreamer/gstlibcamera-controls.cpp.in | 4 +- src/gstreamer/gstlibcamerasrc.cpp | 2 +- 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/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 | 62 ++ src/libcamera/controls.cpp | 194 ++++++ src/libcamera/ipa_data_serializer.cpp | 84 +++ src/libcamera/meson.build | 1 + src/libcamera/metadata_list.cpp | 315 +++++++++ src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 7 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +- .../pipeline/rpi/common/pipeline_base.cpp | 36 +- .../pipeline/rpi/common/pipeline_base.h | 4 +- src/libcamera/pipeline/rpi/pisp/pisp.cpp | 8 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 8 +- src/libcamera/pipeline/simple/simple.cpp | 7 +- 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 | 61 ++ src/libcamera/request.cpp | 2 + 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 | 9 +- test/controls/meson.build | 1 + test/controls/metadata_list.cpp | 171 +++++ .../core_ipa_interface.h.tmpl | 1 + 89 files changed, 2361 insertions(+), 101 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/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.49.0