[v2,2/3] libcamera: request: Move metadata_ to Private
diff mbox series

Message ID 20251126-cam-control-override-v2-2-305024a1f190@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: ipc: ControlLists without valid idmap break IPC
Related show

Commit Message

Jacopo Mondi Nov. 26, 2025, 9:38 a.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>
---
 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

Barnabás Pőcze Dec. 1, 2025, 9:57 a.m. UTC | #1
Hi

2025. 11. 26. 10:38 keltezéssel, Jacopo Mondi írta:
> 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>
> ---
>   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_;

I think it should be just `ControlList metadata_`, no pointer.


>   };
>   
>   } /* namespace libcamera */
> [...]
> 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)

Then

   metadata_(controls::controls)

here.


Regards,
Barnabás Pőcze

>   {
> +	/**
> +	 * \todo Add a validator for metadata controls.
> +	 */
> +	metadata_ = new ControlList(controls::controls);
>   }
>   
> [...]
Jacopo Mondi Dec. 1, 2025, 10:11 a.m. UTC | #2
Hi Barnabás

On Mon, Dec 01, 2025 at 10:57:10AM +0100, Barnabás Pőcze wrote:
> Hi
>
> 2025. 11. 26. 10:38 keltezéssel, Jacopo Mondi írta:
> > 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>
> > ---
> >   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_;
>
> I think it should be just `ControlList metadata_`, no pointer.

it could, but I'm moving code around, not modifying it in this patch,
so  prefer to keep it as it was. We can change it on top ?

>
>
> >   };
> >   } /* namespace libcamera */
> > [...]
> > 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)
>
> Then
>
>   metadata_(controls::controls)
>
> here.
>
>
> Regards,
> Barnabás Pőcze
>
> >   {
> > +	/**
> > +	 * \todo Add a validator for metadata controls.
> > +	 */
> > +	metadata_ = new ControlList(controls::controls);
> >   }
> > [...]
Paul Elder Dec. 1, 2025, 10:23 a.m. UTC | #3
Quoting Jacopo Mondi (2025-11-26 18:38:52)
> 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>

Looks good to me.

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(-)
> 
> 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 9550f54600c4d98e9d110a5e255d1ac85e2b5660..b0fb117b52e2a00fe9b5289c957442c1142fe7c6 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -26,6 +26,7 @@
>  #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/v4l2_subdevice.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>  
> @@ -1151,7 +1152,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 d6b7edcb5a7f9fcb4083d4105a193978e265ef67..49df8fe5b41035b164c3a6828b791ef084ff18c8 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 38bdc6138ed167a2c05f43ca30e60ed549fd953c..68be4cd66050b56e50805336efa5da8dafa4d3b9 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 d7195b6c484d091857a91de709e0e060810c7c32..823160f58d989600ff3213df6bbec2a015d8745f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -25,7 +25,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>
>  
> @@ -44,6 +43,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"
>  
> @@ -452,7 +452,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);
> @@ -1518,8 +1518,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 =
> @@ -1602,7 +1602,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)
>                 LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: "
>                                    << strerror(-ret);
>  
> -       request->metadata().set(controls::ScalerCrop, activeCrop_.value());
> +       request->_d()->metadata().set(controls::ScalerCrop, activeCrop_.value());
>  }
>  
>  void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index c209aa596311a7d902e32caedc3f98cfa3da2b09..4a15839afbc32c3e57caff11904484540a61745e 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -1219,7 +1219,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
> @@ -1490,9 +1490,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;
> @@ -1500,10 +1500,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 4bce4ec4f978cedd7d10d354ce2231c9296a3e39..d9114f1cb954e541b2a35a748a95cca0af59ebbe 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 97009fbc03c64108a23f9edf6df95177593f9035..58cac390eb5be7231101c92fda1caf8e8b088e63 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"
> @@ -926,8 +926,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
> @@ -1014,7 +1014,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 48d6b39d532009bc32a2148f1d35b3a3be0bc0d4..21f2d28399bdfe97e0d97bf2e025721de36607c8 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 5022101505a15af1ebf6a8be4a52bcefb1fccfa0..9b1cfb090a7a274894b90969c4dad361c6748cea 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
>   *
> 
> -- 
> 2.51.1
>

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 9550f54600c4d98e9d110a5e255d1ac85e2b5660..b0fb117b52e2a00fe9b5289c957442c1142fe7c6 100644
--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
@@ -26,6 +26,7 @@ 
 #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/v4l2_subdevice.h"
 #include "libcamera/internal/v4l2_videodevice.h"
 
@@ -1151,7 +1152,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 d6b7edcb5a7f9fcb4083d4105a193978e265ef67..49df8fe5b41035b164c3a6828b791ef084ff18c8 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 38bdc6138ed167a2c05f43ca30e60ed549fd953c..68be4cd66050b56e50805336efa5da8dafa4d3b9 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 d7195b6c484d091857a91de709e0e060810c7c32..823160f58d989600ff3213df6bbec2a015d8745f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -25,7 +25,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>
 
@@ -44,6 +43,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"
 
@@ -452,7 +452,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);
@@ -1518,8 +1518,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 =
@@ -1602,7 +1602,7 @@  void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer)
 		LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: "
 				   << strerror(-ret);
 
-	request->metadata().set(controls::ScalerCrop, activeCrop_.value());
+	request->_d()->metadata().set(controls::ScalerCrop, activeCrop_.value());
 }
 
 void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index c209aa596311a7d902e32caedc3f98cfa3da2b09..4a15839afbc32c3e57caff11904484540a61745e 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -1219,7 +1219,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
@@ -1490,9 +1490,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;
@@ -1500,10 +1500,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 4bce4ec4f978cedd7d10d354ce2231c9296a3e39..d9114f1cb954e541b2a35a748a95cca0af59ebbe 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 97009fbc03c64108a23f9edf6df95177593f9035..58cac390eb5be7231101c92fda1caf8e8b088e63 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"
@@ -926,8 +926,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
@@ -1014,7 +1014,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 48d6b39d532009bc32a2148f1d35b3a3be0bc0d4..21f2d28399bdfe97e0d97bf2e025721de36607c8 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 5022101505a15af1ebf6a8be4a52bcefb1fccfa0..9b1cfb090a7a274894b90969c4dad361c6748cea 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
  *