{"id":26685,"url":"https://patchwork.libcamera.org/api/patches/26685/?format=json","web_url":"https://patchwork.libcamera.org/patch/26685/","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":"<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","date":"2026-05-07T21:37:21","name":"[3/3] ipa: libipa: v4l2_params: Move non-template code to new base class","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"447cb0605d6591890f566709a09437a19183a09f","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/26685/mbox/","series":[{"id":5921,"url":"https://patchwork.libcamera.org/api/series/5921/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5921","date":"2026-05-07T21:37:19","name":"ipa: libipa: Avoid code duplication in V4L2Params","version":1,"mbox":"https://patchwork.libcamera.org/series/5921/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/26685/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/26685/checks/","tags":{},"headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":["parsemail@patchwork.libcamera.org","kbingham@ideasonboard.com"],"Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 533D5BDCBD\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  8 May 2026 08:21:51 +0000 (UTC)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net\n\t[86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9E505ABF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  8 May 2026 10:21:46 +0200 (CEST)","from perceval.ideasonboard.com\n\tby perceval.ideasonboard.com with LMTP id INU/ERcG/WmmahUA4E0KoQ\n\t(envelope-from <libcamera-devel-bounces@lists.libcamera.org>)\n\tfor <kbingham@ideasonboard.com>; Thu, 07 May 2026 23:37:27 +0200","from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\tby perceval.ideasonboard.com (Postfix) with ESMTPS\n\tid 19BA02072;\tThu,  7 May 2026 23:37:27 +0200 (CEST)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 845AA62FE1;\n\tThu,  7 May 2026 23:37:29 +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 DAA6962010\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 23:37:26 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 74C9E1121\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 23:37:22 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778228506;\n\tbh=TPZiJEsWevzE7x+EjZgJuVTh1odB3TVAeX8S7243qvc=;\n\th=From:To:Subject:Date:In-Reply-To:References:List-Id:\n\tList-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\tResent-From:Resent-To:From;\n\tb=PPlXQgIOJYo5TBDJMvnW1klWwfyoQJ76ftaxu1PcJ6FHklHswXD/ZS94sgyUkSbaU\n\tv1jlqUlyC6PhrnSO31LEhU7g+oK7+wGlFWqRo5gfsv07kiUnHIHWiE2TqcgrFFLX6N\n\tdeTTnsL3BuMNGxxdnpK++9QiJckbHQaCSABIZlKI=","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778189842;\n\tbh=TPZiJEsWevzE7x+EjZgJuVTh1odB3TVAeX8S7243qvc=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=dBTjk+CS1I10K0HA8JLqb7vBJbaeO77Bc8O+8d2I6ByxnHO1sISYtsGRjjf8zK0PI\n\tRjleHXU2x+sKwGhx9Ud1D4fSQg04OrE6ObBZE8lw7RoXgbndWEHFoiEURgEHcEyJBs\n\tySmMEAJFtYToghdQLwE7JEQcOiJeivfYRMY4c7zs="],"Authentication-Results":["perceval.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.a=rsa-sha256 header.s=mail header.b=dBTjk+CS; dkim-atps=neutral","lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dBTjk+CS\";\tdkim-atps=neutral"],"From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to new\n\tbase class","Date":"Fri,  8 May 2026 00:37:21 +0300","Message-ID":"<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.53.0","In-Reply-To":"<20260507213721.2137448-1-laurent.pinchart@ideasonboard.com>","References":"<20260507213721.2137448-1-laurent.pinchart@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>","X-TUID":"XjDRYnAro6x4","Resent-From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Resent-To":"parsemail@patchwork.libcamera.org"},"content":"The implementation of multiple V4L2Params member functions does not\ndepend on the class template argument. Move them to a base class, to\navoid inlining them.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++---------\n src/ipa/libipa/v4l2_params.h   |  81 +++++-------------\n src/ipa/rkisp1/params.cpp      |   4 +-\n 3 files changed, 132 insertions(+), 104 deletions(-)","diff":"diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp\nindex d44a366a60b8..476dc0281448 100644\n--- a/src/ipa/libipa/v4l2_params.cpp\n+++ b/src/ipa/libipa/v4l2_params.cpp\n@@ -117,6 +117,115 @@ namespace ipa {\n  * \\brief Memory area reserved for the ISP configuration block\n  */\n \n+/**\n+ * \\class V4L2ParamsBase\n+ * \\brief Base class for V4L2Params\n+ *\n+ * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a\n+ * container for all code that does not depend on the V4L2Params template\n+ * arguments, to avoid duplicate copies of inline code.\n+ */\n+\n+/**\n+ * \\brief Construct an instance of V4L2ParamsBase\n+ * \\param[in] data Reference to the v4l2-buffer memory mapped area\n+ * \\param[in] version The ISP parameters version the implementation supports\n+ */\n+V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version)\n+\t: data_(data)\n+{\n+\tstruct v4l2_isp_params_buffer *params =\n+\t\treinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data());\n+\tparams->data_size = 0;\n+\tparams->version = version;\n+\n+\tused_ = offsetof(struct v4l2_isp_params_buffer, data);\n+}\n+\n+/**\n+ * \\fn V4L2ParamsBase::bytesused()\n+ * \\brief Retrieve the used size of the parameters buffer (in bytes)\n+ *\n+ * The parameters buffer size is mostly used to populate the v4l2_buffer\n+ * bytesused field before queueing the buffer to the ISP.\n+ *\n+ * \\return The number of bytes occupied by the ISP configuration parameters\n+ */\n+\n+/**\n+ * \\brief Populate an ISP configuration block a returns a reference to its\n+ * memory\n+ * \\param[in] type The ISP block identifier enumerated by the IPA module\n+ * \\param[in] blockType The kernel-defined ISP block identifier, used to\n+ * populate the block header\n+ * \\param[in] blockSize The ISP block size, used to populate the block header\n+ *\n+ * Initialize the block header with \\a blockType and \\a blockSize and\n+ * returns a reference to the memory used to store an ISP configuration block.\n+ *\n+ * IPA modules that derive the V4L2Params class shall use this function to\n+ * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T>\n+ * before returning it to the caller.\n+ */\n+Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType,\n+\t\t\t\t    size_t blockSize)\n+{\n+\t/*\n+\t * Look up the block in the cache first. If an algorithm\n+\t * requests the same block type twice, it should get the same\n+\t * block.\n+\t */\n+\tauto cacheIt = blocks_.find(type);\n+\tif (cacheIt != blocks_.end())\n+\t\treturn cacheIt->second;\n+\n+\t/*\n+\t * Make sure we don't run out of space. Assert as otherwise\n+\t * we get a segfault as soon as someone tries to access the\n+\t * empty Span<> returned from here.\n+\t */\n+\tif (blockSize > data_.size() - used_) {\n+\t\tLOG(Fatal)\n+\t\t\t<< \"Parameters buffer out of space; potential version mismatch between driver and libcamera\";\n+\t\treturn {};\n+\t}\n+\n+\t/* Allocate a new block, clear its memory, and initialize its header. */\n+\tSpan<uint8_t> block = data_.subspan(used_, blockSize);\n+\tmemset(block.data(), 0, block.size());\n+\n+\tstruct v4l2_isp_params_block_header *header =\n+\t\treinterpret_cast<struct v4l2_isp_params_block_header *>(block.data());\n+\theader->type = blockType;\n+\theader->size = block.size();\n+\n+\tused_ += block.size();\n+\n+\tstruct v4l2_isp_params_buffer *buffer =\n+\t\treinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data());\n+\tbuffer->data_size += block.size();\n+\n+\t/* Update the cache. */\n+\tblocks_[type] = block;\n+\n+\treturn block;\n+}\n+\n+/**\n+ * \\var V4L2ParamsBase::data_\n+ * \\brief The ISP parameters buffer memory\n+ */\n+\n+/**\n+ * \\var V4L2ParamsBase::used_\n+ * \\brief The number of bytes used in the parameters buffer\n+ */\n+\n+/**\n+ * \\var V4L2ParamsBase::blocks_\n+ * \\brief Cache of ISP configuration blocks\n+ */\n+\n /**\n  * \\class V4L2Params\n  * \\brief Helper class that represent an ISP configuration buffer\n@@ -200,54 +309,12 @@ namespace ipa {\n  * \\param[in] version The ISP parameters version the implementation supports\n  */\n \n-/**\n- * \\fn V4L2Params::bytesused()\n- * \\brief Retrieve the used size of the parameters buffer (in bytes)\n- *\n- * The parameters buffer size is mostly used to populate the v4l2_buffer\n- * bytesused field before queueing the buffer to the ISP.\n- *\n- * \\return The number of bytes occupied by the ISP configuration parameters\n- */\n-\n /**\n  * \\fn V4L2Params::block()\n  * \\brief Retrieve the location of an ISP configuration block a return it\n  * \\return A V4L2ParamsBlock instance that points to the ISP configuration block\n  */\n \n-/**\n- * \\fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize)\n- * \\brief Populate an ISP configuration block a returns a reference to its\n- * memory\n- * \\param[in] type The ISP block identifier enumerated by the IPA module\n- * \\param[in] blockType The kernel-defined ISP block identifier, used to\n- * populate the block header\n- * \\param[in] blockSize The ISP block size, used to populate the block header\n- *\n- * Initialize the block header with \\a blockType and \\a blockSize and\n- * returns a reference to the memory used to store an ISP configuration block.\n- *\n- * IPA modules that derive the V4L2Params class shall use this function to\n- * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T>\n- * before returning it to the caller.\n- */\n-\n-/**\n- * \\var V4L2Params::data_\n- * \\brief The ISP parameters buffer memory\n- */\n-\n-/**\n- * \\var V4L2Params::used_\n- * \\brief The number of bytes used in the parameters buffer\n- */\n-\n-/**\n- * \\var V4L2Params::blocks_\n- * \\brief Cache of ISP configuration blocks\n- */\n-\n } /* namespace ipa */\n \n } /* namespace libcamera */\ndiff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h\nindex 4f84360ee449..5f57167d7646 100644\n--- a/src/ipa/libipa/v4l2_params.h\n+++ b/src/ipa/libipa/v4l2_params.h\n@@ -69,25 +69,34 @@ protected:\n \tSpan<uint8_t> data_;\n };\n \n+class V4L2ParamsBase\n+{\n+public:\n+\tV4L2ParamsBase(Span<uint8_t> data, unsigned int version);\n+\n+\tsize_t bytesused() const { return used_; }\n+\n+protected:\n+\tSpan<uint8_t> block(uint16_t type, unsigned int blockType,\n+\t\t\t    size_t blockSize);\n+\n+\tSpan<uint8_t> data_;\n+\tsize_t used_;\n+\n+\tstd::map<uint16_t, Span<uint8_t>> blocks_;\n+};\n+\n template<typename Traits>\n-class V4L2Params\n+class V4L2Params : public V4L2ParamsBase\n {\n public:\n \tstatic_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>);\n \n \tV4L2Params(Span<uint8_t> data, unsigned int version)\n-\t\t: data_(data)\n+\t\t: V4L2ParamsBase(data, version)\n \t{\n-\t\tstruct v4l2_isp_params_buffer *params =\n-\t\t\treinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data());\n-\t\tparams->data_size = 0;\n-\t\tparams->version = version;\n-\n-\t\tused_ = offsetof(struct v4l2_isp_params_buffer, data);\n \t}\n \n-\tsize_t bytesused() const { return used_; }\n-\n \ttemplate<typename Traits::id_type Id>\n \tauto block()\n \t{\n@@ -96,58 +105,10 @@ public:\n \t\tusing Type = typename Details::type;\n \t\tconstexpr auto kernelId = Details::blockType;\n \n-\t\tauto data = block(utils::to_underlying(Id), kernelId, sizeof(Type));\n+\t\tauto data = V4L2ParamsBase::block(utils::to_underlying(Id),\n+\t\t\t\t\t\t  kernelId, sizeof(Type));\n \t\treturn V4L2ParamsBlock<Type>(data);\n \t}\n-\n-protected:\n-\tSpan<uint8_t> block(uint16_t type, unsigned int blockType,\n-\t\t\t    size_t blockSize)\n-\t{\n-\t\t/*\n-\t\t * Look up the block in the cache first. If an algorithm\n-\t\t * requests the same block type twice, it should get the same\n-\t\t * block.\n-\t\t */\n-\t\tauto cacheIt = blocks_.find(type);\n-\t\tif (cacheIt != blocks_.end())\n-\t\t\treturn cacheIt->second;\n-\n-\t\t/*\n-\t\t * Make sure we don't run out of space. Assert as otherwise\n-\t\t * we get a segfault as soon as someone tries to access the\n-\t\t * empty Span<> returned from here.\n-\t\t */\n-\t\tif (blockSize > data_.size() - used_) {\n-\t\t\tLOG(Fatal)\n-\t\t\t\t<< \"Parameters buffer out of space; potential version mismatch between driver and libcamera\";\n-\t\t\treturn {};\n-\t\t}\n-\n-\t\t/* Allocate a new block, clear its memory, and initialize its header. */\n-\t\tSpan<uint8_t> block = data_.subspan(used_, blockSize);\n-\t\tmemset(block.data(), 0, block.size());\n-\n-\t\tstruct v4l2_isp_params_block_header *header =\n-\t\t\treinterpret_cast<struct v4l2_isp_params_block_header *>(block.data());\n-\t\theader->type = blockType;\n-\t\theader->size = block.size();\n-\n-\t\tused_ += block.size();\n-\n-\t\treinterpret_cast<struct v4l2_isp_params_buffer *>\n-\t\t\t(data_.data())->data_size += block.size();\n-\n-\t\t/* Update the cache. */\n-\t\tblocks_[type] = block;\n-\n-\t\treturn block;\n-\t}\n-\n-\tSpan<uint8_t> data_;\n-\tsize_t used_;\n-\n-\tstd::map<uint16_t, Span<uint8_t>> blocks_;\n };\n \n } /* namespace ipa */\ndiff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp\nindex b8abbdf6ec66..71df0a939de2 100644\n--- a/src/ipa/rkisp1/params.cpp\n+++ b/src/ipa/rkisp1/params.cpp\n@@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type)\n \t\treturn data_.subspan(info.offset, info.size);\n \t}\n \n-\treturn V4L2Params::block(utils::to_underlying(type), info.type,\n-\t\t\t\t info.size);\n+\treturn V4L2ParamsBase::block(utils::to_underlying(type), info.type,\n+\t\t\t\t     info.size);\n }\n \n } /* namespace ipa::rkisp1 */\n","prefixes":["3/3"]}