From dce2ef36f2ef66724ebe0443206a38a56f25d55a Mon Sep 17 00:00:00 2001 From: Jacopo Mondi Date: Tue, 25 Nov 2025 15:07:23 +0100 Subject: [PATCH] libcamera: request: Move metadata_ to Private Address a long standing \todo item that suggested to implement a read-only interface for the Request::metadata() accessor and deflect to the internal implementation for the read-write accessor used by pipeline handlers. Signed-off-by: Jacopo Mondi Acked-by: Paul Elder Reviewed-by: Daniel Scally --- include/libcamera/internal/request.h | 3 ++ include/libcamera/request.h | 3 +- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 3 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 16 ++++---- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++--- .../pipeline/rpi/common/pipeline_base.cpp | 13 ++++--- .../pipeline/rpi/common/pipeline_base.h | 2 +- src/libcamera/pipeline/simple/simple.cpp | 8 ++-- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 6 +-- src/libcamera/pipeline/vimc/vimc.cpp | 6 +-- src/libcamera/pipeline/virtual/virtual.cpp | 4 +- src/libcamera/request.cpp | 37 +++++++++++-------- 13 files changed, 61 insertions(+), 52 deletions(-) diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 78cb99f3..643f67ca 100644 --- a/include/libcamera/internal/request.h +++ b/include/libcamera/internal/request.h @@ -36,6 +36,8 @@ public: Camera *camera() const { return camera_; } bool hasPendingBuffers() const; + ControlList &metadata() { return *metadata_; } + bool completeBuffer(FrameBuffer *buffer); void complete(); void cancel(); @@ -61,6 +63,7 @@ private: std::unordered_set pending_; std::map notifiers_; std::unique_ptr timer_; + ControlList *metadata_; }; } /* namespace libcamera */ diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 0c5939f7..c9aeddb6 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -50,7 +50,7 @@ public: void reuse(ReuseFlag flags = Default); ControlList &controls() { return *controls_; } - ControlList &metadata() { return *metadata_; } + const ControlList &metadata() const; const BufferMap &buffers() const { return bufferMap_; } int addBuffer(const Stream *stream, FrameBuffer *buffer, std::unique_ptr &&fence = {}); @@ -68,7 +68,6 @@ private: LIBCAMERA_DISABLE_COPY(Request) ControlList *controls_; - ControlList *metadata_; BufferMap bufferMap_; const uint64_t cookie_; diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 008155ff..706681fc 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -27,6 +27,7 @@ #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_pipeline.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -1125,7 +1126,7 @@ void PipelineHandlerISI::bufferReady(FrameBuffer *buffer) Request *request = buffer->request(); /* Record the sensor's timestamp in the request metadata. */ - ControlList &metadata = request->metadata(); + ControlList &metadata = request->_d()->metadata(); if (!metadata.contains(controls::SensorTimestamp.id())) metadata.set(controls::SensorTimestamp, buffer->metadata().timestamp); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 695762c7..0190f677 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include @@ -35,6 +34,7 @@ #include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "cio2.h" #include "frames.h" @@ -1249,7 +1249,7 @@ void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata) return; Request *request = info->request; - request->metadata().merge(metadata); + request->_d()->metadata().merge(metadata); info->metadataProcessed = true; if (frameInfos_.tryComplete(info)) @@ -1276,12 +1276,12 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) pipe()->completeBuffer(request, buffer); - request->metadata().set(controls::draft::PipelineDepth, 3); + request->_d()->metadata().set(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_); + request->_d()->metadata().set(controls::ScalerCrop, cropRegion_); if (frameInfos_.tryComplete(info)) pipe()->completeRequest(request); @@ -1321,8 +1321,8 @@ 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); + request->_d()->metadata().set(controls::SensorTimestamp, + buffer->metadata().timestamp); info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); @@ -1416,8 +1416,8 @@ void IPU3CameraData::frameStart(uint32_t sequence) return; } - request->metadata().set(controls::draft::TestPatternMode, - *testPatternMode); + request->_d()->metadata().set(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 cf0cb15f..77f25141 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1528,7 +1528,7 @@ void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; frameInfo.statsDone = true; - frameInfo.request->metadata().merge(metadata); + frameInfo.request->_d()->metadata().merge(metadata); tryComplete(&frameInfo); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 5fd11026..320a4dc5 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include @@ -45,6 +44,7 @@ #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_pipeline.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" #include "libcamera/internal/yaml_parser.h" @@ -507,7 +507,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta if (!info) return; - info->request->metadata().merge(metadata); + info->request->_d()->metadata().merge(metadata); info->metadataProcessed = true; pipe()->tryCompleteRequest(info); @@ -1643,8 +1643,8 @@ 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); + request->_d()->metadata().set(controls::SensorTimestamp, + metadata.timestamp); if (isRaw_) { const ControlList &ctrls = @@ -1686,7 +1686,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) return; } - dewarper_->populateMetadata(&data->mainPathStream_, request->metadata()); + dewarper_->populateMetadata(&data->mainPathStream_, request->_d()->metadata()); } 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 9d65dc83..7de45f4e 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -1221,7 +1221,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); + request->_d()->metadata().merge(metadata); /* * Inform the sensor of the latest colour gains if it has the @@ -1492,9 +1492,9 @@ void CameraData::checkRequestCompleted() void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request *request) { if (auto x = bufferControls.get(controls::SensorTimestamp)) - request->metadata().set(controls::SensorTimestamp, *x); + request->_d()->metadata().set(controls::SensorTimestamp, *x); if (auto x = bufferControls.get(controls::FrameWallClock)) - request->metadata().set(controls::FrameWallClock, *x); + request->_d()->metadata().set(controls::FrameWallClock, *x); if (cropParams_.size()) { std::vector crops; @@ -1502,10 +1502,11 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request for (auto const &[k, v] : cropParams_) crops.push_back(scaleIspCrop(v.ispCrop)); - request->metadata().set(controls::ScalerCrop, crops[0]); + request->_d()->metadata().set(controls::ScalerCrop, crops[0]); if (crops.size() > 1) { - request->metadata().set(controls::rpi::ScalerCrops, - Span(crops.data(), crops.size())); + request->_d()->metadata().set(controls::rpi::ScalerCrops, + Span(crops.data(), + crops.size())); } } } diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h index 15628259..7735d0a9 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -15,7 +15,6 @@ #include #include -#include #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/camera.h" @@ -25,6 +24,7 @@ #include "libcamera/internal/media_device.h" #include "libcamera/internal/media_object.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/v4l2_videodevice.h" #include "libcamera/internal/yaml_parser.h" diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 8f60d3eb..b30b0a12 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include "libcamera/internal/camera.h" @@ -44,6 +43,7 @@ #include "libcamera/internal/global_configuration.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/software_isp/software_isp.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -940,8 +940,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer) } if (request) - request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + request->_d()->metadata().set(controls::SensorTimestamp, + buffer->metadata().timestamp); /* * Queue the captured and the request buffer to the converter or Software @@ -1036,7 +1036,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata if (!info) return; - info->request->metadata().merge(metadata); + info->request->_d()->metadata().merge(metadata); info->metadataProcessed = true; tryCompleteRequest(info->request); } diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index cb8cc82d..3bd51733 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -24,13 +24,13 @@ #include #include #include -#include #include #include "libcamera/internal/camera.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/sysfs.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -895,8 +895,8 @@ 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); + request->_d()->metadata().set(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 cd3370e3..4a03c149 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include @@ -39,6 +38,7 @@ #include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" @@ -618,8 +618,8 @@ void VimcCameraData::imageBufferReady(FrameBuffer *buffer) } /* Record the sensor's timestamp in the request metadata. */ - request->metadata().set(controls::SensorTimestamp, - buffer->metadata().timestamp); + request->_d()->metadata().set(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 7855e8b9..40c35264 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -29,13 +29,13 @@ #include #include #include -#include #include "libcamera/internal/camera.h" #include "libcamera/internal/dma_buf_allocator.h" #include "libcamera/internal/formats.h" #include "libcamera/internal/framebuffer.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/request.h" #include "libcamera/internal/yaml_parser.h" #include "pipeline/virtual/config_parser.h" @@ -366,7 +366,7 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, VirtualCameraData *data = cameraData(camera); const auto timestamp = currentTimestamp(); - request->metadata().set(controls::SensorTimestamp, timestamp); + request->_d()->metadata().set(controls::SensorTimestamp, timestamp); data->invokeMethod(&VirtualCameraData::processRequest, ConnectionTypeQueued, request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 2544a059..a661b2f5 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -57,11 +57,16 @@ LOG_DEFINE_CATEGORY(Request) Request::Private::Private(Camera *camera) : camera_(camera), cancelled_(false) { + /** + * \todo Add a validator for metadata controls. + */ + metadata_ = new ControlList(controls::controls); } Request::Private::~Private() { doCancelRequest(); + delete metadata_; } /** @@ -82,6 +87,12 @@ bool Request::Private::hasPendingBuffers() const return !pending_.empty(); } +/** + * \fn Request::Private::metadata() + * \brief Retrieve the request's metadata + * \return The metadata associated with the request + */ + /** * \brief Complete a buffer for the request * \param[in] buffer The buffer that has completed @@ -359,11 +370,6 @@ Request::Request(Camera *camera, uint64_t cookie) controls_ = new ControlList(camera->controls(), camera->_d()->validator()); - /** - * \todo Add a validator for metadata controls. - */ - metadata_ = new ControlList(controls::controls); - LIBCAMERA_TRACEPOINT(request_construct, this); LOG(Request, Debug) << "Created request - cookie: " << cookie_; @@ -372,8 +378,6 @@ Request::Request(Camera *camera, uint64_t cookie) Request::~Request() { LIBCAMERA_TRACEPOINT(request_destroy, this); - - delete metadata_; delete controls_; } @@ -406,7 +410,7 @@ void Request::reuse(ReuseFlag flags) status_ = RequestPending; controls_->clear(); - metadata_->clear(); + _d()->metadata_->clear(); } /** @@ -425,6 +429,15 @@ void Request::reuse(ReuseFlag flags) * \return A reference to the ControlList in this request */ +/** + * \brief Retrieve the request's metadata + * \return The a const reference to the metadata associated with the request + */ +const ControlList &Request::metadata() const +{ + return *_d()->metadata_; +} + /** * \fn Request::buffers() * \brief Retrieve the request's streams to buffers map @@ -525,14 +538,6 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const return it->second; } -/** - * \fn Request::metadata() - * \brief Retrieve the request's metadata - * \todo Offer a read-only API towards applications while keeping a read/write - * API internally. - * \return The metadata associated with the request - */ - /** * \brief Retrieve the sequence number for the request *