| Message ID | 20241126151203.108407-2-dan.scally@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Daniel Scally (2024-11-26 15:12:02) > Add properties covering the sensor control application delays to both > the static CameraSensorProperties definitions. The values used are > taken from Raspberry Pi's CamHelper class definitions. Where no more > specific values are known the delay struct is defined as empty and > defaults supplied through the getter function. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > Changes in v5: > > - Comment rewording > - Moved the defaultSensorDelays definition inside > CameraSensorLegacy::sensorDelays() > > Changes in v4: > > - getSensorDelays() renamed to sensorDelays() > - sensorDelays() returns a reference to the SensorDelays struct rather > than each of the values. > - Reworded the comments to make the default values seem less acceptable > - Added delay values for AR0144 > > 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. > > 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 | 8 ++ > src/libcamera/sensor/camera_sensor.cpp | 14 +++ > src/libcamera/sensor/camera_sensor_legacy.cpp | 25 ++++ > .../sensor/camera_sensor_properties.cpp | 108 ++++++++++++++++++ > 5 files changed, 157 insertions(+) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index 8aafd82e..bbdb83a1 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -20,6 +20,7 @@ > #include <libcamera/orientation.h> > #include <libcamera/transform.h> > > +#include "libcamera/internal/camera_sensor_properties.h" > #include "libcamera/internal/bayer_format.h" > #include "libcamera/internal/v4l2_subdevice.h" > > @@ -73,6 +74,7 @@ public: > virtual const std::vector<controls::draft::TestPatternModeEnum> & > testPatternModes() const = 0; > virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0; > + virtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0; > }; > > class CameraSensorFactoryBase > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h > index 480ac121..75cb7839 100644 > --- a/include/libcamera/internal/camera_sensor_properties.h > +++ b/include/libcamera/internal/camera_sensor_properties.h > @@ -8,6 +8,7 @@ > #pragma once > > #include <map> > +#include <stdint.h> > #include <string> > > #include <libcamera/control_ids.h> > @@ -20,6 +21,13 @@ struct CameraSensorProperties { > > Size unitCellSize; > std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes; > + > + struct SensorDelays { > + 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..9d2c2ea7 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default; > * pattern mode for every frame thus incurs no performance penalty. > */ > > +/** > + * \fn CameraSensor::sensorDelays() > + * \brief Fetch the sensor delay values > + * > + * This function retreives the sensor control delays for pipeline handlers to > + * use to inform the DelayedControls. If control delays are not specified in the > + * static sensor propertie database, this function returns a reference to a set > + * of default sensor delays provided as best-effort placeholders for the actual > + * sensor specific delays. > + * > + * \return A reference to a struct CameraSensorProperties::SensorDelays holding > + * the delay 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..17d6fa68 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -95,6 +95,7 @@ public: > const std::vector<controls::draft::TestPatternModeEnum> & > testPatternModes() const override { return testPatternModes_; } > int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override; > + const CameraSensorProperties::SensorDelays &sensorDelays() override; > > protected: > std::string logPrefix() const override; > @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties() > initTestPatternModes(); > } > > +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays() > +{ > + static constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 1, > + .vblankDelay = 2, > + .hblankDelay = 2, > + }; A better place to keep this local indeed. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + > + 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."; > + > + return defaultSensorDelays; > + } > + > + return staticProps_->sensorDelays; > +} > + > 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 e2305166..3dc8cf05 100644 > --- a/src/libcamera/sensor/camera_sensor_properties.cpp > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp > @@ -41,6 +41,36 @@ 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 Sensor control application delays. > + * > + * This struct may be defined as empty if the actual sensor delays are not > + * available or have not been measured. Best effort default values are returned > + * in this case. > + */ > + > +/** > + * \struct CameraSensorProperties::SensorDelays > + * \brief Sensor control application delay values > + * > + * This struct holds delay values, expressed in number of frames, between the > + * time a control value is applied to the sensor and the time that value is > + * reflected in the output. For example "2 frames delay" means that parameters > + * set during frame N will take effect for frame N+2 (and by extension a delay > + * of 0 would mean the parameter is applied immediately to the current frame). > + * > + * \var CameraSensorProperties::SensorDelays::exposureDelay > + * \brief Number of frames between application of exposure control and effect > + * > + * \var CameraSensorProperties::SensorDelays::gainDelay > + * \brief Number of frames between application of analogue gain control and effect > + * > + * \var CameraSensorProperties::SensorDelays::vblankDelay > + * \brief Number of frames between application of vblank control and effect > + * > + * \var CameraSensorProperties::SensorDelays::hblankDelay > + * \brief Number of frames between application of hblank control and effect > */ > > /** > @@ -60,6 +90,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBars, 2 }, > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > }, > + .sensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 2, > + .vblankDelay = 2, > + .hblankDelay = 2 > + }, > } }, > { "ar0521", { > .unitCellSize = { 2200, 2200 }, > @@ -69,6 +105,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBars, 2 }, > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > }, > + .sensorDelays = { }, > } }, > { "hi846", { > .unitCellSize = { 1120, 1120 }, > @@ -87,6 +124,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 9: "Resolution Pattern" > */ > }, > + .sensorDelays = { }, > } }, > { "imx214", { > .unitCellSize = { 1120, 1120 }, > @@ -97,6 +135,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > { controls::draft::TestPatternModePn9, 4 }, > }, > + .sensorDelays = { }, > } }, > { "imx219", { > .unitCellSize = { 1120, 1120 }, > @@ -107,6 +146,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,38 +162,67 @@ 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 = { }, > } }, > { "imx462", { > .unitCellSize = { 2900, 2900 }, > .testPatternModes = {}, > + .sensorDelays = { }, > } }, > { "imx477", { > .unitCellSize = { 1550, 1550 }, > .testPatternModes = {}, > + .sensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 2, > + .vblankDelay = 3, > + .hblankDelay = 3 > + }, > } }, > { "imx519", { > .unitCellSize = { 1220, 1220 }, > @@ -161,6 +235,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 }, > @@ -171,6 +251,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 }, > @@ -185,6 +271,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 5: "Color Square" > */ > }, > + .sensorDelays = { }, > } }, > { "ov2740", { > .unitCellSize = { 1400, 1400 }, > @@ -192,6 +279,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1}, > }, > + .sensorDelays = { }, > } }, > { "ov4689", { > .unitCellSize = { 2000, 2000 }, > @@ -205,6 +293,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * colorBarType2 and colorBarType3. > */ > }, > + .sensorDelays = { }, > } }, > { "ov5640", { > .unitCellSize = { 1400, 1400 }, > @@ -212,10 +301,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 }, > @@ -223,6 +319,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > { "ov5675", { > .unitCellSize = { 1120, 1120 }, > @@ -230,6 +327,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > @@ -242,6 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * Rolling Bar". > */ > }, > + .sensorDelays = { }, > } }, > { "ov64a40", { > .unitCellSize = { 1008, 1008 }, > @@ -255,6 +354,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 }, > @@ -268,6 +373,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 4: "Vertical Color Bar Type 4" > */ > }, > + .sensorDelays = { }, > } }, > { "ov8865", { > .unitCellSize = { 1400, 1400 }, > @@ -282,6 +388,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 5: "Color squares with rolling bar" > */ > }, > + .sensorDelays = { }, > } }, > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > @@ -289,6 +396,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > }; > > -- > 2.30.2 >
On Tue, Nov 26, 2024 at 03:12:02PM +0000, Daniel Scally wrote: > Add properties covering the sensor control application delays to both > the static CameraSensorProperties definitions. The values used are > taken from Raspberry Pi's CamHelper class definitions. Where no more > specific values are known the delay struct is defined as empty and > defaults supplied through the getter function. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > Changes in v5: > > - Comment rewording > - Moved the defaultSensorDelays definition inside > CameraSensorLegacy::sensorDelays() > > Changes in v4: > > - getSensorDelays() renamed to sensorDelays() > - sensorDelays() returns a reference to the SensorDelays struct rather > than each of the values. > - Reworded the comments to make the default values seem less acceptable > - Added delay values for AR0144 > > 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. > > 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 | 8 ++ > src/libcamera/sensor/camera_sensor.cpp | 14 +++ > src/libcamera/sensor/camera_sensor_legacy.cpp | 25 ++++ > .../sensor/camera_sensor_properties.cpp | 108 ++++++++++++++++++ > 5 files changed, 157 insertions(+) > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h > index 8aafd82e..bbdb83a1 100644 > --- a/include/libcamera/internal/camera_sensor.h > +++ b/include/libcamera/internal/camera_sensor.h > @@ -20,6 +20,7 @@ > #include <libcamera/orientation.h> > #include <libcamera/transform.h> > > +#include "libcamera/internal/camera_sensor_properties.h" Alphabetical order. I'm surprised checkstyle.py didn't warn you about this. > #include "libcamera/internal/bayer_format.h" > #include "libcamera/internal/v4l2_subdevice.h" > > @@ -73,6 +74,7 @@ public: > virtual const std::vector<controls::draft::TestPatternModeEnum> & > testPatternModes() const = 0; > virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0; > + virtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0; > }; > > class CameraSensorFactoryBase > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h > index 480ac121..75cb7839 100644 > --- a/include/libcamera/internal/camera_sensor_properties.h > +++ b/include/libcamera/internal/camera_sensor_properties.h > @@ -8,6 +8,7 @@ > #pragma once > > #include <map> > +#include <stdint.h> > #include <string> > > #include <libcamera/control_ids.h> > @@ -20,6 +21,13 @@ struct CameraSensorProperties { > > Size unitCellSize; > std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes; > + > + struct SensorDelays { > + uint8_t exposureDelay; > + uint8_t gainDelay; > + uint8_t vblankDelay; > + uint8_t hblankDelay; > + } sensorDelays; We usually separate type definitions from usage. struct CameraSensorProperties { struct SensorDelays { uint8_t exposureDelay; uint8_t gainDelay; uint8_t vblankDelay; uint8_t hblankDelay; }; 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/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 54cf98b2..9d2c2ea7 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default; > * pattern mode for every frame thus incurs no performance penalty. > */ > > +/** > + * \fn CameraSensor::sensorDelays() > + * \brief Fetch the sensor delay values > + * > + * This function retreives the sensor control delays for pipeline handlers to s/retreives/retrieves/ > + * use to inform the DelayedControls. If control delays are not specified in the We don't usually document expected users, unless there's a very tight coupling between a class and its users that is relevant to understanding the API. > + * static sensor propertie database, this function returns a reference to a set s/propertie/properties/ > + * of default sensor delays provided as best-effort placeholders for the actual > + * sensor specific delays. And there's a bit of implementation detail here too. You can simplify this to * This function retrieves the delays that the sensor applies to controls. If * the static properties database doesn't specifiy control delay values for the * sensor, default delays that may be suitable are returned and a warning is * logged. > + * > + * \return A reference to a struct CameraSensorProperties::SensorDelays holding > + * the delay values Doxygen shows the type of the return value, so you can just write * \return The sensor delay 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..17d6fa68 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -95,6 +95,7 @@ public: > const std::vector<controls::draft::TestPatternModeEnum> & > testPatternModes() const override { return testPatternModes_; } > int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override; > + const CameraSensorProperties::SensorDelays &sensorDelays() override; > > protected: > std::string logPrefix() const override; > @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties() > initTestPatternModes(); > } > > +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays() > +{ > + static constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 1, > + .vblankDelay = 2, > + .hblankDelay = 2, > + }; > + > + 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."; > + > + return defaultSensorDelays; > + } > + > + return staticProps_->sensorDelays; > +} > + > 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 e2305166..3dc8cf05 100644 > --- a/src/libcamera/sensor/camera_sensor_properties.cpp > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp > @@ -41,6 +41,36 @@ 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 Sensor control application delays. checkstyle.py flagged this. > + * > + * This struct may be defined as empty if the actual sensor delays are not s/struct/structure/ > + * available or have not been measured. Best effort default values are returned > + * in this case. In this context, it's not clear where the defaults are returned from. I'd drop the last sentence. > + */ > + > +/** > + * \struct CameraSensorProperties::SensorDelays > + * \brief Sensor control application delay values > + * > + * This struct holds delay values, expressed in number of frames, between the s/struct/structure/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * time a control value is applied to the sensor and the time that value is > + * reflected in the output. For example "2 frames delay" means that parameters > + * set during frame N will take effect for frame N+2 (and by extension a delay > + * of 0 would mean the parameter is applied immediately to the current frame). > + * > + * \var CameraSensorProperties::SensorDelays::exposureDelay > + * \brief Number of frames between application of exposure control and effect > + * > + * \var CameraSensorProperties::SensorDelays::gainDelay > + * \brief Number of frames between application of analogue gain control and effect > + * > + * \var CameraSensorProperties::SensorDelays::vblankDelay > + * \brief Number of frames between application of vblank control and effect > + * > + * \var CameraSensorProperties::SensorDelays::hblankDelay > + * \brief Number of frames between application of hblank control and effect > */ > > /** > @@ -60,6 +90,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBars, 2 }, > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > }, > + .sensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 2, > + .vblankDelay = 2, > + .hblankDelay = 2 > + }, > } }, > { "ar0521", { > .unitCellSize = { 2200, 2200 }, > @@ -69,6 +105,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBars, 2 }, > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > }, > + .sensorDelays = { }, > } }, > { "hi846", { > .unitCellSize = { 1120, 1120 }, > @@ -87,6 +124,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 9: "Resolution Pattern" > */ > }, > + .sensorDelays = { }, > } }, > { "imx214", { > .unitCellSize = { 1120, 1120 }, > @@ -97,6 +135,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > { controls::draft::TestPatternModePn9, 4 }, > }, > + .sensorDelays = { }, > } }, > { "imx219", { > .unitCellSize = { 1120, 1120 }, > @@ -107,6 +146,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,38 +162,67 @@ 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 = { }, > } }, > { "imx462", { > .unitCellSize = { 2900, 2900 }, > .testPatternModes = {}, > + .sensorDelays = { }, > } }, > { "imx477", { > .unitCellSize = { 1550, 1550 }, > .testPatternModes = {}, > + .sensorDelays = { > + .exposureDelay = 2, > + .gainDelay = 2, > + .vblankDelay = 3, > + .hblankDelay = 3 > + }, > } }, > { "imx519", { > .unitCellSize = { 1220, 1220 }, > @@ -161,6 +235,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 }, > @@ -171,6 +251,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 }, > @@ -185,6 +271,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 5: "Color Square" > */ > }, > + .sensorDelays = { }, > } }, > { "ov2740", { > .unitCellSize = { 1400, 1400 }, > @@ -192,6 +279,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1}, > }, > + .sensorDelays = { }, > } }, > { "ov4689", { > .unitCellSize = { 2000, 2000 }, > @@ -205,6 +293,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * colorBarType2 and colorBarType3. > */ > }, > + .sensorDelays = { }, > } }, > { "ov5640", { > .unitCellSize = { 1400, 1400 }, > @@ -212,10 +301,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 }, > @@ -223,6 +319,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > { "ov5675", { > .unitCellSize = { 1120, 1120 }, > @@ -230,6 +327,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > @@ -242,6 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * Rolling Bar". > */ > }, > + .sensorDelays = { }, > } }, > { "ov64a40", { > .unitCellSize = { 1008, 1008 }, > @@ -255,6 +354,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 }, > @@ -268,6 +373,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 4: "Vertical Color Bar Type 4" > */ > }, > + .sensorDelays = { }, > } }, > { "ov8865", { > .unitCellSize = { 1400, 1400 }, > @@ -282,6 +388,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > * 5: "Color squares with rolling bar" > */ > }, > + .sensorDelays = { }, > } }, > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > @@ -289,6 +396,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > + .sensorDelays = { }, > } }, > }; >
Hi Laurent On 27/11/2024 06:59, Laurent Pinchart wrote: > On Tue, Nov 26, 2024 at 03:12:02PM +0000, Daniel Scally wrote: >> Add properties covering the sensor control application delays to both >> the static CameraSensorProperties definitions. The values used are >> taken from Raspberry Pi's CamHelper class definitions. Where no more >> specific values are known the delay struct is defined as empty and >> defaults supplied through the getter function. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> Changes in v5: >> >> - Comment rewording >> - Moved the defaultSensorDelays definition inside >> CameraSensorLegacy::sensorDelays() >> >> Changes in v4: >> >> - getSensorDelays() renamed to sensorDelays() >> - sensorDelays() returns a reference to the SensorDelays struct rather >> than each of the values. >> - Reworded the comments to make the default values seem less acceptable >> - Added delay values for AR0144 >> >> 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. >> >> 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 | 8 ++ >> src/libcamera/sensor/camera_sensor.cpp | 14 +++ >> src/libcamera/sensor/camera_sensor_legacy.cpp | 25 ++++ >> .../sensor/camera_sensor_properties.cpp | 108 ++++++++++++++++++ >> 5 files changed, 157 insertions(+) >> >> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h >> index 8aafd82e..bbdb83a1 100644 >> --- a/include/libcamera/internal/camera_sensor.h >> +++ b/include/libcamera/internal/camera_sensor.h >> @@ -20,6 +20,7 @@ >> #include <libcamera/orientation.h> >> #include <libcamera/transform.h> >> >> +#include "libcamera/internal/camera_sensor_properties.h" > Alphabetical order. I'm surprised checkstyle.py didn't warn you about > this. Actually so am I - checkstyle is noisy for this patch because the CameraSensorProperties tables are not its friend. I wondered if I had missed it from that noise but I don't see it there. > >> #include "libcamera/internal/bayer_format.h" >> #include "libcamera/internal/v4l2_subdevice.h" >> >> @@ -73,6 +74,7 @@ public: >> virtual const std::vector<controls::draft::TestPatternModeEnum> & >> testPatternModes() const = 0; >> virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0; >> + virtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0; >> }; >> >> class CameraSensorFactoryBase >> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h >> index 480ac121..75cb7839 100644 >> --- a/include/libcamera/internal/camera_sensor_properties.h >> +++ b/include/libcamera/internal/camera_sensor_properties.h >> @@ -8,6 +8,7 @@ >> #pragma once >> >> #include <map> >> +#include <stdint.h> >> #include <string> >> >> #include <libcamera/control_ids.h> >> @@ -20,6 +21,13 @@ struct CameraSensorProperties { >> >> Size unitCellSize; >> std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes; >> + >> + struct SensorDelays { >> + uint8_t exposureDelay; >> + uint8_t gainDelay; >> + uint8_t vblankDelay; >> + uint8_t hblankDelay; >> + } sensorDelays; > We usually separate type definitions from usage. > > struct CameraSensorProperties { > struct SensorDelays { > uint8_t exposureDelay; > uint8_t gainDelay; > uint8_t vblankDelay; > uint8_t hblankDelay; > }; > > static const CameraSensorProperties *get(const std::string &sensor); > > Size unitCellSize; > std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes; > SensorDelays sensorDelays; > }; Ack >> }; >> >> } /* namespace libcamera */ >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp >> index 54cf98b2..9d2c2ea7 100644 >> --- a/src/libcamera/sensor/camera_sensor.cpp >> +++ b/src/libcamera/sensor/camera_sensor.cpp >> @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default; >> * pattern mode for every frame thus incurs no performance penalty. >> */ >> >> +/** >> + * \fn CameraSensor::sensorDelays() >> + * \brief Fetch the sensor delay values >> + * >> + * This function retreives the sensor control delays for pipeline handlers to > s/retreives/retrieves/ Somewhere my primary school teacher is disappointed. Thanks Dan >> + * use to inform the DelayedControls. If control delays are not specified in the > We don't usually document expected users, unless there's a very tight > coupling between a class and its users that is relevant to understanding > the API. > >> + * static sensor propertie database, this function returns a reference to a set > s/propertie/properties/ > >> + * of default sensor delays provided as best-effort placeholders for the actual >> + * sensor specific delays. > And there's a bit of implementation detail here too. You can simplify > this to > > * This function retrieves the delays that the sensor applies to controls. If > * the static properties database doesn't specifiy control delay values for the > * sensor, default delays that may be suitable are returned and a warning is > * logged. > >> + * >> + * \return A reference to a struct CameraSensorProperties::SensorDelays holding >> + * the delay values > Doxygen shows the type of the return value, so you can just write > > * \return The sensor delay 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..17d6fa68 100644 >> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp >> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp >> @@ -95,6 +95,7 @@ public: >> const std::vector<controls::draft::TestPatternModeEnum> & >> testPatternModes() const override { return testPatternModes_; } >> int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override; >> + const CameraSensorProperties::SensorDelays &sensorDelays() override; >> >> protected: >> std::string logPrefix() const override; >> @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties() >> initTestPatternModes(); >> } >> >> +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays() >> +{ >> + static constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = { >> + .exposureDelay = 2, >> + .gainDelay = 1, >> + .vblankDelay = 2, >> + .hblankDelay = 2, >> + }; >> + >> + 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."; >> + >> + return defaultSensorDelays; >> + } >> + >> + return staticProps_->sensorDelays; >> +} >> + >> 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 e2305166..3dc8cf05 100644 >> --- a/src/libcamera/sensor/camera_sensor_properties.cpp >> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp >> @@ -41,6 +41,36 @@ 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 Sensor control application delays. > checkstyle.py flagged this. > >> + * >> + * This struct may be defined as empty if the actual sensor delays are not > s/struct/structure/ > >> + * available or have not been measured. Best effort default values are returned >> + * in this case. > In this context, it's not clear where the defaults are returned from. > I'd drop the last sentence. > >> + */ >> + >> +/** >> + * \struct CameraSensorProperties::SensorDelays >> + * \brief Sensor control application delay values >> + * >> + * This struct holds delay values, expressed in number of frames, between the > s/struct/structure/ > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> + * time a control value is applied to the sensor and the time that value is >> + * reflected in the output. For example "2 frames delay" means that parameters >> + * set during frame N will take effect for frame N+2 (and by extension a delay >> + * of 0 would mean the parameter is applied immediately to the current frame). >> + * >> + * \var CameraSensorProperties::SensorDelays::exposureDelay >> + * \brief Number of frames between application of exposure control and effect >> + * >> + * \var CameraSensorProperties::SensorDelays::gainDelay >> + * \brief Number of frames between application of analogue gain control and effect >> + * >> + * \var CameraSensorProperties::SensorDelays::vblankDelay >> + * \brief Number of frames between application of vblank control and effect >> + * >> + * \var CameraSensorProperties::SensorDelays::hblankDelay >> + * \brief Number of frames between application of hblank control and effect >> */ >> >> /** >> @@ -60,6 +90,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeColorBars, 2 }, >> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, >> }, >> + .sensorDelays = { >> + .exposureDelay = 2, >> + .gainDelay = 2, >> + .vblankDelay = 2, >> + .hblankDelay = 2 >> + }, >> } }, >> { "ar0521", { >> .unitCellSize = { 2200, 2200 }, >> @@ -69,6 +105,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeColorBars, 2 }, >> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, >> }, >> + .sensorDelays = { }, >> } }, >> { "hi846", { >> .unitCellSize = { 1120, 1120 }, >> @@ -87,6 +124,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * 9: "Resolution Pattern" >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "imx214", { >> .unitCellSize = { 1120, 1120 }, >> @@ -97,6 +135,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, >> { controls::draft::TestPatternModePn9, 4 }, >> }, >> + .sensorDelays = { }, >> } }, >> { "imx219", { >> .unitCellSize = { 1120, 1120 }, >> @@ -107,6 +146,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,38 +162,67 @@ 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 = { }, >> } }, >> { "imx462", { >> .unitCellSize = { 2900, 2900 }, >> .testPatternModes = {}, >> + .sensorDelays = { }, >> } }, >> { "imx477", { >> .unitCellSize = { 1550, 1550 }, >> .testPatternModes = {}, >> + .sensorDelays = { >> + .exposureDelay = 2, >> + .gainDelay = 2, >> + .vblankDelay = 3, >> + .hblankDelay = 3 >> + }, >> } }, >> { "imx519", { >> .unitCellSize = { 1220, 1220 }, >> @@ -161,6 +235,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 }, >> @@ -171,6 +251,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 }, >> @@ -185,6 +271,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * 5: "Color Square" >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "ov2740", { >> .unitCellSize = { 1400, 1400 }, >> @@ -192,6 +279,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1}, >> }, >> + .sensorDelays = { }, >> } }, >> { "ov4689", { >> .unitCellSize = { 2000, 2000 }, >> @@ -205,6 +293,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * colorBarType2 and colorBarType3. >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "ov5640", { >> .unitCellSize = { 1400, 1400 }, >> @@ -212,10 +301,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 }, >> @@ -223,6 +319,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1 }, >> }, >> + .sensorDelays = { }, >> } }, >> { "ov5675", { >> .unitCellSize = { 1120, 1120 }, >> @@ -230,6 +327,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1 }, >> }, >> + .sensorDelays = { }, >> } }, >> { "ov5693", { >> .unitCellSize = { 1400, 1400 }, >> @@ -242,6 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * Rolling Bar". >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "ov64a40", { >> .unitCellSize = { 1008, 1008 }, >> @@ -255,6 +354,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 }, >> @@ -268,6 +373,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * 4: "Vertical Color Bar Type 4" >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "ov8865", { >> .unitCellSize = { 1400, 1400 }, >> @@ -282,6 +388,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> * 5: "Color squares with rolling bar" >> */ >> }, >> + .sensorDelays = { }, >> } }, >> { "ov13858", { >> .unitCellSize = { 1120, 1120 }, >> @@ -289,6 +396,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1 }, >> }, >> + .sensorDelays = { }, >> } }, >> }; >>
diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index 8aafd82e..bbdb83a1 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -20,6 +20,7 @@ #include <libcamera/orientation.h> #include <libcamera/transform.h> +#include "libcamera/internal/camera_sensor_properties.h" #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/v4l2_subdevice.h" @@ -73,6 +74,7 @@ public: virtual const std::vector<controls::draft::TestPatternModeEnum> & testPatternModes() const = 0; virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0; + virtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0; }; class CameraSensorFactoryBase diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h index 480ac121..75cb7839 100644 --- a/include/libcamera/internal/camera_sensor_properties.h +++ b/include/libcamera/internal/camera_sensor_properties.h @@ -8,6 +8,7 @@ #pragma once #include <map> +#include <stdint.h> #include <string> #include <libcamera/control_ids.h> @@ -20,6 +21,13 @@ struct CameraSensorProperties { Size unitCellSize; std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes; + + struct SensorDelays { + 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..9d2c2ea7 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default; * pattern mode for every frame thus incurs no performance penalty. */ +/** + * \fn CameraSensor::sensorDelays() + * \brief Fetch the sensor delay values + * + * This function retreives the sensor control delays for pipeline handlers to + * use to inform the DelayedControls. If control delays are not specified in the + * static sensor propertie database, this function returns a reference to a set + * of default sensor delays provided as best-effort placeholders for the actual + * sensor specific delays. + * + * \return A reference to a struct CameraSensorProperties::SensorDelays holding + * the delay 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..17d6fa68 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -95,6 +95,7 @@ public: const std::vector<controls::draft::TestPatternModeEnum> & testPatternModes() const override { return testPatternModes_; } int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override; + const CameraSensorProperties::SensorDelays &sensorDelays() override; protected: std::string logPrefix() const override; @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties() initTestPatternModes(); } +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays() +{ + static constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = { + .exposureDelay = 2, + .gainDelay = 1, + .vblankDelay = 2, + .hblankDelay = 2, + }; + + 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."; + + return defaultSensorDelays; + } + + return staticProps_->sensorDelays; +} + 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 e2305166..3dc8cf05 100644 --- a/src/libcamera/sensor/camera_sensor_properties.cpp +++ b/src/libcamera/sensor/camera_sensor_properties.cpp @@ -41,6 +41,36 @@ 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 Sensor control application delays. + * + * This struct may be defined as empty if the actual sensor delays are not + * available or have not been measured. Best effort default values are returned + * in this case. + */ + +/** + * \struct CameraSensorProperties::SensorDelays + * \brief Sensor control application delay values + * + * This struct holds delay values, expressed in number of frames, between the + * time a control value is applied to the sensor and the time that value is + * reflected in the output. For example "2 frames delay" means that parameters + * set during frame N will take effect for frame N+2 (and by extension a delay + * of 0 would mean the parameter is applied immediately to the current frame). + * + * \var CameraSensorProperties::SensorDelays::exposureDelay + * \brief Number of frames between application of exposure control and effect + * + * \var CameraSensorProperties::SensorDelays::gainDelay + * \brief Number of frames between application of analogue gain control and effect + * + * \var CameraSensorProperties::SensorDelays::vblankDelay + * \brief Number of frames between application of vblank control and effect + * + * \var CameraSensorProperties::SensorDelays::hblankDelay + * \brief Number of frames between application of hblank control and effect */ /** @@ -60,6 +90,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeColorBars, 2 }, { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, }, + .sensorDelays = { + .exposureDelay = 2, + .gainDelay = 2, + .vblankDelay = 2, + .hblankDelay = 2 + }, } }, { "ar0521", { .unitCellSize = { 2200, 2200 }, @@ -69,6 +105,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeColorBars, 2 }, { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, }, + .sensorDelays = { }, } }, { "hi846", { .unitCellSize = { 1120, 1120 }, @@ -87,6 +124,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * 9: "Resolution Pattern" */ }, + .sensorDelays = { }, } }, { "imx214", { .unitCellSize = { 1120, 1120 }, @@ -97,6 +135,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, { controls::draft::TestPatternModePn9, 4 }, }, + .sensorDelays = { }, } }, { "imx219", { .unitCellSize = { 1120, 1120 }, @@ -107,6 +146,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,38 +162,67 @@ 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 = { }, } }, { "imx462", { .unitCellSize = { 2900, 2900 }, .testPatternModes = {}, + .sensorDelays = { }, } }, { "imx477", { .unitCellSize = { 1550, 1550 }, .testPatternModes = {}, + .sensorDelays = { + .exposureDelay = 2, + .gainDelay = 2, + .vblankDelay = 3, + .hblankDelay = 3 + }, } }, { "imx519", { .unitCellSize = { 1220, 1220 }, @@ -161,6 +235,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 }, @@ -171,6 +251,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 }, @@ -185,6 +271,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * 5: "Color Square" */ }, + .sensorDelays = { }, } }, { "ov2740", { .unitCellSize = { 1400, 1400 }, @@ -192,6 +279,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1}, }, + .sensorDelays = { }, } }, { "ov4689", { .unitCellSize = { 2000, 2000 }, @@ -205,6 +293,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * colorBarType2 and colorBarType3. */ }, + .sensorDelays = { }, } }, { "ov5640", { .unitCellSize = { 1400, 1400 }, @@ -212,10 +301,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 }, @@ -223,6 +319,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1 }, }, + .sensorDelays = { }, } }, { "ov5675", { .unitCellSize = { 1120, 1120 }, @@ -230,6 +327,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1 }, }, + .sensorDelays = { }, } }, { "ov5693", { .unitCellSize = { 1400, 1400 }, @@ -242,6 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * Rolling Bar". */ }, + .sensorDelays = { }, } }, { "ov64a40", { .unitCellSize = { 1008, 1008 }, @@ -255,6 +354,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 }, @@ -268,6 +373,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * 4: "Vertical Color Bar Type 4" */ }, + .sensorDelays = { }, } }, { "ov8865", { .unitCellSize = { 1400, 1400 }, @@ -282,6 +388,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen * 5: "Color squares with rolling bar" */ }, + .sensorDelays = { }, } }, { "ov13858", { .unitCellSize = { 1120, 1120 }, @@ -289,6 +396,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1 }, }, + .sensorDelays = { }, } }, };
Add properties covering the sensor control application delays to both the static CameraSensorProperties definitions. The values used are taken from Raspberry Pi's CamHelper class definitions. Where no more specific values are known the delay struct is defined as empty and defaults supplied through the getter function. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- Changes in v5: - Comment rewording - Moved the defaultSensorDelays definition inside CameraSensorLegacy::sensorDelays() Changes in v4: - getSensorDelays() renamed to sensorDelays() - sensorDelays() returns a reference to the SensorDelays struct rather than each of the values. - Reworded the comments to make the default values seem less acceptable - Added delay values for AR0144 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. 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 | 8 ++ src/libcamera/sensor/camera_sensor.cpp | 14 +++ src/libcamera/sensor/camera_sensor_legacy.cpp | 25 ++++ .../sensor/camera_sensor_properties.cpp | 108 ++++++++++++++++++ 5 files changed, 157 insertions(+)