[{"id":32079,"web_url":"https://patchwork.libcamera.org/comment/32079/","msgid":"","date":"2024-11-08T15:40:34","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\n will need to be rebased on the introduction of CameraSensorFactory\nI can share a rebased branch if you want.\n\nOn Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n> Add properties covering the sensor control application delays to both\n> the list of control values and the static CameraSensorProperties\n\nI don't think this description applies anymore\n\n> definitions. The values used are the defaults that're in use across\n> the library, with deviations from that taken from Raspberry Pi's\n\nMore on this below\n\n> CamHelper class definitions.\n>\n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n>\n> \t- Rather than adding the delays to the properties ControlList, added a\n> \t new function in CameraSensor that allows PipelineHandlers to retreive\n> \t the delay values.\n>\n> include/libcamera/internal/camera_sensor.h | 2 +\n> .../internal/camera_sensor_properties.h | 9 +\n> src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n> .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n> 4 files changed, 211 insertions(+)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index a42c15fa..cdfbf051 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -79,6 +79,8 @@ public:\n> \t\treturn testPatternModes_;\n> \t}\n> \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n> +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n>\n> protected:\n> \tstd::string logPrefix() const override;\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 480ac121..56d5c15d 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -10,6 +10,8 @@\n> #include \n> #include \n>\n> +#include \n> +\n> #include \n> #include \n>\n> @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n>\n> \tSize unitCellSize;\n> \tstd::map testPatternModes;\n> +\n> +\tstruct {\n> +\t\tuint8_t exposureDelay;\n> +\t\tuint8_t gainDelay;\n> +\t\tuint8_t vblankDelay;\n> +\t\tuint8_t hblankDelay;\n> +\t} sensorDelays;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 1b224f19..5d26f3df 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n> \tinitTestPatternModes();\n> }\n>\n> +/**\n> + * \\brief Fetch the sensor delay values\n> + * \\param[out] exposureDelay A variable to set the exposure delay to\n> + * \\param[out] gainDelay A variable to set the gain delay to\n\nAnalogue gain I suppose\n\n> + * \\param[out] vblankDelay A variable to set the vblank delay to\n> + * \\param[out] hblankDelay A variable to set the hblank delay to\n\ns/A variable to set//\ns/delay to/delay\n\n?\n> + *\n> + * This function fills in sensor control delays for pipeline handlers to use to\n> + * inform the DelayedControls. If no static properties are available it fills in\n> + * some widely applicable default values.\n> + */\n> +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n> +{\n> +\t/*\n> +\t * These defaults are applicable to many sensors, however more specific\n> +\t * values can be added to the CameraSensorProperties for a sensor if\n> +\t * required.\n> +\t */\n> +\tif (!staticProps_) {\n> +\t\texposureDelay = 2;\n> +\t\tgainDelay = 1;\n> +\t\tvblankDelay = 2;\n> +\t\thblankDelay = 2;\n> +\t\treturn;\n> +\t}\n> +\n> +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n> +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n> +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n> +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n> +}\n> +\n> void CameraSensor::initTestPatternModes()\n> {\n> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> index 6d4136d0..60d59f79 100644\n> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> * \\brief Map that associates the TestPattern control value with the indexes of\n> * the corresponding sensor test pattern modes as returned by\n> * V4L2_CID_TEST_PATTERN.\n> + *\n> + * \\var CameraSensorProperties::sensorDelays\n> + * \\brief struct holding the number of frames delay between a control value\n> + * set and taking effect for each of exposure, gain, vertical blanking and\n> + * horizontal blanking.\n\nFeels weird to correct a native speaker, but I would write it\ndifferently.\n\n * \\var CameraSensorProperties::sensorDelays\n * \\brief Holds the delays, expressed in number of frames, between the\n * time a control is applied to the sensor and the time it actually takes\n * effect. Delays are recorded for the exposure time, analogue gain,\n * vertical and horizontal blankings controls.\n\nTake in what you like the most\n\n> */\n>\n> /**\n> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n\nOk, now we have defaults in the implementation of the camera sensor\nclass, and most of the values stored here are actually defaults that\nwork in best-effort mode but have not been actually validated against\nthe sensor.\n\nWhat I would do, for sensor where the delayes have not actually been\nmeasured or taken from the datasheet, is to leave sensorDelays empty in\nhere and only populate the ones we're sure about. If we use defaults\npossibly print it out as WARN/INFO that precise per-frame control is\nnot available as the sensor's delays have been default populated.\n\nRecording non-validated information here might give the false\nimpression that we trust the values here recorded and if someone\nnotices issues it might be confusing to see values recorded here that\ndo not match the actual experimental results.\n\n\n> \t\t} },\n> \t\t{ \"ar0521\", {\n> \t\t\t.unitCellSize = { 2200, 2200 },\n> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"hi846\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 9: \"Resolution Pattern\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx214\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx219\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx258\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx283\", {\n> \t\t\t.unitCellSize = { 2400, 2400 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx290\", {\n> \t\t\t.unitCellSize = { 2900, 2900 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx296\", {\n> \t\t\t.unitCellSize = { 3450, 3450 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx327\", {\n> \t\t\t.unitCellSize = { 2900, 2900 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx335\", {\n> \t\t\t.unitCellSize = { 2000, 2000 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx415\", {\n> \t\t\t.unitCellSize = { 1450, 1450 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx477\", {\n> \t\t\t.unitCellSize = { 1550, 1550 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx519\", {\n> \t\t\t.unitCellSize = { 1220, 1220 },\n> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx708\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov2685\", {\n> \t\t\t.unitCellSize = { 1750, 1750 },\n> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 5: \"Color Square\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov2740\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov4689\", {\n> \t\t\t.unitCellSize = { 2000, 2000 },\n> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * colorBarType2 and colorBarType3.\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5640\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5647\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5670\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5675\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5693\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * Rolling Bar\".\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov64a40\", {\n> \t\t\t.unitCellSize = { 1008, 1008 },\n> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov8858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov8865\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 5: \"Color squares with rolling bar\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov13858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t};\n>\n> --\n> 2.30.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8865BBE173\n\tfor ;\n\tFri, 8 Nov 2024 15:40:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC316657AC;\n\tFri, 8 Nov 2024 16:40:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E055A6546C\n\tfor ;\n\tFri, 8 Nov 2024 16:40:37 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 065292E0;\n\tFri, 8 Nov 2024 16:40:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nkQHT7II\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731080428;\n\tbh=DM8M2unWaJb4P6l14rUU9rY+FT4nkZdPnU8G+w+50/A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nkQHT7IIeClQxhUD72aQ6wnmm4LVf7fpLw940CCk/S9WpB/K/nLCnwWczl17M8s59\n\tOrUYc5WF0dfcgL0PcatFSoXPyv35y/dOl9ZfyY0k4g3mAw0bO+PrGc5hM+39GWlAZ1\n\tIjyd+/d5qeyXwOhNHT11PkukEllkQlQ5HBmhxBo4=","Date":"Fri, 8 Nov 2024 16:40:34 +0100","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","Message-ID":"","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241107102508.48322-6-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32116,"web_url":"https://patchwork.libcamera.org/comment/32116/","msgid":"<20241112080921.GA5359@pendragon.ideasonboard.com>","date":"2024-11-12T08:09:21","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nCould you please have a look at \"[PATCH RFC 0/3] Pass sensor delays from\nrkisp1 IPA to the pipeline handler\" and try to reconcile the two ?\n\nOn Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n> Add properties covering the sensor control application delays to both\n> the list of control values and the static CameraSensorProperties\n> definitions. The values used are the defaults that're in use across\n> the library, with deviations from that taken from Raspberry Pi's\n> CamHelper class definitions.\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n> \n> \t- Rather than adding the delays to the properties ControlList, added a\n> \t new function in CameraSensor that allows PipelineHandlers to retreive\n> \t the delay values.\n> \n> include/libcamera/internal/camera_sensor.h | 2 +\n> .../internal/camera_sensor_properties.h | 9 +\n> src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n> .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n> 4 files changed, 211 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index a42c15fa..cdfbf051 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -79,6 +79,8 @@ public:\n> \t\treturn testPatternModes_;\n> \t}\n> \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n> +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n> \n> protected:\n> \tstd::string logPrefix() const override;\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 480ac121..56d5c15d 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -10,6 +10,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> \n> @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n> \n> \tSize unitCellSize;\n> \tstd::map testPatternModes;\n> +\n> +\tstruct {\n> +\t\tuint8_t exposureDelay;\n> +\t\tuint8_t gainDelay;\n> +\t\tuint8_t vblankDelay;\n> +\t\tuint8_t hblankDelay;\n> +\t} sensorDelays;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 1b224f19..5d26f3df 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n> \tinitTestPatternModes();\n> }\n> \n> +/**\n> + * \\brief Fetch the sensor delay values\n> + * \\param[out] exposureDelay A variable to set the exposure delay to\n> + * \\param[out] gainDelay A variable to set the gain delay to\n> + * \\param[out] vblankDelay A variable to set the vblank delay to\n> + * \\param[out] hblankDelay A variable to set the hblank delay to\n> + *\n> + * This function fills in sensor control delays for pipeline handlers to use to\n> + * inform the DelayedControls. If no static properties are available it fills in\n> + * some widely applicable default values.\n> + */\n> +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n> +{\n> +\t/*\n> +\t * These defaults are applicable to many sensors, however more specific\n> +\t * values can be added to the CameraSensorProperties for a sensor if\n> +\t * required.\n> +\t */\n> +\tif (!staticProps_) {\n> +\t\texposureDelay = 2;\n> +\t\tgainDelay = 1;\n> +\t\tvblankDelay = 2;\n> +\t\thblankDelay = 2;\n> +\t\treturn;\n> +\t}\n> +\n> +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n> +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n> +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n> +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n> +}\n> +\n> void CameraSensor::initTestPatternModes()\n> {\n> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> index 6d4136d0..60d59f79 100644\n> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> * \\brief Map that associates the TestPattern control value with the indexes of\n> * the corresponding sensor test pattern modes as returned by\n> * V4L2_CID_TEST_PATTERN.\n> + *\n> + * \\var CameraSensorProperties::sensorDelays\n> + * \\brief struct holding the number of frames delay between a control value\n> + * set and taking effect for each of exposure, gain, vertical blanking and\n> + * horizontal blanking.\n> */\n> \n> /**\n> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ar0521\", {\n> \t\t\t.unitCellSize = { 2200, 2200 },\n> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"hi846\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 9: \"Resolution Pattern\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx214\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx219\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx258\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx283\", {\n> \t\t\t.unitCellSize = { 2400, 2400 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx290\", {\n> \t\t\t.unitCellSize = { 2900, 2900 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx296\", {\n> \t\t\t.unitCellSize = { 3450, 3450 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx327\", {\n> \t\t\t.unitCellSize = { 2900, 2900 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx335\", {\n> \t\t\t.unitCellSize = { 2000, 2000 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx415\", {\n> \t\t\t.unitCellSize = { 1450, 1450 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx477\", {\n> \t\t\t.unitCellSize = { 1550, 1550 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx519\", {\n> \t\t\t.unitCellSize = { 1220, 1220 },\n> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"imx708\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 3,\n> +\t\t\t\t.hblankDelay = 3\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov2685\", {\n> \t\t\t.unitCellSize = { 1750, 1750 },\n> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 5: \"Color Square\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov2740\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov4689\", {\n> \t\t\t.unitCellSize = { 2000, 2000 },\n> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * colorBarType2 and colorBarType3.\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5640\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5647\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5670\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5675\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5693\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * Rolling Bar\".\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov64a40\", {\n> \t\t\t.unitCellSize = { 1008, 1008 },\n> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 2,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov8858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov8865\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t * 5: \"Color squares with rolling bar\"\n> \t\t\t\t */\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov13858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> +\t\t\t.sensorDelays = {\n> +\t\t\t\t.exposureDelay = 2,\n> +\t\t\t\t.gainDelay = 1,\n> +\t\t\t\t.vblankDelay = 2,\n> +\t\t\t\t.hblankDelay = 2\n> +\t\t\t},\n> \t\t} },\n> \t};\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A1D10BE173\n\tfor ;\n\tTue, 12 Nov 2024 08:09:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 526D0657E9;\n\tTue, 12 Nov 2024 09:09:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ECB14657DF\n\tfor ;\n\tTue, 12 Nov 2024 09:09:28 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A6386AF;\n\tTue, 12 Nov 2024 09:09:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"t5g8uZgY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731398956;\n\tbh=Gd09L/ljsE+c2tspegpLAIoTElZuCTobjIo1urAWexY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t5g8uZgYdEZpnsiUXCaALreRDV1nfoCffFbYzLOpMDoqzg/Lcmr9wS2eGI3uMRGTH\n\te5PKW4FebWyLPgiDwOt85fdzN+ekCExL3YM8tUE6ZbKshY5hfB46QoGezvnhXE3+7h\n\ttj5/z10DxyaQr5B4diYcF/k+eyM5atHXfQiKEnqA=","Date":"Tue, 12 Nov 2024 10:09:21 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","Message-ID":"<20241112080921.GA5359@pendragon.ideasonboard.com>","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241107102508.48322-6-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32122,"web_url":"https://patchwork.libcamera.org/comment/32122/","msgid":"","date":"2024-11-12T12:58:18","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Laurent\n\nOn 12/11/2024 08:09, Laurent Pinchart wrote:\n> Hi Dan,\n>\n> Thank you for the patch.\n>\n> Could you please have a look at \"[PATCH RFC 0/3] Pass sensor delays from\n> rkisp1 IPA to the pipeline handler\" and try to reconcile the two ?\n\n\nMikhail and I spoke on that patchset - I also originally added the delays to CameraSensorHelper but \ndecided they were better in CameraSensorProperties and said that I thought that approach was better. \nI think Mikhail was leaning in that direction too, in which case this series can supersede that one.\n\n\nThanks\n\nDan\n\n>\n> On Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n>> Add properties covering the sensor control application delays to both\n>> the list of control values and the static CameraSensorProperties\n>> definitions. The values used are the defaults that're in use across\n>> the library, with deviations from that taken from Raspberry Pi's\n>> CamHelper class definitions.\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> Changes in v2:\n>>\n>> \t- Rather than adding the delays to the properties ControlList, added a\n>> \t new function in CameraSensor that allows PipelineHandlers to retreive\n>> \t the delay values.\n>>\n>> include/libcamera/internal/camera_sensor.h | 2 +\n>> .../internal/camera_sensor_properties.h | 9 +\n>> src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n>> .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n>> 4 files changed, 211 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n>> index a42c15fa..cdfbf051 100644\n>> --- a/include/libcamera/internal/camera_sensor.h\n>> +++ b/include/libcamera/internal/camera_sensor.h\n>> @@ -79,6 +79,8 @@ public:\n>> \t\treturn testPatternModes_;\n>> \t}\n>> \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n>> +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>> +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n>> \n>> protected:\n>> \tstd::string logPrefix() const override;\n>> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n>> index 480ac121..56d5c15d 100644\n>> --- a/include/libcamera/internal/camera_sensor_properties.h\n>> +++ b/include/libcamera/internal/camera_sensor_properties.h\n>> @@ -10,6 +10,8 @@\n>> #include \n>> #include \n>> \n>> +#include \n>> +\n>> #include \n>> #include \n>> \n>> @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n>> \n>> \tSize unitCellSize;\n>> \tstd::map testPatternModes;\n>> +\n>> +\tstruct {\n>> +\t\tuint8_t exposureDelay;\n>> +\t\tuint8_t gainDelay;\n>> +\t\tuint8_t vblankDelay;\n>> +\t\tuint8_t hblankDelay;\n>> +\t} sensorDelays;\n>> };\n>> \n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>> index 1b224f19..5d26f3df 100644\n>> --- a/src/libcamera/sensor/camera_sensor.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>> @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n>> \tinitTestPatternModes();\n>> }\n>> \n>> +/**\n>> + * \\brief Fetch the sensor delay values\n>> + * \\param[out] exposureDelay A variable to set the exposure delay to\n>> + * \\param[out] gainDelay A variable to set the gain delay to\n>> + * \\param[out] vblankDelay A variable to set the vblank delay to\n>> + * \\param[out] hblankDelay A variable to set the hblank delay to\n>> + *\n>> + * This function fills in sensor control delays for pipeline handlers to use to\n>> + * inform the DelayedControls. If no static properties are available it fills in\n>> + * some widely applicable default values.\n>> + */\n>> +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>> +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n>> +{\n>> +\t/*\n>> +\t * These defaults are applicable to many sensors, however more specific\n>> +\t * values can be added to the CameraSensorProperties for a sensor if\n>> +\t * required.\n>> +\t */\n>> +\tif (!staticProps_) {\n>> +\t\texposureDelay = 2;\n>> +\t\tgainDelay = 1;\n>> +\t\tvblankDelay = 2;\n>> +\t\thblankDelay = 2;\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n>> +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n>> +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n>> +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n>> +}\n>> +\n>> void CameraSensor::initTestPatternModes()\n>> {\n>> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n>> index 6d4136d0..60d59f79 100644\n>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n>> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n>> * \\brief Map that associates the TestPattern control value with the indexes of\n>> * the corresponding sensor test pattern modes as returned by\n>> * V4L2_CID_TEST_PATTERN.\n>> + *\n>> + * \\var CameraSensorProperties::sensorDelays\n>> + * \\brief struct holding the number of frames delay between a control value\n>> + * set and taking effect for each of exposure, gain, vertical blanking and\n>> + * horizontal blanking.\n>> */\n>> \n>> /**\n>> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ar0521\", {\n>> \t\t\t.unitCellSize = { 2200, 2200 },\n>> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"hi846\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 9: \"Resolution Pattern\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx214\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx219\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx258\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx283\", {\n>> \t\t\t.unitCellSize = { 2400, 2400 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx290\", {\n>> \t\t\t.unitCellSize = { 2900, 2900 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx296\", {\n>> \t\t\t.unitCellSize = { 3450, 3450 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx327\", {\n>> \t\t\t.unitCellSize = { 2900, 2900 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx335\", {\n>> \t\t\t.unitCellSize = { 2000, 2000 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx415\", {\n>> \t\t\t.unitCellSize = { 1450, 1450 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx477\", {\n>> \t\t\t.unitCellSize = { 1550, 1550 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx519\", {\n>> \t\t\t.unitCellSize = { 1220, 1220 },\n>> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx708\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov2685\", {\n>> \t\t\t.unitCellSize = { 1750, 1750 },\n>> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 5: \"Color Square\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov2740\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov4689\", {\n>> \t\t\t.unitCellSize = { 2000, 2000 },\n>> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * colorBarType2 and colorBarType3.\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5640\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5647\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5670\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5675\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5693\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * Rolling Bar\".\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov64a40\", {\n>> \t\t\t.unitCellSize = { 1008, 1008 },\n>> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov8858\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov8865\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 5: \"Color squares with rolling bar\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov13858\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t};\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1DE3BBE173\n\tfor ;\n\tTue, 12 Nov 2024 12:58:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F4D6657F1;\n\tTue, 12 Nov 2024 13:58:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4DE7657B0\n\tfor ;\n\tTue, 12 Nov 2024 13:58:21 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DF918512;\n\tTue, 12 Nov 2024 13:58:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lQmmOzBC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731416289;\n\tbh=OdOt18oSntvxIM7tUsS+lp0Eb1fUhHWExKO9AvZObQ4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=lQmmOzBCotFvWy8czNfCIe8jjqmyTNHWWDmel9XT3tKt4ncqWMzQnHs+QB66OfEEb\n\t8rPXrRPe8U25EO0LBieqSBD/hjpK+OpkH0Yy3mt6nymDY7JOwWkIfARVQOmnN2hO9V\n\tchJhS6KHQZM+wV0UQiSFHWafAUbOdJwLHZOJjvkg=","Message-ID":"","Date":"Tue, 12 Nov 2024 12:58:18 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>\n\t<20241112080921.GA5359@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20241112080921.GA5359@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32145,"web_url":"https://patchwork.libcamera.org/comment/32145/","msgid":"<87r07f5nqz.fsf@gmail.com>","date":"2024-11-13T12:09:32","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":146,"url":"https://patchwork.libcamera.org/api/people/146/","name":"Mikhail Rudenko","email":"mike.rudenko@gmail.com"},"content":"Hi Dan, Laurent,\n\nOn 2024-11-12 at 12:58 GMT, Dan Scally wrote:\n\n> Hi Laurent\n>\n> On 12/11/2024 08:09, Laurent Pinchart wrote:\n>> Hi Dan,\n>>\n>> Thank you for the patch.\n>>\n>> Could you please have a look at \"[PATCH RFC 0/3] Pass sensor delays from\n>> rkisp1 IPA to the pipeline handler\" and try to reconcile the two ?\n>\n>\n> Mikhail and I spoke on that patchset - I also originally added the\n> delays to CameraSensorHelper but decided they were better in\n> CameraSensorProperties and said that I thought that approach was\n> better. I think Mikhail was leaning in that direction too, in which\n> case this series can supersede that one.\n\nI fully agree with Dan here. The whole path through PH/IPA interface\njust to pass the control delays from CameraSensorHelper to\nDelayedControls (as it was implemented in my RFC) did not look quite\nright to me from the beginning. Since DelayedControls in the PH is the\nonly place where the delays are consumed, I think it does not make much\nsense to define them in IPA/CameraSensor, and CameraSensorProperties may\nactually be a better place to do it.\n\n>\n> Thanks\n>\n> Dan\n>\n>>\n>> On Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n>>> Add properties covering the sensor control application delays to both\n>>> the list of control values and the static CameraSensorProperties\n>>> definitions. The values used are the defaults that're in use across\n>>> the library, with deviations from that taken from Raspberry Pi's\n>>> CamHelper class definitions.\n>>>\n>>> Signed-off-by: Daniel Scally \n>>> ---\n>>> Changes in v2:\n>>>\n>>> \t- Rather than adding the delays to the properties ControlList, added a\n>>> \t new function in CameraSensor that allows PipelineHandlers to retreive\n>>> \t the delay values.\n>>>\n>>> include/libcamera/internal/camera_sensor.h | 2 +\n>>> .../internal/camera_sensor_properties.h | 9 +\n>>> src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n>>> .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n>>> 4 files changed, 211 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n>>> index a42c15fa..cdfbf051 100644\n>>> --- a/include/libcamera/internal/camera_sensor.h\n>>> +++ b/include/libcamera/internal/camera_sensor.h\n>>> @@ -79,6 +79,8 @@ public:\n>>> \t\treturn testPatternModes_;\n>>> \t}\n>>> \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n>>> +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>>> +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n>>> protected:\n>>> \tstd::string logPrefix() const override;\n>>> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n>>> index 480ac121..56d5c15d 100644\n>>> --- a/include/libcamera/internal/camera_sensor_properties.h\n>>> +++ b/include/libcamera/internal/camera_sensor_properties.h\n>>> @@ -10,6 +10,8 @@\n>>> #include \n>>> #include \n>>> +#include \n>>> +\n>>> #include \n>>> #include \n>>> @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n>>> \tSize unitCellSize;\n>>> \tstd::map testPatternModes;\n>>> +\n>>> +\tstruct {\n>>> +\t\tuint8_t exposureDelay;\n>>> +\t\tuint8_t gainDelay;\n>>> +\t\tuint8_t vblankDelay;\n>>> +\t\tuint8_t hblankDelay;\n>>> +\t} sensorDelays;\n>>> };\n>>> } /* namespace libcamera */\n>>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>>> index 1b224f19..5d26f3df 100644\n>>> --- a/src/libcamera/sensor/camera_sensor.cpp\n>>> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>>> @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n>>> \tinitTestPatternModes();\n>>> }\n>>> +/**\n>>> + * \\brief Fetch the sensor delay values\n>>> + * \\param[out] exposureDelay A variable to set the exposure delay to\n>>> + * \\param[out] gainDelay A variable to set the gain delay to\n>>> + * \\param[out] vblankDelay A variable to set the vblank delay to\n>>> + * \\param[out] hblankDelay A variable to set the hblank delay to\n>>> + *\n>>> + * This function fills in sensor control delays for pipeline handlers to use to\n>>> + * inform the DelayedControls. If no static properties are available it fills in\n>>> + * some widely applicable default values.\n>>> + */\n>>> +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>>> +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n>>> +{\n>>> +\t/*\n>>> +\t * These defaults are applicable to many sensors, however more specific\n>>> +\t * values can be added to the CameraSensorProperties for a sensor if\n>>> +\t * required.\n>>> +\t */\n>>> +\tif (!staticProps_) {\n>>> +\t\texposureDelay = 2;\n>>> +\t\tgainDelay = 1;\n>>> +\t\tvblankDelay = 2;\n>>> +\t\thblankDelay = 2;\n>>> +\t\treturn;\n>>> +\t}\n>>> +\n>>> +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n>>> +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n>>> +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n>>> +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n>>> +}\n>>> +\n>>> void CameraSensor::initTestPatternModes()\n>>> {\n>>> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n>>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n>>> index 6d4136d0..60d59f79 100644\n>>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n>>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n>>> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n>>> * \\brief Map that associates the TestPattern control value with the indexes of\n>>> * the corresponding sensor test pattern modes as returned by\n>>> * V4L2_CID_TEST_PATTERN.\n>>> + *\n>>> + * \\var CameraSensorProperties::sensorDelays\n>>> + * \\brief struct holding the number of frames delay between a control value\n>>> + * set and taking effect for each of exposure, gain, vertical blanking and\n>>> + * horizontal blanking.\n>>> */\n>>> /**\n>>> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ar0521\", {\n>>> \t\t\t.unitCellSize = { 2200, 2200 },\n>>> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"hi846\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * 9: \"Resolution Pattern\"\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx214\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx219\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx258\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx283\", {\n>>> \t\t\t.unitCellSize = { 2400, 2400 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx290\", {\n>>> \t\t\t.unitCellSize = { 2900, 2900 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx296\", {\n>>> \t\t\t.unitCellSize = { 3450, 3450 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx327\", {\n>>> \t\t\t.unitCellSize = { 2900, 2900 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx335\", {\n>>> \t\t\t.unitCellSize = { 2000, 2000 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx415\", {\n>>> \t\t\t.unitCellSize = { 1450, 1450 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx477\", {\n>>> \t\t\t.unitCellSize = { 1550, 1550 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 3,\n>>> +\t\t\t\t.hblankDelay = 3\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx519\", {\n>>> \t\t\t.unitCellSize = { 1220, 1220 },\n>>> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 3,\n>>> +\t\t\t\t.hblankDelay = 3\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"imx708\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 3,\n>>> +\t\t\t\t.hblankDelay = 3\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov2685\", {\n>>> \t\t\t.unitCellSize = { 1750, 1750 },\n>>> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * 5: \"Color Square\"\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov2740\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov4689\", {\n>>> \t\t\t.unitCellSize = { 2000, 2000 },\n>>> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * colorBarType2 and colorBarType3.\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov5640\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov5647\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> \t\t\t.testPatternModes = {},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov5670\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov5675\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov5693\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * Rolling Bar\".\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov64a40\", {\n>>> \t\t\t.unitCellSize = { 1008, 1008 },\n>>> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 2,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov8858\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov8865\", {\n>>> \t\t\t.unitCellSize = { 1400, 1400 },\n>>> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t * 5: \"Color squares with rolling bar\"\n>>> \t\t\t\t */\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t\t{ \"ov13858\", {\n>>> \t\t\t.unitCellSize = { 1120, 1120 },\n>>> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>>> \t\t\t},\n>>> +\t\t\t.sensorDelays = {\n>>> +\t\t\t\t.exposureDelay = 2,\n>>> +\t\t\t\t.gainDelay = 1,\n>>> +\t\t\t\t.vblankDelay = 2,\n>>> +\t\t\t\t.hblankDelay = 2\n>>> +\t\t\t},\n>>> \t\t} },\n>>> \t};\n>>>\n\n\n--\nBest regards,\nMikhail Rudenko","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C3153C324C\n\tfor ;\n\tWed, 13 Nov 2024 12:20:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E22D26580E;\n\tWed, 13 Nov 2024 13:20:25 +0100 (CET)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BCD1657CF\n\tfor ;\n\tWed, 13 Nov 2024 13:20:24 +0100 (CET)","by mail-ed1-x529.google.com with SMTP id\n\t4fb4d7f45d1cf-5cece886771so1297562a12.0\n\tfor ;\n\tWed, 13 Nov 2024 04:20:24 -0800 (PST)","from razdolb (basic-fin-2.undef.network. [45.12.135.46])\n\tby smtp.gmail.com with ESMTPSA id\n\t4fb4d7f45d1cf-5cf03b9b661sm7134614a12.36.2024.11.13.04.20.22\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 13 Nov 2024 04:20:22 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"iryPckQi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1731500423; x=1732105223;\n\tdarn=lists.libcamera.org; \n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:from:to:cc:subject:date:message-id:reply-to;\n\tbh=3U2Z9HZi672zrbsPkWVHZjZncGkBQ0AT5UV/nIpsi2Y=;\n\tb=iryPckQiwtmwuoN87Ur/zTlAGrBADrCRnVgeWLPc+fBm6L9lC+QWPjHTalDJhVQpAq\n\t8hF9vHVJ3U41ZFpEccPGSg60fxuhxTrBV7E24mp9zP1KRUz9jlwezYi3QB6g5Fs5HR1U\n\tfIZWk7iIYoGyN3q2StvfqmMdpY69ksVlAy96MQmiVwAyqGxt+QBSvpBQbzohec7zUnRc\n\tPX9Q5hb1h1+DjpnVmw/QGgZFKjiQPx3MgXemaY71Q9ZSiYstzHC1vWfq9xmstlPoRQON\n\tnqTuC1Ltt0xiVWs6PFXuWi3Yj1Tge91w/nFpfrdlEBqxnJXdTfFcGk7S9fMcEemGtJgZ\n\t8RMQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1731500423; x=1732105223;\n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=3U2Z9HZi672zrbsPkWVHZjZncGkBQ0AT5UV/nIpsi2Y=;\n\tb=jNDtl/iERtFgVGxIMQT3ke8AwwD0T6uTgPalJqcQ4ZHBUczNa/l3rS7WmX6jno+714\n\tgkBOiBdBD7+VWUlrPe4DGRwmCZpecnfDeaUFYBipt7HH9K5D+4ZlNmEkmjj5INwbIOYB\n\tXMC0y5dgxlOUj0kAUZHf2CbYQSCQiuxWYPKHsj3SvDiNY65ldKPyVSScoC2Y1p/P0dtH\n\tTK2hWIpBtLqVEtPx20fqdz9zpPVztw2smL2iLNLsnglEmc0hPWEwsGAZYMPAetrYyc/C\n\tVQbVjq6R61PngqxHf3iwBEit6tqZrqcdl/Syxq8OyNmZMhjA1/LT2sKTv+CFhc0vQPPS\n\t0Itw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVA4MP13zO5EbP+dKujNzBws2DM7F1Rcs8mmsoxMIBISlqbczklqgJQ1mDnfqlmeoDqIuZ+cf4z8zzMpCU6fDk=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yzl7AuMR7H34PWSw84PgF/sqYqTS9BtB3T8eAJ3jy4fAKraXixP\n\tqYpxfwpyP2hc+h2zaTh9ljsaH8gmUXBUXSTD3s5w6DxnsxrX9WPyYEXcKQ==","X-Google-Smtp-Source":"AGHT+IGu0d0Yvza0h2yuTKMF2rfP5kFlauz92HWhTSxdEyzb8bJH2ZAsTCGYU1yrPlsl06lZRau6kA==","X-Received":"by 2002:a05:6402:2690:b0:5c9:1cdf:bbae with SMTP id\n\t4fb4d7f45d1cf-5cf0a4fc097mr19276113a12.11.1731500423142; \n\tWed, 13 Nov 2024 04:20:23 -0800 (PST)","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>\n\t<20241112080921.GA5359@pendragon.ideasonboard.com>\n\t","User-agent":"mu4e 1.10.8; emacs 29.4.50","From":"Mikhail Rudenko ","To":"Dan Scally ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","Date":"Wed, 13 Nov 2024 15:09:32 +0300","In-reply-to":"","Message-ID":"<87r07f5nqz.fsf@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32147,"web_url":"https://patchwork.libcamera.org/comment/32147/","msgid":"<08d865c9-316c-421a-a5b3-a9f6079f008d@ideasonboard.com>","date":"2024-11-13T13:38:51","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Morning Jacopo\n\nOn 08/11/2024 15:40, Jacopo Mondi wrote:\n> Hi Dan\n>\n> will need to be rebased on the introduction of CameraSensorFactory\n> I can share a rebased branch if you want.\nIt's ok I've got it\n>\n> On Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n>> Add properties covering the sensor control application delays to both\n>> the list of control values and the static CameraSensorProperties\n> I don't think this description applies anymore\nOops, thanks\n>\n>> definitions. The values used are the defaults that're in use across\n>> the library, with deviations from that taken from Raspberry Pi's\n> More on this below\n>\n>> CamHelper class definitions.\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> Changes in v2:\n>>\n>> \t- Rather than adding the delays to the properties ControlList, added a\n>> \t new function in CameraSensor that allows PipelineHandlers to retreive\n>> \t the delay values.\n>>\n>> include/libcamera/internal/camera_sensor.h | 2 +\n>> .../internal/camera_sensor_properties.h | 9 +\n>> src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n>> .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n>> 4 files changed, 211 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n>> index a42c15fa..cdfbf051 100644\n>> --- a/include/libcamera/internal/camera_sensor.h\n>> +++ b/include/libcamera/internal/camera_sensor.h\n>> @@ -79,6 +79,8 @@ public:\n>> \t\treturn testPatternModes_;\n>> \t}\n>> \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n>> +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>> +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n>>\n>> protected:\n>> \tstd::string logPrefix() const override;\n>> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n>> index 480ac121..56d5c15d 100644\n>> --- a/include/libcamera/internal/camera_sensor_properties.h\n>> +++ b/include/libcamera/internal/camera_sensor_properties.h\n>> @@ -10,6 +10,8 @@\n>> #include \n>> #include \n>>\n>> +#include \n>> +\n>> #include \n>> #include \n>>\n>> @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n>>\n>> \tSize unitCellSize;\n>> \tstd::map testPatternModes;\n>> +\n>> +\tstruct {\n>> +\t\tuint8_t exposureDelay;\n>> +\t\tuint8_t gainDelay;\n>> +\t\tuint8_t vblankDelay;\n>> +\t\tuint8_t hblankDelay;\n>> +\t} sensorDelays;\n>> };\n>>\n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>> index 1b224f19..5d26f3df 100644\n>> --- a/src/libcamera/sensor/camera_sensor.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>> @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n>> \tinitTestPatternModes();\n>> }\n>>\n>> +/**\n>> + * \\brief Fetch the sensor delay values\n>> + * \\param[out] exposureDelay A variable to set the exposure delay to\n>> + * \\param[out] gainDelay A variable to set the gain delay to\n> Analogue gain I suppose\n>\n>> + * \\param[out] vblankDelay A variable to set the vblank delay to\n>> + * \\param[out] hblankDelay A variable to set the hblank delay to\n> s/A variable to set//\n> s/delay to/delay\n>\n> ?\n>> + *\n>> + * This function fills in sensor control delays for pipeline handlers to use to\n>> + * inform the DelayedControls. If no static properties are available it fills in\n>> + * some widely applicable default values.\n>> + */\n>> +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n>> +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n>> +{\n>> +\t/*\n>> +\t * These defaults are applicable to many sensors, however more specific\n>> +\t * values can be added to the CameraSensorProperties for a sensor if\n>> +\t * required.\n>> +\t */\n>> +\tif (!staticProps_) {\n>> +\t\texposureDelay = 2;\n>> +\t\tgainDelay = 1;\n>> +\t\tvblankDelay = 2;\n>> +\t\thblankDelay = 2;\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n>> +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n>> +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n>> +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n>> +}\n>> +\n>> void CameraSensor::initTestPatternModes()\n>> {\n>> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n>> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n>> index 6d4136d0..60d59f79 100644\n>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n>> @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n>> * \\brief Map that associates the TestPattern control value with the indexes of\n>> * the corresponding sensor test pattern modes as returned by\n>> * V4L2_CID_TEST_PATTERN.\n>> + *\n>> + * \\var CameraSensorProperties::sensorDelays\n>> + * \\brief struct holding the number of frames delay between a control value\n>> + * set and taking effect for each of exposure, gain, vertical blanking and\n>> + * horizontal blanking.\n> Feels weird to correct a native speaker\nNot at all! Yours is much clearer\n> , but I would write it\n> differently.\n>\n> * \\var CameraSensorProperties::sensorDelays\n> * \\brief Holds the delays, expressed in number of frames, between the\n> * time a control is applied to the sensor and the time it actually takes\n> * effect. Delays are recorded for the exposure time, analogue gain,\n> * vertical and horizontal blankings controls.\n>\n> Take in what you like the most\n>\n>> */\n>>\n>> /**\n>> @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n> Ok, now we have defaults in the implementation of the camera sensor\n> class, and most of the values stored here are actually defaults that\n> work in best-effort mode but have not been actually validated against\n> the sensor.\nYes\n>\n> What I would do, for sensor where the delayes have not actually been\n> measured or taken from the datasheet, is to leave sensorDelays empty in\n> here and only populate the ones we're sure about. If we use defaults\n> possibly print it out as WARN/INFO that precise per-frame control is\n> not available as the sensor's delays have been default populated.\n>\n> Recording non-validated information here might give the false\n> impression that we trust the values here recorded and if someone\n> notices issues it might be confusing to see values recorded here that\n> do not match the actual experimental results.\n\n\nLeaving them empty generates \"missing initializer for member\" warnings, I can initialise the unknown \nones as empty (I.E. \".sensorDelays = { }\") and check for all zeros in \nCameraSensor::getSensorDelays() and return the defaults with a warning there, if that's ok?\n\n>\n>\n>> \t\t} },\n>> \t\t{ \"ar0521\", {\n>> \t\t\t.unitCellSize = { 2200, 2200 },\n>> @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"hi846\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 9: \"Resolution Pattern\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx214\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx219\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx258\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx283\", {\n>> \t\t\t.unitCellSize = { 2400, 2400 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx290\", {\n>> \t\t\t.unitCellSize = { 2900, 2900 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx296\", {\n>> \t\t\t.unitCellSize = { 3450, 3450 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx327\", {\n>> \t\t\t.unitCellSize = { 2900, 2900 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx335\", {\n>> \t\t\t.unitCellSize = { 2000, 2000 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx415\", {\n>> \t\t\t.unitCellSize = { 1450, 1450 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx477\", {\n>> \t\t\t.unitCellSize = { 1550, 1550 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx519\", {\n>> \t\t\t.unitCellSize = { 1220, 1220 },\n>> @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"imx708\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>> \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 3,\n>> +\t\t\t\t.hblankDelay = 3\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov2685\", {\n>> \t\t\t.unitCellSize = { 1750, 1750 },\n>> @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 5: \"Color Square\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov2740\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov4689\", {\n>> \t\t\t.unitCellSize = { 2000, 2000 },\n>> @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * colorBarType2 and colorBarType3.\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5640\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5647\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5670\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5675\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov5693\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * Rolling Bar\".\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov64a40\", {\n>> \t\t\t.unitCellSize = { 1008, 1008 },\n>> @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 2,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov8858\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov8865\", {\n>> \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t * 5: \"Color squares with rolling bar\"\n>> \t\t\t\t */\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t\t{ \"ov13858\", {\n>> \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>> \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n>> \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n>> \t\t\t},\n>> +\t\t\t.sensorDelays = {\n>> +\t\t\t\t.exposureDelay = 2,\n>> +\t\t\t\t.gainDelay = 1,\n>> +\t\t\t\t.vblankDelay = 2,\n>> +\t\t\t\t.hblankDelay = 2\n>> +\t\t\t},\n>> \t\t} },\n>> \t};\n>>\n>> --\n>> 2.30.2\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 41618C324C\n\tfor ;\n\tWed, 13 Nov 2024 13:38:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 210D665819;\n\tWed, 13 Nov 2024 14:38:56 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EAC3D65816\n\tfor ;\n\tWed, 13 Nov 2024 14:38:54 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8D86D741;\n\tWed, 13 Nov 2024 14:38:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"o9y+iTx7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731505121;\n\tbh=kezNtZm+P+og3TCo9jRLogMavaVP0lMgC3riirjf9Kk=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=o9y+iTx7P3ewS0TexWYYv2AIg6kd5pKTUyYAkHQoK2sl9IOsNhnRvOu4WhxzHyUMg\n\tS5iu6cqArF6DsE7qf+hsBmybr5lwx7f0NDB0y2RRkPgxe0Os4VxQ81NmHpnZ4Z/Z29\n\t8FJ0EfX1GxCEn6AnAwZ+e7qDsGTYkXZztikJaiAo=","Message-ID":"<08d865c9-316c-421a-a5b3-a9f6079f008d@ideasonboard.com>","Date":"Wed, 13 Nov 2024 13:38:51 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32153,"web_url":"https://patchwork.libcamera.org/comment/32153/","msgid":"<4bhb5z7hdxexmroymrrnonpdekazbrxlrhwpuu7kkhavpkfsxc@6k3unesonwu2>","date":"2024-11-13T18:57:11","subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Wed, Nov 13, 2024 at 01:38:51PM +0000, Dan Scally wrote:\n> Morning Jacopo\n>\n> On 08/11/2024 15:40, Jacopo Mondi wrote:\n> > Hi Dan\n> >\n> > will need to be rebased on the introduction of CameraSensorFactory\n> > I can share a rebased branch if you want.\n> It's ok I've got it\n> >\n> > On Thu, Nov 07, 2024 at 10:25:07AM +0000, Daniel Scally wrote:\n> > > Add properties covering the sensor control application delays to both\n> > > the list of control values and the static CameraSensorProperties\n> > I don't think this description applies anymore\n> Oops, thanks\n> >\n> > > definitions. The values used are the defaults that're in use across\n> > > the library, with deviations from that taken from Raspberry Pi's\n> > More on this below\n> >\n> > > CamHelper class definitions.\n> > >\n> > > Signed-off-by: Daniel Scally \n> > > ---\n> > > Changes in v2:\n> > >\n> > > \t- Rather than adding the delays to the properties ControlList, added a\n> > > \t new function in CameraSensor that allows PipelineHandlers to retreive\n> > > \t the delay values.\n> > >\n> > > include/libcamera/internal/camera_sensor.h | 2 +\n> > > .../internal/camera_sensor_properties.h | 9 +\n> > > src/libcamera/sensor/camera_sensor.cpp | 33 ++++\n> > > .../sensor/camera_sensor_properties.cpp | 167 ++++++++++++++++++\n> > > 4 files changed, 211 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > > index a42c15fa..cdfbf051 100644\n> > > --- a/include/libcamera/internal/camera_sensor.h\n> > > +++ b/include/libcamera/internal/camera_sensor.h\n> > > @@ -79,6 +79,8 @@ public:\n> > > \t\treturn testPatternModes_;\n> > > \t}\n> > > \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode);\n> > > +\tvoid getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> > > +\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay);\n> > >\n> > > protected:\n> > > \tstd::string logPrefix() const override;\n> > > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> > > index 480ac121..56d5c15d 100644\n> > > --- a/include/libcamera/internal/camera_sensor_properties.h\n> > > +++ b/include/libcamera/internal/camera_sensor_properties.h\n> > > @@ -10,6 +10,8 @@\n> > > #include \n> > > #include \n> > >\n> > > +#include \n> > > +\n> > > #include \n> > > #include \n> > >\n> > > @@ -20,6 +22,13 @@ struct CameraSensorProperties {\n> > >\n> > > \tSize unitCellSize;\n> > > \tstd::map testPatternModes;\n> > > +\n> > > +\tstruct {\n> > > +\t\tuint8_t exposureDelay;\n> > > +\t\tuint8_t gainDelay;\n> > > +\t\tuint8_t vblankDelay;\n> > > +\t\tuint8_t hblankDelay;\n> > > +\t} sensorDelays;\n> > > };\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> > > index 1b224f19..5d26f3df 100644\n> > > --- a/src/libcamera/sensor/camera_sensor.cpp\n> > > +++ b/src/libcamera/sensor/camera_sensor.cpp\n> > > @@ -391,6 +391,39 @@ void CameraSensor::initStaticProperties()\n> > > \tinitTestPatternModes();\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Fetch the sensor delay values\n> > > + * \\param[out] exposureDelay A variable to set the exposure delay to\n> > > + * \\param[out] gainDelay A variable to set the gain delay to\n> > Analogue gain I suppose\n> >\n> > > + * \\param[out] vblankDelay A variable to set the vblank delay to\n> > > + * \\param[out] hblankDelay A variable to set the hblank delay to\n> > s/A variable to set//\n> > s/delay to/delay\n> >\n> > ?\n> > > + *\n> > > + * This function fills in sensor control delays for pipeline handlers to use to\n> > > + * inform the DelayedControls. If no static properties are available it fills in\n> > > + * some widely applicable default values.\n> > > + */\n> > > +void CameraSensor::getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,\n> > > +\t\t\t\t uint8_t &vblankDelay, uint8_t &hblankDelay)\n> > > +{\n> > > +\t/*\n> > > +\t * These defaults are applicable to many sensors, however more specific\n> > > +\t * values can be added to the CameraSensorProperties for a sensor if\n> > > +\t * required.\n> > > +\t */\n> > > +\tif (!staticProps_) {\n> > > +\t\texposureDelay = 2;\n> > > +\t\tgainDelay = 1;\n> > > +\t\tvblankDelay = 2;\n> > > +\t\thblankDelay = 2;\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\texposureDelay = staticProps_->sensorDelays.exposureDelay;\n> > > +\tgainDelay = staticProps_->sensorDelays.gainDelay;\n> > > +\tvblankDelay = staticProps_->sensorDelays.vblankDelay;\n> > > +\thblankDelay = staticProps_->sensorDelays.hblankDelay;\n> > > +}\n> > > +\n> > > void CameraSensor::initTestPatternModes()\n> > > {\n> > > \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > index 6d4136d0..60d59f79 100644\n> > > --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> > > @@ -41,6 +41,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> > > * \\brief Map that associates the TestPattern control value with the indexes of\n> > > * the corresponding sensor test pattern modes as returned by\n> > > * V4L2_CID_TEST_PATTERN.\n> > > + *\n> > > + * \\var CameraSensorProperties::sensorDelays\n> > > + * \\brief struct holding the number of frames delay between a control value\n> > > + * set and taking effect for each of exposure, gain, vertical blanking and\n> > > + * horizontal blanking.\n> > Feels weird to correct a native speaker\n> Not at all! Yours is much clearer\n> > , but I would write it\n> > differently.\n> >\n> > * \\var CameraSensorProperties::sensorDelays\n> > * \\brief Holds the delays, expressed in number of frames, between the\n> > * time a control is applied to the sensor and the time it actually takes\n> > * effect. Delays are recorded for the exposure time, analogue gain,\n> > * vertical and horizontal blankings controls.\n> >\n> > Take in what you like the most\n> >\n> > > */\n> > >\n> > > /**\n> > > @@ -60,6 +65,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > Ok, now we have defaults in the implementation of the camera sensor\n> > class, and most of the values stored here are actually defaults that\n> > work in best-effort mode but have not been actually validated against\n> > the sensor.\n> Yes\n> >\n> > What I would do, for sensor where the delayes have not actually been\n> > measured or taken from the datasheet, is to leave sensorDelays empty in\n> > here and only populate the ones we're sure about. If we use defaults\n> > possibly print it out as WARN/INFO that precise per-frame control is\n> > not available as the sensor's delays have been default populated.\n> >\n> > Recording non-validated information here might give the false\n> > impression that we trust the values here recorded and if someone\n> > notices issues it might be confusing to see values recorded here that\n> > do not match the actual experimental results.\n>\n>\n> Leaving them empty generates \"missing initializer for member\" warnings, I\n> can initialise the unknown ones as empty (I.E. \".sensorDelays = { }\") and\n> check for all zeros in CameraSensor::getSensorDelays() and return the\n> defaults with a warning there, if that's ok?\n>\n\nThat's what I was thinking, so indeed ok with me :)\n\nThanks\n j\n\n> >\n> >\n> > > \t\t} },\n> > > \t\t{ \"ar0521\", {\n> > > \t\t\t.unitCellSize = { 2200, 2200 },\n> > > @@ -69,6 +80,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"hi846\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -87,6 +104,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * 9: \"Resolution Pattern\"\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx214\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -97,6 +120,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx219\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -107,6 +136,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx258\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -117,34 +152,82 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx283\", {\n> > > \t\t\t.unitCellSize = { 2400, 2400 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx290\", {\n> > > \t\t\t.unitCellSize = { 2900, 2900 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx296\", {\n> > > \t\t\t.unitCellSize = { 3450, 3450 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx327\", {\n> > > \t\t\t.unitCellSize = { 2900, 2900 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx335\", {\n> > > \t\t\t.unitCellSize = { 2000, 2000 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx415\", {\n> > > \t\t\t.unitCellSize = { 1450, 1450 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx477\", {\n> > > \t\t\t.unitCellSize = { 1550, 1550 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 3,\n> > > +\t\t\t\t.hblankDelay = 3\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx519\", {\n> > > \t\t\t.unitCellSize = { 1220, 1220 },\n> > > @@ -157,6 +240,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 3,\n> > > +\t\t\t\t.hblankDelay = 3\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"imx708\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > @@ -167,6 +256,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> > > \t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 3,\n> > > +\t\t\t\t.hblankDelay = 3\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov2685\", {\n> > > \t\t\t.unitCellSize = { 1750, 1750 },\n> > > @@ -181,6 +276,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * 5: \"Color Square\"\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov2740\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > @@ -188,6 +289,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1},\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov4689\", {\n> > > \t\t\t.unitCellSize = { 2000, 2000 },\n> > > @@ -201,6 +308,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * colorBarType2 and colorBarType3.\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov5640\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > @@ -208,10 +321,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov5647\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > \t\t\t.testPatternModes = {},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov5670\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -219,6 +344,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov5675\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -226,6 +357,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov5693\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > @@ -238,6 +375,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * Rolling Bar\".\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov64a40\", {\n> > > \t\t\t.unitCellSize = { 1008, 1008 },\n> > > @@ -251,6 +394,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 2,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov8858\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -264,6 +413,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * 4: \"Vertical Color Bar Type 4\"\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov8865\", {\n> > > \t\t\t.unitCellSize = { 1400, 1400 },\n> > > @@ -278,6 +433,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t * 5: \"Color squares with rolling bar\"\n> > > \t\t\t\t */\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t\t{ \"ov13858\", {\n> > > \t\t\t.unitCellSize = { 1120, 1120 },\n> > > @@ -285,6 +446,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> > > \t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> > > \t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> > > \t\t\t},\n> > > +\t\t\t.sensorDelays = {\n> > > +\t\t\t\t.exposureDelay = 2,\n> > > +\t\t\t\t.gainDelay = 1,\n> > > +\t\t\t\t.vblankDelay = 2,\n> > > +\t\t\t\t.hblankDelay = 2\n> > > +\t\t\t},\n> > > \t\t} },\n> > > \t};\n> > >\n> > > --\n> > > 2.30.2\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8668BC324C\n\tfor ;\n\tWed, 13 Nov 2024 18:57:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9CAE65827;\n\tWed, 13 Nov 2024 19:57:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DA8E657CF\n\tfor ;\n\tWed, 13 Nov 2024 19:57:15 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:6462:5de2:459e:1ee6:26ea:2d31])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87AE5EA7;\n\tWed, 13 Nov 2024 19:57:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mjky9kOY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731524221;\n\tbh=5qXXSsS4vHhfZ99ea4j9LqOSrOzUo/hFu4I6K72bLGc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mjky9kOYrZMFZXsC0s2zM6st1NGHevBTQ1LdlsZzImAcFG7V0TDi5wMHAlbXrpWkh\n\tInBrpMj2bRo+tEVSonS6mZM7mQlfncKUTtq0sje8bgBmakjIy/voeMsydXMcQLdzw2\n\t7TjB7Cvd1kOOOG5ZMMLHX0vKzsqTKZA4RVtFHF80=","Date":"Wed, 13 Nov 2024 19:57:11 +0100","From":"Jacopo Mondi ","To":"Dan Scally ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","Subject":"Re: [PATCH v2 5/6] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","Message-ID":"<4bhb5z7hdxexmroymrrnonpdekazbrxlrhwpuu7kkhavpkfsxc@6k3unesonwu2>","References":"<20241107102508.48322-1-dan.scally@ideasonboard.com>\n\t<20241107102508.48322-6-dan.scally@ideasonboard.com>\n\t\n\t<08d865c9-316c-421a-a5b3-a9f6079f008d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<08d865c9-316c-421a-a5b3-a9f6079f008d@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]