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

14389 diff mbox series

The CameraLens class abstracts camera lens and provides helper
functions to ease interactions with them.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>
---
 Documentation/index.rst                    |   1 +
 Documentation/lens_driver_requirements.rst |  28 ++
 Documentation/meson.build                  |   1 +
 include/libcamera/internal/camera_lens.h   |  61 +++++
 src/libcamera/camera_lens.cpp              | 289 +++++++++++++++++++++
 src/libcamera/meson.build                  |   1 +
 6 files changed, 381 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

Quoting Han-Lin Chen (2021-10-28 11:03:18)
> The CameraLens class abstracts camera lens and provides helper
> functions to ease interactions with them.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>
> ---
>  Documentation/index.rst                    |   1 +
>  Documentation/lens_driver_requirements.rst |  28 ++
>  Documentation/meson.build                  |   1 +
>  include/libcamera/internal/camera_lens.h   |  61 +++++
>  src/libcamera/camera_lens.cpp              | 289 +++++++++++++++++++++
>  src/libcamera/meson.build                  |   1 +
>  6 files changed, 381 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..226cb813
> --- /dev/null
> +++ b/Documentation/lens_driver_requirements.rst
> @@ -0,0 +1,28 @@
> +.. SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +.. _lens-driver-requirements:
> +
> +Lens Driver Requirements
> +==========================

two extra '==' could be removed.

> +
> +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 one or multiple sub-devices exposed in userspace by
> +the lens driver.
> +
> +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

Seeing that there is also V4L2_CID_FOCUS_RELATIVE, I assume that some
drivers might offer a different means to control.

Would an algorithm be able to be more general if it supported RELATIVE?
or are they always expected to know how to operate the lens motor and
have an absolute position request.

I suspect the V4L2_CID_FOCUS_ABSOLUTE is the only way to get the valid
range of movement out anyway, so I think this is appropriate.


> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index df36a808..746af4cf 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -60,6 +60,7 @@ if sphinx.found()
>          'guides/ipa.rst',
>          'guides/pipeline-handler.rst',
>          'guides/tracing.rst',
> +        'lens_driver_requirements.rst',

alphabetical order here please.

>          'index.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..f03de5fc
> --- /dev/null
> +++ b/include/libcamera/internal/camera_lens.h
> @@ -0,0 +1,61 @@
> +/* 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__
> +
> +#include <libcamera/base/class.h>
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/controls.h>
> +#include "libcamera/internal/v4l2_subdevice.h"
> +
> +namespace libcamera {
> +
> +class MediaEntity;
> +
> +class CameraLens : protected Loggable
> +{
> +public:
> +       explicit CameraLens(const MediaEntity *entity);
> +       ~CameraLens();
> +
> +       int init();
> +
> +       const std::string &model() const { return model_; }
> +       const std::string &id() const { return id_; }
> +       const MediaEntity *entity() const { return entity_; }
> +
> +       const ControlInfoMap &controls() const;
> +       ControlList getControls(const std::vector<uint32_t> &ids);
> +       int setControls(ControlList *ctrls);
> +
> +       V4L2Subdevice *device() { return subdev_.get(); }
> +
> +       const ControlList &properties() const { return properties_; }
> +
> +protected:
> +       std::string logPrefix() const override;
> +
> +private:
> +       LIBCAMERA_DISABLE_COPY(CameraLens)
> +
> +       int generateId();
> +       int validateLensDriver();
> +       int initProperties();
> +
> +       const MediaEntity *entity_;
> +       std::unique_ptr<V4L2Subdevice> subdev_;
> +
> +       std::string model_;
> +       std::string id_;
> +
> +       ControlList properties_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_CAMERA_LENS_H__ */
> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> new file mode 100644
> index 00000000..0c80d3c3
> --- /dev/null
> +++ b/src/libcamera/camera_lens.cpp
> @@ -0,0 +1,289 @@
> +/* 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/internal/media_device.h"
> +
> +#include <regex>
> +
> +#include <libcamera/property_ids.h>
> +
> +#include "libcamera/internal/sysfs.h"
> +
> +/**
> + * \file camera_lens.h
> + * \brief A camera lens
> + */
> +
> +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.
> + *
> + * The implementation is currently limited to lens that expose a single V4L2
> + * subdevice. It will be extended to support more complex devices as the needs
> + * arise.
> + */
> +
> +/**
> + * \brief Construct a CameraLens
> + * \param[in] entity The media entity backing the camera lens
> + *
> + * Once constructed the instance must be initialized with init().
> + */
> +CameraLens::CameraLens(const MediaEntity *entity)
> +       : entity_(entity),
> +         properties_(properties::properties)
> +{
> +}
> +
> +/**
> + * \brief Destroy a CameraLens
> + */
> +CameraLens::~CameraLens()
> +{
> +}
> +
> +/**
> + * \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;
> +
> +       ret = initProperties();
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +int CameraLens::validateLensDriver()
> +{
> +       int err = 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";
> +                       err = -EINVAL;
> +               }
> +       }
> +
> +       if (err) {
> +               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";

This goes over 120 chars, perhaps another line break?
Even the message is over 90 chars, so might be better for users if it was split.

> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +int CameraLens::initProperties()
> +{
> +       /*
> +        * Extract the camera lens model name from the media entity name.
> +        *
> +        * There is no standardized naming scheme for lens entities in the
> +        * Linux kernel at the moment.
> +        *
> +        * - The most common rule, used by I2C lens, associates the model
> +        *   name with the I2C bus number and address (e.g. 'dw9714 8-000c').
> +        *
> +        * Other schemes probably exist. As a best effort heuristic, use the
> +        * part of the entity name before the first space if the name contains
> +        * an I2C address, and use the full entity name otherwise.
> +        */
> +       std::string entityName = entity_->name();
> +       std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> +       std::smatch match;
> +
> +       if (std::regex_search(entityName, match, i2cRegex))
> +               model_ = entityName.substr(0, entityName.find(' '));
> +       else
> +               model_ = entityName;
> +
> +       properties_.set(properties::Model, utils::toAscii(model_));
> +
> +       /* Generate a unique ID for the lens. */
> +       int ret = generateId();

Does a lens need an ID like this? Can't it just be referenced as the
object when needed?

Perhaps it will become clearer later.

Will there ever be more than one lens per 'Camera'?


> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +/**
> + * \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
> + */
> +
> +/**
> + * \fn CameraLens::id()
> + * \brief Retrieve the lens ID
> + *
> + * The lens ID is a free-form string that uniquely identifies the lens in
> + * the system.
> + *
> + * \return The lens ID
> + */
> +
> +/**
> + * \fn CameraLens::entity()
> + * \brief Retrieve the lens media entity
> + * \return The lens media entity
> + */
> +
> +/**
> + * \brief Retrieve the supported V4L2 controls and their information
> + *
> + * Control information is updated automatically to reflect the current lens
> + * configuration when the setFormat() function is called, without invalidating
> + * any iterator on the ControlInfoMap.
> + *
> + * \return A map of the V4L2 controls supported by the lens
> + */
> +const ControlInfoMap &CameraLens::controls() const
> +{
> +       return subdev_->controls();
> +}
> +
> +/**
> + * \brief Read V4L2 controls from the lens
> + * \param[in] ids The list of controls to read, specified by their ID
> + *
> + * This function reads the value of all controls contained in \a ids, and
> + * returns their values as a ControlList. The control identifiers are defined by
> + * the V4L2 specification (V4L2_CID_*).
> + *
> + * If any control in \a ids is not supported by the device, is disabled (i.e.
> + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs
> + * during validation of the requested controls, no control is read and this
> + * function returns an empty control list.
> + *
> + * \sa V4L2Device::getControls()
> + *
> + * \return The control values in a ControlList on success, or an empty list on
> + * error
> + */
> +ControlList CameraLens::getControls(const std::vector<uint32_t> &ids)
> +{
> +       return subdev_->getControls(ids);
> +}
> +
> +/**
> + * \brief Write V4L2 controls to the lens
> + * \param[in] ctrls The list of controls to write
> + *
> + * This function writes the value of all controls contained in \a ctrls, and
> + * stores the values actually applied to the device in the corresponding \a
> + * ctrls entry. The control identifiers are defined by the V4L2 specification
> + * (V4L2_CID_*).
> + *
> + * If any control in \a ctrls is not supported by the device, is disabled (i.e.
> + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other
> + * error occurs during validation of the requested controls, no control is
> + * written and this function returns -EINVAL.
> + *
> + * If an error occurs while writing the controls, the index of the first
> + * control that couldn't be written is returned. All controls below that index
> + * are written and their values are updated in \a ctrls, while all other
> + * controls are not written and their values are not changed.
> + *
> + * \sa V4L2Device::setControls()
> + *
> + * \return 0 on success or an error code otherwise
> + * \retval -EINVAL One of the control is not supported or not accessible
> + * \retval i The index of the control that failed
> + */
> +int CameraLens::setControls(ControlList *ctrls)
> +{
> +       return subdev_->setControls(ctrls);
> +}
> +
> +/**
> + * \fn CameraLens::device()
> + * \brief Retrieve the camera lens device
> + * \todo Remove this function by integrating DelayedControl with CameraLens
> + * \return The camera lens device
> + */
> +
> +/**
> + * \fn CameraLens::properties()
> + * \brief Retrieve the camera lens properties
> + * \return The list of camera lens properties
> + */
> +
> +std::string CameraLens::logPrefix() const
> +{
> +       return "'" + entity_->name() + "'";
> +}
> +
> +int CameraLens::generateId()
> +{
> +       const std::string devPath = subdev_->devicePath();
> +
> +       /* Try to get ID from firmware description. */
> +       id_ = sysfs::firmwareNodePath(devPath);
> +       if (!id_.empty())
> +               return 0;
> +
> +       /*
> +        * Virtual lens not described in firmware
> +        *
> +        * Verify it's a platform device and construct ID from the device path
> +        * and model of lens.
> +        */
> +       if (devPath.find("/sys/devices/platform/", 0) == 0) {
> +               id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();
> +               return 0;
> +       }
> +
> +       LOG(CameraLens, Error) << "Can't generate lens ID";
> +       return -EINVAL;
> +}
> +
> +} /* 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',
> -- 
> 2.33.1.1089.g2158813163f-goog
>

Hi Kieran,
Thanks for the review.

On Fri, Oct 29, 2021 at 12:55 AM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Han-Lin Chen (2021-10-28 11:03:18)
> > The CameraLens class abstracts camera lens and provides helper
> > functions to ease interactions with them.
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com>
> > ---
> >  Documentation/index.rst                    |   1 +
> >  Documentation/lens_driver_requirements.rst |  28 ++
> >  Documentation/meson.build                  |   1 +
> >  include/libcamera/internal/camera_lens.h   |  61 +++++
> >  src/libcamera/camera_lens.cpp              | 289 +++++++++++++++++++++
> >  src/libcamera/meson.build                  |   1 +
> >  6 files changed, 381 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..226cb813
> > --- /dev/null
> > +++ b/Documentation/lens_driver_requirements.rst
> > @@ -0,0 +1,28 @@
> > +.. SPDX-License-Identifier: CC-BY-SA-4.0
> > +
> > +.. _lens-driver-requirements:
> > +
> > +Lens Driver Requirements
> > +==========================
>
> two extra '==' could be removed.
>
> > +
> > +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 one or multiple sub-devices exposed in userspace by
> > +the lens driver.
> > +
> > +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
>
> Seeing that there is also V4L2_CID_FOCUS_RELATIVE, I assume that some
> drivers might offer a different means to control.
>
> Would an algorithm be able to be more general if it supported RELATIVE?
> or are they always expected to know how to operate the lens motor and
> have an absolute position request.
>
> I suspect the V4L2_CID_FOCUS_ABSOLUTE is the only way to get the valid
> range of movement out anyway, so I think this is appropriate.
>
>
> > diff --git a/Documentation/meson.build b/Documentation/meson.build
> > index df36a808..746af4cf 100644
> > --- a/Documentation/meson.build
> > +++ b/Documentation/meson.build
> > @@ -60,6 +60,7 @@ if sphinx.found()
> >          'guides/ipa.rst',
> >          'guides/pipeline-handler.rst',
> >          'guides/tracing.rst',
> > +        'lens_driver_requirements.rst',
>
> alphabetical order here please.
>
> >          'index.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..f03de5fc
> > --- /dev/null
> > +++ b/include/libcamera/internal/camera_lens.h
> > @@ -0,0 +1,61 @@
> > +/* 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__
> > +
> > +#include <libcamera/base/class.h>
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/controls.h>
> > +#include "libcamera/internal/v4l2_subdevice.h"
> > +
> > +namespace libcamera {
> > +
> > +class MediaEntity;
> > +
> > +class CameraLens : protected Loggable
> > +{
> > +public:
> > +       explicit CameraLens(const MediaEntity *entity);
> > +       ~CameraLens();
> > +
> > +       int init();
> > +
> > +       const std::string &model() const { return model_; }
> > +       const std::string &id() const { return id_; }
> > +       const MediaEntity *entity() const { return entity_; }
> > +
> > +       const ControlInfoMap &controls() const;
> > +       ControlList getControls(const std::vector<uint32_t> &ids);
> > +       int setControls(ControlList *ctrls);
> > +
> > +       V4L2Subdevice *device() { return subdev_.get(); }
> > +
> > +       const ControlList &properties() const { return properties_; }
> > +
> > +protected:
> > +       std::string logPrefix() const override;
> > +
> > +private:
> > +       LIBCAMERA_DISABLE_COPY(CameraLens)
> > +
> > +       int generateId();
> > +       int validateLensDriver();
> > +       int initProperties();
> > +
> > +       const MediaEntity *entity_;
> > +       std::unique_ptr<V4L2Subdevice> subdev_;
> > +
> > +       std::string model_;
> > +       std::string id_;
> > +
> > +       ControlList properties_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_INTERNAL_CAMERA_LENS_H__ */
> > diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> > new file mode 100644
> > index 00000000..0c80d3c3
> > --- /dev/null
> > +++ b/src/libcamera/camera_lens.cpp
> > @@ -0,0 +1,289 @@
> > +/* 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/internal/media_device.h"
> > +
> > +#include <regex>
> > +
> > +#include <libcamera/property_ids.h>
> > +
> > +#include "libcamera/internal/sysfs.h"
> > +
> > +/**
> > + * \file camera_lens.h
> > + * \brief A camera lens
> > + */
> > +
> > +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.
> > + *
> > + * The implementation is currently limited to lens that expose a single V4L2
> > + * subdevice. It will be extended to support more complex devices as the needs
> > + * arise.
> > + */
> > +
> > +/**
> > + * \brief Construct a CameraLens
> > + * \param[in] entity The media entity backing the camera lens
> > + *
> > + * Once constructed the instance must be initialized with init().
> > + */
> > +CameraLens::CameraLens(const MediaEntity *entity)
> > +       : entity_(entity),
> > +         properties_(properties::properties)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Destroy a CameraLens
> > + */
> > +CameraLens::~CameraLens()
> > +{
> > +}
> > +
> > +/**
> > + * \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;
> > +
> > +       ret = initProperties();
> > +       if (ret)
> > +               return ret;
> > +
> > +       return 0;
> > +}
> > +
> > +int CameraLens::validateLensDriver()
> > +{
> > +       int err = 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";
> > +                       err = -EINVAL;
> > +               }
> > +       }
> > +
> > +       if (err) {
> > +               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";
>
> This goes over 120 chars, perhaps another line break?
> Even the message is over 90 chars, so might be better for users if it was split.
>
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int CameraLens::initProperties()
> > +{
> > +       /*
> > +        * Extract the camera lens model name from the media entity name.
> > +        *
> > +        * There is no standardized naming scheme for lens entities in the
> > +        * Linux kernel at the moment.
> > +        *
> > +        * - The most common rule, used by I2C lens, associates the model
> > +        *   name with the I2C bus number and address (e.g. 'dw9714 8-000c').
> > +        *
> > +        * Other schemes probably exist. As a best effort heuristic, use the
> > +        * part of the entity name before the first space if the name contains
> > +        * an I2C address, and use the full entity name otherwise.
> > +        */
> > +       std::string entityName = entity_->name();
> > +       std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> > +       std::smatch match;
> > +
> > +       if (std::regex_search(entityName, match, i2cRegex))
> > +               model_ = entityName.substr(0, entityName.find(' '));
> > +       else
> > +               model_ = entityName;
> > +
> > +       properties_.set(properties::Model, utils::toAscii(model_));
> > +
> > +       /* Generate a unique ID for the lens. */
> > +       int ret = generateId();
>
> Does a lens need an ID like this? Can't it just be referenced as the
> object when needed?
>
> Perhaps it will become clearer later.
>
> Will there ever be more than one lens per 'Camera'?
It's an interesting question :). I cannot find an actual example.
Since we don't have any use case for Id, I will remove it on the next
patch (and other problems).
>
>
> > +       if (ret)
> > +               return ret;
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * \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
> > + */
> > +
> > +/**
> > + * \fn CameraLens::id()
> > + * \brief Retrieve the lens ID
> > + *
> > + * The lens ID is a free-form string that uniquely identifies the lens in
> > + * the system.
> > + *
> > + * \return The lens ID
> > + */
> > +
> > +/**
> > + * \fn CameraLens::entity()
> > + * \brief Retrieve the lens media entity
> > + * \return The lens media entity
> > + */
> > +
> > +/**
> > + * \brief Retrieve the supported V4L2 controls and their information
> > + *
> > + * Control information is updated automatically to reflect the current lens
> > + * configuration when the setFormat() function is called, without invalidating
> > + * any iterator on the ControlInfoMap.
> > + *
> > + * \return A map of the V4L2 controls supported by the lens
> > + */
> > +const ControlInfoMap &CameraLens::controls() const
> > +{
> > +       return subdev_->controls();
> > +}
> > +
> > +/**
> > + * \brief Read V4L2 controls from the lens
> > + * \param[in] ids The list of controls to read, specified by their ID
> > + *
> > + * This function reads the value of all controls contained in \a ids, and
> > + * returns their values as a ControlList. The control identifiers are defined by
> > + * the V4L2 specification (V4L2_CID_*).
> > + *
> > + * If any control in \a ids is not supported by the device, is disabled (i.e.
> > + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs
> > + * during validation of the requested controls, no control is read and this
> > + * function returns an empty control list.
> > + *
> > + * \sa V4L2Device::getControls()
> > + *
> > + * \return The control values in a ControlList on success, or an empty list on
> > + * error
> > + */
> > +ControlList CameraLens::getControls(const std::vector<uint32_t> &ids)
> > +{
> > +       return subdev_->getControls(ids);
> > +}
> > +
> > +/**
> > + * \brief Write V4L2 controls to the lens
> > + * \param[in] ctrls The list of controls to write
> > + *
> > + * This function writes the value of all controls contained in \a ctrls, and
> > + * stores the values actually applied to the device in the corresponding \a
> > + * ctrls entry. The control identifiers are defined by the V4L2 specification
> > + * (V4L2_CID_*).
> > + *
> > + * If any control in \a ctrls is not supported by the device, is disabled (i.e.
> > + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other
> > + * error occurs during validation of the requested controls, no control is
> > + * written and this function returns -EINVAL.
> > + *
> > + * If an error occurs while writing the controls, the index of the first
> > + * control that couldn't be written is returned. All controls below that index
> > + * are written and their values are updated in \a ctrls, while all other
> > + * controls are not written and their values are not changed.
> > + *
> > + * \sa V4L2Device::setControls()
> > + *
> > + * \return 0 on success or an error code otherwise
> > + * \retval -EINVAL One of the control is not supported or not accessible
> > + * \retval i The index of the control that failed
> > + */
> > +int CameraLens::setControls(ControlList *ctrls)
> > +{
> > +       return subdev_->setControls(ctrls);
> > +}
> > +
> > +/**
> > + * \fn CameraLens::device()
> > + * \brief Retrieve the camera lens device
> > + * \todo Remove this function by integrating DelayedControl with CameraLens
> > + * \return The camera lens device
> > + */
> > +
> > +/**
> > + * \fn CameraLens::properties()
> > + * \brief Retrieve the camera lens properties
> > + * \return The list of camera lens properties
> > + */
> > +
> > +std::string CameraLens::logPrefix() const
> > +{
> > +       return "'" + entity_->name() + "'";
> > +}
> > +
> > +int CameraLens::generateId()
> > +{
> > +       const std::string devPath = subdev_->devicePath();
> > +
> > +       /* Try to get ID from firmware description. */
> > +       id_ = sysfs::firmwareNodePath(devPath);
> > +       if (!id_.empty())
> > +               return 0;
> > +
> > +       /*
> > +        * Virtual lens not described in firmware
> > +        *
> > +        * Verify it's a platform device and construct ID from the device path
> > +        * and model of lens.
> > +        */
> > +       if (devPath.find("/sys/devices/platform/", 0) == 0) {
> > +               id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();
> > +               return 0;
> > +       }
> > +
> > +       LOG(CameraLens, Error) << "Can't generate lens ID";
> > +       return -EINVAL;
> > +}
> > +
> > +} /* 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',
> > --
> > 2.33.1.1089.g2158813163f-goog
> >