[{"id":32385,"web_url":"https://patchwork.libcamera.org/comment/32385/","msgid":"<173263553976.1605529.5715014203230003084@ping.linuxembedded.co.uk>","date":"2024-11-26T15:38:59","subject":"Re: [PATCH v5 1/2] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Daniel Scally (2024-11-26 15:12:02)\n> Add properties covering the sensor control application delays to both\n> the static CameraSensorProperties definitions. The values used are\n> taken from Raspberry Pi's CamHelper class definitions. Where no more\n> specific values are known the delay struct is defined as empty and\n> defaults supplied through the getter function.\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n> Changes in v5:\n> \n>         - Comment rewording\n>         - Moved the defaultSensorDelays definition inside\n>           CameraSensorLegacy::sensorDelays()\n> \n> Changes in v4:\n> \n>         - getSensorDelays() renamed to sensorDelays()\n>         - sensorDelays() returns a reference to the SensorDelays struct rather\n>           than each of the values.\n>         - Reworded the comments to make the default values seem less acceptable\n>         - Added delay values for AR0144\n> \n> Changes in v3:\n> \n>         - Rebased on top of the CameraSensorFactory introduction\n>         - Some rephrasing\n>         - Defined the sensorDelays member as empty where Raspberry Pi didn't\n>           have any specific values. Check for the empty struct in\n>           getSensorDelays() and return the defaults from there with a warning.\n> \n> Changes in v2:\n> \n>         - Rather than adding the delays to the properties ControlList, added a\n>           new function in CameraSensor that allows PipelineHandlers to retreive\n>           the delay values.\n> \n>  include/libcamera/internal/camera_sensor.h    |   2 +\n>  .../internal/camera_sensor_properties.h       |   8 ++\n>  src/libcamera/sensor/camera_sensor.cpp        |  14 +++\n>  src/libcamera/sensor/camera_sensor_legacy.cpp |  25 ++++\n>  .../sensor/camera_sensor_properties.cpp       | 108 ++++++++++++++++++\n>  5 files changed, 157 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 8aafd82e..bbdb83a1 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -20,6 +20,7 @@\n>  #include <libcamera/orientation.h>\n>  #include <libcamera/transform.h>\n>  \n> +#include \"libcamera/internal/camera_sensor_properties.h\"\n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/v4l2_subdevice.h\"\n>  \n> @@ -73,6 +74,7 @@ public:\n>         virtual const std::vector<controls::draft::TestPatternModeEnum> &\n>         testPatternModes() const = 0;\n>         virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0;\n> +       virtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0;\n>  };\n>  \n>  class CameraSensorFactoryBase\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 480ac121..75cb7839 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -8,6 +8,7 @@\n>  #pragma once\n>  \n>  #include <map>\n> +#include <stdint.h>\n>  #include <string>\n>  \n>  #include <libcamera/control_ids.h>\n> @@ -20,6 +21,13 @@ struct CameraSensorProperties {\n>  \n>         Size unitCellSize;\n>         std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;\n> +\n> +       struct SensorDelays {\n> +               uint8_t exposureDelay;\n> +               uint8_t gainDelay;\n> +               uint8_t vblankDelay;\n> +               uint8_t hblankDelay;\n> +       } 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 54cf98b2..9d2c2ea7 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default;\n>   * pattern mode for every frame thus incurs no performance penalty.\n>   */\n>  \n> +/**\n> + * \\fn CameraSensor::sensorDelays()\n> + * \\brief Fetch the sensor delay values\n> + *\n> + * This function retreives the sensor control delays for pipeline handlers to\n> + * use to inform the DelayedControls. If control delays are not specified in the\n> + * static sensor propertie database, this function returns a reference to a set\n> + * of default sensor delays provided as best-effort placeholders for the actual\n> + * sensor specific delays.\n> + *\n> + * \\return A reference to a struct CameraSensorProperties::SensorDelays holding\n> + * the delay values\n> + */\n> +\n>  /**\n>   * \\class CameraSensorFactoryBase\n>   * \\brief Base class for camera sensor factories\n> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> index a9b15c03..17d6fa68 100644\n> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> @@ -95,6 +95,7 @@ public:\n>         const std::vector<controls::draft::TestPatternModeEnum> &\n>         testPatternModes() const override { return testPatternModes_; }\n>         int setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;\n> +       const CameraSensorProperties::SensorDelays &sensorDelays() override;\n>  \n>  protected:\n>         std::string logPrefix() const override;\n> @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties()\n>         initTestPatternModes();\n>  }\n>  \n> +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays()\n> +{\n> +       static constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = {\n> +               .exposureDelay = 2,\n> +               .gainDelay = 1,\n> +               .vblankDelay = 2,\n> +               .hblankDelay = 2,\n> +       };\n\nA better place to keep this local indeed.\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> +\n> +       if (!staticProps_ ||\n> +           (!staticProps_->sensorDelays.exposureDelay &&\n> +            !staticProps_->sensorDelays.gainDelay &&\n> +            !staticProps_->sensorDelays.vblankDelay &&\n> +            !staticProps_->sensorDelays.hblankDelay)) {\n> +               LOG(CameraSensor, Warning)\n> +                       << \"No sensor delays found in static properties. \"\n> +                          \"Assuming unverified defaults.\";\n> +\n> +               return defaultSensorDelays;\n> +       }\n> +\n> +       return staticProps_->sensorDelays;\n> +}\n> +\n>  void CameraSensorLegacy::initTestPatternModes()\n>  {\n>         const 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 e2305166..3dc8cf05 100644\n> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> @@ -41,6 +41,36 @@ 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 Sensor control application delays.\n> + *\n> + * This struct may be defined as empty if the actual sensor delays are not\n> + * available or have not been measured. Best effort default values are returned\n> + * in this case.\n> + */\n> +\n> +/**\n> + * \\struct CameraSensorProperties::SensorDelays\n> + * \\brief Sensor control application delay values\n> + *\n> + * This struct holds delay values, expressed in number of frames, between the\n> + * time a control value is applied to the sensor and the time that value is\n> + * reflected in the output. For example \"2 frames delay\" means that parameters\n> + * set during frame N will take effect for frame N+2 (and by extension a delay\n> + * of 0 would mean the parameter is applied immediately to the current frame).\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::exposureDelay\n> + * \\brief Number of frames between application of exposure control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::gainDelay\n> + * \\brief Number of frames between application of analogue gain control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::vblankDelay\n> + * \\brief Number of frames between application of vblank control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::hblankDelay\n> + * \\brief Number of frames between application of hblank control and effect\n>   */\n>  \n>  /**\n> @@ -60,6 +90,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBars, 2 },\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                         },\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"ar0521\", {\n>                         .unitCellSize = { 2200, 2200 },\n> @@ -69,6 +105,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBars, 2 },\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"hi846\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -87,6 +124,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * 9: \"Resolution Pattern\"\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx214\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -97,6 +135,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                                 { controls::draft::TestPatternModePn9, 4 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx219\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -107,6 +146,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                                 { controls::draft::TestPatternModePn9, 4 },\n>                         },\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 1,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"imx258\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -117,38 +162,67 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                                 { controls::draft::TestPatternModePn9, 4 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx283\", {\n>                         .unitCellSize = { 2400, 2400 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"imx290\", {\n>                         .unitCellSize = { 2900, 2900 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"imx296\", {\n>                         .unitCellSize = { 3450, 3450 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"imx327\", {\n>                         .unitCellSize = { 2900, 2900 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx335\", {\n>                         .unitCellSize = { 2000, 2000 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx415\", {\n>                         .unitCellSize = { 1450, 1450 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx462\", {\n>                         .unitCellSize = { 2900, 2900 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"imx477\", {\n>                         .unitCellSize = { 1550, 1550 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 3,\n> +                               .hblankDelay = 3\n> +                       },\n>                 } },\n>                 { \"imx519\", {\n>                         .unitCellSize = { 1220, 1220 },\n> @@ -161,6 +235,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * these two patterns do not comply with MIPI CCS v1.1 (Section 10.1).\n>                                  */\n>                         },\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 3,\n> +                               .hblankDelay = 3\n> +                       },\n>                 } },\n>                 { \"imx708\", {\n>                         .unitCellSize = { 1400, 1400 },\n> @@ -171,6 +251,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n>                                 { controls::draft::TestPatternModePn9, 4 },\n>                         },\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 3,\n> +                               .hblankDelay = 3\n> +                       },\n>                 } },\n>                 { \"ov2685\", {\n>                         .unitCellSize = { 1750, 1750 },\n> @@ -185,6 +271,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * 5: \"Color Square\"\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov2740\", {\n>                         .unitCellSize = { 1400, 1400 },\n> @@ -192,6 +279,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeOff, 0 },\n>                                 { controls::draft::TestPatternModeColorBars, 1},\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov4689\", {\n>                         .unitCellSize = { 2000, 2000 },\n> @@ -205,6 +293,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * colorBarType2 and colorBarType3.\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov5640\", {\n>                         .unitCellSize = { 1400, 1400 },\n> @@ -212,10 +301,17 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeOff, 0 },\n>                                 { controls::draft::TestPatternModeColorBars, 1 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov5647\", {\n>                         .unitCellSize = { 1400, 1400 },\n>                         .testPatternModes = {},\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"ov5670\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -223,6 +319,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeOff, 0 },\n>                                 { controls::draft::TestPatternModeColorBars, 1 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov5675\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -230,6 +327,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeOff, 0 },\n>                                 { controls::draft::TestPatternModeColorBars, 1 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov5693\", {\n>                         .unitCellSize = { 1400, 1400 },\n> @@ -242,6 +340,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * Rolling Bar\".\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov64a40\", {\n>                         .unitCellSize = { 1008, 1008 },\n> @@ -255,6 +354,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * 4: \"Vertical Color Bar Type 4\"\n>                                  */\n>                         },\n> +                       .sensorDelays = {\n> +                               .exposureDelay = 2,\n> +                               .gainDelay = 2,\n> +                               .vblankDelay = 2,\n> +                               .hblankDelay = 2\n> +                       },\n>                 } },\n>                 { \"ov8858\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -268,6 +373,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * 4: \"Vertical Color Bar Type 4\"\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov8865\", {\n>                         .unitCellSize = { 1400, 1400 },\n> @@ -282,6 +388,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                  * 5: \"Color squares with rolling bar\"\n>                                  */\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>                 { \"ov13858\", {\n>                         .unitCellSize = { 1120, 1120 },\n> @@ -289,6 +396,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n>                                 { controls::draft::TestPatternModeOff, 0 },\n>                                 { controls::draft::TestPatternModeColorBars, 1 },\n>                         },\n> +                       .sensorDelays = { },\n>                 } },\n>         };\n>  \n> -- \n> 2.30.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0BC60C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Nov 2024 15:39:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DECE65FC9;\n\tTue, 26 Nov 2024 16:39:04 +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 1C75A65F9E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Nov 2024 16:39:03 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A5396BE;\n\tTue, 26 Nov 2024 16:38:40 +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=\"EbZSiv15\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732635520;\n\tbh=S59FBkxM5k2+ZxndAPyvMbw3vBSQ6sGiMd5wo6e8Cgg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=EbZSiv15auoGI5Bg7xWMCBeTkRcdnJLgiNfBlM1adIiIGJRir6LWtCzlyePHH1nGF\n\t1CErhvXaGqTuFO8THDGs7wQMN+sLSmHLLXm0zoFFubAHcEWkaQw79yot3NHVQBIm+2\n\t0qg717o/5fmFTt0sb5gBOd6WhMUgZtr4MlKBiuWs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241126151203.108407-2-dan.scally@ideasonboard.com>","References":"<20241126151203.108407-1-dan.scally@ideasonboard.com>\n\t<20241126151203.108407-2-dan.scally@ideasonboard.com>","Subject":"Re: [PATCH v5 1/2] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"mike.rudenko@gmail.com, Daniel Scally <dan.scally@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 26 Nov 2024 15:38:59 +0000","Message-ID":"<173263553976.1605529.5715014203230003084@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32402,"web_url":"https://patchwork.libcamera.org/comment/32402/","msgid":"<20241127065956.GR5461@pendragon.ideasonboard.com>","date":"2024-11-27T06:59:56","subject":"Re: [PATCH v5 1/2] 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":"On Tue, Nov 26, 2024 at 03:12:02PM +0000, Daniel Scally wrote:\n> Add properties covering the sensor control application delays to both\n> the static CameraSensorProperties definitions. The values used are\n> taken from Raspberry Pi's CamHelper class definitions. Where no more\n> specific values are known the delay struct is defined as empty and\n> defaults supplied through the getter function.\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n> Changes in v5:\n> \n> \t- Comment rewording\n> \t- Moved the defaultSensorDelays definition inside\n> \t  CameraSensorLegacy::sensorDelays()\n> \n> Changes in v4:\n> \n> \t- getSensorDelays() renamed to sensorDelays()\n> \t- sensorDelays() returns a reference to the SensorDelays struct rather\n> \t  than each of the values.\n> \t- Reworded the comments to make the default values seem less acceptable\n> \t- Added delay values for AR0144\n> \n> Changes in v3:\n> \n> \t- Rebased on top of the CameraSensorFactory introduction\n> \t- Some rephrasing\n> \t- Defined the sensorDelays member as empty where Raspberry Pi didn't\n> \t  have any specific values. Check for the empty struct in\n> \t  getSensorDelays() and return the defaults from there with a warning.\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       |   8 ++\n>  src/libcamera/sensor/camera_sensor.cpp        |  14 +++\n>  src/libcamera/sensor/camera_sensor_legacy.cpp |  25 ++++\n>  .../sensor/camera_sensor_properties.cpp       | 108 ++++++++++++++++++\n>  5 files changed, 157 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 8aafd82e..bbdb83a1 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -20,6 +20,7 @@\n>  #include <libcamera/orientation.h>\n>  #include <libcamera/transform.h>\n>  \n> +#include \"libcamera/internal/camera_sensor_properties.h\"\n\nAlphabetical order. I'm surprised checkstyle.py didn't warn you about\nthis.\n\n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/v4l2_subdevice.h\"\n>  \n> @@ -73,6 +74,7 @@ public:\n>  \tvirtual const std::vector<controls::draft::TestPatternModeEnum> &\n>  \ttestPatternModes() const = 0;\n>  \tvirtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0;\n> +\tvirtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0;\n>  };\n>  \n>  class CameraSensorFactoryBase\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 480ac121..75cb7839 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -8,6 +8,7 @@\n>  #pragma once\n>  \n>  #include <map>\n> +#include <stdint.h>\n>  #include <string>\n>  \n>  #include <libcamera/control_ids.h>\n> @@ -20,6 +21,13 @@ struct CameraSensorProperties {\n>  \n>  \tSize unitCellSize;\n>  \tstd::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;\n> +\n> +\tstruct SensorDelays {\n> +\t\tuint8_t exposureDelay;\n> +\t\tuint8_t gainDelay;\n> +\t\tuint8_t vblankDelay;\n> +\t\tuint8_t hblankDelay;\n> +\t} sensorDelays;\n\nWe usually separate type definitions from usage.\n\nstruct CameraSensorProperties {\n\tstruct SensorDelays {\n\t\tuint8_t exposureDelay;\n\t\tuint8_t gainDelay;\n\t\tuint8_t vblankDelay;\n\t\tuint8_t hblankDelay;\n\t};\n\n\tstatic const CameraSensorProperties *get(const std::string &sensor);\n\n\tSize unitCellSize;\n\tstd::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;\n\tSensorDelays sensorDelays;\n};\n\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n> index 54cf98b2..9d2c2ea7 100644\n> --- a/src/libcamera/sensor/camera_sensor.cpp\n> +++ b/src/libcamera/sensor/camera_sensor.cpp\n> @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default;\n>   * pattern mode for every frame thus incurs no performance penalty.\n>   */\n>  \n> +/**\n> + * \\fn CameraSensor::sensorDelays()\n> + * \\brief Fetch the sensor delay values\n> + *\n> + * This function retreives the sensor control delays for pipeline handlers to\n\ns/retreives/retrieves/\n\n> + * use to inform the DelayedControls. If control delays are not specified in the\n\nWe don't usually document expected users, unless there's a very tight\ncoupling between a class and its users that is relevant to understanding\nthe API.\n\n> + * static sensor propertie database, this function returns a reference to a set\n\ns/propertie/properties/\n\n> + * of default sensor delays provided as best-effort placeholders for the actual\n> + * sensor specific delays.\n\nAnd there's a bit of implementation detail here too. You can simplify\nthis to\n\n * This function retrieves the delays that the sensor applies to controls. If\n * the static properties database doesn't specifiy control delay values for the\n * sensor, default delays that may be suitable are returned and a warning is\n * logged.\n\n> + *\n> + * \\return A reference to a struct CameraSensorProperties::SensorDelays holding\n> + * the delay values\n\nDoxygen shows the type of the return value, so you can just write\n\n * \\return The sensor delay values\n\n> + */\n> +\n>  /**\n>   * \\class CameraSensorFactoryBase\n>   * \\brief Base class for camera sensor factories\n> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> index a9b15c03..17d6fa68 100644\n> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n> @@ -95,6 +95,7 @@ public:\n>  \tconst std::vector<controls::draft::TestPatternModeEnum> &\n>  \ttestPatternModes() const override { return testPatternModes_; }\n>  \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;\n> +\tconst CameraSensorProperties::SensorDelays &sensorDelays() override;\n>  \n>  protected:\n>  \tstd::string logPrefix() const override;\n> @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties()\n>  \tinitTestPatternModes();\n>  }\n>  \n> +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays()\n> +{\n> +\tstatic constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = {\n> +\t\t.exposureDelay = 2,\n> +\t\t.gainDelay = 1,\n> +\t\t.vblankDelay = 2,\n> +\t\t.hblankDelay = 2,\n> +\t};\n> +\n> +\tif (!staticProps_ ||\n> +\t    (!staticProps_->sensorDelays.exposureDelay &&\n> +\t     !staticProps_->sensorDelays.gainDelay &&\n> +\t     !staticProps_->sensorDelays.vblankDelay &&\n> +\t     !staticProps_->sensorDelays.hblankDelay)) {\n> +\t\tLOG(CameraSensor, Warning)\n> +\t\t\t<< \"No sensor delays found in static properties. \"\n> +\t\t\t   \"Assuming unverified defaults.\";\n> +\n> +\t\treturn defaultSensorDelays;\n> +\t}\n> +\n> +\treturn staticProps_->sensorDelays;\n> +}\n> +\n>  void CameraSensorLegacy::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 e2305166..3dc8cf05 100644\n> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n> @@ -41,6 +41,36 @@ 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 Sensor control application delays.\n\ncheckstyle.py flagged this.\n\n> + *\n> + * This struct may be defined as empty if the actual sensor delays are not\n\ns/struct/structure/\n\n> + * available or have not been measured. Best effort default values are returned\n> + * in this case.\n\nIn this context, it's not clear where the defaults are returned from.\nI'd drop the last sentence.\n\n> + */\n> +\n> +/**\n> + * \\struct CameraSensorProperties::SensorDelays\n> + * \\brief Sensor control application delay values\n> + *\n> + * This struct holds delay values, expressed in number of frames, between the\n\ns/struct/structure/\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> + * time a control value is applied to the sensor and the time that value is\n> + * reflected in the output. For example \"2 frames delay\" means that parameters\n> + * set during frame N will take effect for frame N+2 (and by extension a delay\n> + * of 0 would mean the parameter is applied immediately to the current frame).\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::exposureDelay\n> + * \\brief Number of frames between application of exposure control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::gainDelay\n> + * \\brief Number of frames between application of analogue gain control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::vblankDelay\n> + * \\brief Number of frames between application of vblank control and effect\n> + *\n> + * \\var CameraSensorProperties::SensorDelays::hblankDelay\n> + * \\brief Number of frames between application of hblank control and effect\n>   */\n>  \n>  /**\n> @@ -60,6 +90,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 = 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{ \"ar0521\", {\n>  \t\t\t.unitCellSize = { 2200, 2200 },\n> @@ -69,6 +105,7 @@ 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} },\n>  \t\t{ \"hi846\", {\n>  \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -87,6 +124,7 @@ 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} },\n>  \t\t{ \"imx214\", {\n>  \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -97,6 +135,7 @@ 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} },\n>  \t\t{ \"imx219\", {\n>  \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -107,6 +146,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,38 +162,67 @@ 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} },\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} },\n>  \t\t{ \"imx335\", {\n>  \t\t\t.unitCellSize = { 2000, 2000 },\n>  \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = { },\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} },\n>  \t\t{ \"imx462\", {\n>  \t\t\t.unitCellSize = { 2900, 2900 },\n>  \t\t\t.testPatternModes = {},\n> +\t\t\t.sensorDelays = { },\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> @@ -161,6 +235,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> @@ -171,6 +251,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> @@ -185,6 +271,7 @@ 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} },\n>  \t\t{ \"ov2740\", {\n>  \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -192,6 +279,7 @@ 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} },\n>  \t\t{ \"ov4689\", {\n>  \t\t\t.unitCellSize = { 2000, 2000 },\n> @@ -205,6 +293,7 @@ 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} },\n>  \t\t{ \"ov5640\", {\n>  \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -212,10 +301,17 @@ 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} },\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> @@ -223,6 +319,7 @@ 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} },\n>  \t\t{ \"ov5675\", {\n>  \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -230,6 +327,7 @@ 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} },\n>  \t\t{ \"ov5693\", {\n>  \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -242,6 +340,7 @@ 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} },\n>  \t\t{ \"ov64a40\", {\n>  \t\t\t.unitCellSize = { 1008, 1008 },\n> @@ -255,6 +354,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> @@ -268,6 +373,7 @@ 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} },\n>  \t\t{ \"ov8865\", {\n>  \t\t\t.unitCellSize = { 1400, 1400 },\n> @@ -282,6 +388,7 @@ 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} },\n>  \t\t{ \"ov13858\", {\n>  \t\t\t.unitCellSize = { 1120, 1120 },\n> @@ -289,6 +396,7 @@ 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} },\n>  \t};\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 57D9EBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Nov 2024 07:00:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 56CE06607E;\n\tWed, 27 Nov 2024 08:00:09 +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 AC82465FA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Nov 2024 08:00:07 +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 4AE96792;\n\tWed, 27 Nov 2024 07:59:44 +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=\"r+OkZv/F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732690784;\n\tbh=bs0kHDxx31E1p55ULmZyfp+u7WAFEbe1tw7cCj4iXXA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r+OkZv/F1a1MvC6N/bWfnZXNgfQSNq3/7v0dA2ilib8vRXg6sWzJtvt09kx465OWe\n\twm8Hf5ofprMIy09Gd429GOEX2PlcU/B9lqsXvjjKevySrcq1Xp/RjPWRtk50StYmor\n\tqNGPjFD8sZt85kOgpOmvidyKvKNES5GMTOGnJ8Hs=","Date":"Wed, 27 Nov 2024 08:59:56 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","Subject":"Re: [PATCH v5 1/2] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","Message-ID":"<20241127065956.GR5461@pendragon.ideasonboard.com>","References":"<20241126151203.108407-1-dan.scally@ideasonboard.com>\n\t<20241126151203.108407-2-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241126151203.108407-2-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":32412,"web_url":"https://patchwork.libcamera.org/comment/32412/","msgid":"<97bc9a45-2e9f-4c90-9894-674d5351638f@ideasonboard.com>","date":"2024-11-27T09:35:41","subject":"Re: [PATCH v5 1/2] 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 27/11/2024 06:59, Laurent Pinchart wrote:\n> On Tue, Nov 26, 2024 at 03:12:02PM +0000, Daniel Scally wrote:\n>> Add properties covering the sensor control application delays to both\n>> the static CameraSensorProperties definitions. The values used are\n>> taken from Raspberry Pi's CamHelper class definitions. Where no more\n>> specific values are known the delay struct is defined as empty and\n>> defaults supplied through the getter function.\n>>\n>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n>> ---\n>> Changes in v5:\n>>\n>> \t- Comment rewording\n>> \t- Moved the defaultSensorDelays definition inside\n>> \t  CameraSensorLegacy::sensorDelays()\n>>\n>> Changes in v4:\n>>\n>> \t- getSensorDelays() renamed to sensorDelays()\n>> \t- sensorDelays() returns a reference to the SensorDelays struct rather\n>> \t  than each of the values.\n>> \t- Reworded the comments to make the default values seem less acceptable\n>> \t- Added delay values for AR0144\n>>\n>> Changes in v3:\n>>\n>> \t- Rebased on top of the CameraSensorFactory introduction\n>> \t- Some rephrasing\n>> \t- Defined the sensorDelays member as empty where Raspberry Pi didn't\n>> \t  have any specific values. Check for the empty struct in\n>> \t  getSensorDelays() and return the defaults from there with a warning.\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       |   8 ++\n>>   src/libcamera/sensor/camera_sensor.cpp        |  14 +++\n>>   src/libcamera/sensor/camera_sensor_legacy.cpp |  25 ++++\n>>   .../sensor/camera_sensor_properties.cpp       | 108 ++++++++++++++++++\n>>   5 files changed, 157 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n>> index 8aafd82e..bbdb83a1 100644\n>> --- a/include/libcamera/internal/camera_sensor.h\n>> +++ b/include/libcamera/internal/camera_sensor.h\n>> @@ -20,6 +20,7 @@\n>>   #include <libcamera/orientation.h>\n>>   #include <libcamera/transform.h>\n>>   \n>> +#include \"libcamera/internal/camera_sensor_properties.h\"\n> Alphabetical order. I'm surprised checkstyle.py didn't warn you about\n> this.\n\n\nActually so am I - checkstyle is noisy for this patch because the CameraSensorProperties tables are \nnot its friend. I wondered if I had missed it from that noise but I don't see it there.\n\n>\n>>   #include \"libcamera/internal/bayer_format.h\"\n>>   #include \"libcamera/internal/v4l2_subdevice.h\"\n>>   \n>> @@ -73,6 +74,7 @@ public:\n>>   \tvirtual const std::vector<controls::draft::TestPatternModeEnum> &\n>>   \ttestPatternModes() const = 0;\n>>   \tvirtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0;\n>> +\tvirtual const CameraSensorProperties::SensorDelays &sensorDelays() = 0;\n>>   };\n>>   \n>>   class CameraSensorFactoryBase\n>> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n>> index 480ac121..75cb7839 100644\n>> --- a/include/libcamera/internal/camera_sensor_properties.h\n>> +++ b/include/libcamera/internal/camera_sensor_properties.h\n>> @@ -8,6 +8,7 @@\n>>   #pragma once\n>>   \n>>   #include <map>\n>> +#include <stdint.h>\n>>   #include <string>\n>>   \n>>   #include <libcamera/control_ids.h>\n>> @@ -20,6 +21,13 @@ struct CameraSensorProperties {\n>>   \n>>   \tSize unitCellSize;\n>>   \tstd::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;\n>> +\n>> +\tstruct SensorDelays {\n>> +\t\tuint8_t exposureDelay;\n>> +\t\tuint8_t gainDelay;\n>> +\t\tuint8_t vblankDelay;\n>> +\t\tuint8_t hblankDelay;\n>> +\t} sensorDelays;\n> We usually separate type definitions from usage.\n>\n> struct CameraSensorProperties {\n> \tstruct SensorDelays {\n> \t\tuint8_t exposureDelay;\n> \t\tuint8_t gainDelay;\n> \t\tuint8_t vblankDelay;\n> \t\tuint8_t hblankDelay;\n> \t};\n>\n> \tstatic const CameraSensorProperties *get(const std::string &sensor);\n>\n> \tSize unitCellSize;\n> \tstd::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;\n> \tSensorDelays sensorDelays;\n> };\nAck\n>>   };\n>>   \n>>   } /* namespace libcamera */\n>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp\n>> index 54cf98b2..9d2c2ea7 100644\n>> --- a/src/libcamera/sensor/camera_sensor.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor.cpp\n>> @@ -336,6 +336,20 @@ CameraSensor::~CameraSensor() = default;\n>>    * pattern mode for every frame thus incurs no performance penalty.\n>>    */\n>>   \n>> +/**\n>> + * \\fn CameraSensor::sensorDelays()\n>> + * \\brief Fetch the sensor delay values\n>> + *\n>> + * This function retreives the sensor control delays for pipeline handlers to\n> s/retreives/retrieves/\n\nSomewhere my primary school teacher is disappointed.\n\n\nThanks\n\nDan\n\n>> + * use to inform the DelayedControls. If control delays are not specified in the\n> We don't usually document expected users, unless there's a very tight\n> coupling between a class and its users that is relevant to understanding\n> the API.\n>\n>> + * static sensor propertie database, this function returns a reference to a set\n> s/propertie/properties/\n>\n>> + * of default sensor delays provided as best-effort placeholders for the actual\n>> + * sensor specific delays.\n> And there's a bit of implementation detail here too. You can simplify\n> this to\n>\n>   * This function retrieves the delays that the sensor applies to controls. If\n>   * the static properties database doesn't specifiy control delay values for the\n>   * sensor, default delays that may be suitable are returned and a warning is\n>   * logged.\n>\n>> + *\n>> + * \\return A reference to a struct CameraSensorProperties::SensorDelays holding\n>> + * the delay values\n> Doxygen shows the type of the return value, so you can just write\n>\n>   * \\return The sensor delay values\n>\n>> + */\n>> +\n>>   /**\n>>    * \\class CameraSensorFactoryBase\n>>    * \\brief Base class for camera sensor factories\n>> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp\n>> index a9b15c03..17d6fa68 100644\n>> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp\n>> @@ -95,6 +95,7 @@ public:\n>>   \tconst std::vector<controls::draft::TestPatternModeEnum> &\n>>   \ttestPatternModes() const override { return testPatternModes_; }\n>>   \tint setTestPatternMode(controls::draft::TestPatternModeEnum mode) override;\n>> +\tconst CameraSensorProperties::SensorDelays &sensorDelays() override;\n>>   \n>>   protected:\n>>   \tstd::string logPrefix() const override;\n>> @@ -482,6 +483,30 @@ void CameraSensorLegacy::initStaticProperties()\n>>   \tinitTestPatternModes();\n>>   }\n>>   \n>> +const CameraSensorProperties::SensorDelays &CameraSensorLegacy::sensorDelays()\n>> +{\n>> +\tstatic constexpr CameraSensorProperties::SensorDelays defaultSensorDelays = {\n>> +\t\t.exposureDelay = 2,\n>> +\t\t.gainDelay = 1,\n>> +\t\t.vblankDelay = 2,\n>> +\t\t.hblankDelay = 2,\n>> +\t};\n>> +\n>> +\tif (!staticProps_ ||\n>> +\t    (!staticProps_->sensorDelays.exposureDelay &&\n>> +\t     !staticProps_->sensorDelays.gainDelay &&\n>> +\t     !staticProps_->sensorDelays.vblankDelay &&\n>> +\t     !staticProps_->sensorDelays.hblankDelay)) {\n>> +\t\tLOG(CameraSensor, Warning)\n>> +\t\t\t<< \"No sensor delays found in static properties. \"\n>> +\t\t\t   \"Assuming unverified defaults.\";\n>> +\n>> +\t\treturn defaultSensorDelays;\n>> +\t}\n>> +\n>> +\treturn staticProps_->sensorDelays;\n>> +}\n>> +\n>>   void CameraSensorLegacy::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 e2305166..3dc8cf05 100644\n>> --- a/src/libcamera/sensor/camera_sensor_properties.cpp\n>> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp\n>> @@ -41,6 +41,36 @@ 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 Sensor control application delays.\n> checkstyle.py flagged this.\n>\n>> + *\n>> + * This struct may be defined as empty if the actual sensor delays are not\n> s/struct/structure/\n>\n>> + * available or have not been measured. Best effort default values are returned\n>> + * in this case.\n> In this context, it's not clear where the defaults are returned from.\n> I'd drop the last sentence.\n>\n>> + */\n>> +\n>> +/**\n>> + * \\struct CameraSensorProperties::SensorDelays\n>> + * \\brief Sensor control application delay values\n>> + *\n>> + * This struct holds delay values, expressed in number of frames, between the\n> s/struct/structure/\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n>> + * time a control value is applied to the sensor and the time that value is\n>> + * reflected in the output. For example \"2 frames delay\" means that parameters\n>> + * set during frame N will take effect for frame N+2 (and by extension a delay\n>> + * of 0 would mean the parameter is applied immediately to the current frame).\n>> + *\n>> + * \\var CameraSensorProperties::SensorDelays::exposureDelay\n>> + * \\brief Number of frames between application of exposure control and effect\n>> + *\n>> + * \\var CameraSensorProperties::SensorDelays::gainDelay\n>> + * \\brief Number of frames between application of analogue gain control and effect\n>> + *\n>> + * \\var CameraSensorProperties::SensorDelays::vblankDelay\n>> + * \\brief Number of frames between application of vblank control and effect\n>> + *\n>> + * \\var CameraSensorProperties::SensorDelays::hblankDelay\n>> + * \\brief Number of frames between application of hblank control and effect\n>>    */\n>>   \n>>   /**\n>> @@ -60,6 +90,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 = 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{ \"ar0521\", {\n>>   \t\t\t.unitCellSize = { 2200, 2200 },\n>> @@ -69,6 +105,7 @@ 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} },\n>>   \t\t{ \"hi846\", {\n>>   \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -87,6 +124,7 @@ 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} },\n>>   \t\t{ \"imx214\", {\n>>   \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -97,6 +135,7 @@ 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} },\n>>   \t\t{ \"imx219\", {\n>>   \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -107,6 +146,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,38 +162,67 @@ 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} },\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} },\n>>   \t\t{ \"imx335\", {\n>>   \t\t\t.unitCellSize = { 2000, 2000 },\n>>   \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = { },\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} },\n>>   \t\t{ \"imx462\", {\n>>   \t\t\t.unitCellSize = { 2900, 2900 },\n>>   \t\t\t.testPatternModes = {},\n>> +\t\t\t.sensorDelays = { },\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>> @@ -161,6 +235,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>> @@ -171,6 +251,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>> @@ -185,6 +271,7 @@ 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} },\n>>   \t\t{ \"ov2740\", {\n>>   \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -192,6 +279,7 @@ 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} },\n>>   \t\t{ \"ov4689\", {\n>>   \t\t\t.unitCellSize = { 2000, 2000 },\n>> @@ -205,6 +293,7 @@ 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} },\n>>   \t\t{ \"ov5640\", {\n>>   \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -212,10 +301,17 @@ 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} },\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>> @@ -223,6 +319,7 @@ 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} },\n>>   \t\t{ \"ov5675\", {\n>>   \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -230,6 +327,7 @@ 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} },\n>>   \t\t{ \"ov5693\", {\n>>   \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -242,6 +340,7 @@ 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} },\n>>   \t\t{ \"ov64a40\", {\n>>   \t\t\t.unitCellSize = { 1008, 1008 },\n>> @@ -255,6 +354,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>> @@ -268,6 +373,7 @@ 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} },\n>>   \t\t{ \"ov8865\", {\n>>   \t\t\t.unitCellSize = { 1400, 1400 },\n>> @@ -282,6 +388,7 @@ 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} },\n>>   \t\t{ \"ov13858\", {\n>>   \t\t\t.unitCellSize = { 1120, 1120 },\n>> @@ -289,6 +396,7 @@ 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} },\n>>   \t};\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6F590C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Nov 2024 09:35:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9DDF6660C4;\n\tWed, 27 Nov 2024 10:35:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8ED5F660B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Nov 2024 10:35:44 +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 6F4AD792;\n\tWed, 27 Nov 2024 10:35:21 +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=\"mVG6fhqF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732700121;\n\tbh=kaHsZij+4dpAGclTYlWnXuVK5BSvmZADBs4nGl1CETg=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=mVG6fhqFA3JSZ8jLbYBjYcbUuExFrr3O7zmiM0xyQqJs077nh0DiclJiJdmHzeJIL\n\t1PfkdLmXEyrRkEoqwdyDu0rOKjuHFE+jOsekOSTQnSHD4oevv/fGrJo6I4BBmWb7Ce\n\tDYjD2jLyDSLOdBTAGbpUCXkjLVW9gA/AW252m2Sg=","Message-ID":"<97bc9a45-2e9f-4c90-9894-674d5351638f@ideasonboard.com>","Date":"Wed, 27 Nov 2024 09:35:41 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v5 1/2] libcamera: camera_sensor_properties: Add sensor\n\tcontrol delays","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, mike.rudenko@gmail.com","References":"<20241126151203.108407-1-dan.scally@ideasonboard.com>\n\t<20241126151203.108407-2-dan.scally@ideasonboard.com>\n\t<20241127065956.GR5461@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","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":"<20241127065956.GR5461@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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]