[{"id":20676,"web_url":"https://patchwork.libcamera.org/comment/20676/","msgid":"","date":"2021-11-04T11:49:40","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_lens: Add a\n\tnew class to model a camera lens","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Han-Lin,\n\nOn 29/10/2021 13:59, Han-Lin Chen wrote:\n> The CameraLens class abstracts camera lens and provides helper\n> functions to ease interactions with them.\n> \n> Signed-off-by: Han-Lin Chen \n> ---\n> Documentation/index.rst | 1 +\n> Documentation/lens_driver_requirements.rst | 28 +++\n> Documentation/meson.build | 1 +\n> include/libcamera/internal/camera_lens.h | 60 +++++\n> src/libcamera/camera_lens.cpp | 261 +++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 6 files changed, 352 insertions(+)\n> create mode 100644 Documentation/lens_driver_requirements.rst\n> create mode 100644 include/libcamera/internal/camera_lens.h\n> create mode 100644 src/libcamera/camera_lens.cpp\n> \n> diff --git a/Documentation/index.rst b/Documentation/index.rst\n> index 1f4fc485..0ee10044 100644\n> --- a/Documentation/index.rst\n> +++ b/Documentation/index.rst\n> @@ -21,3 +21,4 @@\n> Tracing guide \n> Environment variables \n> Sensor driver requirements \n> + Lens driver requirements \n> diff --git a/Documentation/lens_driver_requirements.rst b/Documentation/lens_driver_requirements.rst\n> new file mode 100644\n> index 00000000..afc300cf\n> --- /dev/null\n> +++ b/Documentation/lens_driver_requirements.rst\n> @@ -0,0 +1,28 @@\n> +.. SPDX-License-Identifier: CC-BY-SA-4.0\n> +\n> +.. _lens-driver-requirements:\n> +\n> +Lens Driver Requirements\n> +========================\n> +\n> +libcamera handles lens devices in the CameraLens class and defines\n> +a consistent interface through its API towards other library components.\n> +\n> +The CameraLens class uses the V4L2 subdev kernel API to interface with the\n> +camera lens through one or multiple sub-devices exposed in userspace by\n> +the lens driver.\n> +\n> +In order for libcamera to be fully operational and provide all the required\n> +information to interface with the camera lens to applications and pipeline\n> +handlers, a set of mandatory features the driver has to support has been defined.\n> +\n> +Mandatory Requirements\n> +----------------------\n> +\n> +The lens driver is assumed to be fully compliant with the V4L2 specification.\n> +\n> +The lens driver shall support the following V4L2 controls:\n> +\n> +* `V4L2_CID_FOCUS_ABSOLUTE`_\n> +\n> +.. _V4L2_CID_FOCUS_ABSOLUTE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-camera.html\n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index df36a808..6a1ef164 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -61,6 +61,7 @@ if sphinx.found()\n> 'guides/pipeline-handler.rst',\n> 'guides/tracing.rst',\n> 'index.rst',\n> + 'lens_driver_requirements.rst',\n> 'sensor_driver_requirements.rst',\n> '../README.rst',\n> ]\n> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> new file mode 100644\n> index 00000000..c558ea4f\n> --- /dev/null\n> +++ b/include/libcamera/internal/camera_lens.h\n> @@ -0,0 +1,60 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * camera_lens.h - A camera lens\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_CAMERA_LENS_H__\n> +#define __LIBCAMERA_INTERNAL_CAMERA_LENS_H__\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +class MediaEntity;\n> +\n> +class CameraLens : protected Loggable\n> +{\n> +public:\n> +\texplicit CameraLens(const MediaEntity *entity);\n> +\t~CameraLens();\n> +\n> +\tint init();\n> +\n> +\tconst std::string &model() const { return model_; }\n> +\tconst MediaEntity *entity() const { return entity_; }\n> +\n> +\tconst ControlInfoMap &controls() const;\n> +\tControlList getControls(const std::vector &ids);\n> +\tint setControls(ControlList *ctrls);\n> +\n> +\tV4L2Subdevice *device() { return subdev_.get(); }\n> +\n> +\tconst ControlList &properties() const { return properties_; }\n> +\n> +protected:\n> +\tstd::string logPrefix() const override;\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY(CameraLens)\n> +\n> +\tint generateId();\n> +\tint validateLensDriver();\n> +\tint initProperties();\n> +\n> +\tconst MediaEntity *entity_;\n> +\tstd::unique_ptr subdev_;\n> +\n> +\tstd::string model_;\n> +\tstd::string id_;\n> +\n> +\tControlList properties_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_CAMERA_LENS_H__ */\n> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> new file mode 100644\n> index 00000000..8529737c\n> --- /dev/null\n> +++ b/src/libcamera/camera_lens.cpp\n> @@ -0,0 +1,261 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * camera_lens.cpp - A camera lens\n> + */\n> +\n> +#include \"libcamera/internal/camera_lens.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/sysfs.h\"\n> +\n> +/**\n> + * \\file camera_lens.h\n> + * \\brief A camera lens\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(CameraLens)\n> +\n> +/**\n> + * \\class CameraLens\n> + * \\brief A camera lens based on V4L2 subdevices\n> + *\n> + * The CameraLens class eases handling of lens for pipeline handlers by\n> + * hiding the details of the V4L2 subdevice kernel API and caching lens\n> + * information.\n> + *\n> + * The implementation is currently limited to lens that expose a single V4L2\n> + * subdevice. It will be extended to support more complex devices as the needs\n> + * arise.\n> + */\n> +\n> +/**\n> + * \\brief Construct a CameraLens\n> + * \\param[in] entity The media entity backing the camera lens\n> + *\n> + * Once constructed the instance must be initialized with init().\n> + */\n> +CameraLens::CameraLens(const MediaEntity *entity)\n> +\t: entity_(entity),\n> +\t properties_(properties::properties)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Destroy a CameraLens\n> + */\n> +CameraLens::~CameraLens()\n> +{\n> +}\n> +\n\nIf it's empty you may make it a default in the class definition ?\n\n> +/**\n> + * \\brief Initialize the camera lens instance\n> + *\n> + * This function performs the initialisation steps of the CameraLens that may\n> + * fail. It shall be called once and only once after constructing the instance.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int CameraLens::init()\n> +{\n> +\tif (entity_->function() != MEDIA_ENT_F_LENS) {\n> +\t\tLOG(CameraLens, Error)\n> +\t\t\t<< \"Invalid lens function \"\n> +\t\t\t<< utils::hex(entity_->function());\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/* Create and open the subdev. */\n> +\tsubdev_ = std::make_unique(entity_);\n> +\tint ret = subdev_->open();\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = validateLensDriver();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = initProperties();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CameraLens::validateLensDriver()\n> +{\n> +\tint err = 0;\n> +\tstatic constexpr uint32_t mandatoryControls[] = {\n> +\t\tV4L2_CID_FOCUS_ABSOLUTE\n> +\t};\n> +\n> +\tconst ControlInfoMap &controls = subdev_->controls();\n> +\tfor (uint32_t ctrl : mandatoryControls) {\n> +\t\tif (!controls.count(ctrl)) {\n> +\t\t\tLOG(CameraLens, Error)\n> +\t\t\t\t<< \"Mandatory V4L2 control \" << utils::hex(ctrl)\n> +\t\t\t\t<< \" not available\";\n> +\t\t\terr = -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> +\tif (err) {\n> +\t\tLOG(CameraLens, Error)\n> +\t\t\t<< \"The lens kernel driver needs to be fixed\";\n> +\t\tLOG(CameraLens, Error)\n> +\t\t\t<< \"See Documentation/lens_driver_requirements.rst in\n\nYou missed a \" at the end of the line it does not compile.\n\n> +\t\t\t<< \" the libcamera sources for more information\";\n> +\t\treturn err;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CameraLens::initProperties()\n> +{\n> +\t/*\n> +\t * Extract the camera lens model name from the media entity name.\n> +\t *\n> +\t * There is no standardized naming scheme for lens entities in the\n> +\t * Linux kernel at the moment.\n> +\t *\n> +\t * - The most common rule, used by I2C lens, associates the model\n> +\t * name with the I2C bus number and address (e.g. 'dw9714 8-000c').\n> +\t *\n> +\t * Other schemes probably exist. As a best effort heuristic, use the\n> +\t * part of the entity name before the first space if the name contains\n> +\t * an I2C address, and use the full entity name otherwise.\n> +\t */\n> +\tstd::string entityName = entity_->name();\n> +\tstd::regex i2cRegex{ \" [0-9]+-[0-9a-f]{4}\" };\n> +\tstd::smatch match;\n> +\n> +\tif (std::regex_search(entityName, match, i2cRegex))\n> +\t\tmodel_ = entityName.substr(0, entityName.find(' '));\n> +\telse\n> +\t\tmodel_ = entityName;\n> +\n> +\tproperties_.set(properties::Model, utils::toAscii(model_));\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\fn CameraLens::model()\n> + * \\brief Retrieve the lens model name\n> + *\n> + * The lens model name is a free-formed string that uniquely identifies the\n> + * lens model.\n> + *\n> + * \\return The lens model name\n> + */\n> +\n> +/**\n> + * \\fn CameraLens::id()\n> + * \\brief Retrieve the lens ID\n> + *\n> + * The lens ID is a free-form string that uniquely identifies the lens in\n> + * the system.\n> + *\n> + * \\return The lens ID\n> + */\n> +\n> +/**\n> + * \\fn CameraLens::entity()\n> + * \\brief Retrieve the lens media entity\n> + * \\return The lens media entity\n> + */\n> +\n> +/**\n> + * \\brief Retrieve the supported V4L2 controls and their information\n> + *\n> + * Control information is updated automatically to reflect the current lens\n> + * configuration when the setFormat() function is called, without invalidating\n> + * any iterator on the ControlInfoMap.\n> + *\n> + * \\return A map of the V4L2 controls supported by the lens\n> + */\n> +const ControlInfoMap &CameraLens::controls() const\n> +{\n> +\treturn subdev_->controls();\n> +}\n> +\n> +/**\n> + * \\brief Read V4L2 controls from the lens\n> + * \\param[in] ids The list of controls to read, specified by their ID\n> + *\n> + * This function reads the value of all controls contained in \\a ids, and\n> + * returns their values as a ControlList. The control identifiers are defined by\n> + * the V4L2 specification (V4L2_CID_*).\n> + *\n> + * If any control in \\a ids is not supported by the device, is disabled (i.e.\n> + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs\n> + * during validation of the requested controls, no control is read and this\n> + * function returns an empty control list.\n> + *\n> + * \\sa V4L2Device::getControls()\n> + *\n> + * \\return The control values in a ControlList on success, or an empty list on\n> + * error\n> + */\n> +ControlList CameraLens::getControls(const std::vector &ids)\n> +{\n> +\treturn subdev_->getControls(ids);\n> +}\n> +\n> +/**\n> + * \\brief Write V4L2 controls to the lens\n> + * \\param[in] ctrls The list of controls to write\n> + *\n> + * This function writes the value of all controls contained in \\a ctrls, and\n> + * stores the values actually applied to the device in the corresponding \\a\n> + * ctrls entry. The control identifiers are defined by the V4L2 specification\n> + * (V4L2_CID_*).\n> + *\n> + * If any control in \\a ctrls is not supported by the device, is disabled (i.e.\n> + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other\n> + * error occurs during validation of the requested controls, no control is\n> + * written and this function returns -EINVAL.\n> + *\n> + * If an error occurs while writing the controls, the index of the first\n> + * control that couldn't be written is returned. All controls below that index\n> + * are written and their values are updated in \\a ctrls, while all other\n> + * controls are not written and their values are not changed.\n> + *\n> + * \\sa V4L2Device::setControls()\n> + *\n> + * \\return 0 on success or an error code otherwise\n> + * \\retval -EINVAL One of the control is not supported or not accessible\n> + * \\retval i The index of the control that failed\n> + */\n> +int CameraLens::setControls(ControlList *ctrls)\n> +{\n> +\treturn subdev_->setControls(ctrls);\n> +}\n> +\n> +/**\n> + * \\fn CameraLens::device()\n> + * \\brief Retrieve the camera lens device\n> + * \\todo Remove this function by integrating DelayedControl with CameraLens\n> + * \\return The camera lens device\n> + */\n> +\n> +/**\n> + * \\fn CameraLens::properties()\n> + * \\brief Retrieve the camera lens properties\n> + * \\return The list of camera lens properties\n> + */\n> +\n> +std::string CameraLens::logPrefix() const\n> +{\n> +\treturn \"'\" + entity_->name() + \"'\";\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 6727a777..3bee8fee 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -5,6 +5,7 @@ libcamera_sources = files([\n> 'byte_stream_buffer.cpp',\n> 'camera.cpp',\n> 'camera_controls.cpp',\n> + 'camera_lens.cpp',\n> 'camera_manager.cpp',\n> 'camera_sensor.cpp',\n> 'camera_sensor_properties.cpp',\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AC7D8BDB1C\n\tfor ;\n\tThu, 4 Nov 2021 11:49:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6A256033D;\n\tThu, 4 Nov 2021 12:49:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 115AF6032C\n\tfor ;\n\tThu, 4 Nov 2021 12:49:43 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:3875:cc64:ce12:aabc] (unknown\n\t[IPv6:2a01:e0a:169:7140:3875:cc64:ce12:aabc])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 986BDE52;\n\tThu, 4 Nov 2021 12:49:42 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eXS2FQZ4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636026582;\n\tbh=o5raGECujB6WsnWNhTgIbHQXjwqJXeSfMIyyo5L9AR8=;\n\th=Date:From:Subject:To:References:In-Reply-To:From;\n\tb=eXS2FQZ4mH+p5sUPlGDsMMP51XiLH5rxrXyhs3kAVe7uB3TAihtzoqV5Zv2PuFFXJ\n\tZT4s3JQR0OvtGFqBJpLlV6BGTUoxfQ/Ii12vHwrKiC/7fuPHyc6vUmFSKFwYUXvZr9\n\tQuFnl9gEJ+zoailSmWoF27i5cKZ/GLy7R2bTxAoM=","Message-ID":"","Date":"Thu, 4 Nov 2021 12:49:40 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.1.2","From":"Jean-Michel Hautbois ","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211029115917.2467936-1-hanlinchen@chromium.org>\n\t<20211029115917.2467936-3-hanlinchen@chromium.org>","Content-Language":"en-US","In-Reply-To":"<20211029115917.2467936-3-hanlinchen@chromium.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_lens: Add a\n\tnew class to model a camera lens","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20800,"web_url":"https://patchwork.libcamera.org/comment/20800/","msgid":"","date":"2021-11-10T12:46:54","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_lens: Add a\n\tnew class to model a camera lens","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Jean-Michel,\n\nOn Thu, Nov 4, 2021 at 7:49 PM Jean-Michel Hautbois\n wrote:\n>\n> Hi Han-Lin,\n>\n> On 29/10/2021 13:59, Han-Lin Chen wrote:\n> > The CameraLens class abstracts camera lens and provides helper\n> > functions to ease interactions with them.\n> >\n> > Signed-off-by: Han-Lin Chen \n> > ---\n> > Documentation/index.rst | 1 +\n> > Documentation/lens_driver_requirements.rst | 28 +++\n> > Documentation/meson.build | 1 +\n> > include/libcamera/internal/camera_lens.h | 60 +++++\n> > src/libcamera/camera_lens.cpp | 261 +++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > 6 files changed, 352 insertions(+)\n> > create mode 100644 Documentation/lens_driver_requirements.rst\n> > create mode 100644 include/libcamera/internal/camera_lens.h\n> > create mode 100644 src/libcamera/camera_lens.cpp\n> >\n> > diff --git a/Documentation/index.rst b/Documentation/index.rst\n> > index 1f4fc485..0ee10044 100644\n> > --- a/Documentation/index.rst\n> > +++ b/Documentation/index.rst\n> > @@ -21,3 +21,4 @@\n> > Tracing guide \n> > Environment variables \n> > Sensor driver requirements \n> > + Lens driver requirements \n> > diff --git a/Documentation/lens_driver_requirements.rst b/Documentation/lens_driver_requirements.rst\n> > new file mode 100644\n> > index 00000000..afc300cf\n> > --- /dev/null\n> > +++ b/Documentation/lens_driver_requirements.rst\n> > @@ -0,0 +1,28 @@\n> > +.. SPDX-License-Identifier: CC-BY-SA-4.0\n> > +\n> > +.. _lens-driver-requirements:\n> > +\n> > +Lens Driver Requirements\n> > +========================\n> > +\n> > +libcamera handles lens devices in the CameraLens class and defines\n> > +a consistent interface through its API towards other library components.\n> > +\n> > +The CameraLens class uses the V4L2 subdev kernel API to interface with the\n> > +camera lens through one or multiple sub-devices exposed in userspace by\n> > +the lens driver.\n> > +\n> > +In order for libcamera to be fully operational and provide all the required\n> > +information to interface with the camera lens to applications and pipeline\n> > +handlers, a set of mandatory features the driver has to support has been defined.\n> > +\n> > +Mandatory Requirements\n> > +----------------------\n> > +\n> > +The lens driver is assumed to be fully compliant with the V4L2 specification.\n> > +\n> > +The lens driver shall support the following V4L2 controls:\n> > +\n> > +* `V4L2_CID_FOCUS_ABSOLUTE`_\n> > +\n> > +.. _V4L2_CID_FOCUS_ABSOLUTE: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/ext-ctrls-camera.html\n> > diff --git a/Documentation/meson.build b/Documentation/meson.build\n> > index df36a808..6a1ef164 100644\n> > --- a/Documentation/meson.build\n> > +++ b/Documentation/meson.build\n> > @@ -61,6 +61,7 @@ if sphinx.found()\n> > 'guides/pipeline-handler.rst',\n> > 'guides/tracing.rst',\n> > 'index.rst',\n> > + 'lens_driver_requirements.rst',\n> > 'sensor_driver_requirements.rst',\n> > '../README.rst',\n> > ]\n> > diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> > new file mode 100644\n> > index 00000000..c558ea4f\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/camera_lens.h\n> > @@ -0,0 +1,60 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google Inc.\n> > + *\n> > + * camera_lens.h - A camera lens\n> > + */\n> > +#ifndef __LIBCAMERA_INTERNAL_CAMERA_LENS_H__\n> > +#define __LIBCAMERA_INTERNAL_CAMERA_LENS_H__\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \"libcamera/internal/v4l2_subdevice.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +class MediaEntity;\n> > +\n> > +class CameraLens : protected Loggable\n> > +{\n> > +public:\n> > + explicit CameraLens(const MediaEntity *entity);\n> > + ~CameraLens();\n> > +\n> > + int init();\n> > +\n> > + const std::string &model() const { return model_; }\n> > + const MediaEntity *entity() const { return entity_; }\n> > +\n> > + const ControlInfoMap &controls() const;\n> > + ControlList getControls(const std::vector &ids);\n> > + int setControls(ControlList *ctrls);\n> > +\n> > + V4L2Subdevice *device() { return subdev_.get(); }\n> > +\n> > + const ControlList &properties() const { return properties_; }\n> > +\n> > +protected:\n> > + std::string logPrefix() const override;\n> > +\n> > +private:\n> > + LIBCAMERA_DISABLE_COPY(CameraLens)\n> > +\n> > + int generateId();\n> > + int validateLensDriver();\n> > + int initProperties();\n> > +\n> > + const MediaEntity *entity_;\n> > + std::unique_ptr subdev_;\n> > +\n> > + std::string model_;\n> > + std::string id_;\n> > +\n> > + ControlList properties_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_INTERNAL_CAMERA_LENS_H__ */\n> > diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> > new file mode 100644\n> > index 00000000..8529737c\n> > --- /dev/null\n> > +++ b/src/libcamera/camera_lens.cpp\n> > @@ -0,0 +1,261 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google Inc.\n> > + *\n> > + * camera_lens.cpp - A camera lens\n> > + */\n> > +\n> > +#include \"libcamera/internal/camera_lens.h\"\n> > +#include \"libcamera/internal/media_device.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/sysfs.h\"\n> > +\n> > +/**\n> > + * \\file camera_lens.h\n> > + * \\brief A camera lens\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(CameraLens)\n> > +\n> > +/**\n> > + * \\class CameraLens\n> > + * \\brief A camera lens based on V4L2 subdevices\n> > + *\n> > + * The CameraLens class eases handling of lens for pipeline handlers by\n> > + * hiding the details of the V4L2 subdevice kernel API and caching lens\n> > + * information.\n> > + *\n> > + * The implementation is currently limited to lens that expose a single V4L2\n> > + * subdevice. It will be extended to support more complex devices as the needs\n> > + * arise.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Construct a CameraLens\n> > + * \\param[in] entity The media entity backing the camera lens\n> > + *\n> > + * Once constructed the instance must be initialized with init().\n> > + */\n> > +CameraLens::CameraLens(const MediaEntity *entity)\n> > + : entity_(entity),\n> > + properties_(properties::properties)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Destroy a CameraLens\n> > + */\n> > +CameraLens::~CameraLens()\n> > +{\n> > +}\n> > +\n>\n> If it's empty you may make it a default in the class definition ?\nSGTM.\n>\n> > +/**\n> > + * \\brief Initialize the camera lens instance\n> > + *\n> > + * This function performs the initialisation steps of the CameraLens that may\n> > + * fail. It shall be called once and only once after constructing the instance.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int CameraLens::init()\n> > +{\n> > + if (entity_->function() != MEDIA_ENT_F_LENS) {\n> > + LOG(CameraLens, Error)\n> > + << \"Invalid lens function \"\n> > + << utils::hex(entity_->function());\n> > + return -EINVAL;\n> > + }\n> > +\n> > + /* Create and open the subdev. */\n> > + subdev_ = std::make_unique(entity_);\n> > + int ret = subdev_->open();\n> > + if (ret < 0)\n> > + return ret;\n> > +\n> > + ret = validateLensDriver();\n> > + if (ret)\n> > + return ret;\n> > +\n> > + ret = initProperties();\n> > + if (ret)\n> > + return ret;\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int CameraLens::validateLensDriver()\n> > +{\n> > + int err = 0;\n> > + static constexpr uint32_t mandatoryControls[] = {\n> > + V4L2_CID_FOCUS_ABSOLUTE\n> > + };\n> > +\n> > + const ControlInfoMap &controls = subdev_->controls();\n> > + for (uint32_t ctrl : mandatoryControls) {\n> > + if (!controls.count(ctrl)) {\n> > + LOG(CameraLens, Error)\n> > + << \"Mandatory V4L2 control \" << utils::hex(ctrl)\n> > + << \" not available\";\n> > + err = -EINVAL;\n> > + }\n> > + }\n> > +\n> > + if (err) {\n> > + LOG(CameraLens, Error)\n> > + << \"The lens kernel driver needs to be fixed\";\n> > + LOG(CameraLens, Error)\n> > + << \"See Documentation/lens_driver_requirements.rst in\n>\n> You missed a \" at the end of the line it does not compile.\n>\nThanks! Not sure how I could miss that.\n> > + << \" the libcamera sources for more information\";\n> > + return err;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int CameraLens::initProperties()\n> > +{\n> > + /*\n> > + * Extract the camera lens model name from the media entity name.\n> > + *\n> > + * There is no standardized naming scheme for lens entities in the\n> > + * Linux kernel at the moment.\n> > + *\n> > + * - The most common rule, used by I2C lens, associates the model\n> > + * name with the I2C bus number and address (e.g. 'dw9714 8-000c').\n> > + *\n> > + * Other schemes probably exist. As a best effort heuristic, use the\n> > + * part of the entity name before the first space if the name contains\n> > + * an I2C address, and use the full entity name otherwise.\n> > + */\n> > + std::string entityName = entity_->name();\n> > + std::regex i2cRegex{ \" [0-9]+-[0-9a-f]{4}\" };\n> > + std::smatch match;\n> > +\n> > + if (std::regex_search(entityName, match, i2cRegex))\n> > + model_ = entityName.substr(0, entityName.find(' '));\n> > + else\n> > + model_ = entityName;\n> > +\n> > + properties_.set(properties::Model, utils::toAscii(model_));\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\fn CameraLens::model()\n> > + * \\brief Retrieve the lens model name\n> > + *\n> > + * The lens model name is a free-formed string that uniquely identifies the\n> > + * lens model.\n> > + *\n> > + * \\return The lens model name\n> > + */\n> > +\n> > +/**\n> > + * \\fn CameraLens::id()\n> > + * \\brief Retrieve the lens ID\n> > + *\n> > + * The lens ID is a free-form string that uniquely identifies the lens in\n> > + * the system.\n> > + *\n> > + * \\return The lens ID\n> > + */\n> > +\n> > +/**\n> > + * \\fn CameraLens::entity()\n> > + * \\brief Retrieve the lens media entity\n> > + * \\return The lens media entity\n> > + */\n> > +\n> > +/**\n> > + * \\brief Retrieve the supported V4L2 controls and their information\n> > + *\n> > + * Control information is updated automatically to reflect the current lens\n> > + * configuration when the setFormat() function is called, without invalidating\n> > + * any iterator on the ControlInfoMap.\n> > + *\n> > + * \\return A map of the V4L2 controls supported by the lens\n> > + */\n> > +const ControlInfoMap &CameraLens::controls() const\n> > +{\n> > + return subdev_->controls();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Read V4L2 controls from the lens\n> > + * \\param[in] ids The list of controls to read, specified by their ID\n> > + *\n> > + * This function reads the value of all controls contained in \\a ids, and\n> > + * returns their values as a ControlList. The control identifiers are defined by\n> > + * the V4L2 specification (V4L2_CID_*).\n> > + *\n> > + * If any control in \\a ids is not supported by the device, is disabled (i.e.\n> > + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs\n> > + * during validation of the requested controls, no control is read and this\n> > + * function returns an empty control list.\n> > + *\n> > + * \\sa V4L2Device::getControls()\n> > + *\n> > + * \\return The control values in a ControlList on success, or an empty list on\n> > + * error\n> > + */\n> > +ControlList CameraLens::getControls(const std::vector &ids)\n> > +{\n> > + return subdev_->getControls(ids);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Write V4L2 controls to the lens\n> > + * \\param[in] ctrls The list of controls to write\n> > + *\n> > + * This function writes the value of all controls contained in \\a ctrls, and\n> > + * stores the values actually applied to the device in the corresponding \\a\n> > + * ctrls entry. The control identifiers are defined by the V4L2 specification\n> > + * (V4L2_CID_*).\n> > + *\n> > + * If any control in \\a ctrls is not supported by the device, is disabled (i.e.\n> > + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other\n> > + * error occurs during validation of the requested controls, no control is\n> > + * written and this function returns -EINVAL.\n> > + *\n> > + * If an error occurs while writing the controls, the index of the first\n> > + * control that couldn't be written is returned. All controls below that index\n> > + * are written and their values are updated in \\a ctrls, while all other\n> > + * controls are not written and their values are not changed.\n> > + *\n> > + * \\sa V4L2Device::setControls()\n> > + *\n> > + * \\return 0 on success or an error code otherwise\n> > + * \\retval -EINVAL One of the control is not supported or not accessible\n> > + * \\retval i The index of the control that failed\n> > + */\n> > +int CameraLens::setControls(ControlList *ctrls)\n> > +{\n> > + return subdev_->setControls(ctrls);\n> > +}\n> > +\n> > +/**\n> > + * \\fn CameraLens::device()\n> > + * \\brief Retrieve the camera lens device\n> > + * \\todo Remove this function by integrating DelayedControl with CameraLens\n> > + * \\return The camera lens device\n> > + */\n> > +\n> > +/**\n> > + * \\fn CameraLens::properties()\n> > + * \\brief Retrieve the camera lens properties\n> > + * \\return The list of camera lens properties\n> > + */\n> > +\n> > +std::string CameraLens::logPrefix() const\n> > +{\n> > + return \"'\" + entity_->name() + \"'\";\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 6727a777..3bee8fee 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -5,6 +5,7 @@ libcamera_sources = files([\n> > 'byte_stream_buffer.cpp',\n> > 'camera.cpp',\n> > 'camera_controls.cpp',\n> > + 'camera_lens.cpp',\n> > 'camera_manager.cpp',\n> > 'camera_sensor.cpp',\n> > 'camera_sensor_properties.cpp',\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 540F3BF415\n\tfor ;\n\tWed, 10 Nov 2021 12:47:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0AFCE60361;\n\tWed, 10 Nov 2021 13:47:07 +0100 (CET)","from mail-ot1-x329.google.com (mail-ot1-x329.google.com\n\t[IPv6:2607:f8b0:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C8B66033C\n\tfor ;\n\tWed, 10 Nov 2021 13:47:06 +0100 (CET)","by mail-ot1-x329.google.com with SMTP id\n\tl7-20020a0568302b0700b0055ae988dcc8so3649518otv.12\n\tfor ;\n\tWed, 10 Nov 2021 04:47:06 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"BMFWf4Ky\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=i86zAyBs6jwwJUurAg6qE65EccaPWf35/IS1ycK985M=;\n\tb=BMFWf4Kychm/L46vGL6oJrsnyd50yo/LRu0cDqbjW6JKef8Xzy++x4AOa/C5ra71/+\n\toZo6tbZeqyir5b4xyPkQn1GuRJcp3wy45S2jpcomjFNZ1+3ghoABDoLYoo/hBFvvmV+T\n\t+zDt5D/xSEWzwBZYhKI3DwDayLY5huskxf0rA=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=i86zAyBs6jwwJUurAg6qE65EccaPWf35/IS1ycK985M=;\n\tb=5TgEgO5VI2uSi+oglC8k4YgCghQXeTMGx4TLRaUpq/0vKHQW0bMmIvSm2cAXjTnDyZ\n\ta/rMZGqZUFkpcnxlEk5zNkoiO/+7pvnJy2eELwViCLJPa08ATEq2JUl3DCfMcpytAnly\n\tDeyJrkvr6e3LbDhVZCBRtRtYaCXvsQjcVn+ZPGFoymXUp+vaKH62c3oxxdO6nGbZDPWK\n\tcD+rmDvTzYCLSP6+JqezAOr3cEslInf7ezqJFbaKjeRl8ACp+fznGceZrsJMTgo+2eQj\n\tqYkdKtB5NvtNKIXktyQ3fEZ+phOkoZhfAhJn9TLoYMxssRzhb0AYkEuHjV1wJj2wKjzu\n\tE62Q==","X-Gm-Message-State":"AOAM5327HbdfEwK6AkLuRJ6eUtOzSUBlyAPnKsHzkyWNg/zkQq2m6cdq\n\tbi35QmStIc4rN4Qz0EiAc3kQdXPWVefdKFZaG1PwJvHzIjzt3Q==","X-Google-Smtp-Source":"ABdhPJzlq4C9bz9+IrF+3kgCcwWP+gaQ0x/71ZMtmKen0K7U5qFOjxogO5G0V3hP0gY/w1wZJo8CTFKTjbJFMpQwsY4=","X-Received":"by 2002:a9d:2da3:: with SMTP id\n\tg32mr12919161otb.217.1636548424888; \n\tWed, 10 Nov 2021 04:47:04 -0800 (PST)","MIME-Version":"1.0","References":"<20211029115917.2467936-1-hanlinchen@chromium.org>\n\t<20211029115917.2467936-3-hanlinchen@chromium.org>\n\t","In-Reply-To":"","From":"Hanlin Chen ","Date":"Wed, 10 Nov 2021 20:46:54 +0800","Message-ID":"","To":"Jean-Michel Hautbois ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: camera_lens: Add a\n\tnew class to model a camera lens","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]