[libcamera-devel] [PATCH v7 4/5] libcamera: camera_lens: add CameraLens as a member of CameraSensor

[Laurent Pinchart]

Hi Han-lin,

Thank you for the patch.

On Thu, Dec 02, 2021 at 10:03:16PM +0800, Han-Lin Chen wrote:
> Add CameraLens as a member of CameraSenosr. The patch does not implement how

s/CameraSenosr/CameraSensor/

> to link the lens to the specific sensor yet. Only to provide an interface for
> pipeline handler's usage.

It's a good idea, it can be merged right away, and allows rebasing
ongoing developments on top.

> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>  include/libcamera/internal/camera_sensor.h | 5 +++++
>  src/libcamera/camera_sensor.cpp            | 8 ++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d25a1165..8e97a80e 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -24,6 +24,7 @@
>  namespace libcamera {
>  
>  class BayerFormat;
> +class CameraLens;
>  class MediaEntity;
>  
>  class CameraSensor : protected Loggable
> @@ -60,6 +61,8 @@ public:
>  
>  	void updateControlInfo();
>  
> +	CameraLens *lens() { return lens_.get(); }
> +
>  protected:
>  	std::string logPrefix() const override;
>  
> @@ -91,6 +94,8 @@ private:
>  	const BayerFormat *bayerFormat_;
>  
>  	ControlList properties_;
> +
> +	std::unique_ptr<CameraLens> lens_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 6151b32e..b386e7b0 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -20,6 +20,7 @@
>  #include <libcamera/base/utils.h>
>  
>  #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor_properties.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/sysfs.h"
> @@ -787,6 +788,13 @@ void CameraSensor::updateControlInfo()
>  	subdev_->updateControlInfo();
>  }
>  
> +/**
> + * \fn CameraSensor::lens()

I wonder if we should call this focusLens() (and focusLens_, as well as
s/lens/focus lens/ in the rest of this comment block) as zoom lenses are
in scope too.

> + * \brief Retrieve the lens controller
> + *
> + * \return The lens controller. nullptr if no lens is connected to the sensor

s/no lens/no lens controller/

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

> + */
> +
>  std::string CameraSensor::logPrefix() const
>  {
>  	return "'" + entity_->name() + "'";

-- 
Regards,

Laurent Pinchart