[PATCH 5/6] libcamera: camera_sensor_properties: Add sensor control delays

Jacopo Mondi jacopo.mondi at ideasonboard.com
Mon Nov 4 12:14:46 CET 2024


Hi Dan

On Thu, Oct 31, 2024 at 04:07:40PM +0000, Daniel Scally wrote:
> Add properties covering the sensor control application delays to both
> the list of control values and the static CameraSensorProperties
> definitions. The values used are the defaults that're in use across
> the library, with deviations from that taken from Raspberry Pi's
> CamHelper class definitions.
>
> Initialise the properties from the static database during the same
> CameraSensor::initStaticProperties() function that currently handles
> the UnitCellSize.

I welcome this change, but it doesn't seem to be related to the
previous patche ?

>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>
> If anyone has a suggestion for a graceful way to set default values without
> causing warnings due to -Wmissing-field-initializers I'm very interested...
>
>  .../internal/camera_sensor_properties.h       |  13 ++
>  src/libcamera/property_ids_core.yaml          |  25 +++
>  src/libcamera/sensor/camera_sensor.cpp        |   8 +
>  .../sensor/camera_sensor_properties.cpp       | 167 ++++++++++++++++++
>  4 files changed, 213 insertions(+)
>
> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> index 480ac121..cd176b9e 100644
> --- a/include/libcamera/internal/camera_sensor_properties.h
> +++ b/include/libcamera/internal/camera_sensor_properties.h
> @@ -10,6 +10,8 @@
>  #include <map>
>  #include <string>
>
> +#include <stdint.h>
> +
>  #include <libcamera/control_ids.h>
>  #include <libcamera/geometry.h>
>
> @@ -20,6 +22,17 @@ struct CameraSensorProperties {
>
>  	Size unitCellSize;
>  	std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
> +
> +	/*
> +	 * These values are correct for many sensors. Other sensors will need to
> +	 * have the defaults overwritten in their CameraSensorProperties entry.
> +	 */
> +	struct {
> +		uint8_t exposureDelay;
> +		uint8_t gainDelay;
> +		uint8_t vblankDelay;
> +		uint8_t hblankDelay;
> +	} sensorDelays;
>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/property_ids_core.yaml b/src/libcamera/property_ids_core.yaml
> index 834454a4..a01c4014 100644
> --- a/src/libcamera/property_ids_core.yaml
> +++ b/src/libcamera/property_ids_core.yaml
> @@ -701,4 +701,29 @@ controls:
>
>          Different cameras may report identical devices.
>
> +  - ExposureDelay:
> +      type: uint8_t
> +      description: |
> +        The number of frames delay between an Exposure value being configured in
> +        the sensor's registers and taking effect in the output data.
> +
> +  - GainDelay:
> +      type: uint8_t
> +      description: |
> +        The number of frames delay between a Gain value being configured in
> +        the sensor's registers and taking effect in the output data.
> +
> +  - VerticalBlankingDelay:
> +      type: uint8_t
> +      description: |
> +        The number of frames delay between a Horizontal Blanking value being
> +        configured in the sensor's registers and taking effect in the output
> +        data.
> +
> +  - HorizontalBlankingDelay:
> +      type: uint8_t
> +      description: |
> +        The number of frames delay between a Vertical Blanking value being
> +        configured in the sensor's registers and taking effect in the output
> +        data.

Do we want these to be exposed to applications ??

Now that we have control namespacing we can define a namespace for
camera sensor specific controls where to store these information as
well as controls used for the pipeline-IPA communication (once we move
the interface away from V4L2 controls).

>  ...
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 1b224f19..c9a3761e 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -387,6 +387,14 @@ void CameraSensor::initStaticProperties()
>
>  	/* Register the properties retrieved from the sensor database. */
>  	properties_.set(properties::UnitCellSize, staticProps_->unitCellSize);
> +	properties_.set(properties::ExposureDelay,
> +			staticProps_->sensorDelays.exposureDelay);
> +	properties_.set(properties::GainDelay,
> +			staticProps_->sensorDelays.gainDelay);
> +	properties_.set(properties::VerticalBlankingDelay,
> +			staticProps_->sensorDelays.vblankDelay);
> +	properties_.set(properties::HorizontalBlankingDelay,
> +			staticProps_->sensorDelays.hblankDelay);

I don't think these should be part of the properties exposed to
applications.. Or is there a reason to do so I am missing.

>
>  	initTestPatternModes();
>  }
> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> index 6d4136d0..60d59f79 100644
> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
>   * \brief Map that associates the TestPattern control value with the indexes of
>   * the corresponding sensor test pattern modes as returned by
>   * V4L2_CID_TEST_PATTERN.
> + *
> + * \var CameraSensorProperties::sensorDelays
> + * \brief struct holding the number of frames delay between a control value
> + * set and taking effect for each of exposure, gain, vertical blanking and
> + * horizontal blanking.
>   */


Should we make it mandatory for all newly added helpers to specify
delays ? How easy is this information to get ? Do we need a fallback
mechanism ?


>
>  /**
> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBars, 2 },
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ar0521", {
>  			.unitCellSize = { 2200, 2200 },
> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBars, 2 },
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "hi846", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * 9: "Resolution Pattern"
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx214", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx219", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx258", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx283", {
>  			.unitCellSize = { 2400, 2400 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx290", {
>  			.unitCellSize = { 2900, 2900 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx296", {
>  			.unitCellSize = { 3450, 3450 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx327", {
>  			.unitCellSize = { 2900, 2900 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx335", {
>  			.unitCellSize = { 2000, 2000 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx415", {
>  			.unitCellSize = { 1450, 1450 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "imx477", {
>  			.unitCellSize = { 1550, 1550 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 3,
> +				.hblankDelay = 3
> +			},
>  		} },
>  		{ "imx519", {
>  			.unitCellSize = { 1220, 1220 },
> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 3,
> +				.hblankDelay = 3
> +			},
>  		} },
>  		{ "imx708", {
>  			.unitCellSize = { 1400, 1400 },
> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>  				{ controls::draft::TestPatternModePn9, 4 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 3,
> +				.hblankDelay = 3
> +			},
>  		} },
>  		{ "ov2685", {
>  			.unitCellSize = { 1750, 1750 },
> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * 5: "Color Square"
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov2740", {
>  			.unitCellSize = { 1400, 1400 },
> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeOff, 0 },
>  				{ controls::draft::TestPatternModeColorBars, 1},
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov4689", {
>  			.unitCellSize = { 2000, 2000 },
> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * colorBarType2 and colorBarType3.
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov5640", {
>  			.unitCellSize = { 1400, 1400 },
> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeOff, 0 },
>  				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov5647", {
>  			.unitCellSize = { 1400, 1400 },
>  			.testPatternModes = {},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov5670", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeOff, 0 },
>  				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov5675", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeOff, 0 },
>  				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov5693", {
>  			.unitCellSize = { 1400, 1400 },
> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * Rolling Bar".
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov64a40", {
>  			.unitCellSize = { 1008, 1008 },
> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * 4: "Vertical Color Bar Type 4"
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 2,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov8858", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * 4: "Vertical Color Bar Type 4"
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov8865", {
>  			.unitCellSize = { 1400, 1400 },
> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				 * 5: "Color squares with rolling bar"
>  				 */
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  		{ "ov13858", {
>  			.unitCellSize = { 1120, 1120 },
> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  				{ controls::draft::TestPatternModeOff, 0 },
>  				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
> +			.sensorDelays = {
> +				.exposureDelay = 2,
> +				.gainDelay = 1,
> +				.vblankDelay = 2,
> +				.hblankDelay = 2
> +			},
>  		} },
>  	};
>
> --
> 2.30.2
>


More information about the libcamera-devel mailing list