[libcamera-devel] [RFC PATCH 8/8] libcamera: camera_sensor: Parse configuration properties

[Jacopo Mondi]

Hi Kieran,

On Mon, Nov 23, 2020 at 04:43:19PM +0000, Kieran Bingham wrote:
> Search for a camera_sensor.json configuration file specific to libcamera
> and parse it to see if any properties specific to this CameraSensor are
> applicable.

For sake of discussion on the camera sensor database.

I don't think this should be a system-wide configuration file but
rather part of the libcamera sources, with a known path [1] It will
contain details on the sensor that we cannot retrieve easily from the
kernel interface, or that are not even exposed by it, even if we
should keep pushing to get most of those information from a more and
more expressive v4l2 subdev interface.

Currently we have information that we cannot retrieve from the kernel
though. To name a few I can think of
- Colour Filter Arrangement (even if it could be deduced from the RAW
  format components sequence)
- Control delays (see Niklas' delayed controls and RPi's
  StaggeredControls)
- Color tuning information and sensitivites (white/black levels ? Gain
  factors ? Probably we have a V4L2 control for that).

Ideally, we should be able to push all of them into the kernel, but
it's a long process. Having a sensor database helps in that regard,
but makes less pressing the need for decent sensor drivers. One
example is all the size related information. We -could- get most of
them through the V4L2 Selection API, but only a few driver actually
expose them correctly. Recording them in userspace has anyway value,
so I'm a little debated.

Also, if we have a 'camera module' to deal with, the sensor database
(which is not only about sensor anymore) would contain information
relative to the module integration, like lens information.

Then there's the part relative to how the sensor itself is integrated
in the device, and I agree this should be expressed in a system-wide
configuration file. From there we will have other information we
cannot retrieve from the subdev interface (mostly because of the lack
of support in firmware for describing, in example, lens properties, at
least on device tree). In there we will be able to retreive
information on the lens mounted on the sensor, where the sensor is
mounted and its orientation.

Before seeing the integration in CameraSensor, I would like to discuss
and see the desired structure of those files. First thing being how to
identify cameras vs sensor in the two files, if we agree about the
nature and the intended content of those files.

Thanks
  j

[1] being the sensor database a libcamera component, is it worth
parsing it at run time ? I mean, can we pass it through a script, and
generate a .h header with all the information easily accessible at
run-time from C++ ?

>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/internal/camera_sensor.h |  1 +
>  src/libcamera/camera_sensor.cpp            | 50 ++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index b9eba89f00fb..646f24a966bd 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -69,6 +69,7 @@ protected:
>
>  private:
>  	int generateId();
> +	int parseConfigurationFile();
>
>  	const MediaEntity *entity_;
>  	std::unique_ptr<V4L2Subdevice> subdev_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 935de528c496..5a1bda483b9d 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -17,6 +17,7 @@
>
>  #include <libcamera/property_ids.h>
>
> +#include "libcamera/internal/configuration.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/utils.h"
> @@ -251,6 +252,12 @@ int CameraSensor::init()
>  		propertyValue = 0;
>  	properties_.set(properties::Rotation, propertyValue);
>
> +	/*
> +	 * Properties are pulled from a configuration file on a best effort.
> +	 * If the file doesn't exist, or has no properties there is no failure.
> +	 */
> +	parseConfigurationFile();
> +
>  	/* Enumerate, sort and cache media bus codes and sizes. */
>  	formats_ = subdev_->formats(pad_);
>  	if (formats_.empty()) {
> @@ -584,4 +591,47 @@ int CameraSensor::generateId()
>  	return -EINVAL;
>  }
>
> +int CameraSensor::parseConfigurationFile()
> +{
> +	Configuration c;
> +
> +	int ret = c.open("camera_sensor.json");
> +	if (ret) {
> +		LOG(CameraSensor, Debug) << "No configuration file available";
> +		return -ENODATA;
> +	}
> +
> +	/* Find properties based on the Camera model_ */
> +	/* Todo: Spilt parsing out for multiple paths. */
> +
> +	/* Find properties based around the Camera Sensor ID */
> +	json::iterator it = c.data().find(id_);
> +	if (it == c.data().end())
> +		return -ENOENT;
> +
> +	json j = *it;
> +	it = j.find("properties");
> +	if (it == j.end())
> +		return -ENOENT;
> +
> +	json deviceProperties = *it;
> +
> +	for (auto &[key, value] : deviceProperties.items()) {
> +		LOG(CameraSensor, Debug) << "Key: " << key << " Value: " << value;
> +
> +		if (!value.is_number()) {
> +			LOG(CameraSensor, Debug) << "Not a number: " << value;
> +			continue;
> +		}
> +
> +		if (key == "Rotation")
> +			properties_.set(properties::Rotation, value);
> +
> +		if (key == "Location")
> +			properties_.set(properties::Location, value);
> +	}
> +
> +	return 0;
> +}
> +
>  } /* namespace libcamera */
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel