[libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add function to fetch subdev controls

[Laurent Pinchart]

Hi Daniel,

Thank you for the patch.

On Fri, Dec 03, 2021 at 10:42:27PM +0000, Daniel Scally wrote:
> Add a function to the CameraLens class to fetch the V4L2 controls
> for its V4L2 subdev
> 
> Signed-off-by: Daniel Scally <djrscally at gmail.com>
> ---
> Changes in v2:
> 
> 	- New patch
> 
>  include/libcamera/internal/camera_lens.h |  4 ++++
>  src/libcamera/camera_lens.cpp            | 11 +++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h
> index 6f2ea1bc..64794294 100644
> --- a/include/libcamera/internal/camera_lens.h
> +++ b/include/libcamera/internal/camera_lens.h
> @@ -12,6 +12,8 @@
>  #include <libcamera/base/class.h>
>  #include <libcamera/base/log.h>
>  
> +#include <libcamera/controls.h>
> +
>  namespace libcamera {
>  
>  class MediaEntity;
> @@ -28,6 +30,8 @@ public:
>  
>  	const std::string &model() const { return model_; }
>  
> +	const ControlInfoMap &controls() const;
> +
>  protected:
>  	std::string logPrefix() const override;
>  
> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> index 189cb025..7b3b9c24 100644
> --- a/src/libcamera/camera_lens.cpp
> +++ b/src/libcamera/camera_lens.cpp
> @@ -139,4 +139,15 @@ std::string CameraLens::logPrefix() const
>  	return "'" + entity_->name() + "'";
>  }
>  
> +/**
> + * \fn CameraLens::controls()
> + * \brief Retrieve the V4L2 controls of the lens' subdev
> + *
> + * \return A map of the V4L2 controls supported by the sensor

Not a sensor anymore. With that fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I however would like to take one more step, and was wondering if you'd
like to volunteer for it.

I think the CameraSensor and CameraLens classes shouldn't expose V4L2
controls, but a custom set of controls specific to libcamera. This way
we could remove the dependencies on V4L2 in the IPA modules. The
translation between those controls and the V4L2 controls would be local
to the CameraSensor and CameraLens classes.

We would need three controls at the moment, for the analogue gain,
exposure time, and focus position. It could be a good idea to decouple
those controls from the libcamera public controls (exposed by cameras to
applications), but given that we have libcamera controls for those three
already, we could start by using them and see if we need something else
later.

I'm also wondering if we could, this way, combine the sensor controls
and lens controls in a single ControlInfoMap passed to the IPA, and a
single ControlList passed back from the IPA.

> + */
> +const ControlInfoMap &CameraLens::controls() const
> +{
> +	return subdev_->controls();
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart