[v3,2/5] libcamera: request: Move metadata_ to Private
diff mbox series

Message ID 20251202-cam-control-override-v3-2-eacab052798d@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: ipc: ControlLists without valid idmap break IPC
Related show

Commit Message

Jacopo Mondi Dec. 2, 2025, 2:49 p.m. UTC 
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 <jacopo.mondi@ideasonboard.com>
Acked-by: Paul Elder <paul.elder@ideasonboard.com>
---
 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 ++++----
 src/libcamera/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(-)

Comments

Dan Scally Dec. 15, 2025, 11:39 a.m. UTC | #1 
Hi Jacopo

On 02/12/2025 14:49, Jacopo Mondi wrote:
> 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 <jacopo.mondi@ideasonboard.com>
> Acked-by: Paul Elder <paul.elder@ideasonboard.com>
> ---

Only one trivial comment below.

>   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 ++++----
>   src/libcamera/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 78cb99f3604504dfb7145c605835b7493b656ced..643f67cabf5778f7515c0117d06d4e0a3fdec4f8 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<FrameBuffer *> pending_;
>   	std::map<FrameBuffer *, EventNotifier> notifiers_;
>   	std::unique_ptr<Timer> timer_;
> +	ControlList *metadata_;
>   };
>   
>   } /* namespace libcamera */
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 0c5939f7b3336fd089a2fe231f4f6f266cc422f7..c9aeddb62680923dc3490a23dc6c3f70f70cc075 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> &&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 008155ff21a7b398ef6bbe3c1379444ced26bdc5..706681fce5629bc4bd832923294e87a1ac8794cf 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 695762c750f89b5c8d10c221c6a9b3d7bd9ac6e7..0190f677e6794979b04b5729b2ffa88451a08ccb 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -19,7 +19,6 @@
>   #include <libcamera/control_ids.h>
>   #include <libcamera/formats.h>
>   #include <libcamera/property_ids.h>
> -#include <libcamera/request.h>

I think I would keep these rather than rely on the include in libcamera/internal/request.h, but 
either way:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   #include <libcamera/stream.h>
>   
>   #include <libcamera/ipa/ipu3_ipa_interface.h>
> @@ -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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..77f251416f718c8715d9df5d69a9c004e01b6561 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 5fd1102695070667e76daaf868e8d801f0ff70dd..320a4dc5a899b11cf56141a841f63436f0d29a37 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -26,7 +26,6 @@
>   #include <libcamera/formats.h>
>   #include <libcamera/framebuffer.h>
>   #include <libcamera/property_ids.h>
> -#include <libcamera/request.h>
>   #include <libcamera/stream.h>
>   #include <libcamera/transform.h>
>   
> @@ -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 9d65dc83573b88a4c8fd5410594085c21d91b1b1..7de45f4e5d0722a9b6425f01596455f838101900 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<Rectangle> 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<const Rectangle>(crops.data(), crops.size()));
> +			request->_d()->metadata().set(controls::rpi::ScalerCrops,
> +						      Span<const Rectangle>(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 15628259afc6a90511052e8ec463d4d1cad9b30b..7735d0a9ed4311d731b7ed9b01c5970fa5d2b381 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -15,7 +15,6 @@
>   #include <vector>
>   
>   #include <libcamera/controls.h>
> -#include <libcamera/request.h>
>   
>   #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 a06a52926441ba826972b70bc8aef068e354d843..f58208f28149fef561b9bd00d5fab18b2dd8ef9b 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -27,7 +27,6 @@
>   #include <libcamera/camera.h>
>   #include <libcamera/color_space.h>
>   #include <libcamera/control_ids.h>
> -#include <libcamera/request.h>
>   #include <libcamera/stream.h>
>   
>   #include "libcamera/internal/camera.h"
> @@ -41,6 +40,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"
> @@ -927,8 +927,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
> @@ -1015,7 +1015,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 cb8cc82dffd551183fbeccd5041982b1d44e9f77..3bd51733d4005347c0bdfe83db0ea5f827be441e 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -24,13 +24,13 @@
>   #include <libcamera/control_ids.h>
>   #include <libcamera/controls.h>
>   #include <libcamera/property_ids.h>
> -#include <libcamera/request.h>
>   #include <libcamera/stream.h>
>   
>   #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 cd3370e3d41911f6935bead7f2d0dc5b39ee0038..4a03c149a61739d3596c11f91696db3ee5a43568 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -24,7 +24,6 @@
>   #include <libcamera/formats.h>
>   #include <libcamera/framebuffer.h>
>   #include <libcamera/geometry.h>
> -#include <libcamera/request.h>
>   #include <libcamera/stream.h>
>   
>   #include <libcamera/ipa/ipa_interface.h>
> @@ -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 7855e8b9db0c3bdfb3662a0b6e71332ed463a0ed..40c35264c5687c0f94458e19a272612cefb76285 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -29,13 +29,13 @@
>   #include <libcamera/formats.h>
>   #include <libcamera/pixel_format.h>
>   #include <libcamera/property_ids.h>
> -#include <libcamera/request.h>
>   
>   #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 2544a059f6984d930ec909c74e0b621c9fe82726..a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930 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
>    *
>
Jacopo Mondi Dec. 15, 2025, 11:45 a.m. UTC | #2 
Hi Dan
   thanks for the review
On Mon, Dec 15, 2025 at 11:39:42AM +0000, Dan Scally wrote:
> Hi Jacopo
>
> On 02/12/2025 14:49, Jacopo Mondi wrote:
> > 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 <jacopo.mondi@ideasonboard.com>
> > Acked-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
>
> Only one trivial comment below.
>
> >   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 ++++----
> >   src/libcamera/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 78cb99f3604504dfb7145c605835b7493b656ced..643f67cabf5778f7515c0117d06d4e0a3fdec4f8 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<FrameBuffer *> pending_;
> >   	std::map<FrameBuffer *, EventNotifier> notifiers_;
> >   	std::unique_ptr<Timer> timer_;
> > +	ControlList *metadata_;
> >   };
> >   } /* namespace libcamera */
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 0c5939f7b3336fd089a2fe231f4f6f266cc422f7..c9aeddb62680923dc3490a23dc6c3f70f70cc075 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> &&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 008155ff21a7b398ef6bbe3c1379444ced26bdc5..706681fce5629bc4bd832923294e87a1ac8794cf 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 695762c750f89b5c8d10c221c6a9b3d7bd9ac6e7..0190f677e6794979b04b5729b2ffa88451a08ccb 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -19,7 +19,6 @@
> >   #include <libcamera/control_ids.h>
> >   #include <libcamera/formats.h>
> >   #include <libcamera/property_ids.h>
> > -#include <libcamera/request.h>
>
> I think I would keep these rather than rely on the include in
> libcamera/internal/request.h, but either way:

Why do you think so ? The internal header includes the public one,
what's the point of including it both ?

>
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

Thanks
   j

>
> >   #include <libcamera/stream.h>
> >   #include <libcamera/ipa/ipu3_ipa_interface.h>
> > @@ -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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..77f251416f718c8715d9df5d69a9c004e01b6561 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 5fd1102695070667e76daaf868e8d801f0ff70dd..320a4dc5a899b11cf56141a841f63436f0d29a37 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -26,7 +26,6 @@
> >   #include <libcamera/formats.h>
> >   #include <libcamera/framebuffer.h>
> >   #include <libcamera/property_ids.h>
> > -#include <libcamera/request.h>
> >   #include <libcamera/stream.h>
> >   #include <libcamera/transform.h>
> > @@ -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 9d65dc83573b88a4c8fd5410594085c21d91b1b1..7de45f4e5d0722a9b6425f01596455f838101900 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<Rectangle> 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<const Rectangle>(crops.data(), crops.size()));
> > +			request->_d()->metadata().set(controls::rpi::ScalerCrops,
> > +						      Span<const Rectangle>(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 15628259afc6a90511052e8ec463d4d1cad9b30b..7735d0a9ed4311d731b7ed9b01c5970fa5d2b381 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > @@ -15,7 +15,6 @@
> >   #include <vector>
> >   #include <libcamera/controls.h>
> > -#include <libcamera/request.h>
> >   #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 a06a52926441ba826972b70bc8aef068e354d843..f58208f28149fef561b9bd00d5fab18b2dd8ef9b 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -27,7 +27,6 @@
> >   #include <libcamera/camera.h>
> >   #include <libcamera/color_space.h>
> >   #include <libcamera/control_ids.h>
> > -#include <libcamera/request.h>
> >   #include <libcamera/stream.h>
> >   #include "libcamera/internal/camera.h"
> > @@ -41,6 +40,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"
> > @@ -927,8 +927,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
> > @@ -1015,7 +1015,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 cb8cc82dffd551183fbeccd5041982b1d44e9f77..3bd51733d4005347c0bdfe83db0ea5f827be441e 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -24,13 +24,13 @@
> >   #include <libcamera/control_ids.h>
> >   #include <libcamera/controls.h>
> >   #include <libcamera/property_ids.h>
> > -#include <libcamera/request.h>
> >   #include <libcamera/stream.h>
> >   #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 cd3370e3d41911f6935bead7f2d0dc5b39ee0038..4a03c149a61739d3596c11f91696db3ee5a43568 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -24,7 +24,6 @@
> >   #include <libcamera/formats.h>
> >   #include <libcamera/framebuffer.h>
> >   #include <libcamera/geometry.h>
> > -#include <libcamera/request.h>
> >   #include <libcamera/stream.h>
> >   #include <libcamera/ipa/ipa_interface.h>
> > @@ -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 7855e8b9db0c3bdfb3662a0b6e71332ed463a0ed..40c35264c5687c0f94458e19a272612cefb76285 100644
> > --- a/src/libcamera/pipeline/virtual/virtual.cpp
> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> > @@ -29,13 +29,13 @@
> >   #include <libcamera/formats.h>
> >   #include <libcamera/pixel_format.h>
> >   #include <libcamera/property_ids.h>
> > -#include <libcamera/request.h>
> >   #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 2544a059f6984d930ec909c74e0b621c9fe82726..a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930 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
> >    *
> >
>
Dan Scally Dec. 17, 2025, 9:19 a.m. UTC | #3 
Hi Jacopo - sorry for the late reply

On 15/12/2025 11:45, Jacopo Mondi wrote:
> Hi Dan
>     thanks for the review
> On Mon, Dec 15, 2025 at 11:39:42AM +0000, Dan Scally wrote:
>> Hi Jacopo
>>
>> On 02/12/2025 14:49, Jacopo Mondi wrote:
>>> 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 <jacopo.mondi@ideasonboard.com>
>>> Acked-by: Paul Elder <paul.elder@ideasonboard.com>
>>> ---
>>
>> Only one trivial comment below.
>>
>>>    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 ++++----
>>>    src/libcamera/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 78cb99f3604504dfb7145c605835b7493b656ced..643f67cabf5778f7515c0117d06d4e0a3fdec4f8 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<FrameBuffer *> pending_;
>>>    	std::map<FrameBuffer *, EventNotifier> notifiers_;
>>>    	std::unique_ptr<Timer> timer_;
>>> +	ControlList *metadata_;
>>>    };
>>>    } /* namespace libcamera */
>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
>>> index 0c5939f7b3336fd089a2fe231f4f6f266cc422f7..c9aeddb62680923dc3490a23dc6c3f70f70cc075 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> &&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 008155ff21a7b398ef6bbe3c1379444ced26bdc5..706681fce5629bc4bd832923294e87a1ac8794cf 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 695762c750f89b5c8d10c221c6a9b3d7bd9ac6e7..0190f677e6794979b04b5729b2ffa88451a08ccb 100644
>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> @@ -19,7 +19,6 @@
>>>    #include <libcamera/control_ids.h>
>>>    #include <libcamera/formats.h>
>>>    #include <libcamera/property_ids.h>
>>> -#include <libcamera/request.h>
>>
>> I think I would keep these rather than rely on the include in
>> libcamera/internal/request.h, but either way:
> 
> Why do you think so ? The internal header includes the public one,
> what's the point of including it both ?

I lean towards the view that it's better to always directly include the header that defines the 
types a source-file uses on the grounds that it's usually easier to intuit where to find the 
definitions and makes refactoring easier...though in this case of course we're highly unlikely to 
refactor and the fact that both headers are called "request.h" probably erodes the benefits.

Thanks
Dan

> 
>>
>> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> 
> Thanks
>     j
> 
>>
>>>    #include <libcamera/stream.h>
>>>    #include <libcamera/ipa/ipu3_ipa_interface.h>
>>> @@ -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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..77f251416f718c8715d9df5d69a9c004e01b6561 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 5fd1102695070667e76daaf868e8d801f0ff70dd..320a4dc5a899b11cf56141a841f63436f0d29a37 100644
>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>>> @@ -26,7 +26,6 @@
>>>    #include <libcamera/formats.h>
>>>    #include <libcamera/framebuffer.h>
>>>    #include <libcamera/property_ids.h>
>>> -#include <libcamera/request.h>
>>>    #include <libcamera/stream.h>
>>>    #include <libcamera/transform.h>
>>> @@ -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 9d65dc83573b88a4c8fd5410594085c21d91b1b1..7de45f4e5d0722a9b6425f01596455f838101900 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<Rectangle> 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<const Rectangle>(crops.data(), crops.size()));
>>> +			request->_d()->metadata().set(controls::rpi::ScalerCrops,
>>> +						      Span<const Rectangle>(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 15628259afc6a90511052e8ec463d4d1cad9b30b..7735d0a9ed4311d731b7ed9b01c5970fa5d2b381 100644
>>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
>>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
>>> @@ -15,7 +15,6 @@
>>>    #include <vector>
>>>    #include <libcamera/controls.h>
>>> -#include <libcamera/request.h>
>>>    #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 a06a52926441ba826972b70bc8aef068e354d843..f58208f28149fef561b9bd00d5fab18b2dd8ef9b 100644
>>> --- a/src/libcamera/pipeline/simple/simple.cpp
>>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>>> @@ -27,7 +27,6 @@
>>>    #include <libcamera/camera.h>
>>>    #include <libcamera/color_space.h>
>>>    #include <libcamera/control_ids.h>
>>> -#include <libcamera/request.h>
>>>    #include <libcamera/stream.h>
>>>    #include "libcamera/internal/camera.h"
>>> @@ -41,6 +40,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"
>>> @@ -927,8 +927,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
>>> @@ -1015,7 +1015,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 cb8cc82dffd551183fbeccd5041982b1d44e9f77..3bd51733d4005347c0bdfe83db0ea5f827be441e 100644
>>> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
>>> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
>>> @@ -24,13 +24,13 @@
>>>    #include <libcamera/control_ids.h>
>>>    #include <libcamera/controls.h>
>>>    #include <libcamera/property_ids.h>
>>> -#include <libcamera/request.h>
>>>    #include <libcamera/stream.h>
>>>    #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 cd3370e3d41911f6935bead7f2d0dc5b39ee0038..4a03c149a61739d3596c11f91696db3ee5a43568 100644
>>> --- a/src/libcamera/pipeline/vimc/vimc.cpp
>>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
>>> @@ -24,7 +24,6 @@
>>>    #include <libcamera/formats.h>
>>>    #include <libcamera/framebuffer.h>
>>>    #include <libcamera/geometry.h>
>>> -#include <libcamera/request.h>
>>>    #include <libcamera/stream.h>
>>>    #include <libcamera/ipa/ipa_interface.h>
>>> @@ -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 7855e8b9db0c3bdfb3662a0b6e71332ed463a0ed..40c35264c5687c0f94458e19a272612cefb76285 100644
>>> --- a/src/libcamera/pipeline/virtual/virtual.cpp
>>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
>>> @@ -29,13 +29,13 @@
>>>    #include <libcamera/formats.h>
>>>    #include <libcamera/pixel_format.h>
>>>    #include <libcamera/property_ids.h>
>>> -#include <libcamera/request.h>
>>>    #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 2544a059f6984d930ec909c74e0b621c9fe82726..a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930 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
>>>     *
>>>
>>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
index 78cb99f3604504dfb7145c605835b7493b656ced..643f67cabf5778f7515c0117d06d4e0a3fdec4f8 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<FrameBuffer *> pending_;
 	std::map<FrameBuffer *, EventNotifier> notifiers_;
 	std::unique_ptr<Timer> timer_;
+	ControlList *metadata_;
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 0c5939f7b3336fd089a2fe231f4f6f266cc422f7..c9aeddb62680923dc3490a23dc6c3f70f70cc075 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> &&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 008155ff21a7b398ef6bbe3c1379444ced26bdc5..706681fce5629bc4bd832923294e87a1ac8794cf 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 695762c750f89b5c8d10c221c6a9b3d7bd9ac6e7..0190f677e6794979b04b5729b2ffa88451a08ccb 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -19,7 +19,6 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/formats.h>
 #include <libcamera/property_ids.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #include <libcamera/ipa/ipu3_ipa_interface.h>
@@ -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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..77f251416f718c8715d9df5d69a9c004e01b6561 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 5fd1102695070667e76daaf868e8d801f0ff70dd..320a4dc5a899b11cf56141a841f63436f0d29a37 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -26,7 +26,6 @@ 
 #include <libcamera/formats.h>
 #include <libcamera/framebuffer.h>
 #include <libcamera/property_ids.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 #include <libcamera/transform.h>
 
@@ -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 9d65dc83573b88a4c8fd5410594085c21d91b1b1..7de45f4e5d0722a9b6425f01596455f838101900 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<Rectangle> 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<const Rectangle>(crops.data(), crops.size()));
+			request->_d()->metadata().set(controls::rpi::ScalerCrops,
+						      Span<const Rectangle>(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 15628259afc6a90511052e8ec463d4d1cad9b30b..7735d0a9ed4311d731b7ed9b01c5970fa5d2b381 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -15,7 +15,6 @@ 
 #include <vector>
 
 #include <libcamera/controls.h>
-#include <libcamera/request.h>
 
 #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 a06a52926441ba826972b70bc8aef068e354d843..f58208f28149fef561b9bd00d5fab18b2dd8ef9b 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -27,7 +27,6 @@ 
 #include <libcamera/camera.h>
 #include <libcamera/color_space.h>
 #include <libcamera/control_ids.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/camera.h"
@@ -41,6 +40,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"
@@ -927,8 +927,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
@@ -1015,7 +1015,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 cb8cc82dffd551183fbeccd5041982b1d44e9f77..3bd51733d4005347c0bdfe83db0ea5f827be441e 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -24,13 +24,13 @@ 
 #include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
 #include <libcamera/property_ids.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #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 cd3370e3d41911f6935bead7f2d0dc5b39ee0038..4a03c149a61739d3596c11f91696db3ee5a43568 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -24,7 +24,6 @@ 
 #include <libcamera/formats.h>
 #include <libcamera/framebuffer.h>
 #include <libcamera/geometry.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #include <libcamera/ipa/ipa_interface.h>
@@ -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 7855e8b9db0c3bdfb3662a0b6e71332ed463a0ed..40c35264c5687c0f94458e19a272612cefb76285 100644
--- a/src/libcamera/pipeline/virtual/virtual.cpp
+++ b/src/libcamera/pipeline/virtual/virtual.cpp
@@ -29,13 +29,13 @@ 
 #include <libcamera/formats.h>
 #include <libcamera/pixel_format.h>
 #include <libcamera/property_ids.h>
-#include <libcamera/request.h>
 
 #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 2544a059f6984d930ec909c74e0b621c9fe82726..a661b2f5c8ae9ae2bcbab2dcdceeef7dcb8d0930 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
  *