[{"id":38801,"web_url":"https://patchwork.libcamera.org/comment/38801/","msgid":"<4b51bb2e-acc6-48fb-93e3-674137fee6c2@ideasonboard.com>","date":"2026-05-08T08:38:21","subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 05. 07. 23:37 keltezéssel, Laurent Pinchart írta:\n> The implementation of multiple V4L2Params member functions does not\n> depend on the class template argument. Move them to a base class, to\n> avoid inlining them.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n\nLooks ok to me.\n\nReviewed-by: Barnabás Pőcze \n\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(-)\n> \n> diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp\n> index 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 data, unsigned int version)\n> +\t: data_(data)\n> +{\n> +\tstruct v4l2_isp_params_buffer *params =\n> +\t\treinterpret_cast(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\n> + * before returning it to the caller.\n> + */\n> +Span 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 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(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(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\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 */\n> diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h\n> index 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 data_;\n> };\n> \n> +class V4L2ParamsBase\n> +{\n> +public:\n> +\tV4L2ParamsBase(Span data, unsigned int version);\n> +\n> +\tsize_t bytesused() const { return used_; }\n> +\n> +protected:\n> +\tSpan block(uint16_t type, unsigned int blockType,\n> +\t\t\t size_t blockSize);\n> +\n> +\tSpan data_;\n> +\tsize_t used_;\n> +\n> +\tstd::map> blocks_;\n> +};\n> +\n> template\n> -class V4L2Params\n> +class V4L2Params : public V4L2ParamsBase\n> {\n> public:\n> \tstatic_assert(std::is_same_v, uint16_t>);\n> \n> \tV4L2Params(Span 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(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\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(data);\n> \t}\n> -\n> -protected:\n> -\tSpan 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 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(block.data());\n> -\t\theader->type = blockType;\n> -\t\theader->size = block.size();\n> -\n> -\t\tused_ += block.size();\n> -\n> -\t\treinterpret_cast\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 data_;\n> -\tsize_t used_;\n> -\n> -\tstd::map> blocks_;\n> };\n> \n> } /* namespace ipa */\n> diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp\n> index b8abbdf6ec66..71df0a939de2 100644\n> --- a/src/ipa/rkisp1/params.cpp\n> +++ b/src/ipa/rkisp1/params.cpp\n> @@ -128,8 +128,8 @@ Span 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> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 0A87DBE173\n\tfor ;\n\tFri, 8 May 2026 08:38:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1052D6301E;\n\tFri, 8 May 2026 10:38:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A7A262E6A\n\tfor ;\n\tFri, 8 May 2026 10:38:25 +0200 (CEST)","from [192.168.33.85] (185.221.140.217.nat.pool.zt.hu\n\t[185.221.140.217])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 82B68ABF;\n\tFri, 8 May 2026 10:38:20 +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=\"mOdLiqeU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778229500;\n\tbh=g/LG5ZMZNzPy3tVZ9MU0m3LzSOZMYJcnzGXx5JkQt7Y=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=mOdLiqeUGigJ8JeoqHic2am6Wvf/3f3Kdwbctej5KETm34OXGsvVHpz5hLnG6C6O5\n\tnaGusXUT/I9nYoWiuvnhhvcdToEIpGXOzCzj4uuIqMHmbKmkEGBwHTd6cgtY0cohyQ\n\tu1dXQFAQPEZh5sMro3UkBS5Ywzzp9RprPsMFFhwI=","Message-ID":"<4b51bb2e-acc6-48fb-93e3-674137fee6c2@ideasonboard.com>","Date":"Fri, 8 May 2026 10:38:21 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260507213721.2137448-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38805,"web_url":"https://patchwork.libcamera.org/comment/38805/","msgid":"<177823139219.3304204.5665326178129156223@ping.linuxembedded.co.uk>","date":"2026-05-08T09:09:52","subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2026-05-08 09:38:21)\n> 2026. 05. 07. 23:37 keltezéssel, Laurent Pinchart írta:\n> > The implementation of multiple V4L2Params member functions does not\n> > depend on the class template argument. Move them to a base class, to\n> > avoid inlining them.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> \n> Looks ok to me.\n> \n> Reviewed-by: Barnabás Pőcze \n\nReviewed-by: Kieran Bingham \n\n> \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(-)\n> > \n> > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp\n> > index 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 data, unsigned int version)\n> > + : data_(data)\n> > +{\n> > + struct v4l2_isp_params_buffer *params =\n> > + reinterpret_cast(data_.data());\n> > + params->data_size = 0;\n> > + params->version = version;\n> > +\n> > + used_ = 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\n> > + * before returning it to the caller.\n> > + */\n> > +Span V4L2ParamsBase::block(uint16_t type, unsigned int blockType,\n> > + size_t blockSize)\n> > +{\n> > + /*\n> > + * Look up the block in the cache first. If an algorithm\n> > + * requests the same block type twice, it should get the same\n> > + * block.\n> > + */\n> > + auto cacheIt = blocks_.find(type);\n> > + if (cacheIt != blocks_.end())\n> > + return cacheIt->second;\n> > +\n> > + /*\n> > + * Make sure we don't run out of space. Assert as otherwise\n> > + * we get a segfault as soon as someone tries to access the\n> > + * empty Span<> returned from here.\n> > + */\n> > + if (blockSize > data_.size() - used_) {\n> > + LOG(Fatal)\n> > + << \"Parameters buffer out of space; potential version mismatch between driver and libcamera\";\n> > + return {};\n> > + }\n> > +\n> > + /* Allocate a new block, clear its memory, and initialize its header. */\n> > + Span block = data_.subspan(used_, blockSize);\n> > + memset(block.data(), 0, block.size());\n> > +\n> > + struct v4l2_isp_params_block_header *header =\n> > + reinterpret_cast(block.data());\n> > + header->type = blockType;\n> > + header->size = block.size();\n> > +\n> > + used_ += block.size();\n> > +\n> > + struct v4l2_isp_params_buffer *buffer =\n> > + reinterpret_cast(data_.data());\n> > + buffer->data_size += block.size();\n> > +\n> > + /* Update the cache. */\n> > + blocks_[type] = block;\n> > +\n> > + return 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\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 */\n> > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h\n> > index 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> > Span data_;\n> > };\n> > \n> > +class V4L2ParamsBase\n> > +{\n> > +public:\n> > + V4L2ParamsBase(Span data, unsigned int version);\n> > +\n> > + size_t bytesused() const { return used_; }\n> > +\n> > +protected:\n> > + Span block(uint16_t type, unsigned int blockType,\n> > + size_t blockSize);\n> > +\n> > + Span data_;\n> > + size_t used_;\n> > +\n> > + std::map> blocks_;\n> > +};\n> > +\n> > template\n> > -class V4L2Params\n> > +class V4L2Params : public V4L2ParamsBase\n> > {\n> > public:\n> > static_assert(std::is_same_v, uint16_t>);\n> > \n> > V4L2Params(Span data, unsigned int version)\n> > - : data_(data)\n> > + : V4L2ParamsBase(data, version)\n> > {\n> > - struct v4l2_isp_params_buffer *params =\n> > - reinterpret_cast(data_.data());\n> > - params->data_size = 0;\n> > - params->version = version;\n> > -\n> > - used_ = offsetof(struct v4l2_isp_params_buffer, data);\n> > }\n> > \n> > - size_t bytesused() const { return used_; }\n> > -\n> > template\n> > auto block()\n> > {\n> > @@ -96,58 +105,10 @@ public:\n> > using Type = typename Details::type;\n> > constexpr auto kernelId = Details::blockType;\n> > \n> > - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type));\n> > + auto data = V4L2ParamsBase::block(utils::to_underlying(Id),\n> > + kernelId, sizeof(Type));\n> > return V4L2ParamsBlock(data);\n> > }\n> > -\n> > -protected:\n> > - Span block(uint16_t type, unsigned int blockType,\n> > - size_t blockSize)\n> > - {\n> > - /*\n> > - * Look up the block in the cache first. If an algorithm\n> > - * requests the same block type twice, it should get the same\n> > - * block.\n> > - */\n> > - auto cacheIt = blocks_.find(type);\n> > - if (cacheIt != blocks_.end())\n> > - return cacheIt->second;\n> > -\n> > - /*\n> > - * Make sure we don't run out of space. Assert as otherwise\n> > - * we get a segfault as soon as someone tries to access the\n> > - * empty Span<> returned from here.\n> > - */\n> > - if (blockSize > data_.size() - used_) {\n> > - LOG(Fatal)\n> > - << \"Parameters buffer out of space; potential version mismatch between driver and libcamera\";\n> > - return {};\n> > - }\n> > -\n> > - /* Allocate a new block, clear its memory, and initialize its header. */\n> > - Span block = data_.subspan(used_, blockSize);\n> > - memset(block.data(), 0, block.size());\n> > -\n> > - struct v4l2_isp_params_block_header *header =\n> > - reinterpret_cast(block.data());\n> > - header->type = blockType;\n> > - header->size = block.size();\n> > -\n> > - used_ += block.size();\n> > -\n> > - reinterpret_cast\n> > - (data_.data())->data_size += block.size();\n> > -\n> > - /* Update the cache. */\n> > - blocks_[type] = block;\n> > -\n> > - return block;\n> > - }\n> > -\n> > - Span data_;\n> > - size_t used_;\n> > -\n> > - std::map> blocks_;\n> > };\n> > \n> > } /* namespace ipa */\n> > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp\n> > index b8abbdf6ec66..71df0a939de2 100644\n> > --- a/src/ipa/rkisp1/params.cpp\n> > +++ b/src/ipa/rkisp1/params.cpp\n> > @@ -128,8 +128,8 @@ Span RkISP1Params::block(BlockType type)\n> > return data_.subspan(info.offset, info.size);\n> > }\n> > \n> > - return V4L2Params::block(utils::to_underlying(type), info.type,\n> > - info.size);\n> > + return V4L2ParamsBase::block(utils::to_underlying(type), info.type,\n> > + info.size);\n> > }\n> > \n> > } /* namespace ipa::rkisp1 */\n> > --\n> > Regards,\n> > \n> > Laurent Pinchart\n> > \n>","headers":{"Return-Path":"","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 BE9D5BDCBD\n\tfor ;\n\tFri, 8 May 2026 09:09:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF35362FEC;\n\tFri, 8 May 2026 11:09:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB8E762E6A\n\tfor ;\n\tFri, 8 May 2026 11:09:54 +0200 (CEST)","from monstersaurus.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E5CFBCA;\n\tFri, 8 May 2026 11:09:50 +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=\"i5pVbJdn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778231390;\n\tbh=aGEr2lKRawk702d/HvX6xlTOtENMczW06QidwJwyJ9U=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=i5pVbJdnXu3abdGPUdFzO0YnmZm7guUnJ4HcKuj+1rBVE410qtn23NiluZ7fgy4hC\n\tVRdkQrfCXcWtXO0WpBlrUlF1/xqZFv00gVNQVRVyY0AfL/nqyioUrdS2P3TY2kiT17\n\tn2aKl11DWEc1jg/EMAOzQWLHNBziSMbCyE+p4Tho=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<4b51bb2e-acc6-48fb-93e3-674137fee6c2@ideasonboard.com>","References":"<20260507213721.2137448-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>\n\t<4b51bb2e-acc6-48fb-93e3-674137fee6c2@ideasonboard.com>","Subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart , \n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 08 May 2026 10:09:52 +0100","Message-ID":"<177823139219.3304204.5665326178129156223@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38815,"web_url":"https://patchwork.libcamera.org/comment/38815/","msgid":"","date":"2026-05-08T09:45:55","subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Fri, May 08, 2026 at 12:37:21AM +0300, Laurent Pinchart wrote:\n> The implementation of multiple V4L2Params member functions does not\n> depend on the class template argument. Move them to a base class, to\n> avoid inlining them.\n>\n> Signed-off-by: Laurent Pinchart \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(-)\n>\n> diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp\n> index 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 data, unsigned int version)\n> +\t: data_(data)\n> +{\n> +\tstruct v4l2_isp_params_buffer *params =\n> +\t\treinterpret_cast(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\n> + * before returning it to the caller.\n> + */\n> +Span 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 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(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(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\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 */\n> diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h\n> index 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 data_;\n> };\n>\n> +class V4L2ParamsBase\n> +{\n> +public:\n> +\tV4L2ParamsBase(Span data, unsigned int version);\n\nShould the constructor be protected ?\n\n> +\n> +\tsize_t bytesused() const { return used_; }\n> +\n> +protected:\n> +\tSpan block(uint16_t type, unsigned int blockType,\n> +\t\t\t size_t blockSize);\n> +\n> +\tSpan data_;\n> +\tsize_t used_;\n> +\n> +\tstd::map> blocks_;\n> +};\n> +\n> template\n> -class V4L2Params\n> +class V4L2Params : public V4L2ParamsBase\n> {\n> public:\n> \tstatic_assert(std::is_same_v, uint16_t>);\n>\n> \tV4L2Params(Span 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(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\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(data);\n> \t}\n> -\n> -protected:\n> -\tSpan 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 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(block.data());\n> -\t\theader->type = blockType;\n> -\t\theader->size = block.size();\n> -\n> -\t\tused_ += block.size();\n> -\n> -\t\treinterpret_cast\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 data_;\n> -\tsize_t used_;\n> -\n> -\tstd::map> blocks_;\n> };\n>\n> } /* namespace ipa */\n> diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp\n> index b8abbdf6ec66..71df0a939de2 100644\n> --- a/src/ipa/rkisp1/params.cpp\n> +++ b/src/ipa/rkisp1/params.cpp\n> @@ -128,8 +128,8 @@ Span 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> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 007D1BE173\n\tfor ;\n\tFri, 8 May 2026 09:46:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4871A6301E;\n\tFri, 8 May 2026 11:46:00 +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 10CCD62E6A\n\tfor ;\n\tFri, 8 May 2026 11:45:59 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5458DBCA;\n\tFri, 8 May 2026 11:45:54 +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=\"oGXMY4ad\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778233554;\n\tbh=J9+aqB/OW5FMcpPejozVIpVVQFMbuYjo6kZ9k2DmzbM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oGXMY4adMxygLEkmREIzMTIa8gPN6QgGe3yfXpoyprkvArRY3eoM4TIkgGumxPbYJ\n\tFlF/ZjV5rxQ/zl7Djm+X51oAIbEBVDEwdFCjorHCI2qsxPI38k+3npR84g935iuQEl\n\tWlMwdejKKxe5PVP1Ws4J0v2Ts4eNcRtcUAA3h+rU=","Date":"Fri, 8 May 2026 11:45:55 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","Message-ID":"","References":"<20260507213721.2137448-1-laurent.pinchart@ideasonboard.com>\n\t<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":38818,"web_url":"https://patchwork.libcamera.org/comment/38818/","msgid":"<20260508095347.GE2176058@killaraus.ideasonboard.com>","date":"2026-05-08T09:53:47","subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, May 08, 2026 at 11:45:55AM +0200, Jacopo Mondi wrote:\n> On Fri, May 08, 2026 at 12:37:21AM +0300, Laurent Pinchart wrote:\n> > The implementation of multiple V4L2Params member functions does not\n> > depend on the class template argument. Move them to a base class, to\n> > avoid inlining them.\n> >\n> > Signed-off-by: Laurent Pinchart \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(-)\n> >\n> > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp\n> > index 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 data, unsigned int version)\n> > +\t: data_(data)\n> > +{\n> > +\tstruct v4l2_isp_params_buffer *params =\n> > +\t\treinterpret_cast(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\n> > + * before returning it to the caller.\n> > + */\n> > +Span 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 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(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(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\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 */\n> > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h\n> > index 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 data_;\n> > };\n> >\n> > +class V4L2ParamsBase\n> > +{\n> > +public:\n> > +\tV4L2ParamsBase(Span data, unsigned int version);\n> \n> Should the constructor be protected ?\n\nI suppose it won't hurt, so nobody will try to instantiate this base\nclass directly (I'm not sure why anyone would, but who knows ?).\n\n> > +\n> > +\tsize_t bytesused() const { return used_; }\n> > +\n> > +protected:\n> > +\tSpan block(uint16_t type, unsigned int blockType,\n> > +\t\t\t size_t blockSize);\n> > +\n> > +\tSpan data_;\n> > +\tsize_t used_;\n> > +\n> > +\tstd::map> blocks_;\n> > +};\n> > +\n> > template\n> > -class V4L2Params\n> > +class V4L2Params : public V4L2ParamsBase\n> > {\n> > public:\n> > \tstatic_assert(std::is_same_v, uint16_t>);\n> >\n> > \tV4L2Params(Span 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(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\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(data);\n> > \t}\n> > -\n> > -protected:\n> > -\tSpan 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 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(block.data());\n> > -\t\theader->type = blockType;\n> > -\t\theader->size = block.size();\n> > -\n> > -\t\tused_ += block.size();\n> > -\n> > -\t\treinterpret_cast\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 data_;\n> > -\tsize_t used_;\n> > -\n> > -\tstd::map> blocks_;\n> > };\n> >\n> > } /* namespace ipa */\n> > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp\n> > index b8abbdf6ec66..71df0a939de2 100644\n> > --- a/src/ipa/rkisp1/params.cpp\n> > +++ b/src/ipa/rkisp1/params.cpp\n> > @@ -128,8 +128,8 @@ Span 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> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >","headers":{"Return-Path":"","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 1329BBE173\n\tfor ;\n\tFri, 8 May 2026 09:53:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4EBF762FE8;\n\tFri, 8 May 2026 11:53:51 +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 3266D62E6A\n\tfor ;\n\tFri, 8 May 2026 11:53:49 +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 38879BCA;\n\tFri, 8 May 2026 11:53:44 +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=\"Cl4DLMCl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778234024;\n\tbh=yqca2LJrSYSi8CGSJiFPdD0iVsFLa0wqIUniqm1deZk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Cl4DLMClEKfw/qfyt2dqOklHGc1yj/WhIJvRve7ThaHhkXU+w1dBsSql7Tb9harjm\n\t19rS4DbX5BzA7bRHO3F5jzwy8BLdoVbnRIC5P2fKnpLfUVYKxb6qFdONoUY58pX0V6\n\tGRgWRUQsaTaFeLndhckFlM7l+F1wxEa86uZgNWqE=","Date":"Fri, 8 May 2026 12:53:47 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/3] ipa: libipa: v4l2_params: Move non-template code to\n\tnew base class","Message-ID":"<20260508095347.GE2176058@killaraus.ideasonboard.com>","References":"<20260507213721.2137448-1-laurent.pinchart@ideasonboard.com>\n\t<20260507213721.2137448-4-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]