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

Message ID 20251202-cam-control-override-v3-2-eacab052798d@ideasonboard.com
State New
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
> >    *
> >
>

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
  *