[libcamera-devel] [PATCH v4 2/4] libcamera: camera_utils: Add a file for utils common to sensor and lens
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 23 17:51:06 CET 2021
Hi Han-Lin,
Quoting Han-Lin Chen (2021-11-23 12:37:49)
> Add a file to locate utils common to camera sensor and the following lens
> class. The patch also moves two functions from camera_sensor.cpp used to
> generate id and model name for the sensor into the file.
>
I fear I'm about to get shot down by others on this as people's opinions
might differ ...
Splitting the generateID and model out of this to be able to re-use them
sounds like a great idea ... (and the bit I'm scared of)...
I would have thought that generateID() could be done directly by the
V4L2VideoDevice
And the Model ... could be provided by the V4L2SubDevice....
These are the components that know about those details, and any
heuristics there are based on assumptions about those specific device
forms...
Some typo's highlighted during the move below, which aren't a fault of
the move but could be fixed up while moving...
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
> include/libcamera/internal/camera_sensor.h | 1 -
> include/libcamera/internal/camera_utils.h | 22 +++++
> include/libcamera/internal/meson.build | 1 +
> src/libcamera/camera_sensor.cpp | 45 ++---------
> src/libcamera/camera_utils.cpp | 94 ++++++++++++++++++++++
> src/libcamera/meson.build | 1 +
> 6 files changed, 124 insertions(+), 40 deletions(-)
> create mode 100644 include/libcamera/internal/camera_utils.h
> create mode 100644 src/libcamera/camera_utils.cpp
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d25a1165..134108ff 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -66,7 +66,6 @@ protected:
> private:
> LIBCAMERA_DISABLE_COPY(CameraSensor)
>
> - int generateId();
> int validateSensorDriver();
> void initVimcDefaultProperties();
> void initStaticProperties();
> diff --git a/include/libcamera/internal/camera_utils.h b/include/libcamera/internal/camera_utils.h
> new file mode 100644
> index 00000000..04c5e76f
> --- /dev/null
> +++ b/include/libcamera/internal/camera_utils.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_utils.h - Camera related utilities
> + */
> +#ifndef __LIBCAMERA_INTERNAL_CAMERA_UTILS_H__
> +#define __LIBCAMERA_INTERNAL_CAMERA_UTILS_H__
> +
> +#include <string.h>
> +
> +#include "libcamera/internal/v4l2_device.h"
> +
> +namespace libcamera {
> +
> +std::string extractModelFromEntityName(const std::string &entityName);
> +std::string generateIdForV4L2Device(const V4L2Device *dev,
> + const std::string &model);
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_CAMERA_UTILS_H__ */
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 665fd6de..de8630fe 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -16,6 +16,7 @@ libcamera_internal_headers = files([
> 'camera_controls.h',
> 'camera_sensor.h',
> 'camera_sensor_properties.h',
> + 'camera_utils.h',
> 'control_serializer.h',
> 'control_validator.h',
> 'delayed_controls.h',
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..e91c2b0a 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -6,14 +6,12 @@
> */
>
> #include "libcamera/internal/camera_sensor.h"
> -#include "libcamera/internal/media_device.h"
>
> #include <algorithm>
> #include <float.h>
> #include <iomanip>
> #include <limits.h>
> #include <math.h>
> -#include <regex>
> #include <string.h>
>
> #include <libcamera/property_ids.h>
> @@ -22,8 +20,9 @@
>
> #include "libcamera/internal/bayer_format.h"
> #include "libcamera/internal/camera_sensor_properties.h"
> +#include "libcamera/internal/camera_utils.h"
> #include "libcamera/internal/formats.h"
> -#include "libcamera/internal/sysfs.h"
> +#include "libcamera/internal/media_device.h"
>
> /**
> * \file camera_sensor.h
> @@ -366,21 +365,13 @@ int CameraSensor::initProperties()
> * 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;
> -
> + model_ = extractModelFromEntityName(entity_->name());
> properties_.set(properties::Model, utils::toAscii(model_));
>
> /* Generate a unique ID for the sensor. */
> - int ret = generateId();
> - if (ret)
> - return ret;
> + id_ = generateIdForV4L2Device(subdev_.get(), model_);
> + if (id_.empty())
> + return -EINVAL;
>
> /* Initialize the static properties from the sensor database. */
> initStaticProperties();
> @@ -820,28 +811,4 @@ std::string CameraSensor::logPrefix() const
> return "'" + entity_->name() + "'";
> }
>
> -int CameraSensor::generateId()
> -{
> - const std::string devPath = subdev_->devicePath();
> -
> - /* Try to get ID from firmware description. */
> - id_ = sysfs::firmwareNodePath(devPath);
> - if (!id_.empty())
> - return 0;
> -
> - /*
> - * Virtual sensors not described in firmware
> - *
> - * Verify it's a platform device and construct ID from the deive path
> - * and model of sensor.
> - */
> - if (devPath.find("/sys/devices/platform/", 0) == 0) {
> - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();
> - return 0;
> - }
> -
> - LOG(CameraSensor, Error) << "Can't generate sensor ID";
> - return -EINVAL;
> -}
> -
> } /* namespace libcamera */
> diff --git a/src/libcamera/camera_utils.cpp b/src/libcamera/camera_utils.cpp
> new file mode 100644
> index 00000000..b42911b4
> --- /dev/null
> +++ b/src/libcamera/camera_utils.cpp
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * camera_utils.cpp - Camera related utilities
> + */
> +
> +#include "libcamera/internal/camera_utils.h"
> +
> +#include <regex>
> +
> +#include "libcamera/internal/sysfs.h"
> +
> +/**
> + * \file camera_utils.h
> + * \brief Utilities for Camera instances
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \brief Extract the sensor or lens model name from the media entity name
> + * \param[in] entityName The entity name of a lens or sensor
> + *
> + * \return Model name as string
> + */
> +std::string extractModelFromEntityName(const std::string &entityName)
> +{
> + /*
> + * Extract the sensor or lens model name from the media entity name.
> + *
> + * There is no standardized naming scheme for sensor entities in the
> + * Linux kernel at the moment.
> + *
> + * - The most common rule, used by I2C sensors, associates the model
> + * name with the I2C bus number and address (e.g. 'imx219 0-0010').
> + *
> + * - When the sensor exposes multiple subdevs, the model name is
> + * usually followed by a function name, as in the smiapp driver (e.g.
> + * 'jt8ew9 pixel_array 0-0010').
> + *
> + * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> + *
> + * 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::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> + std::smatch match;
> +
> + std::string model;
> + if (std::regex_search(entityName, match, i2cRegex))
> + model = entityName.substr(0, entityName.find(' '));
> + else
> + model = entityName;
> +
> + return model;
> +}
> +
> +/**
> + * \brief Generate ID for V4L2 device
> + * \param[in] dev The V4L2Device
> + * \param[in] model The ModelName
> + *
> + * Contruct ID from the firmware description. If it doesn't exist, contruct it
/Contruct/Construct/
/contruct/contruct/
> + * from the device path and the provided model name.
> + * If both fails, return an empty string.
/fails/fail/
> + *
> + * \return ID as string
> + */
> +std::string generateIdForV4L2Device(const V4L2Device *dev,
> + const std::string &model)
> +{
> + const std::string devPath = dev->devicePath();
> +
> + /* Try to get ID from firmware description. */
> + std::string id = sysfs::firmwareNodePath(devPath);
> + if (!id.empty())
> + return id;
> +
> + /*
> + * Virtual device not described in firmware
> + *
> + * Verify it's a platform device and construct ID from the deive path
/deive/device/
> + * and model of a sensor or lens.
> + */
> + if (devPath.find("/sys/devices/platform/", 0) == 0) {
> + return devPath.substr(strlen("/sys/devices/")) + " " + model;
> + }
> +
> + return "";
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 6727a777..563ed861 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -8,6 +8,7 @@ libcamera_sources = files([
> 'camera_manager.cpp',
> 'camera_sensor.cpp',
> 'camera_sensor_properties.cpp',
> + 'camera_utils.cpp',
> 'controls.cpp',
> 'control_serializer.cpp',
> 'control_validator.cpp',
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>
More information about the libcamera-devel
mailing list