[libcamera-devel] [PATCH v2 7/9] libcamera: Introduce camera sensor database
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 23 18:40:04 CET 2020
Hi Jacopo,
On Wed, Dec 23, 2020 at 05:40:01PM +0100, Jacopo Mondi wrote:
> On Wed, Dec 23, 2020 at 08:56:13AM +0200, Laurent Pinchart wrote:
> > On Wed, Dec 23, 2020 at 07:49:46AM +0200, Laurent Pinchart wrote:
> > > On Fri, Dec 18, 2020 at 05:47:52PM +0100, Jacopo Mondi wrote:
> > > > 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.
> >
>
> Good question.
>
> Are you referring to information contained in cam_helper_xxxx.cpp ?
> Are we sure that information like the number of frames to mis-trust is
> something that should be part of the sensor database. Ideally one
> should be able to expand the database with only the support of the
> sensor's datasheet, am I wrong ? It seems to me that some information
> there are implementation decision specific...
It was very implementation-specific, but with David's recent rework of
the AGC and AWB algorithms, the information exposed by the CamHelper
class has been refactored. It now only carries intrinsic properties of
the camera sensor, with the decision on how to use that information
moved to the IPA itself.
> > > > +
> > > > +} /* 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