[libcamera-devel] [PATCH v4 3/4] libcamera: camera_lens: Add a new class to model a camera lens
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 23 18:10:40 CET 2021
Quoting Han-Lin Chen (2021-11-23 12:37:50)
> 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 | 28 ++++
> Documentation/meson.build | 1 +
> include/libcamera/internal/camera_lens.h | 48 +++++++
> include/libcamera/internal/meson.build | 1 +
> src/libcamera/camera_lens.cpp | 150 +++++++++++++++++++++
> src/libcamera/meson.build | 1 +
> 7 files changed, 230 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..afc300cf
> --- /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
> +========================
> +
> +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
> 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..fe83d4ad
> --- /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__
> +
> +#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();
> + int setFocusPostion(int32_t position);
> +
> + 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 de8630fe..7e9b9998 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',
> 'camera_utils.h',
> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp
> new file mode 100644
> index 00000000..cb7159fe
> --- /dev/null
> +++ b/src/libcamera/camera_lens.cpp
> @@ -0,0 +1,150 @@
> +/* 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 <regex>
I don't think regex is used anymore.
> +#include <libcamera/property_ids.h>
> +
> +#include "libcamera/internal/camera_utils.h"
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/sysfs.h"
nor 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)
> +{
> +}
> +
> +/**
> + * \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;
> + }
> +
> + model_ = extractModelFromEntityName(entity_->name());
> +
> + /* Create and open the subdev. */
> + subdev_ = std::make_unique<V4L2Subdevice>(entity_);
> + int ret = subdev_->open();
> + if (ret < 0)
> + return ret;
This is probably why I think I'd make the model handled by the
V4L2Subdevice.
We /know/ we're creating a V4L2Subdevice here, so we could assign
something like
model_ = subdev_->model();
Or simply have model() call directly onto the subdev which would have
the implementation...
> +
> + ret = validateLensDriver();
> + if (ret)
> + return ret;
> +
> + 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;
> +}
I fear this is going the same way that the TestPatternMode is going on
the CameraSensor.
I don't think we should expose a function for each control and setting
on the devices.
The device should accept the controls, parse it and apply it.
The setFocusPosition helper function is probably good to keep as it is -
but I think a CameraLens should be taking a ControlList (of libcamera
controls) and doing something like:
+/**
+ * \brief Write libcamera controls to the lens
+ * \param[in] list The list of controls to write
+ *
+ * Supported libcamera controls are interpreted and converted to V4L2
+ * controls to write to the device.
+ *
+ * \return 0 on success or an error code otherwise
+ * \retval -EINVAL One of the control is not supported or not accessible
+ */
+int CameraLens::setControls(ControlList *list)
+{
+ if (!list)
+ return -ENODATA;
+
+ const ControlIdMap *idmap = list->idMap();
+ if (idmap != &controls::controls) {
+ LOG(CameraLens, Error) << "Invalid Control List";
+ return -EINVAL;
+ }
+
+ ControlList subdevControls;
+
+ for (const auto &ctrl : *list) {
+ unsigned int id = ctrl.first;
+ const ControlValue &value = ctrl.second;
+
+ // switch case would be nicer here.... but isn't
+ // currently possible
+
+ // Also not a libcamera control yet
+ if (id == controls::FocusAbsolute) {
+ int position = value.get<int>();
+ subdevControls.set(V4L2_CID_FOCUS_ABSOLUTE, position);
+ }
+
+ // Not a real libcamera control currently
+ if (id == controls::FocusRelative) {
+ // Not implemented, showing other control
+ // handling only
+ }
+ }
+
+ return subdev_->setControls(&subdevControls);
+}
+
I think this is what CameraSensor should be doing too, but we have this
awkward precedent that it can already take a V4L2ControlList directly...
Given that a lens can only currently set a position anyway, it's not
such a big deal if this is considered if/when we need more controls
instead, and we keep the direct and simple setting function.
--
Kieran
> +
> +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";
> + return err;
> + }
> +
> + 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
> + */
> +
> +std::string CameraLens::logPrefix() const
> +{
> + return "'" + entity_->name() + "'";
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 563ed861..2100805d 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.34.0.rc2.393.gf8c9666880-goog
>
More information about the libcamera-devel
mailing list