[{"id":35191,"web_url":"https://patchwork.libcamera.org/comment/35191/","msgid":"","date":"2025-07-28T10:55:25","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote:\n> From: Jacopo Mondi \n>\n> Use the newly introduced `metadataAvailable()` function to send metadata\n> items to the application.\n>\n> Signed-off-by: Jacopo Mondi \n> [Adjust commit message, adjust rpi changes.]\n> Signed-off-by: Barnabás Pőcze \n> ---\n> changes in v2:\n> * include rpi changes as well\n> ---\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n> src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n> 9 files changed, 32 insertions(+), 37 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index e452fe2f8..579847367 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n> \tRequest *request = buffer->request();\n>\n> \t/* Record the sensor's timestamp in the request metadata. */\n> -\tControlList &metadata = request->metadata();\n> -\tif (!metadata.contains(controls::SensorTimestamp.id()))\n> -\t\tmetadata.set(controls::SensorTimestamp,\n> -\t\t\t buffer->metadata().timestamp);\n> +\tmetadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>\n> \tcompleteBuffer(request, buffer);\n> \tif (request->hasPendingBuffers())\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index e6a7f675a..0d310b5b4 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n> \t\treturn;\n>\n> \tRequest *request = info->request;\n> -\trequest->metadata().merge(metadata);\n> +\tpipe()->metadataAvailable(request, metadata);\n>\n> \tinfo->metadataProcessed = true;\n> \tif (frameInfos_.tryComplete(info))\n> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>\n> \tpipe()->completeBuffer(request, buffer);\n>\n> -\trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> +\tpipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n> +\n> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n> \tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> \tif (scalerCrop)\n> \t\tcropRegion_ = *scalerCrop;\n> -\trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> +\n> +\tpipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n>\n> \tif (frameInfos_.tryComplete(info))\n> \t\tpipe()->completeRequest(request);\n> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> \t * \\todo The sensor timestamp should be better estimated by connecting\n> \t * to the V4L2Device::frameStart signal.\n> \t */\n> -\trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\tbuffer->metadata().timestamp);\n> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>\n> \tinfo->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n>\n> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \t\treturn;\n> \t}\n>\n> -\trequest->metadata().set(controls::draft::TestPatternMode,\n> -\t\t\t\t*testPatternMode);\n> +\tpipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n> }\n>\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 19980f6d2..d1a107629 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n> \tMaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n>\n> \tframeInfo.statsDone = true;\n> -\tframeInfo.request->metadata().merge(metadata);\n> +\tmetadataAvailable(frameInfo.request, metadata);\n>\n> \ttryComplete(&frameInfo);\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6dce1844d..47ef52699 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n> \tif (!info)\n> \t\treturn;\n>\n> -\tinfo->request->metadata().merge(metadata);\n> +\tpipe()->metadataAvailable(info->request, metadata);\n> \tinfo->metadataProcessed = true;\n>\n> \tpipe()->tryCompleteRequest(info);\n> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> \t\t * \\todo The sensor timestamp should be better estimated by connecting\n> \t\t * to the V4L2Device::frameStart signal.\n> \t\t */\n> -\t\trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\t\tmetadata.timestamp);\n> +\t\tmetadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n>\n> \t\tif (isRaw_) {\n> \t\t\tconst ControlList &ctrls =\n> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> \t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> \t\t\t\t << strerror(-ret);\n>\n> -\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> +\tmetadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n> }\n>\n> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index f2a9a15cd..1d497f6f0 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n> \t/* Add to the Request metadata buffer what the IPA has provided. */\n> \t/* Last thing to do is to fill up the request metadata. */\n> \tRequest *request = requestQueue_.front();\n> -\trequest->metadata().merge(metadata);\n> +\tpipe()->metadataAvailable(request, metadata);\n>\n> \t/*\n> \t * Inform the sensor of the latest colour gains if it has the\n> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n>\n> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n> {\n> -\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> -\t\trequest->metadata().set(controls::SensorTimestamp, *x);\n> -\tif (auto x = bufferControls.get(controls::FrameWallClock))\n> -\t\trequest->metadata().set(controls::FrameWallClock, *x);\n> +\tpipe()->metadataAvailable(request, [&](auto set) {\n> +\t\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> +\t\t\tset(controls::SensorTimestamp, *x);\n> +\t\tif (auto x = bufferControls.get(controls::FrameWallClock))\n> +\t\t\tset(controls::FrameWallClock, *x);\n>\n\nAh here you go\n\nI really wonder if the additional complexity is justified as this\ncould simply be\n\n\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n\t\tmetadataAvailable(request, controls::SensorTimestamp, *x);\n\tif (auto x = bufferControls.get(controls::FrameWallClock))\n\t\tmetadataAvailable.set(request, controls::FrameWallClock, *x);\n\nOr have I missed something ?\n\n> -\tif (cropParams_.size()) {\n> -\t\tstd::vector crops;\n> +\t\tif (cropParams_.size()) {\n> +\t\t\tstd::vector crops;\n>\n> -\t\tfor (auto const &[k, v] : cropParams_)\n> -\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n> +\t\t\tfor (auto const &[k, v] : cropParams_)\n> +\t\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n>\n> -\t\trequest->metadata().set(controls::ScalerCrop, crops[0]);\n> -\t\tif (crops.size() > 1) {\n> -\t\t\trequest->metadata().set(controls::rpi::ScalerCrops,\n> -\t\t\t\t\t\tSpan(crops.data(), crops.size()));\n> +\t\t\tset(controls::ScalerCrop, crops[0]);\n> +\n> +\t\t\tif (crops.size() > 1)\n> +\t\t\t\tset(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n> \t\t}\n> -\t}\n> +\t});\n> }\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 05387ca7c..28399cb67 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t}\n>\n> \tif (request)\n> -\t\trequest->metadata().set(controls::SensorTimestamp,\n> +\t\tpipe->metadataAvailable(request, controls::SensorTimestamp,\n> \t\t\t\t\tbuffer->metadata().timestamp);\n>\n> \t/*\n> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n> \tif (!info)\n> \t\treturn;\n>\n> -\tinfo->request->metadata().merge(metadata);\n> +\tpipe()->metadataAvailable(info->request, metadata);\n> \tinfo->metadataProcessed = true;\n> \ttryCompleteRequest(info->request);\n> }\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 59fb4bd5c..8cea94721 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n> \tRequest *request = buffer->request();\n>\n> \t/* \\todo Use the UVC metadata to calculate a more precise timestamp */\n> -\trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\tbuffer->metadata().timestamp);\n> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>\n> \tpipe()->completeBuffer(request, buffer);\n> \tpipe()->completeRequest(request);\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 9d73e320c..4432de4a0 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t}\n>\n> \t/* Record the sensor's timestamp in the request metadata. */\n> -\trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\tbuffer->metadata().timestamp);\n> +\tpipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>\n> \tpipe->completeBuffer(request, buffer);\n> \tpipe->completeRequest(request);\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 049ebcba5..93c1733d6 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n> \t\tASSERT(found);\n> \t}\n>\n> -\trequest->metadata().set(controls::SensorTimestamp, timestamp);\n> +\tmetadataAvailable(request, controls::SensorTimestamp, timestamp);\n> \tcompleteRequest(request);\n>\n> \treturn 0;\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> --\n> 2.50.1\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 26E95BDCC1\n\tfor ;\n\tMon, 28 Jul 2025 10:55:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2057169156;\n\tMon, 28 Jul 2025 12:55:32 +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 214DB6904C\n\tfor ;\n\tMon, 28 Jul 2025 12:55:30 +0200 (CEST)","from ideasonboard.com (mob-5-90-139-29.net.vodafone.it\n\t[5.90.139.29])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 751A9446;\n\tMon, 28 Jul 2025 12:54:47 +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=\"S1Dug8GY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753700088;\n\tbh=VkHOqlzweUHxX4jq1ALpDvanM5ae4WkD0aDeDnvJ98c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=S1Dug8GYNX4K/o6Rwc/2P8LlDBnbve36BLIlbP6L4SC4McB9mOBP9b6mtjtH2ervW\n\tuyW5iGQ3bGwjej0NIxMuyg//ZIxkpZkCFmdOGbzNOs/432YRK7uEJ5hODjniph2yuQ\n\tmLYDYyOyHAoHofq2fj3hfwFXdFjFHQoZpNFnQf/o=","Date":"Mon, 28 Jul 2025 12:55:25 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tJacopo Mondi ","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","Message-ID":"","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250721104622.1550908-22-barnabas.pocze@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":35194,"web_url":"https://patchwork.libcamera.org/comment/35194/","msgid":"","date":"2025-07-28T11:57:43","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote:\n>> From: Jacopo Mondi \n>>\n>> Use the newly introduced `metadataAvailable()` function to send metadata\n>> items to the application.\n>>\n>> Signed-off-by: Jacopo Mondi \n>> [Adjust commit message, adjust rpi changes.]\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> changes in v2:\n>> * include rpi changes as well\n>> ---\n>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n>> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n>> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n>> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>> src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n>> 9 files changed, 32 insertions(+), 37 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> index e452fe2f8..579847367 100644\n>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n>> \tRequest *request = buffer->request();\n>>\n>> \t/* Record the sensor's timestamp in the request metadata. */\n>> -\tControlList &metadata = request->metadata();\n>> -\tif (!metadata.contains(controls::SensorTimestamp.id()))\n>> -\t\tmetadata.set(controls::SensorTimestamp,\n>> -\t\t\t buffer->metadata().timestamp);\n>> +\tmetadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> \tcompleteBuffer(request, buffer);\n>> \tif (request->hasPendingBuffers())\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index e6a7f675a..0d310b5b4 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n>> \t\treturn;\n>>\n>> \tRequest *request = info->request;\n>> -\trequest->metadata().merge(metadata);\n>> +\tpipe()->metadataAvailable(request, metadata);\n>>\n>> \tinfo->metadataProcessed = true;\n>> \tif (frameInfos_.tryComplete(info))\n>> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>>\n>> \tpipe()->completeBuffer(request, buffer);\n>>\n>> -\trequest->metadata().set(controls::draft::PipelineDepth, 3);\n>> +\tpipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n>> +\n>> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n>> \tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n>> \tif (scalerCrop)\n>> \t\tcropRegion_ = *scalerCrop;\n>> -\trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>> +\n>> +\tpipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n>>\n>> \tif (frameInfos_.tryComplete(info))\n>> \t\tpipe()->completeRequest(request);\n>> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>> \t * \\todo The sensor timestamp should be better estimated by connecting\n>> \t * to the V4L2Device::frameStart signal.\n>> \t */\n>> -\trequest->metadata().set(controls::SensorTimestamp,\n>> -\t\t\t\tbuffer->metadata().timestamp);\n>> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> \tinfo->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n>>\n>> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n>> \t\treturn;\n>> \t}\n>>\n>> -\trequest->metadata().set(controls::draft::TestPatternMode,\n>> -\t\t\t\t*testPatternMode);\n>> +\tpipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n>> }\n>>\n>> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> index 19980f6d2..d1a107629 100644\n>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n>> \tMaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n>>\n>> \tframeInfo.statsDone = true;\n>> -\tframeInfo.request->metadata().merge(metadata);\n>> +\tmetadataAvailable(frameInfo.request, metadata);\n>>\n>> \ttryComplete(&frameInfo);\n>> }\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 6dce1844d..47ef52699 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n>> \tif (!info)\n>> \t\treturn;\n>>\n>> -\tinfo->request->metadata().merge(metadata);\n>> +\tpipe()->metadataAvailable(info->request, metadata);\n>> \tinfo->metadataProcessed = true;\n>>\n>> \tpipe()->tryCompleteRequest(info);\n>> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>> \t\t * \\todo The sensor timestamp should be better estimated by connecting\n>> \t\t * to the V4L2Device::frameStart signal.\n>> \t\t */\n>> -\t\trequest->metadata().set(controls::SensorTimestamp,\n>> -\t\t\t\t\tmetadata.timestamp);\n>> +\t\tmetadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n>>\n>> \t\tif (isRaw_) {\n>> \t\t\tconst ControlList &ctrls =\n>> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>> \t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>> \t\t\t\t << strerror(-ret);\n>>\n>> -\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n>> +\tmetadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n>> }\n>>\n>> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> index f2a9a15cd..1d497f6f0 100644\n>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n>> \t/* Add to the Request metadata buffer what the IPA has provided. */\n>> \t/* Last thing to do is to fill up the request metadata. */\n>> \tRequest *request = requestQueue_.front();\n>> -\trequest->metadata().merge(metadata);\n>> +\tpipe()->metadataAvailable(request, metadata);\n>>\n>> \t/*\n>> \t * Inform the sensor of the latest colour gains if it has the\n>> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n>>\n>> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n>> {\n>> -\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n>> -\t\trequest->metadata().set(controls::SensorTimestamp, *x);\n>> -\tif (auto x = bufferControls.get(controls::FrameWallClock))\n>> -\t\trequest->metadata().set(controls::FrameWallClock, *x);\n>> +\tpipe()->metadataAvailable(request, [&](auto set) {\n>> +\t\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n>> +\t\t\tset(controls::SensorTimestamp, *x);\n>> +\t\tif (auto x = bufferControls.get(controls::FrameWallClock))\n>> +\t\t\tset(controls::FrameWallClock, *x);\n>>\n> \n> Ah here you go\n> \n> I really wonder if the additional complexity is justified as this\n> could simply be\n> \n> \tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> \t\tmetadataAvailable(request, controls::SensorTimestamp, *x);\n> \tif (auto x = bufferControls.get(controls::FrameWallClock))\n> \t\tmetadataAvailable.set(request, controls::FrameWallClock, *x);\n> \n> Or have I missed something ?\n\nThat is what it was in the previous version. I have changed it because I found\nit to be suboptimal that potentially 4 metadata items are reported in quick succession,\nmeaning 4 signal emission, and 4 calls to the application's handler. I would really\nlike to avoid doing that.\n\nAnother option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()`\nwith that, but I find that that is just unnecessary runtime overhead.\n\nSo the approach that I wanted to propose here is having an overload that takes\nan invokable object, which can implement arbitrary logic for setting controls\nhowever it wishes, and when it returns, everything that it has set will be\nreported in a single signal emission.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n>> -\tif (cropParams_.size()) {\n>> -\t\tstd::vector crops;\n>> +\t\tif (cropParams_.size()) {\n>> +\t\t\tstd::vector crops;\n>>\n>> -\t\tfor (auto const &[k, v] : cropParams_)\n>> -\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n>> +\t\t\tfor (auto const &[k, v] : cropParams_)\n>> +\t\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n>>\n>> -\t\trequest->metadata().set(controls::ScalerCrop, crops[0]);\n>> -\t\tif (crops.size() > 1) {\n>> -\t\t\trequest->metadata().set(controls::rpi::ScalerCrops,\n>> -\t\t\t\t\t\tSpan(crops.data(), crops.size()));\n>> +\t\t\tset(controls::ScalerCrop, crops[0]);\n>> +\n>> +\t\t\tif (crops.size() > 1)\n>> +\t\t\t\tset(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n>> \t\t}\n>> -\t}\n>> +\t});\n>> }\n>>\n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 05387ca7c..28399cb67 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t}\n>>\n>> \tif (request)\n>> -\t\trequest->metadata().set(controls::SensorTimestamp,\n>> +\t\tpipe->metadataAvailable(request, controls::SensorTimestamp,\n>> \t\t\t\t\tbuffer->metadata().timestamp);\n>>\n>> \t/*\n>> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n>> \tif (!info)\n>> \t\treturn;\n>>\n>> -\tinfo->request->metadata().merge(metadata);\n>> +\tpipe()->metadataAvailable(info->request, metadata);\n>> \tinfo->metadataProcessed = true;\n>> \ttryCompleteRequest(info->request);\n>> }\n>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> index 59fb4bd5c..8cea94721 100644\n>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \tRequest *request = buffer->request();\n>>\n>> \t/* \\todo Use the UVC metadata to calculate a more precise timestamp */\n>> -\trequest->metadata().set(controls::SensorTimestamp,\n>> -\t\t\t\tbuffer->metadata().timestamp);\n>> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> \tpipe()->completeBuffer(request, buffer);\n>> \tpipe()->completeRequest(request);\n>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>> index 9d73e320c..4432de4a0 100644\n>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t}\n>>\n>> \t/* Record the sensor's timestamp in the request metadata. */\n>> -\trequest->metadata().set(controls::SensorTimestamp,\n>> -\t\t\t\tbuffer->metadata().timestamp);\n>> +\tpipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> \tpipe->completeBuffer(request, buffer);\n>> \tpipe->completeRequest(request);\n>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n>> index 049ebcba5..93c1733d6 100644\n>> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n>> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n>> \t\tASSERT(found);\n>> \t}\n>>\n>> -\trequest->metadata().set(controls::SensorTimestamp, timestamp);\n>> +\tmetadataAvailable(request, controls::SensorTimestamp, timestamp);\n>> \tcompleteRequest(request);\n>>\n>> \treturn 0;\n> \n> Reviewed-by: Jacopo Mondi \n> \n> Thanks\n> j\n> \n>> --\n>> 2.50.1\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 EC2C6BDCC1\n\tfor ;\n\tMon, 28 Jul 2025 11:57:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AFF169181;\n\tMon, 28 Jul 2025 13:57:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E714969158\n\tfor ;\n\tMon, 28 Jul 2025 13:57:46 +0200 (CEST)","from [192.168.33.18] (185.221.140.39.nat.pool.zt.hu\n\t[185.221.140.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0780A379;\n\tMon, 28 Jul 2025 13:57:04 +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=\"UBJY4eT+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753703825;\n\tbh=SG8O4m+LTBe3Iy7QlB/1dHHtQnau/SkfPq6f4UAiMfE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=UBJY4eT+nB49lz8FQYi56qlmJ81vVt0vr1aOftBJjuFTGOZvyItItuVIUDS/MRvct\n\tZynOV/i6u5wHYQgF8apOi/rMixlyfYdtUJM/0Q6aARbPsZq2UB/S9cGt5hr9FilYue\n\t2BaYbP/9iA32T7O+8Dxm8VyEjuk2lHgE8wh9dwcM=","Message-ID":"","Date":"Mon, 28 Jul 2025 13:57:43 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":35197,"web_url":"https://patchwork.libcamera.org/comment/35197/","msgid":"","date":"2025-07-28T12:30:48","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Jul 28, 2025 at 01:57:43PM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote:\n> > > From: Jacopo Mondi \n> > >\n> > > Use the newly introduced `metadataAvailable()` function to send metadata\n> > > items to the application.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > [Adjust commit message, adjust rpi changes.]\n> > > Signed-off-by: Barnabás Pőcze \n> > > ---\n> > > changes in v2:\n> > > * include rpi changes as well\n> > > ---\n> > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n> > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n> > > .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n> > > src/libcamera/pipeline/simple/simple.cpp | 4 +--\n> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n> > > src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n> > > src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n> > > 9 files changed, 32 insertions(+), 37 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > index e452fe2f8..579847367 100644\n> > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n> > > \tRequest *request = buffer->request();\n> > >\n> > > \t/* Record the sensor's timestamp in the request metadata. */\n> > > -\tControlList &metadata = request->metadata();\n> > > -\tif (!metadata.contains(controls::SensorTimestamp.id()))\n> > > -\t\tmetadata.set(controls::SensorTimestamp,\n> > > -\t\t\t buffer->metadata().timestamp);\n> > > +\tmetadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> > >\n> > > \tcompleteBuffer(request, buffer);\n> > > \tif (request->hasPendingBuffers())\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index e6a7f675a..0d310b5b4 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n> > > \t\treturn;\n> > >\n> > > \tRequest *request = info->request;\n> > > -\trequest->metadata().merge(metadata);\n> > > +\tpipe()->metadataAvailable(request, metadata);\n> > >\n> > > \tinfo->metadataProcessed = true;\n> > > \tif (frameInfos_.tryComplete(info))\n> > > @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > >\n> > > \tpipe()->completeBuffer(request, buffer);\n> > >\n> > > -\trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> > > +\tpipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n> > > +\n> > > \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n> > > \tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> > > \tif (scalerCrop)\n> > > \t\tcropRegion_ = *scalerCrop;\n> > > -\trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> > > +\n> > > +\tpipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n> > >\n> > > \tif (frameInfos_.tryComplete(info))\n> > > \t\tpipe()->completeRequest(request);\n> > > @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > > \t * \\todo The sensor timestamp should be better estimated by connecting\n> > > \t * to the V4L2Device::frameStart signal.\n> > > \t */\n> > > -\trequest->metadata().set(controls::SensorTimestamp,\n> > > -\t\t\t\tbuffer->metadata().timestamp);\n> > > +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> > >\n> > > \tinfo->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n> > >\n> > > @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> > > \t\treturn;\n> > > \t}\n> > >\n> > > -\trequest->metadata().set(controls::draft::TestPatternMode,\n> > > -\t\t\t\t*testPatternMode);\n> > > +\tpipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n> > > }\n> > >\n> > > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n> > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > index 19980f6d2..d1a107629 100644\n> > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n> > > \tMaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n> > >\n> > > \tframeInfo.statsDone = true;\n> > > -\tframeInfo.request->metadata().merge(metadata);\n> > > +\tmetadataAvailable(frameInfo.request, metadata);\n> > >\n> > > \ttryComplete(&frameInfo);\n> > > }\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 6dce1844d..47ef52699 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n> > > \tif (!info)\n> > > \t\treturn;\n> > >\n> > > -\tinfo->request->metadata().merge(metadata);\n> > > +\tpipe()->metadataAvailable(info->request, metadata);\n> > > \tinfo->metadataProcessed = true;\n> > >\n> > > \tpipe()->tryCompleteRequest(info);\n> > > @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> > > \t\t * \\todo The sensor timestamp should be better estimated by connecting\n> > > \t\t * to the V4L2Device::frameStart signal.\n> > > \t\t */\n> > > -\t\trequest->metadata().set(controls::SensorTimestamp,\n> > > -\t\t\t\t\tmetadata.timestamp);\n> > > +\t\tmetadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n> > >\n> > > \t\tif (isRaw_) {\n> > > \t\t\tconst ControlList &ctrls =\n> > > @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> > > \t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> > > \t\t\t\t << strerror(-ret);\n> > >\n> > > -\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> > > +\tmetadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n> > > }\n> > >\n> > > void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index f2a9a15cd..1d497f6f0 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n> > > \t/* Add to the Request metadata buffer what the IPA has provided. */\n> > > \t/* Last thing to do is to fill up the request metadata. */\n> > > \tRequest *request = requestQueue_.front();\n> > > -\trequest->metadata().merge(metadata);\n> > > +\tpipe()->metadataAvailable(request, metadata);\n> > >\n> > > \t/*\n> > > \t * Inform the sensor of the latest colour gains if it has the\n> > > @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n> > >\n> > > void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n> > > {\n> > > -\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> > > -\t\trequest->metadata().set(controls::SensorTimestamp, *x);\n> > > -\tif (auto x = bufferControls.get(controls::FrameWallClock))\n> > > -\t\trequest->metadata().set(controls::FrameWallClock, *x);\n> > > +\tpipe()->metadataAvailable(request, [&](auto set) {\n> > > +\t\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> > > +\t\t\tset(controls::SensorTimestamp, *x);\n> > > +\t\tif (auto x = bufferControls.get(controls::FrameWallClock))\n> > > +\t\t\tset(controls::FrameWallClock, *x);\n> > >\n> >\n> > Ah here you go\n> >\n> > I really wonder if the additional complexity is justified as this\n> > could simply be\n> >\n> > \tif (auto x = bufferControls.get(controls::SensorTimestamp))\n> > \t\tmetadataAvailable(request, controls::SensorTimestamp, *x);\n> > \tif (auto x = bufferControls.get(controls::FrameWallClock))\n> > \t\tmetadataAvailable.set(request, controls::FrameWallClock, *x);\n> >\n> > Or have I missed something ?\n>\n> That is what it was in the previous version. I have changed it because I found\n> it to be suboptimal that potentially 4 metadata items are reported in quick succession,\n> meaning 4 signal emission, and 4 calls to the application's handler. I would really\n> like to avoid doing that.\n\nAck, thanks for clarifying\n\nHowever, what confuses me is that this function already receives a\nControlList and the above two lines, if I'm not mistaken are\nequivalent to\n\n metadataAvailable.set(request, bufferControls);\n\n>\n> Another option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()`\n> with that, but I find that that is just unnecessary runtime overhead.\n>\n> So the approach that I wanted to propose here is having an overload that takes\n> an invokable object, which can implement arbitrary logic for setting controls\n> however it wishes, and when it returns, everything that it has set will be\n> reported in a single signal emission.\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n> >\n> > > -\tif (cropParams_.size()) {\n> > > -\t\tstd::vector crops;\n> > > +\t\tif (cropParams_.size()) {\n> > > +\t\t\tstd::vector crops;\n> > >\n> > > -\t\tfor (auto const &[k, v] : cropParams_)\n> > > -\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n> > > +\t\t\tfor (auto const &[k, v] : cropParams_)\n> > > +\t\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n> > >\n> > > -\t\trequest->metadata().set(controls::ScalerCrop, crops[0]);\n> > > -\t\tif (crops.size() > 1) {\n> > > -\t\t\trequest->metadata().set(controls::rpi::ScalerCrops,\n> > > -\t\t\t\t\t\tSpan(crops.data(), crops.size()));\n> > > +\t\t\tset(controls::ScalerCrop, crops[0]);\n> > > +\n> > > +\t\t\tif (crops.size() > 1)\n> > > +\t\t\t\tset(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n> > > \t\t}\n\nBut yes, emitting a single Camera::metadataAvailable for all the\ncontrols handled here would require creating a single control list,\ncopying all of them in that list and then calling metadataAvailable().\n\nI wonder if we shouldn't split the population of the metadata list\nfrom the emission of the signal and give pipelines control over it,\nwhich is basically what you're doing it by providing to pipelines a\nMetadataSetter they can use to populate the list without emitting the\nsignal...\n\n\n\n> > > -\t}\n> > > +\t});\n> > > }\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 05387ca7c..28399cb67 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > > \t}\n> > >\n> > > \tif (request)\n> > > -\t\trequest->metadata().set(controls::SensorTimestamp,\n> > > +\t\tpipe->metadataAvailable(request, controls::SensorTimestamp,\n> > > \t\t\t\t\tbuffer->metadata().timestamp);\n> > >\n> > > \t/*\n> > > @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n> > > \tif (!info)\n> > > \t\treturn;\n> > >\n> > > -\tinfo->request->metadata().merge(metadata);\n> > > +\tpipe()->metadataAvailable(info->request, metadata);\n> > > \tinfo->metadataProcessed = true;\n> > > \ttryCompleteRequest(info->request);\n> > > }\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 59fb4bd5c..8cea94721 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n> > > \tRequest *request = buffer->request();\n> > >\n> > > \t/* \\todo Use the UVC metadata to calculate a more precise timestamp */\n> > > -\trequest->metadata().set(controls::SensorTimestamp,\n> > > -\t\t\t\tbuffer->metadata().timestamp);\n> > > +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> > >\n> > > \tpipe()->completeBuffer(request, buffer);\n> > > \tpipe()->completeRequest(request);\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index 9d73e320c..4432de4a0 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n> > > \t}\n> > >\n> > > \t/* Record the sensor's timestamp in the request metadata. */\n> > > -\trequest->metadata().set(controls::SensorTimestamp,\n> > > -\t\t\t\tbuffer->metadata().timestamp);\n> > > +\tpipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> > >\n> > > \tpipe->completeBuffer(request, buffer);\n> > > \tpipe->completeRequest(request);\n> > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> > > index 049ebcba5..93c1733d6 100644\n> > > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > > @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n> > > \t\tASSERT(found);\n> > > \t}\n> > >\n> > > -\trequest->metadata().set(controls::SensorTimestamp, timestamp);\n> > > +\tmetadataAvailable(request, controls::SensorTimestamp, timestamp);\n> > > \tcompleteRequest(request);\n> > >\n> > > \treturn 0;\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n> >\n> > > --\n> > > 2.50.1\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 8E6D0BDCC1\n\tfor ;\n\tMon, 28 Jul 2025 12:30:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A691869164;\n\tMon, 28 Jul 2025 14:30:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45B2D69158\n\tfor ;\n\tMon, 28 Jul 2025 14:30:57 +0200 (CEST)","from ideasonboard.com (mob-5-90-139-29.net.vodafone.it\n\t[5.90.139.29])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F149F446;\n\tMon, 28 Jul 2025 14:30:13 +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=\"tXCa6+im\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753705815;\n\tbh=Tge+8YhGR1IbBx0XzC1Qv3bqPnVdIkuq6E/3tPc6gSg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tXCa6+im0xiUAiY4JrAjUMgLbqfjPYtRDYluMY91ipHJ5+geQ4HYibQrCU3t651Br\n\tT4IeQVy86TvowEjflAQCNw9diik0Up6SiHhuIH+rRhuPZDITP757kudqxHEviHej9X\n\tb63PCcbmsc8PccpfokAVbEF+46qoDCTYs1LDyu9k=","Date":"Mon, 28 Jul 2025 14:30:48 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","Message-ID":"","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","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\" "}},{"id":35254,"web_url":"https://patchwork.libcamera.org/comment/35254/","msgid":"<5890ae0a-9213-42f2-9aec-20ac12e19e1a@ideasonboard.com>","date":"2025-07-30T14:50:10","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 07. 28. 14:30 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Jul 28, 2025 at 01:57:43PM +0200, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote:\n>>>> From: Jacopo Mondi \n>>>>\n>>>> Use the newly introduced `metadataAvailable()` function to send metadata\n>>>> items to the application.\n>>>>\n>>>> Signed-off-by: Jacopo Mondi \n>>>> [Adjust commit message, adjust rpi changes.]\n>>>> Signed-off-by: Barnabás Pőcze \n>>>> ---\n>>>> changes in v2:\n>>>> * include rpi changes as well\n>>>> ---\n>>>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n>>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n>>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n>>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n>>>> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n>>>> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n>>>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n>>>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>>>> src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n>>>> 9 files changed, 32 insertions(+), 37 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>>> index e452fe2f8..579847367 100644\n>>>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>>>> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n>>>> \tRequest *request = buffer->request();\n>>>>\n>>>> \t/* Record the sensor's timestamp in the request metadata. */\n>>>> -\tControlList &metadata = request->metadata();\n>>>> -\tif (!metadata.contains(controls::SensorTimestamp.id()))\n>>>> -\t\tmetadata.set(controls::SensorTimestamp,\n>>>> -\t\t\t buffer->metadata().timestamp);\n>>>> +\tmetadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>>>\n>>>> \tcompleteBuffer(request, buffer);\n>>>> \tif (request->hasPendingBuffers())\n>>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> index e6a7f675a..0d310b5b4 100644\n>>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n>>>> \t\treturn;\n>>>>\n>>>> \tRequest *request = info->request;\n>>>> -\trequest->metadata().merge(metadata);\n>>>> +\tpipe()->metadataAvailable(request, metadata);\n>>>>\n>>>> \tinfo->metadataProcessed = true;\n>>>> \tif (frameInfos_.tryComplete(info))\n>>>> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>>>>\n>>>> \tpipe()->completeBuffer(request, buffer);\n>>>>\n>>>> -\trequest->metadata().set(controls::draft::PipelineDepth, 3);\n>>>> +\tpipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n>>>> +\n>>>> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n>>>> \tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n>>>> \tif (scalerCrop)\n>>>> \t\tcropRegion_ = *scalerCrop;\n>>>> -\trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>>>> +\n>>>> +\tpipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n>>>>\n>>>> \tif (frameInfos_.tryComplete(info))\n>>>> \t\tpipe()->completeRequest(request);\n>>>> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>>>> \t * \\todo The sensor timestamp should be better estimated by connecting\n>>>> \t * to the V4L2Device::frameStart signal.\n>>>> \t */\n>>>> -\trequest->metadata().set(controls::SensorTimestamp,\n>>>> -\t\t\t\tbuffer->metadata().timestamp);\n>>>> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>>>\n>>>> \tinfo->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n>>>>\n>>>> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n>>>> \t\treturn;\n>>>> \t}\n>>>>\n>>>> -\trequest->metadata().set(controls::draft::TestPatternMode,\n>>>> -\t\t\t\t*testPatternMode);\n>>>> +\tpipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n>>>> }\n>>>>\n>>>> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n>>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> index 19980f6d2..d1a107629 100644\n>>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n>>>> \tMaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n>>>>\n>>>> \tframeInfo.statsDone = true;\n>>>> -\tframeInfo.request->metadata().merge(metadata);\n>>>> +\tmetadataAvailable(frameInfo.request, metadata);\n>>>>\n>>>> \ttryComplete(&frameInfo);\n>>>> }\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> index 6dce1844d..47ef52699 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n>>>> \tif (!info)\n>>>> \t\treturn;\n>>>>\n>>>> -\tinfo->request->metadata().merge(metadata);\n>>>> +\tpipe()->metadataAvailable(info->request, metadata);\n>>>> \tinfo->metadataProcessed = true;\n>>>>\n>>>> \tpipe()->tryCompleteRequest(info);\n>>>> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t * \\todo The sensor timestamp should be better estimated by connecting\n>>>> \t\t * to the V4L2Device::frameStart signal.\n>>>> \t\t */\n>>>> -\t\trequest->metadata().set(controls::SensorTimestamp,\n>>>> -\t\t\t\t\tmetadata.timestamp);\n>>>> +\t\tmetadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n>>>>\n>>>> \t\tif (isRaw_) {\n>>>> \t\t\tconst ControlList &ctrls =\n>>>> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>>>> \t\t\t\t << strerror(-ret);\n>>>>\n>>>> -\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n>>>> +\tmetadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n>>>> }\n>>>>\n>>>> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n>>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> index f2a9a15cd..1d497f6f0 100644\n>>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>>>> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n>>>> \t/* Add to the Request metadata buffer what the IPA has provided. */\n>>>> \t/* Last thing to do is to fill up the request metadata. */\n>>>> \tRequest *request = requestQueue_.front();\n>>>> -\trequest->metadata().merge(metadata);\n>>>> +\tpipe()->metadataAvailable(request, metadata);\n>>>>\n>>>> \t/*\n>>>> \t * Inform the sensor of the latest colour gains if it has the\n>>>> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n>>>>\n>>>> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n>>>> {\n>>>> -\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n>>>> -\t\trequest->metadata().set(controls::SensorTimestamp, *x);\n>>>> -\tif (auto x = bufferControls.get(controls::FrameWallClock))\n>>>> -\t\trequest->metadata().set(controls::FrameWallClock, *x);\n>>>> +\tpipe()->metadataAvailable(request, [&](auto set) {\n>>>> +\t\tif (auto x = bufferControls.get(controls::SensorTimestamp))\n>>>> +\t\t\tset(controls::SensorTimestamp, *x);\n>>>> +\t\tif (auto x = bufferControls.get(controls::FrameWallClock))\n>>>> +\t\t\tset(controls::FrameWallClock, *x);\n>>>>\n>>>\n>>> Ah here you go\n>>>\n>>> I really wonder if the additional complexity is justified as this\n>>> could simply be\n>>>\n>>> \tif (auto x = bufferControls.get(controls::SensorTimestamp))\n>>> \t\tmetadataAvailable(request, controls::SensorTimestamp, *x);\n>>> \tif (auto x = bufferControls.get(controls::FrameWallClock))\n>>> \t\tmetadataAvailable.set(request, controls::FrameWallClock, *x);\n>>>\n>>> Or have I missed something ?\n>>\n>> That is what it was in the previous version. I have changed it because I found\n>> it to be suboptimal that potentially 4 metadata items are reported in quick succession,\n>> meaning 4 signal emission, and 4 calls to the application's handler. I would really\n>> like to avoid doing that.\n> \n> Ack, thanks for clarifying\n> \n> However, what confuses me is that this function already receives a\n> ControlList and the above two lines, if I'm not mistaken are\n> equivalent to\n> \n> metadataAvailable.set(request, bufferControls);\n> \n>>\n>> Another option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()`\n>> with that, but I find that that is just unnecessary runtime overhead.\n>>\n>> So the approach that I wanted to propose here is having an overload that takes\n>> an invokable object, which can implement arbitrary logic for setting controls\n>> however it wishes, and when it returns, everything that it has set will be\n>> reported in a single signal emission.\n>>\n>>\n>> Regards,\n>> Barnabás Pőcze\n>>\n>>\n>>>\n>>>> -\tif (cropParams_.size()) {\n>>>> -\t\tstd::vector crops;\n>>>> +\t\tif (cropParams_.size()) {\n>>>> +\t\t\tstd::vector crops;\n>>>>\n>>>> -\t\tfor (auto const &[k, v] : cropParams_)\n>>>> -\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n>>>> +\t\t\tfor (auto const &[k, v] : cropParams_)\n>>>> +\t\t\t\tcrops.push_back(scaleIspCrop(v.ispCrop));\n>>>>\n>>>> -\t\trequest->metadata().set(controls::ScalerCrop, crops[0]);\n>>>> -\t\tif (crops.size() > 1) {\n>>>> -\t\t\trequest->metadata().set(controls::rpi::ScalerCrops,\n>>>> -\t\t\t\t\t\tSpan(crops.data(), crops.size()));\n>>>> +\t\t\tset(controls::ScalerCrop, crops[0]);\n>>>> +\n>>>> +\t\t\tif (crops.size() > 1)\n>>>> +\t\t\t\tset(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n>>>> \t\t}\n> \n> But yes, emitting a single Camera::metadataAvailable for all the\n> controls handled here would require creating a single control list,\n> copying all of them in that list and then calling metadataAvailable().\n> \n> I wonder if we shouldn't split the population of the metadata list\n> from the emission of the signal and give pipelines control over it,\n> which is basically what you're doing it by providing to pipelines a\n> MetadataSetter they can use to populate the list without emitting the\n> signal...\n\nYes, that's the idea. I believe this \"lambda\" approach is much to misuse than\nproviding e.g. more separate objects where attention needs to be paid to lifetimes\nand calling the right methods at the right times, etc.\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> \n> \n>>>> -\t}\n>>>> +\t});\n>>>> }\n>>>>\n>>>> } /* namespace libcamera */\n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 05387ca7c..28399cb67 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t}\n>>>>\n>>>> \tif (request)\n>>>> -\t\trequest->metadata().set(controls::SensorTimestamp,\n>>>> +\t\tpipe->metadataAvailable(request, controls::SensorTimestamp,\n>>>> \t\t\t\t\tbuffer->metadata().timestamp);\n>>>>\n>>>> \t/*\n>>>> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n>>>> \tif (!info)\n>>>> \t\treturn;\n>>>>\n>>>> -\tinfo->request->metadata().merge(metadata);\n>>>> +\tpipe()->metadataAvailable(info->request, metadata);\n>>>> \tinfo->metadataProcessed = true;\n>>>> \ttryCompleteRequest(info->request);\n>>>> }\n>>>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>>> index 59fb4bd5c..8cea94721 100644\n>>>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>>>> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \tRequest *request = buffer->request();\n>>>>\n>>>> \t/* \\todo Use the UVC metadata to calculate a more precise timestamp */\n>>>> -\trequest->metadata().set(controls::SensorTimestamp,\n>>>> -\t\t\t\tbuffer->metadata().timestamp);\n>>>> +\tpipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>>>\n>>>> \tpipe()->completeBuffer(request, buffer);\n>>>> \tpipe()->completeRequest(request);\n>>>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> index 9d73e320c..4432de4a0 100644\n>>>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>>>> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t}\n>>>>\n>>>> \t/* Record the sensor's timestamp in the request metadata. */\n>>>> -\trequest->metadata().set(controls::SensorTimestamp,\n>>>> -\t\t\t\tbuffer->metadata().timestamp);\n>>>> +\tpipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>>>\n>>>> \tpipe->completeBuffer(request, buffer);\n>>>> \tpipe->completeRequest(request);\n>>>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n>>>> index 049ebcba5..93c1733d6 100644\n>>>> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n>>>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n>>>> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n>>>> \t\tASSERT(found);\n>>>> \t}\n>>>>\n>>>> -\trequest->metadata().set(controls::SensorTimestamp, timestamp);\n>>>> +\tmetadataAvailable(request, controls::SensorTimestamp, timestamp);\n>>>> \tcompleteRequest(request);\n>>>>\n>>>> \treturn 0;\n>>>\n>>> Reviewed-by: Jacopo Mondi \n>>>\n>>> Thanks\n>>> j\n>>>\n>>>> --\n>>>> 2.50.1\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 66057BDC71\n\tfor ;\n\tWed, 30 Jul 2025 14:50:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CEB36920B;\n\tWed, 30 Jul 2025 16:50:13 +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 EBF9E69207\n\tfor ;\n\tWed, 30 Jul 2025 16:50:11 +0200 (CEST)","from [192.168.33.11] (185.182.214.105.nat.pool.zt.hu\n\t[185.182.214.105])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7373EC7A;\n\tWed, 30 Jul 2025 16:49:27 +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=\"ra9L+eLE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753886968;\n\tbh=+ffMEsmWLAh0MkIjVA/3jFbC8KnUQUtp3bce//cHFew=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=ra9L+eLEudZZ2gr7JESZZLMqoKaPZymubx+f2tj6KL3vpJNOwXUXeFzCRPv+rkcQV\n\tuTA4vRgIXw2Gsc5YNTFAtRt6wCFQePiOOFdlmCHE3YoHjRxa4S3DusQR+OdEwzQU6/\n\tBN8jdAZNBlC7pmQ3SN77eqsnmpBm4torEf97bdEQ=","Message-ID":"<5890ae0a-9213-42f2-9aec-20ac12e19e1a@ideasonboard.com>","Date":"Wed, 30 Jul 2025 16:50:10 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>\n\t\n\t\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":35892,"web_url":"https://patchwork.libcamera.org/comment/35892/","msgid":"<175820066673.2127323.2752531515419494969@neptunite.rasen.tech>","date":"2025-09-18T13:04:26","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-07-21 19:46:21)\n> From: Jacopo Mondi \n> \n> Use the newly introduced `metadataAvailable()` function to send metadata\n> items to the application.\n> \n> Signed-off-by: Jacopo Mondi \n> [Adjust commit message, adjust rpi changes.]\n> Signed-off-by: Barnabás Pőcze \n> ---\n> changes in v2:\n> * include rpi changes as well\n> ---\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n> src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n> 9 files changed, 32 insertions(+), 37 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index e452fe2f8..579847367 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n> Request *request = buffer->request();\n> \n> /* Record the sensor's timestamp in the request metadata. */\n> - ControlList &metadata = request->metadata();\n> - if (!metadata.contains(controls::SensorTimestamp.id()))\n> - metadata.set(controls::SensorTimestamp,\n> - buffer->metadata().timestamp);\n> + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> \n> completeBuffer(request, buffer);\n> if (request->hasPendingBuffers())\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index e6a7f675a..0d310b5b4 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n> return;\n> \n> Request *request = info->request;\n> - request->metadata().merge(metadata);\n> + pipe()->metadataAvailable(request, metadata);\n> \n> info->metadataProcessed = true;\n> if (frameInfos_.tryComplete(info))\n> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \n> pipe()->completeBuffer(request, buffer);\n> \n> - request->metadata().set(controls::draft::PipelineDepth, 3);\n> + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n> +\n> /* \\todo Actually apply the scaler crop region to the ImgU. */\n> const auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> if (scalerCrop)\n> cropRegion_ = *scalerCrop;\n> - request->metadata().set(controls::ScalerCrop, cropRegion_);\n> +\n> + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n> \n> if (frameInfos_.tryComplete(info))\n> pipe()->completeRequest(request);\n> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> * \\todo The sensor timestamp should be better estimated by connecting\n> * to the V4L2Device::frameStart signal.\n> */\n> - request->metadata().set(controls::SensorTimestamp,\n> - buffer->metadata().timestamp);\n> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> \n> info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n> \n> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> return;\n> }\n> \n> - request->metadata().set(controls::draft::TestPatternMode,\n> - *testPatternMode);\n> + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 19980f6d2..d1a107629 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n> MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n> \n> frameInfo.statsDone = true;\n> - frameInfo.request->metadata().merge(metadata);\n> + metadataAvailable(frameInfo.request, metadata);\n> \n> tryComplete(&frameInfo);\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6dce1844d..47ef52699 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n> if (!info)\n> return;\n> \n> - info->request->metadata().merge(metadata);\n> + pipe()->metadataAvailable(info->request, metadata);\n> info->metadataProcessed = true;\n> \n> pipe()->tryCompleteRequest(info);\n> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> * \\todo The sensor timestamp should be better estimated by connecting\n> * to the V4L2Device::frameStart signal.\n> */\n> - request->metadata().set(controls::SensorTimestamp,\n> - metadata.timestamp);\n> + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n> \n> if (isRaw_) {\n> const ControlList &ctrls =\n> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n> LOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> << strerror(-ret);\n> \n> - request->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n> }\n> \n> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index f2a9a15cd..1d497f6f0 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n> /* Add to the Request metadata buffer what the IPA has provided. */\n> /* Last thing to do is to fill up the request metadata. */\n> Request *request = requestQueue_.front();\n> - request->metadata().merge(metadata);\n> + pipe()->metadataAvailable(request, metadata);\n> \n> /*\n> * Inform the sensor of the latest colour gains if it has the\n> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n> \n> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n> {\n> - if (auto x = bufferControls.get(controls::SensorTimestamp))\n> - request->metadata().set(controls::SensorTimestamp, *x);\n> - if (auto x = bufferControls.get(controls::FrameWallClock))\n> - request->metadata().set(controls::FrameWallClock, *x);\n> + pipe()->metadataAvailable(request, [&](auto set) {\n> + if (auto x = bufferControls.get(controls::SensorTimestamp))\n> + set(controls::SensorTimestamp, *x);\n> + if (auto x = bufferControls.get(controls::FrameWallClock))\n> + set(controls::FrameWallClock, *x);\n> \n> - if (cropParams_.size()) {\n> - std::vector crops;\n> + if (cropParams_.size()) {\n> + std::vector crops;\n> \n> - for (auto const &[k, v] : cropParams_)\n> - crops.push_back(scaleIspCrop(v.ispCrop));\n> + for (auto const &[k, v] : cropParams_)\n> + crops.push_back(scaleIspCrop(v.ispCrop));\n> \n> - request->metadata().set(controls::ScalerCrop, crops[0]);\n> - if (crops.size() > 1) {\n> - request->metadata().set(controls::rpi::ScalerCrops,\n> - Span(crops.data(), crops.size()));\n> + set(controls::ScalerCrop, crops[0]);\n> +\n> + if (crops.size() > 1)\n> + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n\nAm I paranoid for being worried that there's no check of crops.data().size\nagainst what we allocated in the MetadataListPlan? Or maybe it's fine because\nthe way that ScalerCrops is handled that'll never happen?\n\nEither way,\n\nReviewed-by: Paul Elder \n\n> }\n> - }\n> + });\n> }\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 05387ca7c..28399cb67 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> }\n> \n> if (request)\n> - request->metadata().set(controls::SensorTimestamp,\n> + pipe->metadataAvailable(request, controls::SensorTimestamp,\n> buffer->metadata().timestamp);\n> \n> /*\n> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n> if (!info)\n> return;\n> \n> - info->request->metadata().merge(metadata);\n> + pipe()->metadataAvailable(info->request, metadata);\n> info->metadataProcessed = true;\n> tryCompleteRequest(info->request);\n> }\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 59fb4bd5c..8cea94721 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n> Request *request = buffer->request();\n> \n> /* \\todo Use the UVC metadata to calculate a more precise timestamp */\n> - request->metadata().set(controls::SensorTimestamp,\n> - buffer->metadata().timestamp);\n> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> \n> pipe()->completeBuffer(request, buffer);\n> pipe()->completeRequest(request);\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 9d73e320c..4432de4a0 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n> }\n> \n> /* Record the sensor's timestamp in the request metadata. */\n> - request->metadata().set(controls::SensorTimestamp,\n> - buffer->metadata().timestamp);\n> + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n> \n> pipe->completeBuffer(request, buffer);\n> pipe->completeRequest(request);\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 049ebcba5..93c1733d6 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n> ASSERT(found);\n> }\n> \n> - request->metadata().set(controls::SensorTimestamp, timestamp);\n> + metadataAvailable(request, controls::SensorTimestamp, timestamp);\n> completeRequest(request);\n> \n> return 0;\n> -- \n> 2.50.1\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 D83AAC328C\n\tfor ;\n\tThu, 18 Sep 2025 13:04:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DC4B062C3B;\n\tThu, 18 Sep 2025 15:04:34 +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 E6E7B62C3B\n\tfor ;\n\tThu, 18 Sep 2025 15:04:32 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:7cf2:5f58:dd2a:9ec1])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id AD865842;\n\tThu, 18 Sep 2025 15:03:12 +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=\"gvndRBlh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758200593;\n\tbh=Wj/ySscWW5nc5sQt1oAhdz+Qti/0esmzn7DlMAQ9Yko=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gvndRBlhiS975U20OX6j91NMvZSH9lZMQ0DaTHgJXTgqEzsan8DHlWOc/3yJw6P9x\n\tlRwGcFMTQayiT9AlPkFYqTHtC8OD6ekl9GIsZ8/QNSEDWpgIQkl8gNymFGhBPnsYqi\n\t2uvvV/nhaclCmVY3eOna9amSgz4DhFBYS8uGd/t0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","From":"Paul Elder ","Cc":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 18 Sep 2025 22:04:26 +0900","Message-ID":"<175820066673.2127323.2752531515419494969@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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":35895,"web_url":"https://patchwork.libcamera.org/comment/35895/","msgid":"","date":"2025-09-18T13:14:33","subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 09. 18. 15:04 keltezéssel, Paul Elder írta:\n> Quoting Barnabás Pőcze (2025-07-21 19:46:21)\n>> From: Jacopo Mondi \n>>\n>> Use the newly introduced `metadataAvailable()` function to send metadata\n>> items to the application.\n>>\n>> Signed-off-by: Jacopo Mondi \n>> [Adjust commit message, adjust rpi changes.]\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> changes in v2:\n>> * include rpi changes as well\n>> ---\n>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +---\n>> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++-----\n>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +-\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++---\n>> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++---------\n>> src/libcamera/pipeline/simple/simple.cpp | 4 +--\n>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +-\n>> src/libcamera/pipeline/virtual/virtual.cpp | 2 +-\n>> 9 files changed, 32 insertions(+), 37 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> index e452fe2f8..579847367 100644\n>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer)\n>> Request *request = buffer->request();\n>>\n>> /* Record the sensor's timestamp in the request metadata. */\n>> - ControlList &metadata = request->metadata();\n>> - if (!metadata.contains(controls::SensorTimestamp.id()))\n>> - metadata.set(controls::SensorTimestamp,\n>> - buffer->metadata().timestamp);\n>> + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> completeBuffer(request, buffer);\n>> if (request->hasPendingBuffers())\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index e6a7f675a..0d310b5b4 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)\n>> return;\n>>\n>> Request *request = info->request;\n>> - request->metadata().merge(metadata);\n>> + pipe()->metadataAvailable(request, metadata);\n>>\n>> info->metadataProcessed = true;\n>> if (frameInfos_.tryComplete(info))\n>> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>>\n>> pipe()->completeBuffer(request, buffer);\n>>\n>> - request->metadata().set(controls::draft::PipelineDepth, 3);\n>> + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3);\n>> +\n>> /* \\todo Actually apply the scaler crop region to the ImgU. */\n>> const auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n>> if (scalerCrop)\n>> cropRegion_ = *scalerCrop;\n>> - request->metadata().set(controls::ScalerCrop, cropRegion_);\n>> +\n>> + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_);\n>>\n>> if (frameInfos_.tryComplete(info))\n>> pipe()->completeRequest(request);\n>> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>> * \\todo The sensor timestamp should be better estimated by connecting\n>> * to the V4L2Device::frameStart signal.\n>> */\n>> - request->metadata().set(controls::SensorTimestamp,\n>> - buffer->metadata().timestamp);\n>> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);\n>>\n>> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n>> return;\n>> }\n>>\n>> - request->metadata().set(controls::draft::TestPatternMode,\n>> - *testPatternMode);\n>> + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode);\n>> }\n>>\n>> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, \"ipu3\")\n>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> index 19980f6d2..d1a107629 100644\n>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,\n>> MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId];\n>>\n>> frameInfo.statsDone = true;\n>> - frameInfo.request->metadata().merge(metadata);\n>> + metadataAvailable(frameInfo.request, metadata);\n>>\n>> tryComplete(&frameInfo);\n>> }\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 6dce1844d..47ef52699 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta\n>> if (!info)\n>> return;\n>>\n>> - info->request->metadata().merge(metadata);\n>> + pipe()->metadataAvailable(info->request, metadata);\n>> info->metadataProcessed = true;\n>>\n>> pipe()->tryCompleteRequest(info);\n>> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>> * \\todo The sensor timestamp should be better estimated by connecting\n>> * to the V4L2Device::frameStart signal.\n>> */\n>> - request->metadata().set(controls::SensorTimestamp,\n>> - metadata.timestamp);\n>> + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp);\n>>\n>> if (isRaw_) {\n>> const ControlList &ctrls =\n>> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)\n>> LOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>> << strerror(-ret);\n>>\n>> - request->metadata().set(controls::ScalerCrop, activeCrop_.value());\n>> + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value());\n>> }\n>>\n>> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> index f2a9a15cd..1d497f6f0 100644\n>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n>> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata)\n>> /* Add to the Request metadata buffer what the IPA has provided. */\n>> /* Last thing to do is to fill up the request metadata. */\n>> Request *request = requestQueue_.front();\n>> - request->metadata().merge(metadata);\n>> + pipe()->metadataAvailable(request, metadata);\n>>\n>> /*\n>> * Inform the sensor of the latest colour gains if it has the\n>> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted()\n>>\n>> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request)\n>> {\n>> - if (auto x = bufferControls.get(controls::SensorTimestamp))\n>> - request->metadata().set(controls::SensorTimestamp, *x);\n>> - if (auto x = bufferControls.get(controls::FrameWallClock))\n>> - request->metadata().set(controls::FrameWallClock, *x);\n>> + pipe()->metadataAvailable(request, [&](auto set) {\n>> + if (auto x = bufferControls.get(controls::SensorTimestamp))\n>> + set(controls::SensorTimestamp, *x);\n>> + if (auto x = bufferControls.get(controls::FrameWallClock))\n>> + set(controls::FrameWallClock, *x);\n>>\n>> - if (cropParams_.size()) {\n>> - std::vector crops;\n>> + if (cropParams_.size()) {\n>> + std::vector crops;\n>>\n>> - for (auto const &[k, v] : cropParams_)\n>> - crops.push_back(scaleIspCrop(v.ispCrop));\n>> + for (auto const &[k, v] : cropParams_)\n>> + crops.push_back(scaleIspCrop(v.ispCrop));\n>>\n>> - request->metadata().set(controls::ScalerCrop, crops[0]);\n>> - if (crops.size() > 1) {\n>> - request->metadata().set(controls::rpi::ScalerCrops,\n>> - Span(crops.data(), crops.size()));\n>> + set(controls::ScalerCrop, crops[0]);\n>> +\n>> + if (crops.size() > 1)\n>> + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() });\n> \n> Am I paranoid for being worried that there's no check of crops.data().size\n> against what we allocated in the MetadataListPlan? Or maybe it's fine because\n> the way that ScalerCrops is handled that'll never happen?\n\n`ScalerCrops` is supposed to have as many elements as many streams there are,\nand the metadata plan is updated when the camera is configured. So I think\nthis shouldn't be an issue.\n\n\n> \n> Either way,\n> \n> Reviewed-by: Paul Elder \n> \n>> }\n>> - }\n>> + });\n>> }\n>>\n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 05387ca7c..28399cb67 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> }\n>>\n>> if (request)\n>> - request->metadata().set(controls::SensorTimestamp,\n>> + pipe->metadataAvailable(request, controls::SensorTimestamp,\n>> buffer->metadata().timestamp);\n>>\n>> /*\n>> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata\n>> if (!info)\n>> return;\n>>\n>> - info->request->metadata().merge(metadata);\n>> + pipe()->metadataAvailable(info->request, metadata);\n>> info->metadataProcessed = true;\n>> tryCompleteRequest(info->request);\n>> }\n>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> index 59fb4bd5c..8cea94721 100644\n>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n>> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer)\n>> Request *request = buffer->request();\n>>\n>> /* \\todo Use the UVC metadata to calculate a more precise timestamp */\n>> - request->metadata().set(controls::SensorTimestamp,\n>> - buffer->metadata().timestamp);\n>> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> pipe()->completeBuffer(request, buffer);\n>> pipe()->completeRequest(request);\n>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>> index 9d73e320c..4432de4a0 100644\n>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer)\n>> }\n>>\n>> /* Record the sensor's timestamp in the request metadata. */\n>> - request->metadata().set(controls::SensorTimestamp,\n>> - buffer->metadata().timestamp);\n>> + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp);\n>>\n>> pipe->completeBuffer(request, buffer);\n>> pipe->completeRequest(request);\n>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n>> index 049ebcba5..93c1733d6 100644\n>> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n>> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n>> ASSERT(found);\n>> }\n>>\n>> - request->metadata().set(controls::SensorTimestamp, timestamp);\n>> + metadataAvailable(request, controls::SensorTimestamp, timestamp);\n>> completeRequest(request);\n>>\n>> return 0;\n>> --\n>> 2.50.1\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 50712C328C\n\tfor ;\n\tThu, 18 Sep 2025 13:14:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 595FA6936F;\n\tThu, 18 Sep 2025 15:14:39 +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 12A1862C3B\n\tfor ;\n\tThu, 18 Sep 2025 15:14:37 +0200 (CEST)","from [192.168.33.22] (185.221.142.115.nat.pool.zt.hu\n\t[185.221.142.115])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E2489FC;\n\tThu, 18 Sep 2025 15:13:17 +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=\"JPdMIPYi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758201197;\n\tbh=vV83cifSBmVWCrWKU7XN8cNoL698sjJq/ditjxezXmI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=JPdMIPYi0vU/3PSt8SeY/Sy70UjE3skSsp5I/5xfje9TkkqMz3eBgjDWMa2qmKhxp\n\tvcd9TDJXwm9KMYrf5bO47QQBg2iS8QozfN3qTVG5/zCx8p0nKYb/ObCRMuqzOisDp1\n\tyLqsvRuUIQbL+Y5KYMnLhNiv1RUQXUfFx0Zh07Rc=","Message-ID":"","Date":"Thu, 18 Sep 2025 15:14:33 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 21/22] libcamera: pipeline: Use\n\t`metadataAvailable()`","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Jacopo Mondi ","References":"<20250721104622.1550908-1-barnabas.pocze@ideasonboard.com>\n\t<20250721104622.1550908-22-barnabas.pocze@ideasonboard.com>\n\t\n\t<175820066673.2127323.2752531515419494969@neptunite.rasen.tech>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<175820066673.2127323.2752531515419494969@neptunite.rasen.tech>","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\" "}}]