[{"id":36844,"web_url":"https://patchwork.libcamera.org/comment/36844/","msgid":"<176315318770.83626.11045797068827695003@localhost>","date":"2025-11-14T20:46:27","subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nQuoting Jacopo Mondi (2025-11-14 15:17:01)\n> There is a margin between the maximum achievable shutter time and the frame\n> length. The value is sensor-dependent and should be used to adjust the\n> maximum achievable exposure time whenever the frame length changes.\n> \n> Introduce in the CameraSensorHelper the notion of exposureMargin_ as the\n> minimum difference in number of lines between the current frame length and\n> the maximum achievable shutter time and the maxShutterTime() function that\n> returns the maximum achievable shutter time give the current maximum frame\n> length.\n> \n> This feature is already implemented in the RPi CamHelper class hierarchy\n> with the name of frameIntegrationDiff.\n> \n> Populate the CameraSensorHelper instances with an exposureMargin_. The\n> value of the exposure margin comes from the mainline driver version of each\n> sensor, and it has been compared with the frameIntegrationDiff_ value in\n> the rpi camera helpers.\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/ipa/libipa/camera_sensor_helper.cpp | 80 +++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/camera_sensor_helper.h   |  4 ++\n>  2 files changed, 84 insertions(+)\n> \n> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> index e3e3e535541384ba862ba2045114a69db7efff34..3c3e24c1cdefa4bca059352482bb29654a37b42f 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> @@ -154,6 +154,36 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n>         return g;\n>  }\n>  \n> +/**\n> + * \\brief Compute the maximum shutter time given the maximum frame duration\n> + * \\param[in] maxFrameDuration The maximum frame duration\n> + * \\param[in] lineDuration The current sensor line duration\n> + *\n> + * This function returns the maximum achievable shutter time by subtracting to\n> + * \\a maxFrameDuration the difference between the frame length and the maximum\n> + * achievable integration time.\n> + *\n> + * \\todo The line duration should be a property of the CameraSensorHelper class\n> + * instead of being provided by the IPA.\n> + *\n> + * \\return The maximum achievable shutter time for the current sensor\n> + * configuration\n> + */\n> +utils::Duration CameraSensorHelper::maxShutterTime(utils::Duration maxFrameDuration,\n> +                                                  utils::Duration lineDuration) const\n\nA little bikeshedding on the name. In commit d0478c41f432 (\"libcamera:\nRename \"shutter speed\" to \"exposure time\"\") all occurrences of shutter\nwere replaced by exposure. Should this be renamed accordingly?\n\nOtherwise the patch looks good to me.\n\nBest regards,\nStefan\n\n> +{\n> +       /* Use a static to rate-limit the error message. */\n> +       static uint32_t exposureMargin = exposureMargin_.has_value()\n> +                                      ? exposureMargin_.value() : 0;\n> +       if (!exposureMargin_.has_value() && !exposureMargin) {\n> +               LOG(CameraSensorHelper, Warning)\n> +                       << \"Exposure margin not known. Default to 4\";\n> +               exposureMargin = 4;\n> +       }\n> +\n> +       return maxFrameDuration - exposureMargin * lineDuration;\n> +}\n> +\n>  /**\n>   * \\struct CameraSensorHelper::AnalogueGainLinear\n>   * \\brief Analogue gain constants for the linear gain model\n> @@ -229,6 +259,13 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n>   * sensor specific. Use this variable to store the values at init time.\n>   */\n>  \n> +/**\n> + * \\var CameraSensorHelper::exposureMargin_\n> + * \\brief The smallest margin between the integration time and the frame lenght\n> + * in lines\n> + * \\sa CameraSensorHelper::maxShutterTime()\n> + */\n> +\n>  /**\n>   * \\class CameraSensorHelperFactoryBase\n>   * \\brief Base class for camera sensor helper factories\n> @@ -385,6 +422,7 @@ public:\n>         {\n>                 /* Power-on default value: 168 at 12bits. */\n>                 blackLevel_ = 2688;\n> +               exposureMargin_ = 4;\n>         }\n>  \n>         uint32_t gainCode(double gain) const override\n> @@ -474,6 +512,11 @@ REGISTER_CAMERA_SENSOR_HELPER(\"ar0144\", CameraSensorHelperAr0144)\n>  class CameraSensorHelperAr0521 : public CameraSensorHelper\n>  {\n>  public:\n> +       CameraSensorHelperAr0521()\n> +       {\n> +               exposureMargin_ = 4;\n> +       }\n> +\n>         uint32_t gainCode(double gain) const override\n>         {\n>                 gain = std::clamp(gain, 1.0, 15.5);\n> @@ -504,6 +547,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"gc05a2\", CameraSensorHelperGc05a2)\n> @@ -516,6 +560,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> +               exposureMargin_ = 16;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"gc08a3\", CameraSensorHelperGc08a3)\n> @@ -526,6 +571,7 @@ public:\n>         CameraSensorHelperHm1246()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };\n> +               exposureMargin_ = 2;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"hm1246\", CameraSensorHelperHm1246)\n> @@ -538,6 +584,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> +               exposureMargin_ = 10;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx214\", CameraSensorHelperImx214)\n> @@ -550,6 +597,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 256, -1, 256 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx219\", CameraSensorHelperImx219)\n> @@ -562,6 +610,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> +               exposureMargin_ = 10;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx258\", CameraSensorHelperImx258)\n> @@ -574,6 +623,7 @@ public:\n>                 /* From datasheet: 0x32 at 10bits. */\n>                 blackLevel_ = 3200;\n>                 gain_ = AnalogueGainLinear{ 0, 2048, -1, 2048 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx283\", CameraSensorHelperImx283)\n> @@ -586,6 +636,7 @@ public:\n>                 /* From datasheet: 0xf0 at 12bits. */\n>                 blackLevel_ = 3840;\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 2;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx290\", CameraSensorHelperImx290)\n> @@ -596,6 +647,11 @@ public:\n>         CameraSensorHelperImx296()\n>         {\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.1) };\n> +               /*\n> +                * The driver doesn't apply any margin. Use the value\n> +                * in RPi's CamHelper.\n> +                */\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx296\", CameraSensorHelperImx296)\n> @@ -613,6 +669,7 @@ public:\n>                 /* From datasheet: 0x32 at 10bits. */\n>                 blackLevel_ = 3200;\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 9;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> @@ -623,6 +680,7 @@ public:\n>         CameraSensorHelperImx415()\n>         {\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> @@ -638,6 +696,7 @@ public:\n>         CameraSensorHelperImx477()\n>         {\n>                 gain_ = AnalogueGainLinear{ 0, 1024, -1, 1024 };\n> +               exposureMargin_ = 22;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx477\", CameraSensorHelperImx477)\n> @@ -663,6 +722,7 @@ public:\n>                  * This has been validated with some empirical testing only.\n>                  */\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov2685\", CameraSensorHelperOv2685)\n> @@ -673,6 +733,7 @@ public:\n>         CameraSensorHelperOv2740()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov2740\", CameraSensorHelperOv2740)\n> @@ -685,6 +746,7 @@ public:\n>                 /* From datasheet: 0x40 at 12bits. */\n>                 blackLevel_ = 1024;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov4689\", CameraSensorHelperOv4689)\n> @@ -697,6 +759,14 @@ public:\n>                 /* From datasheet: 0x10 at 10bits. */\n>                 blackLevel_ = 1024;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               /*\n> +                * Very convoluted in the driver that however applies a margin\n> +                * of 4 lines when setting vts.\n> +                *\n> +                *      cap_vts = cap_shutter + 4;\n> +                *      ret = ov5640_set_vts(sensor, cap_vts);\n> +                */\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5640\", CameraSensorHelperOv5640)\n> @@ -707,6 +777,7 @@ public:\n>         CameraSensorHelperOv5647()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5647\", CameraSensorHelperOv5647)\n> @@ -717,6 +788,7 @@ public:\n>         CameraSensorHelperOv5670()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5670\", CameraSensorHelperOv5670)\n> @@ -729,6 +801,7 @@ public:\n>                 /* From Linux kernel driver: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5675\", CameraSensorHelperOv5675)\n> @@ -739,6 +812,7 @@ public:\n>         CameraSensorHelperOv5693()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5693\", CameraSensorHelperOv5693)\n> @@ -749,6 +823,7 @@ public:\n>         CameraSensorHelperOv64a40()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 32;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov64a40\", CameraSensorHelperOv64a40)\n> @@ -765,6 +840,7 @@ public:\n>                  * See: https://patchwork.linuxtv.org/project/linux-media/patch/20221106171129.166892-2-nicholas@rothemail.net/#142267\n>                  */\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov8858\", CameraSensorHelperOv8858)\n> @@ -775,6 +851,7 @@ public:\n>         CameraSensorHelperOv8865()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov8865\", CameraSensorHelperOv8865)\n> @@ -785,6 +862,7 @@ public:\n>         CameraSensorHelperOv13858()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov13858\", CameraSensorHelperOv13858)\n> @@ -797,6 +875,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> +               exposureMargin_ = 64;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"vd55g1\", CameraSensorHelperVd55g1)\n> @@ -809,6 +888,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> +               exposureMargin_ = 75;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"vd56g3\", CameraSensorHelperVd56g3)\n> diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h\n> index bd3d0beec77f006b68fecf45eee850c5283fefa5..a1cf4bc334ad3b9a51d26b345bd5f0630c7ae87c 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.h\n> +++ b/src/ipa/libipa/camera_sensor_helper.h\n> @@ -15,6 +15,7 @@\n>  #include <vector>\n>  \n>  #include <libcamera/base/class.h>\n> +#include <libcamera/base/utils.h>\n>  \n>  namespace libcamera {\n>  \n> @@ -30,6 +31,8 @@ public:\n>         virtual uint32_t gainCode(double gain) const;\n>         virtual double gain(uint32_t gainCode) const;\n>         double quantizeGain(double gain, double *quantizationGain) const;\n> +       utils::Duration maxShutterTime(utils::Duration maxFrameDuration,\n> +                                      utils::Duration lineDuration) const;\n>  \n>  protected:\n>         struct AnalogueGainLinear {\n> @@ -46,6 +49,7 @@ protected:\n>  \n>         std::optional<int16_t> blackLevel_;\n>         std::variant<std::monostate, AnalogueGainLinear, AnalogueGainExp> gain_;\n> +       std::optional<uint32_t> exposureMargin_;\n>  \n>  private:\n>         LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelper)\n> \n> -- \n> 2.51.1\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 C789AC3263\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 Nov 2025 20:46:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10C69609D8;\n\tFri, 14 Nov 2025 21:46:33 +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 A517C606E6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 Nov 2025 21:46:30 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:3cb4:651a:fe0:73ab])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6A8AE2D8;\n\tFri, 14 Nov 2025 21:44:29 +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=\"nPrhbbpw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763153069;\n\tbh=Oa/jQ2/ALROiAxmLagJ/cxzLq4JyzC5rMGVPbH/0DCs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=nPrhbbpw7dqRwPUVe7btKmFZkl4TJQYltleKRmvyBCGRLNCKnPNqYeYn0nobZVpb7\n\tbC6RZ7bPwYUrjdrGQsAZDUhdsws6w/gxHDPPUh/UROzIK0vjcFpldk5nE5yam5ue1n\n\tC/oNGRyCuv3bT5myzrP9MRDPeVTNZ2skSpxD+9s4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251114-exposure-limits-v3-6-b7c07feba026@ideasonboard.com>","References":"<20251114-exposure-limits-v3-0-b7c07feba026@ideasonboard.com>\n\t<20251114-exposure-limits-v3-6-b7c07feba026@ideasonboard.com>","Subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","From":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Niklas =?utf-8?b?U8O2?=\n\t=?utf-8?q?derlund?= <niklas.soderlund@ragnatech.se>, Robert Mader\n\t<robert.mader@collabora.com>, libcamera-devel@lists.libcamera.org","Date":"Fri, 14 Nov 2025 21:46:27 +0100","Message-ID":"<176315318770.83626.11045797068827695003@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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":36845,"web_url":"https://patchwork.libcamera.org/comment/36845/","msgid":"<mghssg3tepvdo4tjuledgwcb6pievujrztlmu4dzs63sjuuby7@shljclo3db6h>","date":"2025-11-15T18:10:04","subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Stefan\n\nOn Fri, Nov 14, 2025 at 09:46:27PM +0100, Stefan Klug wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> Quoting Jacopo Mondi (2025-11-14 15:17:01)\n> > There is a margin between the maximum achievable shutter time and the frame\n> > length. The value is sensor-dependent and should be used to adjust the\n> > maximum achievable exposure time whenever the frame length changes.\n> >\n> > Introduce in the CameraSensorHelper the notion of exposureMargin_ as the\n> > minimum difference in number of lines between the current frame length and\n> > the maximum achievable shutter time and the maxShutterTime() function that\n> > returns the maximum achievable shutter time give the current maximum frame\n> > length.\n> >\n> > This feature is already implemented in the RPi CamHelper class hierarchy\n> > with the name of frameIntegrationDiff.\n> >\n> > Populate the CameraSensorHelper instances with an exposureMargin_. The\n> > value of the exposure margin comes from the mainline driver version of each\n> > sensor, and it has been compared with the frameIntegrationDiff_ value in\n> > the rpi camera helpers.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  src/ipa/libipa/camera_sensor_helper.cpp | 80 +++++++++++++++++++++++++++++++++\n> >  src/ipa/libipa/camera_sensor_helper.h   |  4 ++\n> >  2 files changed, 84 insertions(+)\n> >\n> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> > index e3e3e535541384ba862ba2045114a69db7efff34..3c3e24c1cdefa4bca059352482bb29654a37b42f 100644\n> > --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> > @@ -154,6 +154,36 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n> >         return g;\n> >  }\n> >\n> > +/**\n> > + * \\brief Compute the maximum shutter time given the maximum frame duration\n> > + * \\param[in] maxFrameDuration The maximum frame duration\n> > + * \\param[in] lineDuration The current sensor line duration\n> > + *\n> > + * This function returns the maximum achievable shutter time by subtracting to\n> > + * \\a maxFrameDuration the difference between the frame length and the maximum\n> > + * achievable integration time.\n> > + *\n> > + * \\todo The line duration should be a property of the CameraSensorHelper class\n> > + * instead of being provided by the IPA.\n> > + *\n> > + * \\return The maximum achievable shutter time for the current sensor\n> > + * configuration\n> > + */\n> > +utils::Duration CameraSensorHelper::maxShutterTime(utils::Duration maxFrameDuration,\n> > +                                                  utils::Duration lineDuration) const\n>\n> A little bikeshedding on the name. In commit d0478c41f432 (\"libcamera:\n> Rename \"shutter speed\" to \"exposure time\"\") all occurrences of shutter\n> were replaced by exposure. Should this be renamed accordingly?\n\nYou know, I was pretty sure we were going in the oppostite direction\nand we were slowly replacing all usage of \"exposure time\" with\n\"shutter time\"!\n\nI'll switch back to exposure time everywhere in this series and also\nupdate the Mali IPA which uses \"shutter time\" for its variables.\n\nThanks for noticing!\n   j\n\n>\n> Otherwise the patch looks good to me.\n>\n> Best regards,\n> Stefan\n>\n> > +{\n> > +       /* Use a static to rate-limit the error message. */\n> > +       static uint32_t exposureMargin = exposureMargin_.has_value()\n> > +                                      ? exposureMargin_.value() : 0;\n> > +       if (!exposureMargin_.has_value() && !exposureMargin) {\n> > +               LOG(CameraSensorHelper, Warning)\n> > +                       << \"Exposure margin not known. Default to 4\";\n> > +               exposureMargin = 4;\n> > +       }\n> > +\n> > +       return maxFrameDuration - exposureMargin * lineDuration;\n> > +}\n> > +\n> >  /**\n> >   * \\struct CameraSensorHelper::AnalogueGainLinear\n> >   * \\brief Analogue gain constants for the linear gain model\n> > @@ -229,6 +259,13 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n> >   * sensor specific. Use this variable to store the values at init time.\n> >   */\n> >\n> > +/**\n> > + * \\var CameraSensorHelper::exposureMargin_\n> > + * \\brief The smallest margin between the integration time and the frame lenght\n> > + * in lines\n> > + * \\sa CameraSensorHelper::maxShutterTime()\n> > + */\n> > +\n> >  /**\n> >   * \\class CameraSensorHelperFactoryBase\n> >   * \\brief Base class for camera sensor helper factories\n> > @@ -385,6 +422,7 @@ public:\n> >         {\n> >                 /* Power-on default value: 168 at 12bits. */\n> >                 blackLevel_ = 2688;\n> > +               exposureMargin_ = 4;\n> >         }\n> >\n> >         uint32_t gainCode(double gain) const override\n> > @@ -474,6 +512,11 @@ REGISTER_CAMERA_SENSOR_HELPER(\"ar0144\", CameraSensorHelperAr0144)\n> >  class CameraSensorHelperAr0521 : public CameraSensorHelper\n> >  {\n> >  public:\n> > +       CameraSensorHelperAr0521()\n> > +       {\n> > +               exposureMargin_ = 4;\n> > +       }\n> > +\n> >         uint32_t gainCode(double gain) const override\n> >         {\n> >                 gain = std::clamp(gain, 1.0, 15.5);\n> > @@ -504,6 +547,7 @@ public:\n> >                 /* From datasheet: 64 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"gc05a2\", CameraSensorHelperGc05a2)\n> > @@ -516,6 +560,7 @@ public:\n> >                 /* From datasheet: 64 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> > +               exposureMargin_ = 16;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"gc08a3\", CameraSensorHelperGc08a3)\n> > @@ -526,6 +571,7 @@ public:\n> >         CameraSensorHelperHm1246()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };\n> > +               exposureMargin_ = 2;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"hm1246\", CameraSensorHelperHm1246)\n> > @@ -538,6 +584,7 @@ public:\n> >                 /* From datasheet: 64 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> > +               exposureMargin_ = 10;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx214\", CameraSensorHelperImx214)\n> > @@ -550,6 +597,7 @@ public:\n> >                 /* From datasheet: 64 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 0, 256, -1, 256 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx219\", CameraSensorHelperImx219)\n> > @@ -562,6 +610,7 @@ public:\n> >                 /* From datasheet: 0x40 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> > +               exposureMargin_ = 10;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx258\", CameraSensorHelperImx258)\n> > @@ -574,6 +623,7 @@ public:\n> >                 /* From datasheet: 0x32 at 10bits. */\n> >                 blackLevel_ = 3200;\n> >                 gain_ = AnalogueGainLinear{ 0, 2048, -1, 2048 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx283\", CameraSensorHelperImx283)\n> > @@ -586,6 +636,7 @@ public:\n> >                 /* From datasheet: 0xf0 at 12bits. */\n> >                 blackLevel_ = 3840;\n> >                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> > +               exposureMargin_ = 2;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx290\", CameraSensorHelperImx290)\n> > @@ -596,6 +647,11 @@ public:\n> >         CameraSensorHelperImx296()\n> >         {\n> >                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.1) };\n> > +               /*\n> > +                * The driver doesn't apply any margin. Use the value\n> > +                * in RPi's CamHelper.\n> > +                */\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx296\", CameraSensorHelperImx296)\n> > @@ -613,6 +669,7 @@ public:\n> >                 /* From datasheet: 0x32 at 10bits. */\n> >                 blackLevel_ = 3200;\n> >                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> > +               exposureMargin_ = 9;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> > @@ -623,6 +680,7 @@ public:\n> >         CameraSensorHelperImx415()\n> >         {\n> >                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> > @@ -638,6 +696,7 @@ public:\n> >         CameraSensorHelperImx477()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 0, 1024, -1, 1024 };\n> > +               exposureMargin_ = 22;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"imx477\", CameraSensorHelperImx477)\n> > @@ -663,6 +722,7 @@ public:\n> >                  * This has been validated with some empirical testing only.\n> >                  */\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov2685\", CameraSensorHelperOv2685)\n> > @@ -673,6 +733,7 @@ public:\n> >         CameraSensorHelperOv2740()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov2740\", CameraSensorHelperOv2740)\n> > @@ -685,6 +746,7 @@ public:\n> >                 /* From datasheet: 0x40 at 12bits. */\n> >                 blackLevel_ = 1024;\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov4689\", CameraSensorHelperOv4689)\n> > @@ -697,6 +759,14 @@ public:\n> >                 /* From datasheet: 0x10 at 10bits. */\n> >                 blackLevel_ = 1024;\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> > +               /*\n> > +                * Very convoluted in the driver that however applies a margin\n> > +                * of 4 lines when setting vts.\n> > +                *\n> > +                *      cap_vts = cap_shutter + 4;\n> > +                *      ret = ov5640_set_vts(sensor, cap_vts);\n> > +                */\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov5640\", CameraSensorHelperOv5640)\n> > @@ -707,6 +777,7 @@ public:\n> >         CameraSensorHelperOv5647()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov5647\", CameraSensorHelperOv5647)\n> > @@ -717,6 +788,7 @@ public:\n> >         CameraSensorHelperOv5670()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov5670\", CameraSensorHelperOv5670)\n> > @@ -729,6 +801,7 @@ public:\n> >                 /* From Linux kernel driver: 0x40 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov5675\", CameraSensorHelperOv5675)\n> > @@ -739,6 +812,7 @@ public:\n> >         CameraSensorHelperOv5693()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov5693\", CameraSensorHelperOv5693)\n> > @@ -749,6 +823,7 @@ public:\n> >         CameraSensorHelperOv64a40()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 32;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov64a40\", CameraSensorHelperOv64a40)\n> > @@ -765,6 +840,7 @@ public:\n> >                  * See: https://patchwork.linuxtv.org/project/linux-media/patch/20221106171129.166892-2-nicholas@rothemail.net/#142267\n> >                  */\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 4;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov8858\", CameraSensorHelperOv8858)\n> > @@ -775,6 +851,7 @@ public:\n> >         CameraSensorHelperOv8865()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov8865\", CameraSensorHelperOv8865)\n> > @@ -785,6 +862,7 @@ public:\n> >         CameraSensorHelperOv13858()\n> >         {\n> >                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> > +               exposureMargin_ = 8;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"ov13858\", CameraSensorHelperOv13858)\n> > @@ -797,6 +875,7 @@ public:\n> >                 /* From datasheet: 0x40 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> > +               exposureMargin_ = 64;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"vd55g1\", CameraSensorHelperVd55g1)\n> > @@ -809,6 +888,7 @@ public:\n> >                 /* From datasheet: 0x40 at 10bits. */\n> >                 blackLevel_ = 4096;\n> >                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> > +               exposureMargin_ = 75;\n> >         }\n> >  };\n> >  REGISTER_CAMERA_SENSOR_HELPER(\"vd56g3\", CameraSensorHelperVd56g3)\n> > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h\n> > index bd3d0beec77f006b68fecf45eee850c5283fefa5..a1cf4bc334ad3b9a51d26b345bd5f0630c7ae87c 100644\n> > --- a/src/ipa/libipa/camera_sensor_helper.h\n> > +++ b/src/ipa/libipa/camera_sensor_helper.h\n> > @@ -15,6 +15,7 @@\n> >  #include <vector>\n> >\n> >  #include <libcamera/base/class.h>\n> > +#include <libcamera/base/utils.h>\n> >\n> >  namespace libcamera {\n> >\n> > @@ -30,6 +31,8 @@ public:\n> >         virtual uint32_t gainCode(double gain) const;\n> >         virtual double gain(uint32_t gainCode) const;\n> >         double quantizeGain(double gain, double *quantizationGain) const;\n> > +       utils::Duration maxShutterTime(utils::Duration maxFrameDuration,\n> > +                                      utils::Duration lineDuration) const;\n> >\n> >  protected:\n> >         struct AnalogueGainLinear {\n> > @@ -46,6 +49,7 @@ protected:\n> >\n> >         std::optional<int16_t> blackLevel_;\n> >         std::variant<std::monostate, AnalogueGainLinear, AnalogueGainExp> gain_;\n> > +       std::optional<uint32_t> exposureMargin_;\n> >\n> >  private:\n> >         LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelper)\n> >\n> > --\n> > 2.51.1\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 61EADC3241\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 15 Nov 2025 18:10:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66CD960A8B;\n\tSat, 15 Nov 2025 19:10:12 +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 B4C9E608CF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 15 Nov 2025 19:10:10 +0100 (CET)","from ideasonboard.com (mob-5-90-140-85.net.vodafone.it\n\t[5.90.140.85])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3629382A;\n\tSat, 15 Nov 2025 19:08:07 +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=\"mZo1hd2g\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763230088;\n\tbh=ew6tkad5Hyojta0qCB2ZaqkdFzlQfrsEbhLJCCd1l78=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mZo1hd2g9kZUlbn9qJOTTvZ+SRtBQwZqPCkHglRHECLLDq2YnBHOvAdBv4EtLbUUi\n\tJVq7O9wx7jZnp52JK7LWV7A5TKEEgjjXMel9RaFBux+nMK4JqarqpF0Fdb1i2mnJg3\n\tGKD8nF6iIpAeFJQj1pMxya1d8IIW3Aq1SEWnCB+8=","Date":"Sat, 15 Nov 2025 19:10:04 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,  Niklas =?utf-8?b?U8O2?=\n\t=?utf-8?q?derlund?= <niklas.soderlund@ragnatech.se>, Robert Mader\n\t<robert.mader@collabora.com>,  libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","Message-ID":"<mghssg3tepvdo4tjuledgwcb6pievujrztlmu4dzs63sjuuby7@shljclo3db6h>","References":"<20251114-exposure-limits-v3-0-b7c07feba026@ideasonboard.com>\n\t<20251114-exposure-limits-v3-6-b7c07feba026@ideasonboard.com>\n\t<176315318770.83626.11045797068827695003@localhost>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<176315318770.83626.11045797068827695003@localhost>","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":38120,"web_url":"https://patchwork.libcamera.org/comment/38120/","msgid":"<177039183855.607498.2115337495460049614@neptunite.rasen.tech>","date":"2026-02-06T15:30:38","subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-11-14 23:17:01)\n> There is a margin between the maximum achievable shutter time and the frame\n> length. The value is sensor-dependent and should be used to adjust the\n> maximum achievable exposure time whenever the frame length changes.\n> \n> Introduce in the CameraSensorHelper the notion of exposureMargin_ as the\n> minimum difference in number of lines between the current frame length and\n> the maximum achievable shutter time and the maxShutterTime() function that\n> returns the maximum achievable shutter time give the current maximum frame\n> length.\n> \n> This feature is already implemented in the RPi CamHelper class hierarchy\n> with the name of frameIntegrationDiff.\n> \n> Populate the CameraSensorHelper instances with an exposureMargin_. The\n> value of the exposure margin comes from the mainline driver version of each\n> sensor, and it has been compared with the frameIntegrationDiff_ value in\n> the rpi camera helpers.\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/ipa/libipa/camera_sensor_helper.cpp | 80 +++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/camera_sensor_helper.h   |  4 ++\n>  2 files changed, 84 insertions(+)\n> \n> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\n> index e3e3e535541384ba862ba2045114a69db7efff34..3c3e24c1cdefa4bca059352482bb29654a37b42f 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.cpp\n> +++ b/src/ipa/libipa/camera_sensor_helper.cpp\n> @@ -154,6 +154,36 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n>         return g;\n>  }\n>  \n> +/**\n> + * \\brief Compute the maximum shutter time given the maximum frame duration\n> + * \\param[in] maxFrameDuration The maximum frame duration\n> + * \\param[in] lineDuration The current sensor line duration\n> + *\n> + * This function returns the maximum achievable shutter time by subtracting to\n\ns/to/from/\n\n> + * \\a maxFrameDuration the difference between the frame length and the maximum\n> + * achievable integration time.\n> + *\n> + * \\todo The line duration should be a property of the CameraSensorHelper class\n> + * instead of being provided by the IPA.\n> + *\n> + * \\return The maximum achievable shutter time for the current sensor\n> + * configuration\n> + */\n> +utils::Duration CameraSensorHelper::maxShutterTime(utils::Duration maxFrameDuration,\n\nI'll let other people bikeshed the name :)\n\n> +                                                  utils::Duration lineDuration) const\n> +{\n> +       /* Use a static to rate-limit the error message. */\n> +       static uint32_t exposureMargin = exposureMargin_.has_value()\n> +                                      ? exposureMargin_.value() : 0;\n> +       if (!exposureMargin_.has_value() && !exposureMargin) {\n> +               LOG(CameraSensorHelper, Warning)\n> +                       << \"Exposure margin not known. Default to 4\";\n> +               exposureMargin = 4;\n> +       }\n> +\n> +       return maxFrameDuration - exposureMargin * lineDuration;\n> +}\n> +\n>  /**\n>   * \\struct CameraSensorHelper::AnalogueGainLinear\n>   * \\brief Analogue gain constants for the linear gain model\n> @@ -229,6 +259,13 @@ double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain)\n>   * sensor specific. Use this variable to store the values at init time.\n>   */\n>  \n> +/**\n> + * \\var CameraSensorHelper::exposureMargin_\n> + * \\brief The smallest margin between the integration time and the frame lenght\n\ns/lenght/length/\n\nOtherwise, looks good to me.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> + * in lines\n> + * \\sa CameraSensorHelper::maxShutterTime()\n> + */\n> +\n>  /**\n>   * \\class CameraSensorHelperFactoryBase\n>   * \\brief Base class for camera sensor helper factories\n> @@ -385,6 +422,7 @@ public:\n>         {\n>                 /* Power-on default value: 168 at 12bits. */\n>                 blackLevel_ = 2688;\n> +               exposureMargin_ = 4;\n>         }\n>  \n>         uint32_t gainCode(double gain) const override\n> @@ -474,6 +512,11 @@ REGISTER_CAMERA_SENSOR_HELPER(\"ar0144\", CameraSensorHelperAr0144)\n>  class CameraSensorHelperAr0521 : public CameraSensorHelper\n>  {\n>  public:\n> +       CameraSensorHelperAr0521()\n> +       {\n> +               exposureMargin_ = 4;\n> +       }\n> +\n>         uint32_t gainCode(double gain) const override\n>         {\n>                 gain = std::clamp(gain, 1.0, 15.5);\n> @@ -504,6 +547,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"gc05a2\", CameraSensorHelperGc05a2)\n> @@ -516,6 +560,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 100, 0, 0, 1024 };\n> +               exposureMargin_ = 16;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"gc08a3\", CameraSensorHelperGc08a3)\n> @@ -526,6 +571,7 @@ public:\n>         CameraSensorHelperHm1246()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 16, 0, 16 };\n> +               exposureMargin_ = 2;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"hm1246\", CameraSensorHelperHm1246)\n> @@ -538,6 +584,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> +               exposureMargin_ = 10;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx214\", CameraSensorHelperImx214)\n> @@ -550,6 +597,7 @@ public:\n>                 /* From datasheet: 64 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 256, -1, 256 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx219\", CameraSensorHelperImx219)\n> @@ -562,6 +610,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 512, -1, 512 };\n> +               exposureMargin_ = 10;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx258\", CameraSensorHelperImx258)\n> @@ -574,6 +623,7 @@ public:\n>                 /* From datasheet: 0x32 at 10bits. */\n>                 blackLevel_ = 3200;\n>                 gain_ = AnalogueGainLinear{ 0, 2048, -1, 2048 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx283\", CameraSensorHelperImx283)\n> @@ -586,6 +636,7 @@ public:\n>                 /* From datasheet: 0xf0 at 12bits. */\n>                 blackLevel_ = 3840;\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 2;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx290\", CameraSensorHelperImx290)\n> @@ -596,6 +647,11 @@ public:\n>         CameraSensorHelperImx296()\n>         {\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.1) };\n> +               /*\n> +                * The driver doesn't apply any margin. Use the value\n> +                * in RPi's CamHelper.\n> +                */\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx296\", CameraSensorHelperImx296)\n> @@ -613,6 +669,7 @@ public:\n>                 /* From datasheet: 0x32 at 10bits. */\n>                 blackLevel_ = 3200;\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 9;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx335\", CameraSensorHelperImx335)\n> @@ -623,6 +680,7 @@ public:\n>         CameraSensorHelperImx415()\n>         {\n>                 gain_ = AnalogueGainExp{ 1.0, expGainDb(0.3) };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx415\", CameraSensorHelperImx415)\n> @@ -638,6 +696,7 @@ public:\n>         CameraSensorHelperImx477()\n>         {\n>                 gain_ = AnalogueGainLinear{ 0, 1024, -1, 1024 };\n> +               exposureMargin_ = 22;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"imx477\", CameraSensorHelperImx477)\n> @@ -663,6 +722,7 @@ public:\n>                  * This has been validated with some empirical testing only.\n>                  */\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov2685\", CameraSensorHelperOv2685)\n> @@ -673,6 +733,7 @@ public:\n>         CameraSensorHelperOv2740()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov2740\", CameraSensorHelperOv2740)\n> @@ -685,6 +746,7 @@ public:\n>                 /* From datasheet: 0x40 at 12bits. */\n>                 blackLevel_ = 1024;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov4689\", CameraSensorHelperOv4689)\n> @@ -697,6 +759,14 @@ public:\n>                 /* From datasheet: 0x10 at 10bits. */\n>                 blackLevel_ = 1024;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               /*\n> +                * Very convoluted in the driver that however applies a margin\n> +                * of 4 lines when setting vts.\n> +                *\n> +                *      cap_vts = cap_shutter + 4;\n> +                *      ret = ov5640_set_vts(sensor, cap_vts);\n> +                */\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5640\", CameraSensorHelperOv5640)\n> @@ -707,6 +777,7 @@ public:\n>         CameraSensorHelperOv5647()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5647\", CameraSensorHelperOv5647)\n> @@ -717,6 +788,7 @@ public:\n>         CameraSensorHelperOv5670()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5670\", CameraSensorHelperOv5670)\n> @@ -729,6 +801,7 @@ public:\n>                 /* From Linux kernel driver: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5675\", CameraSensorHelperOv5675)\n> @@ -739,6 +812,7 @@ public:\n>         CameraSensorHelperOv5693()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 16 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov5693\", CameraSensorHelperOv5693)\n> @@ -749,6 +823,7 @@ public:\n>         CameraSensorHelperOv64a40()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 32;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov64a40\", CameraSensorHelperOv64a40)\n> @@ -765,6 +840,7 @@ public:\n>                  * See: https://patchwork.linuxtv.org/project/linux-media/patch/20221106171129.166892-2-nicholas@rothemail.net/#142267\n>                  */\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 4;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov8858\", CameraSensorHelperOv8858)\n> @@ -775,6 +851,7 @@ public:\n>         CameraSensorHelperOv8865()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov8865\", CameraSensorHelperOv8865)\n> @@ -785,6 +862,7 @@ public:\n>         CameraSensorHelperOv13858()\n>         {\n>                 gain_ = AnalogueGainLinear{ 1, 0, 0, 128 };\n> +               exposureMargin_ = 8;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"ov13858\", CameraSensorHelperOv13858)\n> @@ -797,6 +875,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> +               exposureMargin_ = 64;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"vd55g1\", CameraSensorHelperVd55g1)\n> @@ -809,6 +888,7 @@ public:\n>                 /* From datasheet: 0x40 at 10bits. */\n>                 blackLevel_ = 4096;\n>                 gain_ = AnalogueGainLinear{ 0, 32, -1, 32 };\n> +               exposureMargin_ = 75;\n>         }\n>  };\n>  REGISTER_CAMERA_SENSOR_HELPER(\"vd56g3\", CameraSensorHelperVd56g3)\n> diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h\n> index bd3d0beec77f006b68fecf45eee850c5283fefa5..a1cf4bc334ad3b9a51d26b345bd5f0630c7ae87c 100644\n> --- a/src/ipa/libipa/camera_sensor_helper.h\n> +++ b/src/ipa/libipa/camera_sensor_helper.h\n> @@ -15,6 +15,7 @@\n>  #include <vector>\n>  \n>  #include <libcamera/base/class.h>\n> +#include <libcamera/base/utils.h>\n>  \n>  namespace libcamera {\n>  \n> @@ -30,6 +31,8 @@ public:\n>         virtual uint32_t gainCode(double gain) const;\n>         virtual double gain(uint32_t gainCode) const;\n>         double quantizeGain(double gain, double *quantizationGain) const;\n> +       utils::Duration maxShutterTime(utils::Duration maxFrameDuration,\n> +                                      utils::Duration lineDuration) const;\n>  \n>  protected:\n>         struct AnalogueGainLinear {\n> @@ -46,6 +49,7 @@ protected:\n>  \n>         std::optional<int16_t> blackLevel_;\n>         std::variant<std::monostate, AnalogueGainLinear, AnalogueGainExp> gain_;\n> +       std::optional<uint32_t> exposureMargin_;\n>  \n>  private:\n>         LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelper)\n> \n> -- \n> 2.51.1\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 BA209C32E7\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Feb 2026 15:30:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1C00620A0;\n\tFri,  6 Feb 2026 16:30:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F5A96209B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Feb 2026 16:30:44 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:5247:7b72:2b7:10da])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 352412E0;\n\tFri,  6 Feb 2026 16:29:59 +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=\"bB+r9t4d\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1770391800;\n\tbh=q3Q3OWi8NiIrnTsQIPp9avz3sxTnnpc9kK6wUfBLYYY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=bB+r9t4diFTpb5SoQrXB16ACCZQsxhNZb8LkzDOdNCHFeMKdiOnUl6jM2H4Ykz3sO\n\tDxNNOB8umGoJ8BVuDtPkLLwt0nM7miMHWlZEYH8y/RY+8emwo1jJpJgRPWhPIY3EGx\n\t4rR1tBW9QKow3XBs4l2NayRAZgga83XzugWmJOns=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251114-exposure-limits-v3-6-b7c07feba026@ideasonboard.com>","References":"<20251114-exposure-limits-v3-0-b7c07feba026@ideasonboard.com>\n\t<20251114-exposure-limits-v3-6-b7c07feba026@ideasonboard.com>","Subject":"Re: [PATCH v3 06/19] ipa: camera_sensor_helper: Introduce\n\tmaxShutterTime()","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, Niklas =?utf-8?b?U8O2?=\n\t=?utf-8?q?derlund?= <niklas.soderlund@ragnatech.se>, Robert Mader\n\t<robert.mader@collabora.com>, libcamera-devel@lists.libcamera.org","Date":"Sat, 07 Feb 2026 00:30:38 +0900","Message-ID":"<177039183855.607498.2115337495460049614@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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>"}}]