[libcamera-devel] [PATCH v2 7/9] libcamera: Introduce camera sensor database
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 23 07:56:13 CET 2020
Hi Jacopo,
Another comment.
On Wed, Dec 23, 2020 at 07:49:46AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> On Fri, Dec 18, 2020 at 05:47:52PM +0100, Jacopo Mondi wrote:
>
> Thank you for the patch.
>
> > Introduce a 'database' of camera sensor information, which contains
> > a list of camera sensor properties which is not possible, or desirable,
> > to retrieve at run-time.
> >
> > The camera sensor database is a global read-only library object.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > include/libcamera/internal/meson.build | 1 +
> > include/libcamera/internal/sensor_database.h | 37 +++++++
> > src/libcamera/meson.build | 1 +
> > src/libcamera/sensor_database.cpp | 106 +++++++++++++++++++
> > 4 files changed, 145 insertions(+)
> > create mode 100644 include/libcamera/internal/sensor_database.h
> > create mode 100644 src/libcamera/sensor_database.cpp
> >
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index 7cde023f7c3a..ac08f0761238 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -36,6 +36,7 @@ libcamera_internal_headers = files([
> > 'process.h',
> > 'pub_key.h',
> > 'semaphore.h',
> > + 'sensor_database.h',
> > 'sysfs.h',
> > 'thread.h',
> > 'timer.h',
> > diff --git a/include/libcamera/internal/sensor_database.h b/include/libcamera/internal/sensor_database.h
> > new file mode 100644
> > index 000000000000..b7d3a6e7468e
> > --- /dev/null
> > +++ b/include/libcamera/internal/sensor_database.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * sensor_database.h - Database of camera sensor properties
> > + */
> > +#ifndef __LIBCAMERA_SENSOR_DATABASE_H__
> > +#define __LIBCAMERA_SENSOR_DATABASE_H__
> > +
> > +#include <initializer_list>
> > +#include <unordered_map>
> > +
> > +#include <libcamera/controls.h>
> > +#include <libcamera/geometry.h>
> > +
> > +namespace libcamera {
> > +
> > +class SensorDatabase : private std::unordered_map<std::string, const ControlList>
> > +{
> > +private:
> > + using Map = std::unordered_map<std::string, const ControlList>;
> > +
> > +public:
> > + SensorDatabase(std::initializer_list<Map::value_type> items)
> > + : Map(items)
> > + {
> > + }
> > +
> > + bool contains(Map::key_type sensor) const;
> > + const ControlList &properties(Map::key_type sensor) const;
>
> Should both functions take a const reference as argument ?
>
> > +};
> > +
> > +extern const SensorDatabase sensorDatabase;
>
> To reduce the exposure of details to the outside of this class, what
> would you think of
>
> class SensorDatabase
> {
> public:
> static bool contains(const std::string &model);
> static const ControlList &properties(const std::string &model);
> };
>
> ? Everything else could be moved from the header file to the .cpp file.
> No instance would need to be created, the functions could operate on a
> static map.
I wonder if the sensor database should actually expose a ControList, or
if we should use a custom structure instead. One of my short term goals
with this is to move the sensor static information stored in the RPi
IPA, and there are no corresponding libcamera properties.
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_SENSOR_DATABASE_H__ */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 387d5d88ecae..54f3b81ad7b2 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -41,6 +41,7 @@ libcamera_sources = files([
> > 'pub_key.cpp',
> > 'request.cpp',
> > 'semaphore.cpp',
> > + 'sensor_database.cpp',
> > 'signal.cpp',
> > 'stream.cpp',
> > 'sysfs.cpp',
> > diff --git a/src/libcamera/sensor_database.cpp b/src/libcamera/sensor_database.cpp
> > new file mode 100644
> > index 000000000000..b90eb45ed46d
> > --- /dev/null
> > +++ b/src/libcamera/sensor_database.cpp
> > @@ -0,0 +1,106 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * sensor_database.cpp - Database of camera sensor properties
> > + */
> > +
> > +#include "libcamera/internal/sensor_database.h"
> > +
> > +#include <libcamera/property_ids.h>
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \file sensor_database.h
> > + * \brief Database of camera sensor properties
> > + *
> > + * The camera sensor database collects information on camera sensors
> > + * which is not possible or desirable to retrieve at run-time.
> > + */
> > +
> > +/**
> > + * \class SensorDatabase
> > + * \brief Database of camera sensor properties
> > + *
> > + * The database is indexed using the camera sensor model, as reported by the
> > + * properties::Model property, and for each supported sensor it contains a
> > + * list of properties.
> > + *
> > + * The database is statically allocated and cannot be modified at runtime, and
> > + * only provides two methods: one to verify if a sensor is supported
> > + * (SensorDatabase::contains()) and one to retrieve the sensor properties
> > + * (SensorDatabase::properties()).
> > + */
> > +
> > +/**
> > + * \fn SensorDatabase::SensorDatabase(std::initializer_list<Map::value_type> items)
> > + * \brief Contruct the sensor database with a list of sensor properties
> > + * \param[in] items The list of sensor properties
> > + */
> > +
> > +/**
> > + * \brief Verify if a sensor is part of the database
> > + * \param sensor The sensor model name
> > + * \return True if the sensor has an associated list of properties in the
> > + * database, false otherwise
> > + */
> > +bool SensorDatabase::contains(Map::key_type sensor) const
> > +{
> > + return Map::find(sensor) != Map::end();
> > +}
> > +
> > +/**
> > + * \brief Retrieve the properties associated with a sensor
> > + * \param sensor The sensor model name
> > + * \return The properties list associated with a sensor or an empty list if the
> > + * sensor is not supported
> > + */
> > +const ControlList &SensorDatabase::properties(Map::key_type sensor) const
> > +{
> > + static ControlList empty{};
> > +
> > + auto it = Map::find(sensor);
> > + if (it != Map::end())
> > + return it->second;
> > +
> > + return empty;
> > +}
> > +
> > +/**
> > + * \brief Sony IMX219 sensor properties
> > + */
> > +extern const ControlList imx219Properties = {
> > + { &properties::draft::SensorPhysicalSize, Span<const float>({ 5.095, 4.93 }) },
> > + { &properties::UnitCellSize, Size(1120, 1120) },
> > +};
> > +
> > +/**
> > + * \brief Omnivision ov5670 sensor properties
> > + */
> > +extern const ControlList ov5670Properties = {
> > + { &properties::draft::SensorPhysicalSize, Span<const float>({ 2.9457, 2.214 }) },
> > + { &properties::UnitCellSize, Size(1120, 1120) }
> > +};
> > +
> > +/**
> > + * \brief Omnivision 13858 sensor properties
> > + */
> > +extern const ControlList ov13858Properties = {
> > + { &properties::draft::SensorPhysicalSize, Span<const float>({ 4.7497, 3.53549 }) },
> > + { &properties::UnitCellSize, Size(1120, 1120) }
> > +};
>
> We'll probably need a mechanism to ensure that all properties are
> included for all sensors, but that can come later.
>
> > +
> > +#define SENSOR_ENTRY(_sensor) \
> > + { #_sensor, _sensor ## Properties }
> > +
> > +/**
> > + * \brief Database of sensor properties
> > + */
> > +extern const SensorDatabase sensorDatabase = {
> > + SENSOR_ENTRY(imx219),
> > + SENSOR_ENTRY(ov5670),
> > + SENSOR_ENTRY(ov13858),
> > +};
> > +
> > +} /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list