Cover Letter Detail
Show a cover letter.
GET /api/covers/24903/?format=api
{ "id": 24903, "url": "https://patchwork.libcamera.org/api/covers/24903/?format=api", "web_url": "https://patchwork.libcamera.org/cover/24903/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20251030165816.1095180-1-barnabas.pocze@ideasonboard.com>", "date": "2025-10-30T16:57:54", "name": "[RFC,v3,00/22] libcamera: Add `MetadataList`", "submitter": { "id": 216, "url": "https://patchwork.libcamera.org/api/people/216/?format=api", "name": "Barnabás Pőcze", "email": "barnabas.pocze@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/cover/24903/mbox/", "series": [ { "id": 5545, "url": "https://patchwork.libcamera.org/api/series/5545/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5545", "date": "2025-10-30T16:57:54", "name": "libcamera: Add `MetadataList`", "version": 3, "mbox": "https://patchwork.libcamera.org/series/5545/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/covers/24903/comments/", "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 14F85C3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Oct 2025 16:58:23 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1128A608DF;\n\tThu, 30 Oct 2025 17:58:22 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 82B16603ED\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Oct 2025 17:58:20 +0100 (CET)", "from pb-laptop.local (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5528A6F3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Oct 2025 17:56:30 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DSczqx/6\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761843390;\n\tbh=4IjRo9YqWJ9aoHWHOx81J9aOhKEa6rBuIYMBUzzk12g=;\n\th=From:To:Subject:Date:From;\n\tb=DSczqx/6Lsliisk09a4WHvZBF3shOz5AMKuiHkKuFGlIr2SVvVeiktJYM1Davv8BW\n\t5LUZGNR71j0+ajABHyPkdn5dAGT4AXXcJOe6z4XXTd0716LmkO7yHi1Lhrl2gEAb8n\n\t/oD80F36OQLMJoT3a95j1x3qnjqfr1+6QqmkvQIw=", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[RFC PATCH v3 00/22] libcamera: Add `MetadataList`", "Date": "Thu, 30 Oct 2025 17:57:54 +0100", "Message-ID": "<20251030165816.1095180-1-barnabas.pocze@ideasonboard.com>", "X-Mailer": "git-send-email 2.51.1", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The main purpose of this patch series is to introduce a mechanism for pipeline handlers\nby which they can report metadata items before a request completes. That part is based\non https://patchwork.libcamera.org/cover/22226/, which itself was created in response\nto https://patchwork.libcamera.org/cover/22111/. Most of the former patch series is\nintegrated here.\n\nIn contrast, this patch series introduces a new type `MetadataList` that is used for\nstoring the metadata items, instead of a `ControlList`. This is needed to solve the\ninherent synchronization issues caused by the fact that user callbacks (e.g. `requestComplete`)\nare invoked synchronously. Specifically, the newly introduced `metadataAvailable` signal\nshould provide a way for users to defer the processing of the completed metadata items\nto a separate thread (as blocking the internal libcamera thread is undesired). Using\na `ControlList` would not be ideal.\n\nThis patch series also adds `Documentation/design/metadata-list.rst`. I believe reading\nthat (first) provides useful information to better understand the reasons and design\nof the implemented mechanism. I tried to put most of the information in the commit\nmessages or actual documentation as this cover letter will be lost to time.\n\n\n### TODOs\n\nThere are some TODOs in the code at the moment, those still has to be discussed.\n\nThe old `ControlList` metadata list is not removed yet for testing,\nbut everything uses the new metadata list in `Request::metadata2()`.\n\nThe pipeline handlers guide is also not updated yet.\n\n\n### MetadataList::merge()\n\nThe function has been modified to be transactional, i.e. either commit every\nitem or none. And PipelineHandler::metadataAvailable(const ControlList&)\nhas been changed to log a fatal error if mergin fails.\n\n\n---\n\n# v2 (https://patchwork.libcamera.org/cover/23868/):\n\n\n### TODOs\n\nThere are some TODOs in the code at the moment, those still has to be discussed.\n\nThe old `ControlList` metadata list is not removed yet for testing,\nbut everything uses the new metadata list in `Request::metadata2()`.\n\nThe pipeline handlers guide is also not updated yet.\n\n\n### user interface\n\nThe `MetadataListPlan` is now exposed in `Camera::metadata()` for applications.\n(E.g. for pipewire to be able to convert libcamera metadata and send it alongside\nthe buffers.)\n\n\n### rpi\n\nNo special handling is needed anymore, after https://patchwork.libcamera.org/cover/23474/\nhas been merged.\n\n\n### debug metadata\n\nThe current approach is to automatically inject debug metadata controls into\na camera's metadata plan. Dynamically sized controls are added with a hard-coded\nsize of 32.\n\n\n---\n\n# v1 (https://patchwork.libcamera.org/cover/23481/):\n\n### TODOs\n\nThere are some TODOs in the code at the moment, those still has to be discussed.\n\nThe old `ControlList` metadata list is not removed yet for testing,\nbut everything uses the new metadata list in `Request::metadata2()`.\n\nThe pipeline handlers guide is also not updated yet.\n\n\n### user interface\n\nAn important question remaining to be answered is what exactly should be exposed to user applications\nand how. For example, should `MetadataListPLan` be accessible? There are arguments to be made for that:\nfor example, if an application has to know beforehand which metadata items are reported in order to\nallocate memory for them (e.g. pipewire). I see this as very similar to the `ControlInfoMap` of the camera.\n\nSimilarly, e.g. `MetadataList::iterator` has an inline `operator++`. I don't think going through a shared\nlibrary for an operator increment is ideal, but an argument could be made that doing so enables\nthe structure to be changed. I believe it is more likely than not that it will have to be changed\nat least initially. Since libcamera does not promise any stability at the moment, I think this is\nnot a great concern at the moment.\n\n\n### rpi\n\nThe rpi related changes conflict with https://patchwork.libcamera.org/cover/23474/\nsome things can be simplified after that is accepted.\n\n\n### testing\n\nI was only able to test the uvc, rkisp1, rpi/vc4, vimc, virtual pipeline handlers.\nThe rest compile and all, but I am not entirely sure everything is in the right\nplace (especially in the `simple` pipeline handler).\n\n\n### debug metadata\n\nOne could argue, that this implementation is based on one of the simplest ideas considered,\nbut that naturally has some drawbacks. One such issues arises when considering \"debug metadata\".\nThe current implementation requires the set of all possibly reported metadata items to be known.\nThis means that using debug metadata will become a lot less convenient, as the new metadata has\nto be added to a `MetadataListPlan` object. As opposed to the current situation where metadata\nitems can be just set, and `gen-debug-controls.py` will automagically generate them.\nA couple ideas to address this situation:\n\n (1) do nothing\n * by far the simplest\n * inconvenient for debug metadata\n (2) decouple \"debug metadata\" from \"normal metadata\"\n * no early reporting (probably not too important)\n * can keep using `ControlList`\n * how to expose and to which (all?) applications?\n * needs some, possibly quite a bit of, modifications in the IPA interface\n (3) different `MetadataList` implementation\n * I have a prototype for another implementation\n - somewhat more complex\n - needs to allocate on demand\n - provides the essentially same interface and guarantees\n - can accommodate any number of metadata items just like a `ControlList`\n - no `MetadataListPlan` needed\n\n\n### alternative implementation\n\nThere is a prototype for an alternative implementation that implements basically the\nsame interface, but is a bit more complicated. Comparison of the here proposed (\"contiguous\")\nmetadata list, and the more complex (\"chunked\") metadata list is as follows:\n\n cont. chunk.\n==============================+===================================+==========================+\ncomplexity | simpler | more complex |\nmemory allocation | single one when constructed | on demand |\ncontiguous<1> | yes | no |\nbacking data structure<2> | sorted array | chaining hash table<3> |\niterator step | constant time | constant time |\nwasted space | <4> | <5> |\n\n<1>: The reason why this is highlighted is that initially I expected that if\n a serialized version of the `ControlList` is ever introduced, then it should\n use the same format as this metadata list. This would allow the currently completed\n items in the metadata list to be accessed through a hypothetical non-owning\n `ControlListView` type referring to the storage of the metadata list. Such a\n `ControlList` would most likely use a contiguous chunk of memory, so having\n contiguous storage helps compatibility. Nonetheless, I am becoming less sure\n that is is actually useful; and with sufficient complexity the non-contiguous\n version can also be supported.\n\n<2>: for lookup, wrt. the numeric identifier of the control\n\n<3>: fixed number buckets, making it resizable is not trivial\n\n<4>: Space for all metadata items is allocated, so if there are metadata items with large space requirements\n (e.g. `rpi::PispStatsOutput` - 23200 bytes) that are seldom reported, then that space is essentially wasted.\n\n<5>: (I think) bounded by (items / 2 * chunk_size), but with the right parameters\n all \"usually\" reported metadata can fit in one chunk.\n\nv2: https://patchwork.libcamera.org/cover/23868/\nv1: https://patchwork.libcamera.org/cover/23481/\n\nBarnabás Pőcze (16):\n libcamera: controls: Strings are arrays\n libcamera: controls: Add `ControlValueView`\n libcamera: base: Add file for C++20 polyfills\n libcamera: base: cxx20: Add `type_identity{,_t}`\n libcamera: base: cxx20: Add `has_single_bit()`\n libcamera: base: Add alignment utility functions\n libcamera: Add `MetadataList`\n Documentation: design: Document `MetadataList`\n libcamera: ipa_data_serializer: Support `MetadataListPlan`\n libcamera: camera: Store `MetadataListPlan` in `Camera::Private`\n libcamera: request: Store `MetadataList`\n [DNI] apps: cam: Print `MetadataListPlan` of camera\n [DNI] apps: cam: Print `MetadataList` of `Request` as well\n libcamera: pipeline_handler: Inject \"debug\" metadata\n libcamera: pipeline: Fill `MetadataListPlan` of cameras\n libcamera: request: Swap the two metadata lists\n\nJacopo Mondi (6):\n libcamera: camera: Introduce metadataAvailable signal\n guides: application: Document Camera::metadataAvailable\n libcamera: pipeline_handler: Add metadataAvailable() function\n guides: pipeline_handler: Document PipelineHandler::metadataAvailable\n [DNI] apps: cam: Use Camera::metadataAvailable signal\n libcamera: pipeline: Use `metadataAvailable()`\n\n Documentation/design/metadata-list.rst | 266 ++++++++\n .../guides/application-developer.rst | 13 +-\n Documentation/guides/pipeline-handler.rst | 21 +-\n Documentation/index.rst | 3 +-\n Documentation/meson.build | 1 +\n include/libcamera/base/internal/align.h | 135 ++++\n include/libcamera/base/internal/cxx20.h | 58 ++\n include/libcamera/base/meson.build | 8 +\n include/libcamera/camera.h | 3 +\n include/libcamera/controls.h | 70 ++-\n include/libcamera/internal/camera.h | 2 +\n include/libcamera/internal/controls.h | 39 ++\n include/libcamera/internal/pipeline_handler.h | 49 ++\n .../internal/software_isp/software_isp.h | 3 +-\n include/libcamera/ipa/core.mojom | 1 +\n include/libcamera/ipa/ipu3.mojom | 3 +-\n include/libcamera/ipa/mali-c55.mojom | 3 +-\n include/libcamera/ipa/raspberrypi.mojom | 1 +\n include/libcamera/ipa/rkisp1.mojom | 3 +-\n include/libcamera/ipa/soft.mojom | 3 +-\n include/libcamera/meson.build | 2 +\n include/libcamera/metadata_list.h | 529 ++++++++++++++++\n include/libcamera/metadata_list_plan.h | 110 ++++\n include/libcamera/request.h | 9 +-\n src/android/camera_device.cpp | 2 +-\n src/apps/cam/camera_session.cpp | 39 +-\n src/apps/cam/file_sink.cpp | 2 +-\n src/apps/cam/file_sink.h | 4 +-\n src/apps/common/dng_writer.cpp | 2 +-\n src/apps/common/dng_writer.h | 4 +-\n src/apps/qcam/main_window.cpp | 2 +-\n src/apps/qcam/main_window.h | 3 +-\n src/gstreamer/gstlibcamera-controls.cpp.in | 4 +-\n src/ipa/ipu3/algorithms/agc.cpp | 4 +\n src/ipa/ipu3/algorithms/awb.cpp | 12 +\n src/ipa/ipu3/algorithms/awb.h | 1 +\n src/ipa/ipu3/ipa_context.cpp | 3 +\n src/ipa/ipu3/ipa_context.h | 3 +\n src/ipa/ipu3/ipu3.cpp | 8 +-\n src/ipa/mali-c55/algorithms/agc.cpp | 5 +\n src/ipa/mali-c55/algorithms/awb.cpp | 7 +\n src/ipa/mali-c55/algorithms/awb.h | 1 +\n src/ipa/mali-c55/algorithms/blc.cpp | 2 +\n src/ipa/mali-c55/ipa_context.h | 3 +\n src/ipa/mali-c55/mali-c55.cpp | 6 +-\n src/ipa/rkisp1/algorithms/agc.cpp | 10 +\n src/ipa/rkisp1/algorithms/awb.cpp | 4 +\n src/ipa/rkisp1/algorithms/blc.cpp | 2 +\n src/ipa/rkisp1/algorithms/ccm.cpp | 2 +\n src/ipa/rkisp1/algorithms/goc.cpp | 2 +\n src/ipa/rkisp1/algorithms/lux.cpp | 8 +-\n src/ipa/rkisp1/ipa_context.h | 1 +\n src/ipa/rkisp1/rkisp1.cpp | 8 +-\n src/ipa/rpi/common/ipa_base.cpp | 34 +\n src/ipa/rpi/pisp/pisp.cpp | 5 +-\n src/ipa/rpi/vc4/vc4.cpp | 4 +-\n src/ipa/simple/algorithms/agc.cpp | 8 +\n src/ipa/simple/algorithms/agc.h | 1 +\n src/ipa/simple/algorithms/awb.cpp | 8 +\n src/ipa/simple/algorithms/awb.h | 1 +\n src/ipa/simple/algorithms/blc.cpp | 3 +\n src/ipa/simple/algorithms/ccm.cpp | 3 +\n src/ipa/simple/algorithms/lut.cpp | 3 +\n src/ipa/simple/ipa_context.h | 2 +\n src/ipa/simple/soft_simple.cpp | 8 +-\n src/libcamera/camera.cpp | 75 +++\n src/libcamera/controls.cpp | 270 +++++---\n src/libcamera/ipa_data_serializer.cpp | 94 +++\n src/libcamera/meson.build | 1 +\n src/libcamera/metadata_list.cpp | 594 ++++++++++++++++++\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 7 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 21 +-\n src/libcamera/pipeline/mali-c55/mali-c55.cpp | 4 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +-\n .../pipeline/rpi/common/pipeline_base.cpp | 37 +-\n src/libcamera/pipeline/simple/simple.cpp | 6 +-\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +-\n src/libcamera/pipeline/vimc/vimc.cpp | 5 +-\n .../pipeline/virtual/config_parser.cpp | 3 +\n src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n src/libcamera/pipeline_handler.cpp | 135 ++++\n src/libcamera/request.cpp | 8 +-\n src/libcamera/software_isp/software_isp.cpp | 6 +-\n src/py/libcamera/py_helpers.cpp | 4 +-\n src/py/libcamera/py_helpers.h | 2 +-\n src/py/libcamera/py_main.cpp | 2 +-\n test/controls/meson.build | 12 +-\n test/controls/metadata_list.cpp | 205 ++++++\n test/controls/metadata_list_iter_uaf.cpp | 36 ++\n .../core_ipa_interface.h.tmpl | 1 +\n 90 files changed, 2941 insertions(+), 174 deletions(-)\n create mode 100644 Documentation/design/metadata-list.rst\n create mode 100644 include/libcamera/base/internal/align.h\n create mode 100644 include/libcamera/base/internal/cxx20.h\n create mode 100644 include/libcamera/internal/controls.h\n create mode 100644 include/libcamera/metadata_list.h\n create mode 100644 include/libcamera/metadata_list_plan.h\n create mode 100644 src/libcamera/metadata_list.cpp\n create mode 100644 test/controls/metadata_list.cpp\n create mode 100644 test/controls/metadata_list_iter_uaf.cpp\n\n--\n2.51.1" }