{"id":21540,"url":"https://patchwork.libcamera.org/api/patches/21540/?format=json","web_url":"https://patchwork.libcamera.org/patch/21540/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20241008153031.429906-3-stefan.klug@ideasonboard.com>","date":"2024-10-08T15:29:40","name":"[v3,2/7] libcamera: Add a DebugMetadata helper","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"3718cb248426f45c5f07e24f55622059c9b243f7","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/?format=json","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/21540/mbox/","series":[{"id":4667,"url":"https://patchwork.libcamera.org/api/series/4667/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4667","date":"2024-10-08T15:29:38","name":"Add support for IPA debugging metadata","version":3,"mbox":"https://patchwork.libcamera.org/series/4667/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/21540/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/21540/checks/","tags":{},"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 2B07FBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  8 Oct 2024 15:30:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A27DF6536A;\n\tTue,  8 Oct 2024 17:30:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CDFC6536C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Oct 2024 17:30:42 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:d:f30b:aa60:fabf])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4206ADB2;\n\tTue,  8 Oct 2024 17:29:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XB8EdfLj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728401345;\n\tbh=Re0mu06kI6Sfde7TA4UbsLGtdKbF48gupVAj5GIZQ8M=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=XB8EdfLjRXOkyAC3H7tfFML7dJWmhTZbTD+BEaph26OGwoQ8blXva8e7u33D3DK1W\n\t1olbr/gFZe1vMlFDl7Ai/6m6P+QEOqkcZhteRJxJ+QRfjvzTZOrOm9rw5ZaYJseFIf\n\tsNCWk5yC96LDcVW6YlDkfMzabkT5AKy6kY3bFaC0=","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","Subject":"[PATCH v3 2/7] libcamera: Add a DebugMetadata helper","Date":"Tue,  8 Oct 2024 17:29:40 +0200","Message-ID":"<20241008153031.429906-3-stefan.klug@ideasonboard.com>","X-Mailer":"git-send-email 2.43.0","In-Reply-To":"<20241008153031.429906-1-stefan.klug@ideasonboard.com>","References":"<20241008153031.429906-1-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","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":"Debug metadata often occurs in places where the metadata control list is\nnot available e.g. in queueRequest() or processStatsBuffer() or even in\na class far away from the metadata handling code. It is therefore\ndifficult to add debug metadata without adding lots of boilerplate\ncode. This can be mitigated by recording the metadata and forwarding it\nto the metadata control list when it becomes available. To solve the\nissue of code that is far away from the metadata context, add a chaining\nmechanism to allow loose coupling at runtime.\n\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n\n---\nChanges in v3:\n- Rename checkForEnable by enableByControl\n- Remove explicit template params in set\n- Improve documentation\n\nChanges in v2:\n- Replace assignments of ControlList by a moveEntries function\n- Replace assignUpstream by setParent\n- Improve documentation\n---\n include/libcamera/internal/debug_controls.h |  46 ++++++\n include/libcamera/internal/meson.build      |   1 +\n src/libcamera/control_ids_core.yaml         |   5 +\n src/libcamera/debug_controls.cpp            | 164 ++++++++++++++++++++\n src/libcamera/meson.build                   |   1 +\n 5 files changed, 217 insertions(+)\n create mode 100644 include/libcamera/internal/debug_controls.h\n create mode 100644 src/libcamera/debug_controls.cpp","diff":"diff --git a/include/libcamera/internal/debug_controls.h b/include/libcamera/internal/debug_controls.h\nnew file mode 100644\nindex 000000000000..0b049f48e246\n--- /dev/null\n+++ b/include/libcamera/internal/debug_controls.h\n@@ -0,0 +1,46 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2024, Google Inc.\n+ *\n+ * Debug metadata helpers\n+ */\n+\n+#pragma once\n+\n+#include <libcamera/control_ids.h>\n+\n+namespace libcamera {\n+\n+class DebugMetadata\n+{\n+public:\n+\tDebugMetadata() = default;\n+\n+\tvoid enableByControl(const ControlList &controls);\n+\tvoid enable(bool enable = true);\n+\tvoid setParent(DebugMetadata *parent);\n+\tvoid moveEntries(ControlList &list);\n+\n+\ttemplate<typename T, typename V>\n+\tvoid set(const Control<T> &ctrl, const V &value)\n+\t{\n+\t\tif (parent_) {\n+\t\t\tparent_->set(ctrl, value);\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tif (!enabled_)\n+\t\t\treturn;\n+\n+\t\tcache_.set(ctrl, value);\n+\t}\n+\n+\tvoid set(unsigned int id, const ControlValue &value);\n+\n+private:\n+\tbool enabled_ = false;\n+\tDebugMetadata *parent_ = nullptr;\n+\tControlList cache_;\n+};\n+\n+} /* namespace libcamera */\ndiff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\nindex 1c5eef9cab80..1dddcd50c90b 100644\n--- a/include/libcamera/internal/meson.build\n+++ b/include/libcamera/internal/meson.build\n@@ -14,6 +14,7 @@ libcamera_internal_headers = files([\n     'control_serializer.h',\n     'control_validator.h',\n     'converter.h',\n+    'debug_controls.h',\n     'delayed_controls.h',\n     'device_enumerator.h',\n     'device_enumerator_sysfs.h',\ndiff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\nindex 1b1bd9507d25..d34a2d068b60 100644\n--- a/src/libcamera/control_ids_core.yaml\n+++ b/src/libcamera/control_ids_core.yaml\n@@ -968,4 +968,9 @@ controls:\n         The default gamma value must be 2.2 which closely mimics sRGB gamma.\n         Note that this is camera gamma, so it is applied as 1.0/gamma.\n \n+  - DebugMetadataEnable:\n+      type: bool\n+      description: |\n+        Enable or disable the debug metadata.\n+\n ...\ndiff --git a/src/libcamera/debug_controls.cpp b/src/libcamera/debug_controls.cpp\nnew file mode 100644\nindex 000000000000..27a05592a97a\n--- /dev/null\n+++ b/src/libcamera/debug_controls.cpp\n@@ -0,0 +1,164 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2024, Google Inc.\n+ *\n+ * Helper to easily record debug metadata inside libcamera.\n+ */\n+\n+#include \"libcamera/internal/debug_controls.h\"\n+\n+#include <libcamera/base/log.h>\n+\n+namespace libcamera {\n+\n+LOG_DEFINE_CATEGORY(DebugControls)\n+\n+/**\n+ * \\file debug_controls.h\n+ * \\brief Helper to easily record debug metadata inside libcamera\n+ */\n+\n+/**\n+ * \\class DebugMetadata\n+ * \\brief Helper to record metadata for later use\n+ *\n+ * Metadata is a useful tool for debugging the internal state of libcamera. It\n+ * has the benefit that it is easy to use and related tooling is readily\n+ * available. The difficulty is that the metadata control list is often not\n+ * directly available (either because the variable to debug lives inside\n+ * process() of an IPA or inside a closed algorithm class with no direct access\n+ * to the ipa and therefore the metadata list).\n+ *\n+ * This class helps in both cases. It allows to forward the data to a parent or\n+ * alternatively record the data and at a later point in time copy it to the\n+ * metadata list when it becomes available. Both mechanisms allow easy reuse and\n+ * loose coupling.\n+ *\n+ * The idea is to instantiate a DebugMetadata object in every class/algorithm\n+ * where debug metadata shall be recorded (the inner object). If the IPA doesn't\n+ * support debug metadata, the object is still usable, but the debug data gets\n+ * dropped. If the IPA supports debug metadata it will either register a parent\n+ * DebugMetadata object on the inner object or manually retrieve the data using\n+ * enable()/moveToList().\n+ *\n+ * The concepts of forwarding to a parent and recording for later retrieval are\n+ * mutually exclusive and the parent takes precedence. E.g. it is not allowed to\n+ * enable a DebugMetadata object, log entries to it and later set the parent.\n+ *\n+ * This is done to keep the path open for using other means of data transport\n+ * (like tracing). For every tracing event a corresponding context needs to be\n+ * available on set() time. The parent can be treated as such, the top level\n+ * object (the one where enable() get's called) also lives in a place where that\n+ * information is also available.\n+ */\n+\n+/**\n+ * \\fn DebugMetadata::enableByControl()\n+ * \\brief Enable based on controls::DebugMetadataEnable in the supplied\n+ * ControlList\n+ * \\param[in] controls The supplied ControlList\n+ *\n+ * Looks for controls::DebugMetadataEnable and enables or disables debug\n+ * metadata handling accordingly.\n+ */\n+void DebugMetadata::enableByControl(const ControlList &controls)\n+{\n+\tconst auto &ctrl = controls.get(controls::DebugMetadataEnable);\n+\tif (ctrl)\n+\t\tenable(*ctrl);\n+}\n+\n+/**\n+ * \\fn DebugMetadata::enable()\n+ * \\brief Enable or disable metadata handling\n+ * \\param[in] enable The enable state\n+ *\n+ * When \\a enable is true, all calls to set() get cached and can later be\n+ * retrieved using moveEntries(). When \\a enable is false, the cache gets\n+ * cleared and no further metadata is recorded.\n+ *\n+ * Forwarding to a parent is independent of the enabled state.\n+ */\n+void DebugMetadata::enable(bool enable)\n+{\n+\tenabled_ = enable;\n+\tif (!enabled_)\n+\t\tcache_.clear();\n+}\n+\n+/**\n+ * \\fn DebugMetadata::setParent()\n+ * \\brief Set the parent metadata handler to \\a parent\n+ * \\param[in] parent Pointer to the parent handler\n+ *\n+ * When a \\a parent is set, all further calls to set() are unconditionally\n+ * forwarded to that instance.\n+ *\n+ * The parent can be reset by passing a nullptr.\n+ */\n+void DebugMetadata::setParent(DebugMetadata *parent)\n+{\n+\tparent_ = parent;\n+\n+\tif (!parent_)\n+\t\treturn;\n+\n+\tif (!cache_.empty())\n+\t\tLOG(DebugControls, Error)\n+\t\t\t<< \"Controls were recorded before setting a parent.\"\n+\t\t\t<< \" These are dropped.\";\n+\n+\tcache_.clear();\n+}\n+\n+/**\n+ * \\fn DebugMetadata::moveEntries()\n+ * \\brief Move all cached entries into control list \\a list\n+ * \\param[in] list The control list\n+ *\n+ * This function moves all entries into the list specified by \\a list. Duplicate\n+ * entries in \\a list get overwritten.\n+ */\n+void DebugMetadata::moveEntries(ControlList &list)\n+{\n+\tlist.merge(std::move(cache_), ControlList::MergePolicy::OverwriteExisting);\n+\tcache_.clear();\n+}\n+\n+/**\n+ * \\fn DebugMetadata::set(const Control<T> &ctrl, const V &value)\n+ * \\brief Set the value of \\a ctrl to \\a value\n+ * \\param[in] ctrl The control to set\n+ * \\param[in] value The control value\n+ *\n+ * If a parent is set, the value gets passed there unconditionally. Otherwise it\n+ * gets cached if the instance is enabled or dropped silently when disabled.\n+ *\n+ * \\sa enable()\n+ */\n+\n+/**\n+ * \\fn DebugMetadata::set(unsigned int id, const ControlValue &value)\n+ * \\brief Set the value of control \\a id to \\a value\n+ * \\param[in] id The id of the control\n+ * \\param[in] value The control value\n+ *\n+ * If a parent is set, the value gets passed there unconditionally. Otherwise it\n+ * gets cached if the instance is enabled or dropped silently when disabled.\n+ *\n+ * \\sa enable()\n+ */\n+void DebugMetadata::set(unsigned int id, const ControlValue &value)\n+{\n+\tif (parent_) {\n+\t\tparent_->set(id, value);\n+\t\treturn;\n+\t}\n+\n+\tif (!enabled_)\n+\t\treturn;\n+\n+\tcache_.set(id, value);\n+}\n+\n+} /* namespace libcamera */\ndiff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\nindex aa9ab0291854..f7b5ee8dcc34 100644\n--- a/src/libcamera/meson.build\n+++ b/src/libcamera/meson.build\n@@ -25,6 +25,7 @@ libcamera_internal_sources = files([\n     'control_validator.cpp',\n     'converter.cpp',\n     'delayed_controls.cpp',\n+    'debug_controls.cpp',\n     'device_enumerator.cpp',\n     'device_enumerator_sysfs.cpp',\n     'dma_buf_allocator.cpp',\n","prefixes":["v3","2/7"]}