[libcamera-devel] [PATCH v4 4/6] libcamera: camera_sensor: Add V4L2 control operations

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Jun 19, 2019 at 01:08:56PM +0200, Jacopo Mondi wrote:
> Add operations to get and set control and to retrieve the informations
> on a V4L2 control. For simple camera sensors, the operations are
> directly called on the underlying V4L2 subdevice.

This makes CameraSensor V4L2-specific, but I think that's fair for now.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp       | 58 +++++++++++++++++++++++++++
>  src/libcamera/include/camera_sensor.h |  6 +++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index a804a68c9d91..28a32a96ef7b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -247,6 +247,64 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
>  	return subdev_->setFormat(0, format);
>  }
>  
> +/**
> + * \brief Retrieve the V4L2 control info from the sensor's subdevice
> + * \param id The V4L2 control id

s/control id/control ID/

> + *
> + * Retrieve the V4L2ControlInfo from the V4L2 subdevice backing the
> + * camera sensor.
> + *
> + * More complex camera sensors, which expose multiple subdevices should
> + * override this method.
> + *

Then you'll have to declare it as virtual. For now let's not bother, you
can drop this sentence. Same for the two methods below.

> + * \sa V4L2Device::getControlInfo()
> + *
> + * \return The V4L2ControlInfo associated to the V4L2 control with \a id or
> + * nullptr if the control is not supported.

Could you update the documentation for those three functions similarly
to how you'll update V4L2Device after my review ?

> + */
> +V4L2ControlInfo *CameraSensor::getControlInfo(unsigned int id)
> +{
> +	return subdev_->getControlInfo(id);
> +}
> +
> +/**
> + * \brief Read a list of controls from the sensor's subdevice
> + * \param ctrls The list of controls to read
> + *
> + * Read V4L2 controls values on the V4L2 subdevice backing the
> + * camera sensor.
> + *
> + * More complex camera sensors, which expose multiple subdevices should
> + * override this method.
> + *
> + * \sa V4L2Device::getControls()
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::getControls(V4L2Controls *ctrls)
> +{
> +	return subdev_->getControls(ctrls);
> +}
> +
> +/**
> + * \brief Write a list of controls from the sensor's subdevice
> + * \param ctrls The list of controls to write
> + *
> + * Write V4L2 controls values on the V4L2 subdevice backing the
> + * camera sensor.
> + *
> + * More complex camera sensors, which expose multiple subdevices should
> + * override this method.
> + *
> + * \sa V4L2Device::setControls()
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::setControls(V4L2Controls *ctrls)
> +{
> +	return subdev_->setControls(ctrls);
> +}
> +
>  std::string CameraSensor::logPrefix() const
>  {
>  	return "'" + subdev_->entity()->name() + "'";
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index b823480241a7..ed6bb16f5686 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -17,6 +17,8 @@
>  namespace libcamera {
>  
>  class MediaEntity;
> +class V4L2ControlInfo;
> +class V4L2Controls;
>  class V4L2Subdevice;
>  
>  struct V4L2SubdeviceFormat;
> @@ -41,6 +43,10 @@ public:
>  				      const Size &size) const;
>  	int setFormat(V4L2SubdeviceFormat *format);
>  
> +	V4L2ControlInfo *getControlInfo(unsigned int id);

	const V4L2ControlInfo *getControlInfo(unsigned int id) const;

> +	int getControls(V4L2Controls *ctrls);
> +	int setControls(V4L2Controls *ctrls);
> +
>  protected:
>  	std::string logPrefix() const;
>  

-- 
Regards,

Laurent Pinchart