[libcamera-devel] [PATCH v3 05/14] libcamera: camera: Provide a list of ControlInfo

[Laurent Pinchart]

Hi Jacopo,

On Mon, Jul 01, 2019 at 06:48:14PM +0200, Jacopo Mondi wrote:
> On Mon, Jul 01, 2019 at 02:38:08AM +0300, Laurent Pinchart wrote:
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> > Extend the Camera class to expose the controls it supports. Each
> > pipeline should generate a list of controls supported by each camera it
> > creates. These are represented by a ControlInfoMap, and an associated
> > ControlList of default values.
> >
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/camera.h               |  3 +++
> >  include/libcamera/controls.h             |  2 ++
> >  src/libcamera/camera.cpp                 | 12 ++++++++++++
> >  src/libcamera/controls.cpp               |  5 +++++
> >  src/libcamera/include/pipeline_handler.h |  4 ++++
> >  src/libcamera/pipeline_handler.cpp       | 19 +++++++++++++++++++
> >  6 files changed, 45 insertions(+)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index fb2f7ba3423c..6d693d9a6c7a 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -12,6 +12,7 @@
> >  #include <set>
> >  #include <string>
> >
> > +#include <libcamera/controls.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/signal.h>
> >  #include <libcamera/stream.h>
> > @@ -83,6 +84,8 @@ public:
> >  	int acquire();
> >  	int release();
> >
> > +	const ControlInfoMap &controls();
> 
> can't this be called on const instances?
> Or there is no such a thing as a const Camera instances? Same question
> for the pipeline handler operation, but in that case I doubt we'll
> have any const pipeline handler.

If we want to call this on const Camera instance we need const
PipelineHandler instances, and it trickles down to more classes, until
it creates compilation errors that were hard to fix last time I tried.
As we have no const Camera instance, I don't think it's an issue.

> This apart
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > +
> >  	const std::set<Stream *> &streams() const;
> >  	std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles);
> >  	int configure(CameraConfiguration *config);
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index ad2d49d522c5..9b37dfb16b89 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -99,6 +99,8 @@ static inline bool operator!=(const ControlInfo &lhs, const ControlId &rhs)
> >  	return !(lhs == rhs);
> >  }
> >
> > +using ControlInfoMap = std::unordered_map<ControlId, ControlInfo>;
> > +
> >  class ControlList
> >  {
> >  private:
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 617ea99cdf71..592dfd39eacc 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -548,6 +548,18 @@ int Camera::release()
> >  	return 0;
> >  }
> >
> > +/**
> > + * \brief Retrieve the list of controls supported by the camera
> > + *
> > + * Camera controls remain constant through the lifetime of the camera.
> > + *
> > + * \return A ControlInfoMap listing the controls supported by the camera
> > + */
> > +const ControlInfoMap &Camera::controls()
> > +{
> > +	return pipe_->controls(this);
> > +}
> > +
> >  /**
> >   * \brief Retrieve all the camera's stream information
> >   *
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 22db2b93eff2..e4c41b97a354 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -315,6 +315,11 @@ bool operator==(const ControlInfo &lhs, const ControlId &rhs)
> >  	return lhs.id() == rhs;
> >  }
> >
> > +/**
> > + * \typedef ControlInfoMap
> > + * \brief A map of ControlId to ControlInfo
> > + */
> > +
> >  /**
> >   * \class ControlList
> >   * \brief Associates a list of ControlIds with their values for a Camera.
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index a4cbc593a179..f836d5d1a600 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -14,6 +14,7 @@
> >  #include <string>
> >  #include <vector>
> >
> > +#include <libcamera/controls.h>
> >  #include <libcamera/stream.h>
> >
> >  namespace libcamera {
> > @@ -41,6 +42,7 @@ public:
> >  	Camera *camera_;
> >  	PipelineHandler *pipe_;
> >  	std::list<Request *> queuedRequests_;
> > +	ControlInfoMap controlInfo_;
> >
> >  private:
> >  	CameraData(const CameraData &) = delete;
> > @@ -60,6 +62,8 @@ public:
> >  	bool lock();
> >  	void unlock();
> >
> > +	const ControlInfoMap &controls(Camera *camera);
> > +
> >  	virtual CameraConfiguration *generateConfiguration(Camera *camera,
> >  		const StreamRoles &roles) = 0;
> >  	virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index c91ef2f7d336..0283e4e5ad51 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -88,6 +88,14 @@ LOG_DEFINE_CATEGORY(Pipeline)
> >   * PipelineHandler::completeRequest()
> >   */
> >
> > +/**
> > + * \var CameraData::controlInfo_
> > + * \brief The set of controls supported by the camera
> > + *
> > + * The control information shall be initialised by the pipeline handler when
> > + * creating the camera, and shall not be modified afterwards.
> > + */
> > +
> >  /**
> >   * \class PipelineHandler
> >   * \brief Create and manage cameras based on a set of media devices
> > @@ -217,6 +225,17 @@ void PipelineHandler::unlock()
> >  		media->unlock();
> >  }
> >
> > +/**
> > + * \brief Retrieve the list of controls for a camera
> > + * \param[in] camera The camera
> > + * \return A ControlInfoMap listing the controls support by \a camera
> > + */
> > +const ControlInfoMap &PipelineHandler::controls(Camera *camera)
> > +{
> > +	CameraData *data = cameraData(camera);
> > +	return data->controlInfo_;
> > +}
> > +
> >  /**
> >   * \fn PipelineHandler::generateConfiguration()
> >   * \brief Generate a camera configuration for a specified camera

-- 
Regards,

Laurent Pinchart