[PATCH v3 5/6] libcamera: camera_sensor_properties: Add sensor control delays
Dan Scally
dan.scally at ideasonboard.com
Fri Nov 15 14:35:54 CET 2024
Hi Kieran
On 15/11/2024 12:10, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-11-15 07:46:27)
>> Add properties covering the sensor control application delays to both
>> 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.
>>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>> Changes in v3:
>>
>> - Rebased on top of the CameraSensorFactory introduction
>> - Some rephrasing
>> - Defined the sensorDelays member as empty where Raspberry Pi didn't
>> have any specific values. Check for the empty struct in
>> getSensorDelays() and return the defaults from there with a warning.
> Sounds resonable to me.
>
>
>> Changes in v2:
>>
>> - Rather than adding the delays to the properties ControlList, added a
>> new function in CameraSensor that allows PipelineHandlers to retreive
>> the delay values.
>>
>> include/libcamera/internal/camera_sensor.h | 2 +
>> .../internal/camera_sensor_properties.h | 9 +++
>> src/libcamera/sensor/camera_sensor.cpp | 13 +++
>> src/libcamera/sensor/camera_sensor_legacy.cpp | 33 ++++++++
>> .../sensor/camera_sensor_properties.cpp | 79 +++++++++++++++++++
>> 5 files changed, 136 insertions(+)
>>
>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
>> index 8aafd82e..a9b2d494 100644
>> --- a/include/libcamera/internal/camera_sensor.h
>> +++ b/include/libcamera/internal/camera_sensor.h
>> @@ -73,6 +73,8 @@ public:
>> virtual const std::vector<controls::draft::TestPatternModeEnum> &
>> testPatternModes() const = 0;
>> virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0;
>> + virtual void getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,
>> + uint8_t &vblankDelay, uint8_t &hblankDelay) = 0;
> Could/Should this return a const pointer to a struct SensorDelays now ?
Yeah maybe...that would add a requirement for the struct to be available to CameraSensor (which is a
Factory class now) - I don't really have any strong feelings either way, so as long as that's fine
then I'll make the change.
>
>> };
>>
>> class CameraSensorFactoryBase
>> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
>> index 480ac121..56d5c15d 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,13 @@ struct CameraSensorProperties {
>>
>> Size unitCellSize;
>> std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
>> +
>> + struct {
>> + uint8_t exposureDelay;
>> + uint8_t gainDelay;
>> + uint8_t vblankDelay;
>> + uint8_t hblankDelay;
>> + } sensorDelays;
>> };
>>
>> } /* namespace libcamera */
>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
>> index 54cf98b2..61420873 100644
>> --- a/src/libcamera/sensor/camera_sensor.cpp
>> +++ b/src/libcamera/sensor/camera_sensor.cpp
>> @@ -336,6 +336,19 @@ CameraSensor::~CameraSensor() = default;
>> * pattern mode for every frame thus incurs no performance penalty.
>> */
>>
>> +/**
>> + * \fn CameraSensor::getSensorDelays()
>> + * \brief Fetch the sensor delay values
>> + * \param[out] exposureDelay The exposure delay
>> + * \param[out] gainDelay The analogue gain delay
>> + * \param[out] vblankDelay The vblank delay
>> + * \param[out] hblankDelay The hblank delay
>> + *
>> + * This function fills in sensor control delays for pipeline handlers to use to
>> + * inform the DelayedControls. If no static properties are available it fills in
>> + * some widely applicable default values.
>> + */
>> +
>> /**
>> * \class CameraSensorFactoryBase
>> * \brief Base class for camera sensor factories
>> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp
>> index a9b15c03..84972f02 100644
>> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp
>> @@ -95,6 +95,8 @@ public:
>> const std::vector<controls::draft::TestPatternModeEnum> &
>> testPatternModes() const override { return testPatternModes_; }
>> int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;
>> + void getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,
>> + uint8_t &vblankDelay, uint8_t &hblankDelay) override;
>>
>> protected:
>> std::string logPrefix() const override;
>> @@ -482,6 +484,37 @@ void CameraSensorLegacy::initStaticProperties()
>> initTestPatternModes();
>> }
>>
>> +void CameraSensorLegacy::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,
>> + uint8_t &vblankDelay, uint8_t &hblankDelay)
>> +{
>> +
>> + /*
>> + * These defaults are applicable to many sensors, however more specific
>> + * values can be added to the CameraSensorProperties for a sensor if
>> + * required.
>> + */
>> + if (!staticProps_ ||
>> + (!staticProps_->sensorDelays.exposureDelay &&
>> + !staticProps_->sensorDelays.gainDelay &&
>> + !staticProps_->sensorDelays.vblankDelay &&
>> + !staticProps_->sensorDelays.hblankDelay)) {
>> + LOG(CameraSensor, Warning)
>> + << "No sensor delays found in static properties. "
>> + "Assuming unverified defaults.";
>> +
>> + exposureDelay = 2;
>> + gainDelay = 1;
>> + vblankDelay = 2;
>> + hblankDelay = 2;
>> + return;
>> + }
>> +
>> + exposureDelay = staticProps_->sensorDelays.exposureDelay;
>> + gainDelay = staticProps_->sensorDelays.gainDelay;
>> + vblankDelay = staticProps_->sensorDelays.vblankDelay;
>> + hblankDelay = staticProps_->sensorDelays.hblankDelay;
> Which would make this just
>
> return staticProps_->sensorDelays;
>
>> +}
> I'm glad this function prints a warning!
>
>> +
>> void CameraSensorLegacy::initTestPatternModes()
>> {
>> const auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);
>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
>> index 6d4136d0..6dda7ba9 100644
>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp
>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
>> @@ -41,6 +41,13 @@ 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 Holds the delays, expressed in number of frames, between the time a
>> + * control is applied to the sensor and the time it actually takes effect.
>> + * Delays are recorded for the exposure time, analogue gain, vertical and
>> + * horizontal blanking. These values may be defined as empty, in which case the
>> + * CameraSensor derivative should provide default values.
>> */
>>
>> /**
>> @@ -60,6 +67,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeColorBars, 2 },
>> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "ar0521", {
>> .unitCellSize = { 2200, 2200 },
>> @@ -69,6 +77,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeColorBars, 2 },
>> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "hi846", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -87,6 +96,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * 9: "Resolution Pattern"
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "imx214", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -97,6 +107,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>> { controls::draft::TestPatternModePn9, 4 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "imx219", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -107,6 +118,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 +134,62 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
>> { controls::draft::TestPatternModePn9, 4 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "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 = { },
>> } },
>> { "imx335", {
>> .unitCellSize = { 2000, 2000 },
>> .testPatternModes = {},
>> + .sensorDelays = { },
>> } },
>> { "imx415", {
>> .unitCellSize = { 1450, 1450 },
>> .testPatternModes = {},
>> + .sensorDelays = { },
>> } },
>> { "imx477", {
>> .unitCellSize = { 1550, 1550 },
>> .testPatternModes = {},
>> + .sensorDelays = {
>> + .exposureDelay = 2,
>> + .gainDelay = 2,
>> + .vblankDelay = 3,
>> + .hblankDelay = 3
>> + },
>> } },
>> { "imx519", {
>> .unitCellSize = { 1220, 1220 },
>> @@ -157,6 +202,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 +218,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 +238,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * 5: "Color Square"
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov2740", {
>> .unitCellSize = { 1400, 1400 },
>> @@ -188,6 +246,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeOff, 0 },
>> { controls::draft::TestPatternModeColorBars, 1},
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov4689", {
>> .unitCellSize = { 2000, 2000 },
>> @@ -201,6 +260,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * colorBarType2 and colorBarType3.
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov5640", {
>> .unitCellSize = { 1400, 1400 },
>> @@ -208,10 +268,17 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeOff, 0 },
>> { controls::draft::TestPatternModeColorBars, 1 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov5647", {
>> .unitCellSize = { 1400, 1400 },
>> .testPatternModes = {},
>> + .sensorDelays = {
>> + .exposureDelay = 2,
>> + .gainDelay = 2,
>> + .vblankDelay = 2,
>> + .hblankDelay = 2
>> + },
>> } },
>> { "ov5670", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -219,6 +286,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeOff, 0 },
>> { controls::draft::TestPatternModeColorBars, 1 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov5675", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -226,6 +294,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeOff, 0 },
>> { controls::draft::TestPatternModeColorBars, 1 },
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov5693", {
>> .unitCellSize = { 1400, 1400 },
>> @@ -238,6 +307,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * Rolling Bar".
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov64a40", {
>> .unitCellSize = { 1008, 1008 },
>> @@ -251,6 +321,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 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * 4: "Vertical Color Bar Type 4"
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov8865", {
>> .unitCellSize = { 1400, 1400 },
>> @@ -278,6 +355,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> * 5: "Color squares with rolling bar"
>> */
>> },
>> + .sensorDelays = { },
>> } },
>> { "ov13858", {
>> .unitCellSize = { 1120, 1120 },
>> @@ -285,6 +363,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>> { controls::draft::TestPatternModeOff, 0 },
>> { controls::draft::TestPatternModeColorBars, 1 },
>> },
>> + .sensorDelays = { },
>> } },
>> };
>>
>> --
>> 2.30.2
>>
More information about the libcamera-devel
mailing list