[PATCH 5/6] libcamera: camera_sensor_properties: Add sensor control delays
Dan Scally
dan.scally at ideasonboard.com
Wed Nov 6 10:42:13 CET 2024
Hi Jacopo
On 04/11/2024 11:14, Jacopo Mondi wrote:
> 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 ?
Only in the sense that they all stemmed from Kieran's review of the C55 series
>
>> 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 ??
I have no strong feelings on that I think; what makes you think that they shouldn't be?
>
> 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 ?
Indeed a way to specify a default would be helpful...that was possible in the CameraSensorHelper as
Mikhail did it but I couldn't see a way for here. I think they need to be experimentally verified.
>
>
>> /**
>> @@ -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