[libcamera-devel] [PATCH v5 3/4] libcamera: camera_lens: Add a new class to model a camera lens

[Laurent Pinchart]

Hi Han-lin,

Thank you for the patch.

On Fri, Nov 26, 2021 at 07:29:02PM +0800, Han-Lin Chen wrote:
> The CameraLens class abstracts camera lens and provides helper
> functions to ease interactions with them.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  Documentation/index.rst                    |   1 +
>  Documentation/lens_driver_requirements.rst |  27 ++++
>  Documentation/meson.build                  |   1 +
>  include/libcamera/internal/camera_lens.h   |  48 +++++++
>  include/libcamera/internal/meson.build     |   1 +
>  src/libcamera/camera_lens.cpp              | 142 +++++++++++++++++++++
>  src/libcamera/meson.build                  |   1 +
>  7 files changed, 221 insertions(+)
>  create mode 100644 Documentation/lens_driver_requirements.rst
>  create mode 100644 include/libcamera/internal/camera_lens.h
>  create mode 100644 src/libcamera/camera_lens.cpp
> 
> diff --git a/Documentation/index.rst b/Documentation/index.rst
> index 1f4fc485..0ee10044 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -21,3 +21,4 @@
>     Tracing guide <guides/tracing>
>     Environment variables <environment_variables>
>     Sensor driver requirements <sensor_driver_requirements>
> +   Lens driver requirements <lens_driver_requirements>
> diff --git a/Documentation/lens_driver_requirements.rst b/Documentation/lens_driver_requirements.rst
> new file mode 100644
> index 00000000..21e778fd
> --- /dev/null
> +++ b/Documentation/lens_driver_requirements.rst
> @@ -0,0 +1,27 @@
> +.. SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +.. _lens-driver-requirements:
> +
> +Lens Driver Requirements
> +========================
> +
> +libcamera handles lens devices in the CameraLens class and defines
> +a consistent interface through its API towards other library components.
> +
> +The CameraLens class uses the V4L2 subdev kernel API to interface with the
> +camera lens through a sub-devices exposed in userspace by the lens driver.

s/sub-devices/sub-device/
s/exposed in/exposed to/

> +
> +In order for libcamera to be fully operational and provide all the required
> +information to interface with the camera lens to applications and pipeline
> +handlers, a set of mandatory features the driver has to support has been defined.
> +
> +Mandatory Requirements
> +----------------------
> +
> +The lens driver is assumed to be fully compliant with the V4L2 specification.
> +
> +The lens driver shall support the following V4L2 controls:
> +
> +* `V4L2_CID_FOCUS_ABSOLUTE`_
> +
> +.. _V4L2_CID_FOCUS_ABSOLUTE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-camera.html
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 4c972675..33af82aa 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -66,6 +66,7 @@ if sphinx.found()
>          'guides/pipeline-handler.rst',
>          'guides/tracing.rst',
>          'index.rst',
> +        'lens_driver_requirements.rst',
>          'sensor_driver_requirements.rst',
>         '../README.rst',
>      ]
> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h
> new file mode 100644
> index 00000000..3597e929
> --- /dev/null
> +++ b/include/libcamera/internal/camera_lens.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_lens.h - A camera lens
> + */
> +#ifndef __LIBCAMERA_INTERNAL_CAMERA_LENS_H__
> +#define __LIBCAMERA_INTERNAL_CAMERA_LENS_H__

This should be #pragma once now.

> +
> +#include <memory>
> +#include <string>
> +
> +#include <libcamera/base/class.h>
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +class MediaEntity;
> +class V4L2Subdevice;
> +
> +class CameraLens : protected Loggable
> +{
> +public:
> +	explicit CameraLens(const MediaEntity *entity);
> +	~CameraLens();
> +
> +	int init();
> +	int setFocusPostion(int32_t position);

Can the position be negative ?

With these issues addressed,

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

> +
> +	const std::string &model() const { return model_; }
> +
> +protected:
> +	std::string logPrefix() const override;
> +
> +private:
> +	LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraLens)
> +
> +	int validateLensDriver();
> +
> +	const MediaEntity *entity_;
> +	std::unique_ptr<V4L2Subdevice> subdev_;
> +
> +	std::string model_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_CAMERA_LENS_H__ */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 665fd6de..a96bbb95 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -14,6 +14,7 @@ libcamera_internal_headers = files([
>      'byte_stream_buffer.h',
>      'camera.h',
>      'camera_controls.h',
> +    'camera_lens.h',
>      'camera_sensor.h',
>      'camera_sensor_properties.h',
>      'control_serializer.h',
> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> new file mode 100644
> index 00000000..189cb025
> --- /dev/null
> +++ b/src/libcamera/camera_lens.cpp
> @@ -0,0 +1,142 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_lens.cpp - A camera lens
> + */
> +
> +#include "libcamera/internal/camera_lens.h"
> +
> +#include <libcamera/base/utils.h>
> +
> +#include "libcamera/internal/v4l2_subdevice.h"
> +
> +/**
> + * \file camera_lens.h
> + * \brief A camera lens controller
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(CameraLens)
> +
> +/**
> + * \class CameraLens
> + * \brief A camera lens based on V4L2 subdevices
> + *
> + * The CameraLens class eases handling of lens for pipeline handlers by
> + * hiding the details of the V4L2 subdevice kernel API and caching lens
> + * information.
> + */
> +
> +/**
> + * \brief Construct a CameraLens
> + * \param[in] entity The media entity backing the camera lens controller
> + *
> + * Once constructed the instance must be initialized with init().
> + */
> +CameraLens::CameraLens(const MediaEntity *entity)
> +	: entity_(entity)
> +{
> +}
> +
> +/**
> + * \brief Destroy a CameraLens
> + */
> +CameraLens::~CameraLens() = default;
> +
> +/**
> + * \brief Initialize the camera lens instance
> + *
> + * This function performs the initialisation steps of the CameraLens that may
> + * fail. It shall be called once and only once after constructing the instance.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraLens::init()
> +{
> +	if (entity_->function() != MEDIA_ENT_F_LENS) {
> +		LOG(CameraLens, Error)
> +			<< "Invalid lens function "
> +			<< utils::hex(entity_->function());
> +		return -EINVAL;
> +	}
> +
> +	/* Create and open the subdev. */
> +	subdev_ = std::make_unique<V4L2Subdevice>(entity_);
> +	int ret = subdev_->open();
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = validateLensDriver();
> +	if (ret)
> +		return ret;
> +
> +	model_ = subdev_->model();
> +	return 0;
> +}
> +
> +/**
> + * \brief This function sets the focal point of the lens to a specific position.
> + * \param[in] position The focal point of the lens
> + *
> + * This function sets the value of focal point of the lens as in \a position.
> + *
> + * \return 0 on success or -EINVAL otherwise
> + */
> +int CameraLens::setFocusPostion(int32_t position)
> +{
> +	ControlList lensCtrls(subdev_->controls());
> +	lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, static_cast<int32_t>(position));
> +
> +	if (subdev_->setControls(&lensCtrls))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +int CameraLens::validateLensDriver()
> +{
> +	int ret = 0;
> +	static constexpr uint32_t mandatoryControls[] = {
> +		V4L2_CID_FOCUS_ABSOLUTE,
> +	};
> +
> +	const ControlInfoMap &controls = subdev_->controls();
> +	for (uint32_t ctrl : mandatoryControls) {
> +		if (!controls.count(ctrl)) {
> +			LOG(CameraLens, Error)
> +				<< "Mandatory V4L2 control " << utils::hex(ctrl)
> +				<< " not available";
> +			ret = -EINVAL;
> +		}
> +	}
> +
> +	if (ret) {
> +		LOG(CameraLens, Error)
> +			<< "The lens kernel driver needs to be fixed";
> +		LOG(CameraLens, Error)
> +			<< "See Documentation/lens_driver_requirements.rst in"
> +			<< " the libcamera sources for more information";
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * \fn CameraLens::model()
> + * \brief Retrieve the lens model name
> + *
> + * The lens model name is a free-formed string that uniquely identifies the
> + * lens model.
> + *
> + * \return The lens model name
> + */
> +
> +std::string CameraLens::logPrefix() const
> +{
> +	return "'" + entity_->name() + "'";
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 6727a777..3bee8fee 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_sources = files([
>      'byte_stream_buffer.cpp',
>      'camera.cpp',
>      'camera_controls.cpp',
> +    'camera_lens.cpp',
>      'camera_manager.cpp',
>      'camera_sensor.cpp',
>      'camera_sensor_properties.cpp',

-- 
Regards,

Laurent Pinchart