[libcamera-devel] [PATCH v5 05/10] libcamera: request: Add metadata information

[Laurent Pinchart]

Hi Jacopo,

On Tue, Oct 08, 2019 at 10:35:42AM +0200, Jacopo Mondi wrote:
> On Tue, Oct 08, 2019 at 02:45:29AM +0200, Niklas Söderlund wrote:
> > A new ControlList container is needed to hold metadata coming out of
> > the IPA. The list of supported controls in this list is expected to
> > grow, so for now do not add a validator for the list.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/request.h |  2 ++
> >  src/libcamera/request.cpp   | 14 ++++++++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index e3db5243aaf3cf30..2d5a5964e99eb75f 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -37,6 +37,7 @@ public:
> >  	~Request();
> >
> >  	ControlList &controls() { return *controls_; }
> > +	ControlList &metadata() { return *metadata_; }
> 
> Just thinking out loud, should the returned & be a const ? Might
> application want to change the content of the metadata array ?

Applications shouldn't change it, but pipeline handlers need write
access to the metadata. I think we need a better interface for this, in
order to set tighter permissions for applications, but that's a broad
issue with the current code base, so I don't think it needs to be fixed
as part of this patch series.

> >  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> >  	int addBuffer(std::unique_ptr<Buffer> buffer);
> >  	Buffer *findBuffer(Stream *stream) const;
> > @@ -58,6 +59,7 @@ private:
> >  	Camera *camera_;
> >  	CameraControlValidator *validator_;
> >  	ControlList *controls_;
> > +	ControlList *metadata_;
> >  	std::map<Stream *, Buffer *> bufferMap_;
> >  	std::unordered_set<Buffer *> pending_;
> >
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index 19f6d0b9a0aeb03c..23d3ab6f422c406e 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -65,6 +65,11 @@ Request::Request(Camera *camera, uint64_t cookie)
> >  	 */
> >  	validator_ = new CameraControlValidator(camera);
> >  	controls_ = new ControlList(validator_);
> > +
> > +	/**
> > +	 * \todo: Add a validator for metadata controls.
> > +	 */
> > +	metadata_ = new ControlList(nullptr);
> >  }
> >
> >  Request::~Request()
> > @@ -74,6 +79,7 @@ Request::~Request()
> >  		delete buffer;
> >  	}
> >
> > +	delete metadata_;
> >  	delete controls_;
> >  	delete validator_;
> >  }
> > @@ -161,6 +167,14 @@ Buffer *Request::findBuffer(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
> > + */
> > +
> >  /**
> >   * \fn Request::cookie()
> >   * \brief Retrieve the cookie set when the request was created

-- 
Regards,

Laurent Pinchart