Patch Detail
Show a patch.
GET /api/1.1/patches/21525/?format=api
{ "id": 21525, "url": "https://patchwork.libcamera.org/api/1.1/patches/21525/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21525/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20241007095425.211158-3-stefan.klug@ideasonboard.com>", "date": "2024-10-07T09:54:06", "name": "[v2,2/7] libcamera: Add a DebugMetadata helper", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "89b87b73d6443305e4520c77e4359f8814e7f70b", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/1.1/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21525/mbox/", "series": [ { "id": 4661, "url": "https://patchwork.libcamera.org/api/1.1/series/4661/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4661", "date": "2024-10-07T09:54:04", "name": "Add support for IPA debugging metadata", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4661/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21525/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21525/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 8FADCBD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 7 Oct 2024 09:54:40 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6503B63535;\n\tMon, 7 Oct 2024 11:54:38 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61CB16352D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 7 Oct 2024 11:54:36 +0200 (CEST)", "from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:ba12:1296:516b:1122])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D32C49E;\n\tMon, 7 Oct 2024 11:53:00 +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=\"TfksGh/e\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728294780;\n\tbh=eh7WhZirsJVemqom/sbb5vduqK8tqmE9IEIX1lhhN0Q=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=TfksGh/eQAHtbNnWGWuonTUhnaAnG080mqd6tX9gLmI+Au02+rMbORowDK6htKpSz\n\tVGRJnrLsP/BE23cdVgpZnSgVHKJjKRpT+xV8gfce0rmQlgr96QDt8jfgOzrcUWlbth\n\t85S2e3QOWxMt6ShaRBPEp3EdIFdVh7dDYwbAVh4M=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH v2 2/7] libcamera: Add a DebugMetadata helper", "Date": "Mon, 7 Oct 2024 11:54:06 +0200", "Message-ID": "<20241007095425.211158-3-stefan.klug@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20241007095425.211158-1-stefan.klug@ideasonboard.com>", "References": "<20241007095425.211158-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 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/control_ids_debug.yaml | 3 +-\n src/libcamera/debug_controls.cpp | 145 ++++++++++++++++++++\n src/libcamera/meson.build | 1 +\n 6 files changed, 199 insertions(+), 2 deletions(-)\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..00457d60b1a2\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 checkForEnable(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<T, V>(ctrl, value);\n+\t\t\treturn;\n+\t\t}\n+\n+\t\tif (!enabled_)\n+\t\t\treturn;\n+\n+\t\tcache_.set<T, V>(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/control_ids_debug.yaml b/src/libcamera/control_ids_debug.yaml\nindex 62f742c58129..797532712099 100644\n--- a/src/libcamera/control_ids_debug.yaml\n+++ b/src/libcamera/control_ids_debug.yaml\n@@ -3,5 +3,4 @@\n %YAML 1.1\n ---\n vendor: debug\n-controls:\n-\n+controls: []\ndiff --git a/src/libcamera/debug_controls.cpp b/src/libcamera/debug_controls.cpp\nnew file mode 100644\nindex 000000000000..84157aca5a74\n--- /dev/null\n+++ b/src/libcamera/debug_controls.cpp\n@@ -0,0 +1,145 @@\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+ * When one wants to record debug metadata, the metadata list is often not\n+ * directly available (either because we are inside process() of an IPA or\n+ * because we are in a closed module). This class allows to record the data and\n+ * at a later point in time forward it either to another DebugMetadata instance\n+ * or to a ControlList.\n+ */\n+\n+/**\n+ * \\fn DebugMetadata::checkForEnable\n+ * \\brief Check for DebugMetadataEnable in the supplied 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::checkForEnable(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 Enables or disabled metadata handling\n+ * \\param[in] enable The enable state\n+ *\n+ * Enables or disables metadata handling according to \\a enable. When \\a enable\n+ * is true, all calls to set() get cached and can later be retrieved using \\a\n+ * DebugMetadata::moveEntries(). When \\a enable is false, the cache gets cleared\n+ * 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 Assign a parent metadata handler\n+ * \\param[in] parent Pointer to the parent handler\n+ *\n+ * When a \\a parent gets set, all further calls to DebugMetadata::set are\n+ * forwarded to that instance. It is not allowed to enable a DebugMetadata\n+ * object, log entries to it and later set the parent. This is done to keep a\n+ * path open for switching to tracing infrastructure later. For tracing one\n+ * would need some kind of \"context\" identifier that needs to be available on\n+ * set() time. The parent can be treated as such. The top level object\n+ * (the one where enable() get's called) lives in a place where that information\n+ * is also available.\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 a list\n+ * \\param[in] list The list\n+ *\n+ * Moves all entries into the list specified by \\a list. Duplicate entries in\n+ * \\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 a value\n+ * \\param[in] ctrl The ctrl to set\n+ * \\param[in] value The control value\n+ *\n+ * Sets the debug metadata for \\a ctrl to value \\a value. If a parent is set,\n+ * the value gets passed there unconditionally. Otherwise it gets cached if the\n+ * instance is enabled or dropped silently when disabled.\n+ */\n+\n+/**\n+ * \\fn DebugMetadata::set(unsigned int id, const ControlValue &value)\n+ * \\brief Set a value\n+ * \\param[in] id The id of the control\n+ * \\param[in] value The control value\n+ *\n+ * Sets the debug metadata for \\a id to value \\a value. If a parent is set,\n+ * the value gets passed there unconditionally. Otherwise it gets cached if the\n+ * instance is enabled or dropped silently when disabled.\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": [ "v2", "2/7" ] }