[libcamera-devel] [PATCH v3 2/4] libcamera: camera_sensor: Register static properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2020-12-28 17:52:01 +0100, Jacopo Mondi wrote:
> Register static properties, retrieved inspecting the sensor
> database, in the CameraSensor class.
> 
> Static properties are overridden by properties retrieved from
> the kernel interface at run-time if any overlap between the two
> sets occurs.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/internal/camera_sensor.h |  1 +
>  src/libcamera/camera_sensor.cpp            | 21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 10877b9d1d9d..2790c55b6178 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -71,6 +71,7 @@ protected:
>  private:
>  	int generateId();
>  	int validateSensorDriver();
> +	void initStaticProperties();
>  	int initProperties();
>  	int initControls();
>  
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index ea57943647b1..d86fc211c4f6 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -20,6 +20,7 @@
>  
>  #include "libcamera/internal/bayer_format.h"
>  #include "libcamera/internal/formats.h"
> +#include "libcamera/internal/sensor_database.h"
>  #include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/utils.h"
>  
> @@ -295,6 +296,21 @@ int CameraSensor::validateSensorDriver()
>  	return 0;
>  }
>  
> +void CameraSensor::initStaticProperties()
> +{
> +	const SensorInfo *info = SensorDatabase::get(model_);
> +	if (!info) {
> +		LOG(CameraSensor, Warning)
> +			<< "No static properties available for '" << model_ << "'";
> +		LOG(CameraSensor, Warning)
> +			<< "Please consider updating the sensor database";

I think this could be a single LOG().

> +		return;
> +	}
> +
> +	/* Register the properties retrieved from the sensor database. */
> +	properties_.set(properties::UnitCellSize, info->unitCellSize);

This implies all future data members of SensorInfo are mandatory. Would 
it make sens to add a 'valid' flag for each data member and only set the 
property if it's actually recorded in the sensor database?

> +}
> +
>  int CameraSensor::initProperties()
>  {
>  	/*
> @@ -332,7 +348,10 @@ int CameraSensor::initProperties()
>  	if (ret)
>  		return ret;
>  
> -	/* Retrieve and store the camera sensor properties. */
> +	/* Initialize the static properties from the sensor database. */
> +	initStaticProperties();
> +
> +	/* Retrieve and register properties from the kernel interface. */
>  	const ControlInfoMap &controls = subdev_->controls();
>  	int32_t propertyValue;
>  
> -- 
> 2.29.2
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund