[libcamera-devel] [PATCH v4 2/4] libcamera: camera_utils: Add a file for utils common to sensor and lens
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Nov 24 06:20:39 CET 2021
Hello,
On Tue, Nov 23, 2021 at 04:51:06PM +0000, Kieran Bingham wrote:
> 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 ...
Don't we scared :-) (mental self-note: I need to work on communication
skills if I'm scaring people)
> 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)...
generateIdForV4L2Device() is only used in CameraSensor in this patch
series, so I'd keep the function in that class for now.
> I would have thought that generateID() could be done directly by the
> V4L2VideoDevice
>
> And the Model ... could be provided by the V4L2SubDevice....
I like this idea I think. There's a small risk we'll need different
heuristics for different types of subdevs, but I'd rather work on the
kernel side to improve this (adding a model field to media_entity would
be useful for instance).
> 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',
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list