[PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor control delays
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Wed Nov 13 19:57:11 CET 2024
Hi Dan
On Wed, Nov 13, 2024 at 01:38:51PM +0000, Dan Scally wrote:
> Morning Jacopo
>
> On 08/11/2024 15:40, Jacopo Mondi wrote:
> > Hi Dan
> >
> > will need to be rebased on the introduction of CameraSensorFactory
> > I can share a rebased branch if you want.
> It's ok I've got it
> >
> > On Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:
> > > Add properties covering the sensor control application delays to both
> > > the list of control values and the static CameraSensorProperties
> > I don't think this description applies anymore
> Oops, thanks
> >
> > > definitions. The values used are the defaults that're in use across
> > > the library, with deviations from that taken from Raspberry Pi's
> > More on this below
> >
> > > CamHelper class definitions.
> > >
> > > Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> > > ---
> > > 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 | 33 ++++
> > > .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++
> > > 4 files changed, 211 insertions(+)
> > >
> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > > index a42c15fa..cdfbf051 100644
> > > --- a/include/libcamera/internal/camera_sensor.h
> > > +++ b/include/libcamera/internal/camera_sensor.h
> > > @@ -79,6 +79,8 @@ public:
> > > return testPatternModes_;
> > > }
> > > int setTestPatternMode(controls::draft::TestPatternModeEnum mode);
> > > + void getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,
> > > + uint8_t &vblankDelay, uint8_t &hblankDelay);
> > >
> > > protected:
> > > std::string logPrefix() const override;
> > > 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 1b224f19..5d26f3df 100644
> > > --- a/src/libcamera/sensor/camera_sensor.cpp
> > > +++ b/src/libcamera/sensor/camera_sensor.cpp
> > > @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()
> > > initTestPatternModes();
> > > }
> > >
> > > +/**
> > > + * \brief Fetch the sensor delay values
> > > + * \param[out] exposureDelay A variable to set the exposure delay to
> > > + * \param[out] gainDelay A variable to set the gain delay to
> > Analogue gain I suppose
> >
> > > + * \param[out] vblankDelay A variable to set the vblank delay to
> > > + * \param[out] hblankDelay A variable to set the hblank delay to
> > s/A variable to set//
> > s/delay to/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.
> > > + */
> > > +void CameraSensor::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_) {
> > > + 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;
> > > +}
> > > +
> > > void CameraSensor::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..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.
> > Feels weird to correct a native speaker
> Not at all! Yours is much clearer
> > , but I would write it
> > differently.
> >
> > * \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 blankings controls.
> >
> > Take in what you like the most
> >
> > > */
> > >
> > > /**
> > > @@ -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
> > > + },
> > Ok, now we have defaults in the implementation of the camera sensor
> > class, and most of the values stored here are actually defaults that
> > work in best-effort mode but have not been actually validated against
> > the sensor.
> Yes
> >
> > What I would do, for sensor where the delayes have not actually been
> > measured or taken from the datasheet, is to leave sensorDelays empty in
> > here and only populate the ones we're sure about. If we use defaults
> > possibly print it out as WARN/INFO that precise per-frame control is
> > not available as the sensor's delays have been default populated.
> >
> > Recording non-validated information here might give the false
> > impression that we trust the values here recorded and if someone
> > notices issues it might be confusing to see values recorded here that
> > do not match the actual experimental results.
>
>
> Leaving them empty generates "missing initializer for member" warnings, I
> can initialise the unknown ones as empty (I.E. ".sensorDelays = { }") and
> check for all zeros in CameraSensor::getSensorDelays() and return the
> defaults with a warning there, if that's ok?
>
That's what I was thinking, so indeed ok with me :)
Thanks
j
> >
> >
> > > } },
> > > { "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