[libcamera-devel] [PATCH v3 09/23] libcamera: camera_sensor_properties: Add sensor delays

Kieran Bingham kieran.bingham at ideasonboard.com
Fri Jul 1 00:37:32 CEST 2022


Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:48)
> Add to the camera sensor properties a map of control identifiers to
> their latencies.
> 
> Some camera sensor controls take a variable number of frames to take
> effect, hence they should be set in advance. The newly introduced map
> records that information as a property of the camera sensor class.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  .../internal/camera_sensor_properties.h       |  4 ++
>  .../camera_sensor_properties.cpp              | 40 +++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> index 1ee3cb994106..2404b0674323 100644
> --- a/include/libcamera/internal/camera_sensor_properties.h
> +++ b/include/libcamera/internal/camera_sensor_properties.h
> @@ -9,6 +9,7 @@
>  
>  #include <map>
>  #include <string>
> +#include <unordered_map>
>  
>  #include <libcamera/control_ids.h>
>  #include <libcamera/geometry.h>
> @@ -16,10 +17,13 @@
>  namespace libcamera {
>  
>  struct CameraSensorProperties {
> +       using SensorDelays = std::unordered_map<const ControlId *, unsigned int>;
> +
>         static const CameraSensorProperties *get(const std::string &sensor);
>  
>         Size unitCellSize;
>         std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
> +       SensorDelays sensorDelays;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor/camera_sensor_properties.cpp b/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> index e5f27f06eb1d..4b5ab716abe2 100644
> --- a/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> @@ -13,6 +13,8 @@
>  
>  #include <libcamera/control_ids.h>
>  
> +#include <libcamera/internal/control_ids.h>
> +
>  /**
>   * \file camera_sensor_properties.h
>   * \brief Database of camera sensor properties
> @@ -41,6 +43,18 @@ 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 Map that associates control identifiers with their latencies,
> + * expressed in frame periods.
> + */
> +
> +/**
> + * \typedef CameraSensorProperties::SensorDelays
> + * \brief Camera sensor controls delays
> + *
> + * Type to represent a map of camera sensor controls associated with their
> + * latencies, expressed in frame periods.
>   */
>  

I wonder if defining a 'common' struct of sensor delays here could ease
referencing it in the default case.

static const SensorDelays commonDelays = {
    { &controls::internal::FrameDuration, 2 },
    { &controls::internal::ExposureTime, 2 },
    { &controls::internal::AnalogueGain, 1 },
};

Or ideally, if we knew 'all ccs' cameras or such used these delays we
could have a better name ?

defaultDelays ?

I know I commented on this before saying if we don't have the numbers we
should leave the the delays as zero, but I think there I meant on an
individual entry.

But perhaps now I see this I'm changing my mind too - and leaning
towards a default set can easily be defined and referenced, and
overridden in the case of a difference?


>  /**
> @@ -69,6 +83,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                  * 9: "Resolution Pattern"
>                                  */
>                         },
> +                       .sensorDelays = { },

then
			.sensorDelays = &commonDelays;
?

Though this is the hi846, so we should identify what the delays really
are there.

>                 } },
>                 { "imx219", {
>                         .unitCellSize = { 1120, 1120 },
> @@ -79,6 +94,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>                                 { controls::draft::TestPatternModePn9, 4 },
>                         },
> +                       .sensorDelays = {
> +                               { &controls::internal::FrameDuration, 2 },
> +                               { &controls::internal::ExposureTime, 2 },
> +                               { &controls::internal::AnalogueGain, 1 },
> +                       },

But this one could be: 
			.sensorDelays = &commonDelays;

>                 } },
>                 { "imx258", {
>                         .unitCellSize = { 1120, 1120 },
> @@ -89,18 +109,26 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>                                 { controls::draft::TestPatternModePn9, 4 },
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "imx290", {
>                         .unitCellSize = { 2900, 2900 },
>                         .testPatternModes = {},
> +                       .sensorDelays = {
> +                               { &controls::internal::FrameDuration, 2 },
> +                               { &controls::internal::ExposureTime, 2 },
> +                               { &controls::internal::AnalogueGain, 2 },
> +                       },
>                 } },
>                 { "imx296", {
>                         .unitCellSize = { 3450, 3450 },
>                         .testPatternModes = {},
> +                       .sensorDelays = { },

The IMX296 is listed in RPi's cam_helper_imx296 without an override.
That means in their implementation that this is expected to use

	.sensorDelays = &commonDelays;

>                 } },
>                 { "imx477", {
>                         .unitCellSize = { 1550, 1550 },
>                         .testPatternModes = {},
> +                       .sensorDelays = { },

cam_helper_imx477 has the following overrides:

	exposure_delay = 2;
	gain_delay = 2;
	vblank_delay = 3;

>                 } },
>                 { "ov2740", {
>                         .unitCellSize = { 1400, 1400 },
> @@ -108,6 +136,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeOff, 0 },
>                                 { controls::draft::TestPatternModeColorBars, 1},
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov5640", {
>                         .unitCellSize = { 1400, 1400 },
> @@ -115,10 +144,16 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeOff, 0 },
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov5647", {
>                         .unitCellSize = { 1400, 1400 },
>                         .testPatternModes = {},
> +                       .sensorDelays = {
> +                               { &controls::internal::FrameDuration, 2 },
> +                               { &controls::internal::ExposureTime, 2 },
> +                               { &controls::internal::AnalogueGain, 2 },
> +                       },
>                 } },
>                 { "ov5670", {
>                         .unitCellSize = { 1120, 1120 },
> @@ -126,6 +161,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeOff, 0 },
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov5675", {
>                         .unitCellSize = { 1120, 1120 },
> @@ -133,6 +169,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeOff, 0 },
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov5693", {
>                         .unitCellSize = { 1400, 1400 },
> @@ -145,6 +182,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                  * Rolling Bar".
>                                  */
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov8865", {
>                         .unitCellSize = { 1400, 1400 },
> @@ -159,6 +197,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                  * 5: "Color squares with rolling bar"
>                                  */
>                         },
> +                       .sensorDelays = { },
>                 } },
>                 { "ov13858", {
>                         .unitCellSize = { 1120, 1120 },
> @@ -166,6 +205,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>                                 { controls::draft::TestPatternModeOff, 0 },
>                                 { controls::draft::TestPatternModeColorBars, 1 },
>                         },
> +                       .sensorDelays = { },
>                 } },
>         };
>  
> -- 
> 2.36.1
>


More information about the libcamera-devel mailing list