libcamera: request: Move metadata_ to Private

Address a long standing \todo item that suggested to implement a
read-only interface for the Request::metadata() accessor and deflect to
the internal implementation for the read-write accessor used by pipeline
handlers.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Acked-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

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 */

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_;

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);

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")

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);
 }

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)

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()));
 		}
 	}
 }

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"
 

src/libcamera/pipeline/simple/simple.cpp

@@ -30,7 +30,6 @@
 #include <libcamera/control_ids.h>
 #include <libcamera/geometry.h>
 #include <libcamera/pixel_format.h>
-#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/camera.h"
@@ -44,6 +43,7 @@
 #include "libcamera/internal/global_configuration.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
+#include "libcamera/internal/request.h"
 #include "libcamera/internal/software_isp/software_isp.h"
 #include "libcamera/internal/v4l2_subdevice.h"
 #include "libcamera/internal/v4l2_videodevice.h"
@@ -940,8 +940,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
 	}
 
 	if (request)
-		request->metadata().set(controls::SensorTimestamp,
-					buffer->metadata().timestamp);
+		request->_d()->metadata().set(controls::SensorTimestamp,
+					      buffer->metadata().timestamp);
 
 	/*
 	 * Queue the captured and the request buffer to the converter or Software
@@ -1036,7 +1036,7 @@ void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata
 	if (!info)
 		return;
 
-	info->request->metadata().merge(metadata);
+	info->request->_d()->metadata().merge(metadata);
 	info->metadataProcessed = true;
 	tryCompleteRequest(info->request);
 }

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);

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);

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);
 

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
  *