[libcamera-devel] [RFC PATCH v2 6/9] libcamera: request: Add a ControlList

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Sun, Jun 23, 2019 at 05:36:52PM +0200, Niklas Söderlund wrote:
> On 2019-06-21 17:13:58 +0100, Kieran Bingham wrote:
> > Provide a ControlList on request objects to facilitate setting controls
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  include/libcamera/request.h |  3 +++
> >  src/libcamera/request.cpp   | 10 ++++++++++
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 58de6f00a554..8075270a9a12 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -11,6 +11,7 @@
> >  #include <unordered_set>
> >  
> >  #include <libcamera/signal.h>
> > +#include <libcamera/controls.h>
> 
> Alphabetical order please.
> 
> With this fixed,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
> >  
> >  namespace libcamera {
> >  
> > @@ -32,6 +33,7 @@ public:
> >  	Request(const Request &) = delete;
> >  	Request &operator=(const Request &) = delete;
> >  
> > +	ControlList &controls() { return controls_; }
> >  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> >  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
> >  	Buffer *findBuffer(Stream *stream) const;
> > @@ -50,6 +52,7 @@ private:
> >  	bool completeBuffer(Buffer *buffer);
> >  
> >  	Camera *camera_;
> > +	ControlList controls_;
> >  	std::map<Stream *, Buffer *> bufferMap_;
> >  	std::unordered_set<Buffer *> pending_;
> >  
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index fa3ee46da440..9a0409b46017 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -52,6 +52,16 @@ Request::Request(Camera *camera)
> >  {
> >  }
> >  
> > +/**
> > + * \fn Request::controls()
> > + * \brief Retrieve the request's ControlList
> > + *
> > + * Return a reference to the ControlList that stores all the controls relevant
> > + * to this request.
> > + *
> > + * \return A reference to the ControlList in this request
> > + */

We need more documentation, with a focus on how this API should be used
by applications. I also expect that you will need to reset the controls
once the request complete, as request objects can be reused.

> > +
> >  /**
> >   * \fn Request::buffers()
> >   * \brief Retrieve the request's streams to buffers map

-- 
Regards,

Laurent Pinchart