[libcamera-devel] [PATCH v2 7/9] libcamera: Introduce camera sensor database

[Laurent Pinchart]

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.

> +
> +} /* 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