| Message ID | 20250721104622.1550908-22-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote: > From: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Use the newly introduced `metadataAvailable()` function to send metadata > items to the application. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > [Adjust commit message, adjust rpi changes.] > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > changes in v2: > * include rpi changes as well > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +--- > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++----- > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++--------- > src/libcamera/pipeline/simple/simple.cpp | 4 +-- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +- > src/libcamera/pipeline/vimc/vimc.cpp | 3 +- > src/libcamera/pipeline/virtual/virtual.cpp | 2 +- > 9 files changed, 32 insertions(+), 37 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index e452fe2f8..579847367 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) > Request *request = buffer->request(); > > /* Record the sensor's timestamp in the request metadata. */ > - ControlList &metadata = request->metadata(); > - if (!metadata.contains(controls::SensorTimestamp.id())) > - metadata.set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > completeBuffer(request, buffer); > if (request->hasPendingBuffers()) > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index e6a7f675a..0d310b5b4 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) > return; > > Request *request = info->request; > - request->metadata().merge(metadata); > + pipe()->metadataAvailable(request, metadata); > > info->metadataProcessed = true; > if (frameInfos_.tryComplete(info)) > @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > pipe()->completeBuffer(request, buffer); > > - request->metadata().set(controls::draft::PipelineDepth, 3); > + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3); > + > /* \todo Actually apply the scaler crop region to the ImgU. */ > const auto &scalerCrop = request->controls().get(controls::ScalerCrop); > if (scalerCrop) > cropRegion_ = *scalerCrop; > - request->metadata().set(controls::ScalerCrop, cropRegion_); > + > + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_); > > if (frameInfos_.tryComplete(info)) > pipe()->completeRequest(request); > @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) > * \todo The sensor timestamp should be better estimated by connecting > * to the V4L2Device::frameStart signal. > */ > - request->metadata().set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); > > @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) > return; > } > > - request->metadata().set(controls::draft::TestPatternMode, > - *testPatternMode); > + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode); > } > > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "ipu3") > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > index 19980f6d2..d1a107629 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, > MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; > > frameInfo.statsDone = true; > - frameInfo.request->metadata().merge(metadata); > + metadataAvailable(frameInfo.request, metadata); > > tryComplete(&frameInfo); > } > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 6dce1844d..47ef52699 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > if (!info) > return; > > - info->request->metadata().merge(metadata); > + pipe()->metadataAvailable(info->request, metadata); > info->metadataProcessed = true; > > pipe()->tryCompleteRequest(info); > @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > * \todo The sensor timestamp should be better estimated by connecting > * to the V4L2Device::frameStart signal. > */ > - request->metadata().set(controls::SensorTimestamp, > - metadata.timestamp); > + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp); > > if (isRaw_) { > const ControlList &ctrls = > @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: " > << strerror(-ret); > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value()); > } > > void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > index f2a9a15cd..1d497f6f0 100644 > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata) > /* Add to the Request metadata buffer what the IPA has provided. */ > /* Last thing to do is to fill up the request metadata. */ > Request *request = requestQueue_.front(); > - request->metadata().merge(metadata); > + pipe()->metadataAvailable(request, metadata); > > /* > * Inform the sensor of the latest colour gains if it has the > @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted() > > void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) > { > - if (auto x = bufferControls.get(controls::SensorTimestamp)) > - request->metadata().set(controls::SensorTimestamp, *x); > - if (auto x = bufferControls.get(controls::FrameWallClock)) > - request->metadata().set(controls::FrameWallClock, *x); > + pipe()->metadataAvailable(request, [&](auto set) { > + if (auto x = bufferControls.get(controls::SensorTimestamp)) > + set(controls::SensorTimestamp, *x); > + if (auto x = bufferControls.get(controls::FrameWallClock)) > + set(controls::FrameWallClock, *x); > Ah here you go I really wonder if the additional complexity is justified as this could simply be if (auto x = bufferControls.get(controls::SensorTimestamp)) metadataAvailable(request, controls::SensorTimestamp, *x); if (auto x = bufferControls.get(controls::FrameWallClock)) metadataAvailable.set(request, controls::FrameWallClock, *x); Or have I missed something ? > - if (cropParams_.size()) { > - std::vector<Rectangle> crops; > + if (cropParams_.size()) { > + std::vector<Rectangle> crops; > > - for (auto const &[k, v] : cropParams_) > - crops.push_back(scaleIspCrop(v.ispCrop)); > + for (auto const &[k, v] : cropParams_) > + crops.push_back(scaleIspCrop(v.ispCrop)); > > - request->metadata().set(controls::ScalerCrop, crops[0]); > - if (crops.size() > 1) { > - request->metadata().set(controls::rpi::ScalerCrops, > - Span<const Rectangle>(crops.data(), crops.size())); > + set(controls::ScalerCrop, crops[0]); > + > + if (crops.size() > 1) > + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() }); > } > - } > + }); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 05387ca7c..28399cb67 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) > } > > if (request) > - request->metadata().set(controls::SensorTimestamp, > + pipe->metadataAvailable(request, controls::SensorTimestamp, > buffer->metadata().timestamp); > > /* > @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata > if (!info) > return; > > - info->request->metadata().merge(metadata); > + pipe()->metadataAvailable(info->request, metadata); > info->metadataProcessed = true; > tryCompleteRequest(info->request); > } > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 59fb4bd5c..8cea94721 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) > Request *request = buffer->request(); > > /* \todo Use the UVC metadata to calculate a more precise timestamp */ > - request->metadata().set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > pipe()->completeBuffer(request, buffer); > pipe()->completeRequest(request); > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index 9d73e320c..4432de4a0 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) > } > > /* Record the sensor's timestamp in the request metadata. */ > - request->metadata().set(controls::SensorTimestamp, > - buffer->metadata().timestamp); > + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > pipe->completeBuffer(request, buffer); > pipe->completeRequest(request); > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > index 049ebcba5..93c1733d6 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > ASSERT(found); > } > > - request->metadata().set(controls::SensorTimestamp, timestamp); > + metadataAvailable(request, controls::SensorTimestamp, timestamp); > completeRequest(request); > > return 0; Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > -- > 2.50.1 >
Hi 2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote: >> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >> >> Use the newly introduced `metadataAvailable()` function to send metadata >> items to the application. >> >> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >> [Adjust commit message, adjust rpi changes.] >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> changes in v2: >> * include rpi changes as well >> --- >> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +--- >> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++----- >> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +- >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- >> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++--------- >> src/libcamera/pipeline/simple/simple.cpp | 4 +-- >> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +- >> src/libcamera/pipeline/vimc/vimc.cpp | 3 +- >> src/libcamera/pipeline/virtual/virtual.cpp | 2 +- >> 9 files changed, 32 insertions(+), 37 deletions(-) >> >> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> index e452fe2f8..579847367 100644 >> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) >> Request *request = buffer->request(); >> >> /* Record the sensor's timestamp in the request metadata. */ >> - ControlList &metadata = request->metadata(); >> - if (!metadata.contains(controls::SensorTimestamp.id())) >> - metadata.set(controls::SensorTimestamp, >> - buffer->metadata().timestamp); >> + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >> >> completeBuffer(request, buffer); >> if (request->hasPendingBuffers()) >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >> index e6a7f675a..0d310b5b4 100644 >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) >> return; >> >> Request *request = info->request; >> - request->metadata().merge(metadata); >> + pipe()->metadataAvailable(request, metadata); >> >> info->metadataProcessed = true; >> if (frameInfos_.tryComplete(info)) >> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) >> >> pipe()->completeBuffer(request, buffer); >> >> - request->metadata().set(controls::draft::PipelineDepth, 3); >> + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3); >> + >> /* \todo Actually apply the scaler crop region to the ImgU. */ >> const auto &scalerCrop = request->controls().get(controls::ScalerCrop); >> if (scalerCrop) >> cropRegion_ = *scalerCrop; >> - request->metadata().set(controls::ScalerCrop, cropRegion_); >> + >> + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_); >> >> if (frameInfos_.tryComplete(info)) >> pipe()->completeRequest(request); >> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) >> * \todo The sensor timestamp should be better estimated by connecting >> * to the V4L2Device::frameStart signal. >> */ >> - request->metadata().set(controls::SensorTimestamp, >> - buffer->metadata().timestamp); >> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >> >> info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); >> >> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) >> return; >> } >> >> - request->metadata().set(controls::draft::TestPatternMode, >> - *testPatternMode); >> + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode); >> } >> >> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "ipu3") >> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> index 19980f6d2..d1a107629 100644 >> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, >> MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; >> >> frameInfo.statsDone = true; >> - frameInfo.request->metadata().merge(metadata); >> + metadataAvailable(frameInfo.request, metadata); >> >> tryComplete(&frameInfo); >> } >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> index 6dce1844d..47ef52699 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta >> if (!info) >> return; >> >> - info->request->metadata().merge(metadata); >> + pipe()->metadataAvailable(info->request, metadata); >> info->metadataProcessed = true; >> >> pipe()->tryCompleteRequest(info); >> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) >> * \todo The sensor timestamp should be better estimated by connecting >> * to the V4L2Device::frameStart signal. >> */ >> - request->metadata().set(controls::SensorTimestamp, >> - metadata.timestamp); >> + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp); >> >> if (isRaw_) { >> const ControlList &ctrls = >> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) >> LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: " >> << strerror(-ret); >> >> - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); >> + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value()); >> } >> >> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) >> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >> index f2a9a15cd..1d497f6f0 100644 >> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata) >> /* Add to the Request metadata buffer what the IPA has provided. */ >> /* Last thing to do is to fill up the request metadata. */ >> Request *request = requestQueue_.front(); >> - request->metadata().merge(metadata); >> + pipe()->metadataAvailable(request, metadata); >> >> /* >> * Inform the sensor of the latest colour gains if it has the >> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted() >> >> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) >> { >> - if (auto x = bufferControls.get(controls::SensorTimestamp)) >> - request->metadata().set(controls::SensorTimestamp, *x); >> - if (auto x = bufferControls.get(controls::FrameWallClock)) >> - request->metadata().set(controls::FrameWallClock, *x); >> + pipe()->metadataAvailable(request, [&](auto set) { >> + if (auto x = bufferControls.get(controls::SensorTimestamp)) >> + set(controls::SensorTimestamp, *x); >> + if (auto x = bufferControls.get(controls::FrameWallClock)) >> + set(controls::FrameWallClock, *x); >> > > Ah here you go > > I really wonder if the additional complexity is justified as this > could simply be > > if (auto x = bufferControls.get(controls::SensorTimestamp)) > metadataAvailable(request, controls::SensorTimestamp, *x); > if (auto x = bufferControls.get(controls::FrameWallClock)) > metadataAvailable.set(request, controls::FrameWallClock, *x); > > Or have I missed something ? That is what it was in the previous version. I have changed it because I found it to be suboptimal that potentially 4 metadata items are reported in quick succession, meaning 4 signal emission, and 4 calls to the application's handler. I would really like to avoid doing that. Another option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()` with that, but I find that that is just unnecessary runtime overhead. So the approach that I wanted to propose here is having an overload that takes an invokable object, which can implement arbitrary logic for setting controls however it wishes, and when it returns, everything that it has set will be reported in a single signal emission. Regards, Barnabás Pőcze > >> - if (cropParams_.size()) { >> - std::vector<Rectangle> crops; >> + if (cropParams_.size()) { >> + std::vector<Rectangle> crops; >> >> - for (auto const &[k, v] : cropParams_) >> - crops.push_back(scaleIspCrop(v.ispCrop)); >> + for (auto const &[k, v] : cropParams_) >> + crops.push_back(scaleIspCrop(v.ispCrop)); >> >> - request->metadata().set(controls::ScalerCrop, crops[0]); >> - if (crops.size() > 1) { >> - request->metadata().set(controls::rpi::ScalerCrops, >> - Span<const Rectangle>(crops.data(), crops.size())); >> + set(controls::ScalerCrop, crops[0]); >> + >> + if (crops.size() > 1) >> + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() }); >> } >> - } >> + }); >> } >> >> } /* namespace libcamera */ >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index 05387ca7c..28399cb67 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) >> } >> >> if (request) >> - request->metadata().set(controls::SensorTimestamp, >> + pipe->metadataAvailable(request, controls::SensorTimestamp, >> buffer->metadata().timestamp); >> >> /* >> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata >> if (!info) >> return; >> >> - info->request->metadata().merge(metadata); >> + pipe()->metadataAvailable(info->request, metadata); >> info->metadataProcessed = true; >> tryCompleteRequest(info->request); >> } >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> index 59fb4bd5c..8cea94721 100644 >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) >> Request *request = buffer->request(); >> >> /* \todo Use the UVC metadata to calculate a more precise timestamp */ >> - request->metadata().set(controls::SensorTimestamp, >> - buffer->metadata().timestamp); >> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >> >> pipe()->completeBuffer(request, buffer); >> pipe()->completeRequest(request); >> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp >> index 9d73e320c..4432de4a0 100644 >> --- a/src/libcamera/pipeline/vimc/vimc.cpp >> +++ b/src/libcamera/pipeline/vimc/vimc.cpp >> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) >> } >> >> /* Record the sensor's timestamp in the request metadata. */ >> - request->metadata().set(controls::SensorTimestamp, >> - buffer->metadata().timestamp); >> + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >> >> pipe->completeBuffer(request, buffer); >> pipe->completeRequest(request); >> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp >> index 049ebcba5..93c1733d6 100644 >> --- a/src/libcamera/pipeline/virtual/virtual.cpp >> +++ b/src/libcamera/pipeline/virtual/virtual.cpp >> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, >> ASSERT(found); >> } >> >> - request->metadata().set(controls::SensorTimestamp, timestamp); >> + metadataAvailable(request, controls::SensorTimestamp, timestamp); >> completeRequest(request); >> >> return 0; > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Thanks > j > >> -- >> 2.50.1 >>
Hi Barnabás On Mon, Jul 28, 2025 at 01:57:43PM +0200, Barnabás Pőcze wrote: > Hi > > 2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta: > > Hi Barnabás > > > > On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote: > > > From: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > Use the newly introduced `metadataAvailable()` function to send metadata > > > items to the application. > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > [Adjust commit message, adjust rpi changes.] > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > --- > > > changes in v2: > > > * include rpi changes as well > > > --- > > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +--- > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++----- > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +- > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > > > .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++--------- > > > src/libcamera/pipeline/simple/simple.cpp | 4 +-- > > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +- > > > src/libcamera/pipeline/vimc/vimc.cpp | 3 +- > > > src/libcamera/pipeline/virtual/virtual.cpp | 2 +- > > > 9 files changed, 32 insertions(+), 37 deletions(-) > > > > > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > index e452fe2f8..579847367 100644 > > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) > > > Request *request = buffer->request(); > > > > > > /* Record the sensor's timestamp in the request metadata. */ > > > - ControlList &metadata = request->metadata(); > > > - if (!metadata.contains(controls::SensorTimestamp.id())) > > > - metadata.set(controls::SensorTimestamp, > > > - buffer->metadata().timestamp); > > > + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > > > > > completeBuffer(request, buffer); > > > if (request->hasPendingBuffers()) > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > index e6a7f675a..0d310b5b4 100644 > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) > > > return; > > > > > > Request *request = info->request; > > > - request->metadata().merge(metadata); > > > + pipe()->metadataAvailable(request, metadata); > > > > > > info->metadataProcessed = true; > > > if (frameInfos_.tryComplete(info)) > > > @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > > > > > pipe()->completeBuffer(request, buffer); > > > > > > - request->metadata().set(controls::draft::PipelineDepth, 3); > > > + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3); > > > + > > > /* \todo Actually apply the scaler crop region to the ImgU. */ > > > const auto &scalerCrop = request->controls().get(controls::ScalerCrop); > > > if (scalerCrop) > > > cropRegion_ = *scalerCrop; > > > - request->metadata().set(controls::ScalerCrop, cropRegion_); > > > + > > > + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_); > > > > > > if (frameInfos_.tryComplete(info)) > > > pipe()->completeRequest(request); > > > @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) > > > * \todo The sensor timestamp should be better estimated by connecting > > > * to the V4L2Device::frameStart signal. > > > */ > > > - request->metadata().set(controls::SensorTimestamp, > > > - buffer->metadata().timestamp); > > > + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > > > > > info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); > > > > > > @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) > > > return; > > > } > > > > > > - request->metadata().set(controls::draft::TestPatternMode, > > > - *testPatternMode); > > > + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode); > > > } > > > > > > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "ipu3") > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > index 19980f6d2..d1a107629 100644 > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, > > > MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; > > > > > > frameInfo.statsDone = true; > > > - frameInfo.request->metadata().merge(metadata); > > > + metadataAvailable(frameInfo.request, metadata); > > > > > > tryComplete(&frameInfo); > > > } > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > index 6dce1844d..47ef52699 100644 > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta > > > if (!info) > > > return; > > > > > > - info->request->metadata().merge(metadata); > > > + pipe()->metadataAvailable(info->request, metadata); > > > info->metadataProcessed = true; > > > > > > pipe()->tryCompleteRequest(info); > > > @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > > * \todo The sensor timestamp should be better estimated by connecting > > > * to the V4L2Device::frameStart signal. > > > */ > > > - request->metadata().set(controls::SensorTimestamp, > > > - metadata.timestamp); > > > + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp); > > > > > > if (isRaw_) { > > > const ControlList &ctrls = > > > @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) > > > LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: " > > > << strerror(-ret); > > > > > > - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); > > > + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value()); > > > } > > > > > > void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) > > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > > > index f2a9a15cd..1d497f6f0 100644 > > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp > > > @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata) > > > /* Add to the Request metadata buffer what the IPA has provided. */ > > > /* Last thing to do is to fill up the request metadata. */ > > > Request *request = requestQueue_.front(); > > > - request->metadata().merge(metadata); > > > + pipe()->metadataAvailable(request, metadata); > > > > > > /* > > > * Inform the sensor of the latest colour gains if it has the > > > @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted() > > > > > > void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) > > > { > > > - if (auto x = bufferControls.get(controls::SensorTimestamp)) > > > - request->metadata().set(controls::SensorTimestamp, *x); > > > - if (auto x = bufferControls.get(controls::FrameWallClock)) > > > - request->metadata().set(controls::FrameWallClock, *x); > > > + pipe()->metadataAvailable(request, [&](auto set) { > > > + if (auto x = bufferControls.get(controls::SensorTimestamp)) > > > + set(controls::SensorTimestamp, *x); > > > + if (auto x = bufferControls.get(controls::FrameWallClock)) > > > + set(controls::FrameWallClock, *x); > > > > > > > Ah here you go > > > > I really wonder if the additional complexity is justified as this > > could simply be > > > > if (auto x = bufferControls.get(controls::SensorTimestamp)) > > metadataAvailable(request, controls::SensorTimestamp, *x); > > if (auto x = bufferControls.get(controls::FrameWallClock)) > > metadataAvailable.set(request, controls::FrameWallClock, *x); > > > > Or have I missed something ? > > That is what it was in the previous version. I have changed it because I found > it to be suboptimal that potentially 4 metadata items are reported in quick succession, > meaning 4 signal emission, and 4 calls to the application's handler. I would really > like to avoid doing that. Ack, thanks for clarifying However, what confuses me is that this function already receives a ControlList and the above two lines, if I'm not mistaken are equivalent to metadataAvailable.set(request, bufferControls); > > Another option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()` > with that, but I find that that is just unnecessary runtime overhead. > > So the approach that I wanted to propose here is having an overload that takes > an invokable object, which can implement arbitrary logic for setting controls > however it wishes, and when it returns, everything that it has set will be > reported in a single signal emission. > > > Regards, > Barnabás Pőcze > > > > > > > - if (cropParams_.size()) { > > > - std::vector<Rectangle> crops; > > > + if (cropParams_.size()) { > > > + std::vector<Rectangle> crops; > > > > > > - for (auto const &[k, v] : cropParams_) > > > - crops.push_back(scaleIspCrop(v.ispCrop)); > > > + for (auto const &[k, v] : cropParams_) > > > + crops.push_back(scaleIspCrop(v.ispCrop)); > > > > > > - request->metadata().set(controls::ScalerCrop, crops[0]); > > > - if (crops.size() > 1) { > > > - request->metadata().set(controls::rpi::ScalerCrops, > > > - Span<const Rectangle>(crops.data(), crops.size())); > > > + set(controls::ScalerCrop, crops[0]); > > > + > > > + if (crops.size() > 1) > > > + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() }); > > > } But yes, emitting a single Camera::metadataAvailable for all the controls handled here would require creating a single control list, copying all of them in that list and then calling metadataAvailable(). I wonder if we shouldn't split the population of the metadata list from the emission of the signal and give pipelines control over it, which is basically what you're doing it by providing to pipelines a MetadataSetter they can use to populate the list without emitting the signal... > > > - } > > > + }); > > > } > > > > > > } /* namespace libcamera */ > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > > index 05387ca7c..28399cb67 100644 > > > --- a/src/libcamera/pipeline/simple/simple.cpp > > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > > @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) > > > } > > > > > > if (request) > > > - request->metadata().set(controls::SensorTimestamp, > > > + pipe->metadataAvailable(request, controls::SensorTimestamp, > > > buffer->metadata().timestamp); > > > > > > /* > > > @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata > > > if (!info) > > > return; > > > > > > - info->request->metadata().merge(metadata); > > > + pipe()->metadataAvailable(info->request, metadata); > > > info->metadataProcessed = true; > > > tryCompleteRequest(info->request); > > > } > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > index 59fb4bd5c..8cea94721 100644 > > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) > > > Request *request = buffer->request(); > > > > > > /* \todo Use the UVC metadata to calculate a more precise timestamp */ > > > - request->metadata().set(controls::SensorTimestamp, > > > - buffer->metadata().timestamp); > > > + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > > > > > pipe()->completeBuffer(request, buffer); > > > pipe()->completeRequest(request); > > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > > > index 9d73e320c..4432de4a0 100644 > > > --- a/src/libcamera/pipeline/vimc/vimc.cpp > > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > > > @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) > > > } > > > > > > /* Record the sensor's timestamp in the request metadata. */ > > > - request->metadata().set(controls::SensorTimestamp, > > > - buffer->metadata().timestamp); > > > + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); > > > > > > pipe->completeBuffer(request, buffer); > > > pipe->completeRequest(request); > > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp > > > index 049ebcba5..93c1733d6 100644 > > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > > @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > > ASSERT(found); > > > } > > > > > > - request->metadata().set(controls::SensorTimestamp, timestamp); > > > + metadataAvailable(request, controls::SensorTimestamp, timestamp); > > > completeRequest(request); > > > > > > return 0; > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > Thanks > > j > > > > > -- > > > 2.50.1 > > > >
Hi 2025. 07. 28. 14:30 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Mon, Jul 28, 2025 at 01:57:43PM +0200, Barnabás Pőcze wrote: >> Hi >> >> 2025. 07. 28. 12:55 keltezéssel, Jacopo Mondi írta: >>> Hi Barnabás >>> >>> On Mon, Jul 21, 2025 at 12:46:21PM +0200, Barnabás Pőcze wrote: >>>> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>> >>>> Use the newly introduced `metadataAvailable()` function to send metadata >>>> items to the application. >>>> >>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>> [Adjust commit message, adjust rpi changes.] >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>>> --- >>>> changes in v2: >>>> * include rpi changes as well >>>> --- >>>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 5 +--- >>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++++----- >>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +- >>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- >>>> .../pipeline/rpi/common/pipeline_base.cpp | 29 ++++++++++--------- >>>> src/libcamera/pipeline/simple/simple.cpp | 4 +-- >>>> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +- >>>> src/libcamera/pipeline/vimc/vimc.cpp | 3 +- >>>> src/libcamera/pipeline/virtual/virtual.cpp | 2 +- >>>> 9 files changed, 32 insertions(+), 37 deletions(-) >>>> >>>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >>>> index e452fe2f8..579847367 100644 >>>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >>>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp >>>> @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) >>>> Request *request = buffer->request(); >>>> >>>> /* Record the sensor's timestamp in the request metadata. */ >>>> - ControlList &metadata = request->metadata(); >>>> - if (!metadata.contains(controls::SensorTimestamp.id())) >>>> - metadata.set(controls::SensorTimestamp, >>>> - buffer->metadata().timestamp); >>>> + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >>>> >>>> completeBuffer(request, buffer); >>>> if (request->hasPendingBuffers()) >>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> index e6a7f675a..0d310b5b4 100644 >>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >>>> @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) >>>> return; >>>> >>>> Request *request = info->request; >>>> - request->metadata().merge(metadata); >>>> + pipe()->metadataAvailable(request, metadata); >>>> >>>> info->metadataProcessed = true; >>>> if (frameInfos_.tryComplete(info)) >>>> @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) >>>> >>>> pipe()->completeBuffer(request, buffer); >>>> >>>> - request->metadata().set(controls::draft::PipelineDepth, 3); >>>> + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3); >>>> + >>>> /* \todo Actually apply the scaler crop region to the ImgU. */ >>>> const auto &scalerCrop = request->controls().get(controls::ScalerCrop); >>>> if (scalerCrop) >>>> cropRegion_ = *scalerCrop; >>>> - request->metadata().set(controls::ScalerCrop, cropRegion_); >>>> + >>>> + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_); >>>> >>>> if (frameInfos_.tryComplete(info)) >>>> pipe()->completeRequest(request); >>>> @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) >>>> * \todo The sensor timestamp should be better estimated by connecting >>>> * to the V4L2Device::frameStart signal. >>>> */ >>>> - request->metadata().set(controls::SensorTimestamp, >>>> - buffer->metadata().timestamp); >>>> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >>>> >>>> info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); >>>> >>>> @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) >>>> return; >>>> } >>>> >>>> - request->metadata().set(controls::draft::TestPatternMode, >>>> - *testPatternMode); >>>> + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode); >>>> } >>>> >>>> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "ipu3") >>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>> index 19980f6d2..d1a107629 100644 >>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>> @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, >>>> MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; >>>> >>>> frameInfo.statsDone = true; >>>> - frameInfo.request->metadata().merge(metadata); >>>> + metadataAvailable(frameInfo.request, metadata); >>>> >>>> tryComplete(&frameInfo); >>>> } >>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>>> index 6dce1844d..47ef52699 100644 >>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>>> @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta >>>> if (!info) >>>> return; >>>> >>>> - info->request->metadata().merge(metadata); >>>> + pipe()->metadataAvailable(info->request, metadata); >>>> info->metadataProcessed = true; >>>> >>>> pipe()->tryCompleteRequest(info); >>>> @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) >>>> * \todo The sensor timestamp should be better estimated by connecting >>>> * to the V4L2Device::frameStart signal. >>>> */ >>>> - request->metadata().set(controls::SensorTimestamp, >>>> - metadata.timestamp); >>>> + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp); >>>> >>>> if (isRaw_) { >>>> const ControlList &ctrls = >>>> @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) >>>> LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: " >>>> << strerror(-ret); >>>> >>>> - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); >>>> + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value()); >>>> } >>>> >>>> void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) >>>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >>>> index f2a9a15cd..1d497f6f0 100644 >>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp >>>> @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata) >>>> /* Add to the Request metadata buffer what the IPA has provided. */ >>>> /* Last thing to do is to fill up the request metadata. */ >>>> Request *request = requestQueue_.front(); >>>> - request->metadata().merge(metadata); >>>> + pipe()->metadataAvailable(request, metadata); >>>> >>>> /* >>>> * Inform the sensor of the latest colour gains if it has the >>>> @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted() >>>> >>>> void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) >>>> { >>>> - if (auto x = bufferControls.get(controls::SensorTimestamp)) >>>> - request->metadata().set(controls::SensorTimestamp, *x); >>>> - if (auto x = bufferControls.get(controls::FrameWallClock)) >>>> - request->metadata().set(controls::FrameWallClock, *x); >>>> + pipe()->metadataAvailable(request, [&](auto set) { >>>> + if (auto x = bufferControls.get(controls::SensorTimestamp)) >>>> + set(controls::SensorTimestamp, *x); >>>> + if (auto x = bufferControls.get(controls::FrameWallClock)) >>>> + set(controls::FrameWallClock, *x); >>>> >>> >>> Ah here you go >>> >>> I really wonder if the additional complexity is justified as this >>> could simply be >>> >>> if (auto x = bufferControls.get(controls::SensorTimestamp)) >>> metadataAvailable(request, controls::SensorTimestamp, *x); >>> if (auto x = bufferControls.get(controls::FrameWallClock)) >>> metadataAvailable.set(request, controls::FrameWallClock, *x); >>> >>> Or have I missed something ? >> >> That is what it was in the previous version. I have changed it because I found >> it to be suboptimal that potentially 4 metadata items are reported in quick succession, >> meaning 4 signal emission, and 4 calls to the application's handler. I would really >> like to avoid doing that. > > Ack, thanks for clarifying > > However, what confuses me is that this function already receives a > ControlList and the above two lines, if I'm not mistaken are > equivalent to > > metadataAvailable.set(request, bufferControls); > >> >> Another option could be do construct a `ControlList` and do a single `pipe()->metadataAvailable()` >> with that, but I find that that is just unnecessary runtime overhead. >> >> So the approach that I wanted to propose here is having an overload that takes >> an invokable object, which can implement arbitrary logic for setting controls >> however it wishes, and when it returns, everything that it has set will be >> reported in a single signal emission. >> >> >> Regards, >> Barnabás Pőcze >> >> >>> >>>> - if (cropParams_.size()) { >>>> - std::vector<Rectangle> crops; >>>> + if (cropParams_.size()) { >>>> + std::vector<Rectangle> crops; >>>> >>>> - for (auto const &[k, v] : cropParams_) >>>> - crops.push_back(scaleIspCrop(v.ispCrop)); >>>> + for (auto const &[k, v] : cropParams_) >>>> + crops.push_back(scaleIspCrop(v.ispCrop)); >>>> >>>> - request->metadata().set(controls::ScalerCrop, crops[0]); >>>> - if (crops.size() > 1) { >>>> - request->metadata().set(controls::rpi::ScalerCrops, >>>> - Span<const Rectangle>(crops.data(), crops.size())); >>>> + set(controls::ScalerCrop, crops[0]); >>>> + >>>> + if (crops.size() > 1) >>>> + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() }); >>>> } > > But yes, emitting a single Camera::metadataAvailable for all the > controls handled here would require creating a single control list, > copying all of them in that list and then calling metadataAvailable(). > > I wonder if we shouldn't split the population of the metadata list > from the emission of the signal and give pipelines control over it, > which is basically what you're doing it by providing to pipelines a > MetadataSetter they can use to populate the list without emitting the > signal... Yes, that's the idea. I believe this "lambda" approach is much to misuse than providing e.g. more separate objects where attention needs to be paid to lifetimes and calling the right methods at the right times, etc. Regards, Barnabás Pőcze > > > >>>> - } >>>> + }); >>>> } >>>> >>>> } /* namespace libcamera */ >>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >>>> index 05387ca7c..28399cb67 100644 >>>> --- a/src/libcamera/pipeline/simple/simple.cpp >>>> +++ b/src/libcamera/pipeline/simple/simple.cpp >>>> @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) >>>> } >>>> >>>> if (request) >>>> - request->metadata().set(controls::SensorTimestamp, >>>> + pipe->metadataAvailable(request, controls::SensorTimestamp, >>>> buffer->metadata().timestamp); >>>> >>>> /* >>>> @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata >>>> if (!info) >>>> return; >>>> >>>> - info->request->metadata().merge(metadata); >>>> + pipe()->metadataAvailable(info->request, metadata); >>>> info->metadataProcessed = true; >>>> tryCompleteRequest(info->request); >>>> } >>>> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>>> index 59fb4bd5c..8cea94721 100644 >>>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp >>>> @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) >>>> Request *request = buffer->request(); >>>> >>>> /* \todo Use the UVC metadata to calculate a more precise timestamp */ >>>> - request->metadata().set(controls::SensorTimestamp, >>>> - buffer->metadata().timestamp); >>>> + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >>>> >>>> pipe()->completeBuffer(request, buffer); >>>> pipe()->completeRequest(request); >>>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp >>>> index 9d73e320c..4432de4a0 100644 >>>> --- a/src/libcamera/pipeline/vimc/vimc.cpp >>>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp >>>> @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) >>>> } >>>> >>>> /* Record the sensor's timestamp in the request metadata. */ >>>> - request->metadata().set(controls::SensorTimestamp, >>>> - buffer->metadata().timestamp); >>>> + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); >>>> >>>> pipe->completeBuffer(request, buffer); >>>> pipe->completeRequest(request); >>>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp >>>> index 049ebcba5..93c1733d6 100644 >>>> --- a/src/libcamera/pipeline/virtual/virtual.cpp >>>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp >>>> @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, >>>> ASSERT(found); >>>> } >>>> >>>> - request->metadata().set(controls::SensorTimestamp, timestamp); >>>> + metadataAvailable(request, controls::SensorTimestamp, timestamp); >>>> completeRequest(request); >>>> >>>> return 0; >>> >>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> >>> Thanks >>> j >>> >>>> -- >>>> 2.50.1 >>>> >>
diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index e452fe2f8..579847367 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -1098,10 +1098,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) Request *request = buffer->request(); /* Record the sensor's timestamp in the request metadata. */ - ControlList &metadata = request->metadata(); - if (!metadata.contains(controls::SensorTimestamp.id())) - metadata.set(controls::SensorTimestamp, - buffer->metadata().timestamp); + metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); completeBuffer(request, buffer); if (request->hasPendingBuffers()) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e6a7f675a..0d310b5b4 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1250,7 +1250,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) return; Request *request = info->request; - request->metadata().merge(metadata); + pipe()->metadataAvailable(request, metadata); info->metadataProcessed = true; if (frameInfos_.tryComplete(info)) @@ -1277,12 +1277,14 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) pipe()->completeBuffer(request, buffer); - request->metadata().set(controls::draft::PipelineDepth, 3); + pipe()->metadataAvailable(request, controls::draft::PipelineDepth, 3); + /* \todo Actually apply the scaler crop region to the ImgU. */ const auto &scalerCrop = request->controls().get(controls::ScalerCrop); if (scalerCrop) cropRegion_ = *scalerCrop; - request->metadata().set(controls::ScalerCrop, cropRegion_); + + pipe()->metadataAvailable(request, controls::ScalerCrop, cropRegion_); if (frameInfos_.tryComplete(info)) pipe()->completeRequest(request); @@ -1322,8 +1324,7 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) * \todo The sensor timestamp should be better estimated by connecting * to the V4L2Device::frameStart signal. */ - request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); @@ -1417,8 +1418,7 @@ void IPU3CameraData::frameStart(uint32_t sequence) return; } - request->metadata().set(controls::draft::TestPatternMode, - *testPatternMode); + pipe()->metadataAvailable(request, controls::draft::TestPatternMode, *testPatternMode); } REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3, "ipu3") diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index 19980f6d2..d1a107629 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1523,7 +1523,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; frameInfo.statsDone = true; - frameInfo.request->metadata().merge(metadata); + metadataAvailable(frameInfo.request, metadata); tryComplete(&frameInfo); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 6dce1844d..47ef52699 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -451,7 +451,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta if (!info) return; - info->request->metadata().merge(metadata); + pipe()->metadataAvailable(info->request, metadata); info->metadataProcessed = true; pipe()->tryCompleteRequest(info); @@ -1497,8 +1497,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) * \todo The sensor timestamp should be better estimated by connecting * to the V4L2Device::frameStart signal. */ - request->metadata().set(controls::SensorTimestamp, - metadata.timestamp); + metadataAvailable(request, controls::SensorTimestamp, metadata.timestamp); if (isRaw_) { const ControlList &ctrls = @@ -1581,7 +1580,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: " << strerror(-ret); - request->metadata().set(controls::ScalerCrop, activeCrop_.value()); + metadataAvailable(request, controls::ScalerCrop, activeCrop_.value()); } void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index f2a9a15cd..1d497f6f0 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -1225,7 +1225,7 @@ void CameraData::metadataReady(const ControlList &metadata) /* Add to the Request metadata buffer what the IPA has provided. */ /* Last thing to do is to fill up the request metadata. */ Request *request = requestQueue_.front(); - request->metadata().merge(metadata); + pipe()->metadataAvailable(request, metadata); /* * Inform the sensor of the latest colour gains if it has the @@ -1495,23 +1495,24 @@ void CameraData::checkRequestCompleted() void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) { - if (auto x = bufferControls.get(controls::SensorTimestamp)) - request->metadata().set(controls::SensorTimestamp, *x); - if (auto x = bufferControls.get(controls::FrameWallClock)) - request->metadata().set(controls::FrameWallClock, *x); + pipe()->metadataAvailable(request, [&](auto set) { + if (auto x = bufferControls.get(controls::SensorTimestamp)) + set(controls::SensorTimestamp, *x); + if (auto x = bufferControls.get(controls::FrameWallClock)) + set(controls::FrameWallClock, *x); - if (cropParams_.size()) { - std::vector<Rectangle> crops; + if (cropParams_.size()) { + std::vector<Rectangle> crops; - for (auto const &[k, v] : cropParams_) - crops.push_back(scaleIspCrop(v.ispCrop)); + for (auto const &[k, v] : cropParams_) + crops.push_back(scaleIspCrop(v.ispCrop)); - request->metadata().set(controls::ScalerCrop, crops[0]); - if (crops.size() > 1) { - request->metadata().set(controls::rpi::ScalerCrops, - Span<const Rectangle>(crops.data(), crops.size())); + set(controls::ScalerCrop, crops[0]); + + if (crops.size() > 1) + set(controls::rpi::ScalerCrops, { crops.data(), crops.size() }); } - } + }); } } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 05387ca7c..28399cb67 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -909,7 +909,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) } if (request) - request->metadata().set(controls::SensorTimestamp, + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); /* @@ -997,7 +997,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata if (!info) return; - info->request->metadata().merge(metadata); + pipe()->metadataAvailable(info->request, metadata); info->metadataProcessed = true; tryCompleteRequest(info->request); } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 59fb4bd5c..8cea94721 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -894,8 +894,7 @@ void UVCCameraData::imageBufferReady(FrameBuffer *buffer) Request *request = buffer->request(); /* \todo Use the UVC metadata to calculate a more precise timestamp */ - request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + pipe()->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); pipe()->completeBuffer(request, buffer); pipe()->completeRequest(request); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index 9d73e320c..4432de4a0 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -617,8 +617,7 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) } /* Record the sensor's timestamp in the request metadata. */ - request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + pipe->metadataAvailable(request, controls::SensorTimestamp, buffer->metadata().timestamp); pipe->completeBuffer(request, buffer); pipe->completeRequest(request); diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp index 049ebcba5..93c1733d6 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -331,7 +331,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, ASSERT(found); } - request->metadata().set(controls::SensorTimestamp, timestamp); + metadataAvailable(request, controls::SensorTimestamp, timestamp); completeRequest(request); return 0;