[{"id":27132,"web_url":"https://patchwork.libcamera.org/comment/27132/","msgid":"<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","date":"2023-05-24T09:04:10","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Benjamin\n\nOn Tue, May 23, 2023 at 07:04:43PM +0200, Benjamin Bara via libcamera-devel wrote:\n> From: Benjamin Bara <benjamin.bara@skidata.com>\n>\n> Set the vertical blanking of a sensor accordingly to a targeted\n> framerate.\n>\n> E.g. gst_libcamera_clamp_and_set_frameduration() sets\n> FrameDurationLimits to the initControls_, which are then passed to\n> Camera::start() (and from there to PipelineHandler::start()).\n>\n> Example (imx327: 1080p@25; default/minimum hBlank: 280):\n> vBlank = 0.04 * 148500000 / 2200 - 1080 = 1620\n> which results to:\n> 16390.268661 (25.00 fps) cam0-stream0 seq: 000001 bytesused: 2073600/1036800\n> 16390.308661 (25.00 fps) cam0-stream0 seq: 000002 bytesused: 2073600/1036800\n>\n> When doing this on sensors where the allowed exposure range depends on\n> vblank (e.g. on the imx327), the upper exposure limit is increased (\n> from 1123 to 2698 in the example above).\n>\n> As the sensor controls are \"cached\" in the IPA context, they must be\n> updated. This is done by passing the updated values via the start()\n> method.\n>\n> In general, this could be done independently of the IPA.\n>\n> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>\n> ---\n> Some checks are still missing in this version, but I wanted to check\n> first if my approach fits and if this is something worth to look further\n> into.\n\nI think the approach is right. So far frame durations set at start()\ntime are ignored on RkISP1 and this changeset is certainly welcome.\n\nBy pieces:\n1) Adding sensor controls to IPA::start() I think it's correct\n\n2) Updating vblank in ph before calling IPA::start()\n\n   Here I would consider a different approach\n\n   The call stack you have implemented is\n\n   ph::start() {\n      compute VBLANK\n      set VBLANK on sensor\n\n      ipa->start(v4l2_controls)\n                                IPA::start(v4l2_controls) {\n                                        update exposure limits\n\n                                        setControls(0)\n                                                ctrls(EXPOSURE, ...)\n                                                ctrls(GAIN)\n                                                setSensorControls.emit()\n                                }\n\n      setSensorControls()\n                delayedCtrl_->push(ctrls);\n   }\n\n   This seems convenient, as applying VBLANK on the sensor before\n   calling the IPA guarantees EXPOSURE is up-to-date with the newly\n   computed vertical blanking.\n\n   I wonder if we shouldn't instead remove the call to setControls(0)\n   in IPA::start() and return a list of v4l2 controls from\n   IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n   and GAIN are computed all on the IPA side by re-using\n   updateControls() which re-computes the limits for the\n   Camera::controls as well.\n\n   Your new call stack would be like\n\n   ph::start(ctrls)\n        ipa->start(ctrls, data->controlInfo_)\n\n                                        IPA::start(ctrls, cameraCtrs)\n                                                recompute VBLANK\n                                                recompute EXPOSURE\n                                                recompute GAIN\n\n                                                setSensorCtrl.emit(sensorCtrls)\n\n        setSensorControls()\n                delayedCtrl_->push()\n\n                                                updateControlInfo(cameraCtrls)\n\n    If I'm not mistaken, with your current implementation the\n    Camera::controls limits are not updated after start(), right ?\n\n    Assume you set a very short FrameDurationsLimit and that\n    controls::Exposure needs to be shrink because of that. Am I wrong\n    that after start() this will not be refelected in\n    Camera::controls::Exposure ? Could you quickly test it, it\n    shouldn't be too hard, just give gst a very high 'framerate' which\n    requires shrinking the exposure time..\n\n    The only drawback I see with my proposal is that the\n    re-computation of the EXPOSURE v4l2 control limits has to be done\n    manually in the IPA instead of relaying on it being done by the\n    driver when setting a new VBLANK as per your current\n    implementation.\n\n    What do you think ?\n\nA few minor style issues below\n\n>\n> I also saw that the getBlanking() method inside of the rpi IPA is doing\n> something similar, but I guess this can be done independent of the\n> sensor and IPA.\n>\n> Many thanks and best regards,\n> Benjamin\n> ---\n>  include/libcamera/ipa/rkisp1.mojom       |  2 +-\n>  src/ipa/rkisp1/rkisp1.cpp                | 18 ++++++++++++++--\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 +++++++++++++++++++++++++++++++-\n>  3 files changed, 53 insertions(+), 4 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index 1009e970..0e0df9e8 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -19,7 +19,7 @@ interface IPARkISP1Interface {\n>  \t     libcamera.IPACameraSensorInfo sensorInfo,\n>  \t     libcamera.ControlInfoMap sensorControls)\n>  \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> -\tstart() => (int32 ret);\n> +\tstart(libcamera.ControlInfoMap sensorControls) => (int32 ret);\n>  \tstop();\n>\n>  \tconfigure(IPAConfigInfo configInfo,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 6544c925..2c092efa 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -53,7 +53,7 @@ public:\n>  \t\t const IPACameraSensorInfo &sensorInfo,\n>  \t\t const ControlInfoMap &sensorControls,\n>  \t\t ControlInfoMap *ipaControls) override;\n> -\tint start() override;\n> +\tint start(const ControlInfoMap &sensorControls) override;\n>  \tvoid stop() override;\n>\n>  \tint configure(const IPAConfigInfo &ipaConfig,\n> @@ -197,8 +197,22 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n>  \treturn 0;\n>  }\n>\n> -int IPARkISP1::start()\n> +int IPARkISP1::start(const ControlInfoMap &sensorControls)\n>  {\n> +\t/*\n> +\t * it might be possible that VBLANK has been changed and that this has\n> +\t * an impact on the exposure limits. therefore re-set them here.\n> +\t */\n> +\tconst auto itExp = sensorControls.find(V4L2_CID_EXPOSURE);\n> +\tint32_t minExposure = itExp->second.min().get<int32_t>();\n> +\tint32_t maxExposure = itExp->second.max().get<int32_t>();\n> +\tcontext_.configuration.sensor.minShutterSpeed =\n> +\t\tminExposure * context_.configuration.sensor.lineDuration;\n> +\tcontext_.configuration.sensor.maxShutterSpeed =\n> +\t\tmaxExposure * context_.configuration.sensor.lineDuration;\n> +\tLOG(IPARkISP1, Debug)\n> +\t\t<< \"Exposure: [\" << minExposure << \", \" << maxExposure << \"]\";\n> +\n>  \tsetControls(0);\n>\n>  \treturn 0;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 8a30fe06..f9b3a3f7 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -914,12 +914,47 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>  \tRkISP1CameraData *data = cameraData(camera);\n>  \tint ret;\n>\n> +\t/*\n> +\t * \\todo Move this to a IPA-indepent location where a camera sensor\n> +\t * instance is available and the targeted frame duration is known.\n> +\t * Additionally, the IPA's sensor controls must be set accordingly.\n> +\t */\n> +\tauto frameDurations = controls->get(controls::FrameDurationLimits);\n> +\tif (frameDurations && frameDurations->size() == 2) {\n> +\t\tControlList sensorControls = data->sensor_->controls();\n> +\t\tControlList ctrls;\n> +\t\tIPACameraSensorInfo sensorInfo;\n> +\t\tif (data->sensor_->sensorInfo(&sensorInfo)) {\n> +\t\t\tLOG(RkISP1, Error) << \"couldn't fetch sensor info\";\n> +\t\t}\n\nNo braces for single line if statements. Also this should probably be\na fatal error\n\n> +\n> +\t\t/*\n> +\t\t * setup vertical blanking for target frame rate:\n> +\t\t * frameWidth = width + hBlank\n> +\t\t * frameHeight = height + vBlank\n> +\t\t * frameDuration = frameWidth * frameHeight / pixelRate\n> +\t\t * =>\n> +\t\t * vBlank = frameDuration [us] * pixelRate / frameWidth - height\n> +\t\t */\n> +\t\tuint32_t frameWidth = sensorInfo.minLineLength;\n> +\t\tuint32_t height = sensorInfo.outputSize.height;\n> +\t\tuint64_t pixelRate = sensorInfo.pixelRate;\n> +\t\tuint32_t maxFrameDuration = (*frameDurations)[1];\n> +\t\tint32_t vBlank = maxFrameDuration * pixelRate / (frameWidth * 1000000) - height;\n> +\t\tLOG(RkISP1, Debug) << \"Setting VBLANK to \" << vBlank;\n> +\t\tctrls.set(V4L2_CID_VBLANK, vBlank);\n> +\t\tdata->sensor_->setControls(&ctrls);\n> +\t\tdata->sensor_->updateControlInfo();\n> +\t} else {\n> +\t\tLOG(RkISP1, Debug) << \"Skip setting VBLANK\";\n> +\t}\n\nThis could be skipped. Maybe we should make sure\nframeDurations->size() == 2 and fail if that's not the case. But if\n!frameDurations I don't think we need a debug printout\n\nThanks\n   j\n\n> +\n>  \t/* Allocate buffers for internal pipeline usage. */\n>  \tret = allocateBuffers(camera);\n>  \tif (ret)\n>  \t\treturn ret;\n>\n> -\tret = data->ipa_->start();\n> +\tret = data->ipa_->start(data->sensor_->controls());\n>  \tif (ret) {\n>  \t\tfreeBuffers(camera);\n>  \t\tLOG(RkISP1, Error)\n>\n> ---\n> base-commit: e8fccaea46b9e545282cd37d54b1acb168608a46\n> change-id: 20230523-rkisp1-vblank-3ad862689e4a\n>\n> Best regards,\n> --\n> Benjamin Bara <benjamin.bara@skidata.com>\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 4F171C3284\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 May 2023 09:04:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AA4C626F5;\n\tWed, 24 May 2023 11:04:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 701AF626F4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 May 2023 11:04:13 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8AF24DB;\n\tWed, 24 May 2023 11:03:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1684919055;\n\tbh=OEi6fUhvF75eLslWlh7wK0ZFAzi3MyKOgolu6JNpywc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fzzzAkcH+DJRuVJK+h79/xbLNy7xs1r4NtBDb3M+8eEu65vSEd1UDWu3mQluZJuAA\n\tUTrJYCcrbPJwRiw7pX1ghNaD3jHsqJimXUrXsK57Ibw0i8KNEnZ0jQzzyQR+3hzgbD\n\tg3TdukehCP97bSy5hCgOzLpDBLyVsHk7VjNbxVpv31uwKDKbvGCJIOFDXi44O39GvF\n\tSjFhgiU0i+9gu7LQJ7/DAbq1PXyvX6XrtNiD+xensZtHi5Jw2jrIOPiveTwyhe26Pc\n\tG84iik8BsFyskou0NdXNqaasAkQqa9nTkvD1UvfYtnvMiN8DFj6bi45JKyK0MT+QB4\n\tMPoDuLFrz0HXw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1684919036;\n\tbh=OEi6fUhvF75eLslWlh7wK0ZFAzi3MyKOgolu6JNpywc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AuA20PDSb6oSLt/KZ9Za7+2N6SsehZSW7SIeSxdUkZMkfHXchz3mCMZzVi4FsHpUA\n\ttZAe68duoe9sdhmUWPy6OO4bWaMr3ROGJ+S7qOCRVEx/d1RWdRNSe/U3jl4p3QMPA/\n\tM/EO+fqKkiFb2wnya+k+65U9dhMFmTzK2Cyn/1nI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AuA20PDS\"; dkim-atps=neutral","Date":"Wed, 24 May 2023 11:04:10 +0200","To":"Benjamin Bara <bbara93@gmail.com>","Message-ID":"<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27292,"web_url":"https://patchwork.libcamera.org/comment/27292/","msgid":"<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","date":"2023-06-06T17:27:12","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":163,"url":"https://patchwork.libcamera.org/api/people/163/","name":"Benjamin Bara","email":"bbara93@gmail.com"},"content":"Hi Jacopo,\n\nthanks for the feedback and sorry for the late response.\n\nOn Wed, 24 May 2023 at 11:04, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>    I wonder if we shouldn't instead remove the call to setControls(0)\n>    in IPA::start() and return a list of v4l2 controls from\n>    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n>    and GAIN are computed all on the IPA side by re-using\n>    updateControls() which re-computes the limits for the\n>    Camera::controls as well.\n>\n>     If I'm not mistaken, with your current implementation the\n>     Camera::controls limits are not updated after start(), right ?\n\nExactly, they aren't.\nAs I am fairly new to libcamera and so far only used libcamera in\ncombination with gst-launch: Is it possible to change the frame duration\nafter start() is called? Because IMHO, vblank is static, as long as the\nframe duration is static. Obviously, if the frame duration limit is\ndynamic after start() is called, then it would make sense to also have\nvblank recalculated afterwards. Under my assumption of a static frame\nduration, I guess it would even make sense to put it \"before\" or outside\nof the IPA-specific ph::start(), as it is just related to the camera\nsensor, and independent of the IPA - but I guess start() is the first\ncall to libcamera where the frame durations are actually known.\n\n>     The only drawback I see with my proposal is that the\n>     re-computation of the EXPOSURE v4l2 control limits has to be done\n>     manually in the IPA instead of relaying on it being done by the\n>     driver when setting a new VBLANK as per your current\n>     implementation.\n\nYes, I think so too. This needs to be implemented per-sensor in the\nhelper I guess. I skimmed a little bit through the camera sensor drivers\nand it looks like most of the drivers adapt the v4l2 exposure limits as\nsoon as vblank is changed (except e.g. imx258 or imx415). So I guess at\nleast it seems to be quite reliable.\n\nSo IMHO, for the \"given frame duration limit\" case, I guess it just\nboils down to the question if the limits can change after calling\nstart(). For other use cases, like reducing the frame rate to increase\nexposure when in saturation (or similar), your suggestion might fit\nbetter. What do you think?\n\nThanks and regards,\nBenjamin","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 1EF15C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Jun 2023 17:27:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 744DB62892;\n\tTue,  6 Jun 2023 19:27:26 +0200 (CEST)","from mail-ej1-x634.google.com (mail-ej1-x634.google.com\n\t[IPv6:2a00:1450:4864:20::634])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1AD662709\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Jun 2023 19:27:24 +0200 (CEST)","by mail-ej1-x634.google.com with SMTP id\n\ta640c23a62f3a-977e0fbd742so346619466b.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 06 Jun 2023 10:27:24 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686072446;\n\tbh=eChPgrLawCyawp1bc/OuLuXEpdMOdrsPtVZye5hxe/E=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=0AJXMUSrO+HCOz5SwPq0kurmKsYI4fvy2wUGwCWVKcF0DWp9RrsgtQzhA3/86j4dE\n\tjxzAMLiytR/VrltX0+367iE/yaVIXEDUZP+UpEcpisfohjp2YLhSJMfjHMxYnEdGMY\n\tQgkuO7/y+7NcEbfv3oVVzZYOGM0fwBlJp1yWJcmwEePN3H8eerUOzQTtql1QbQmKRa\n\txy6H2AMpvfYzaup5BJ8MVsiHJZDMXFp5B6Lh2NZTwpa9+gSqcTusA4V/xKy2bBuxge\n\tfy/QGCC6JJWFNavyRtYHeeudXoU+eROdNmwzL3UlZlupH2DGDIxnQmMJ3Dd92jmK74\n\tN5l2gzWFEvK2w==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20221208; t=1686072444; x=1688664444;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=BG9q/dmJ064Z8PpyIKXtdVk8ywA+Y8sIBs3S0GtmZ2w=;\n\tb=ht9q83sPbRsAJkCGBR7ZMCBYfCQLq5mdYeZrgUlq6p1tzQCNmlo+6FyFEd5OSPxIKD\n\tLvz7N8Eg+lV32vL8+9sDosip45t7C7E9ogdlv9iH8+E06KaZAYHG7aRAJH73HVm6e6J0\n\tfj71tUSEW4rkOQizgso+N5tA8Z5XVHsh81/2ChaF+0OKPTL64CvlhtBuRB/uusSOc5be\n\tVKKionX4Z4heIGqbaI7fr6TpL0Y8StoN2Ho2Z+kNooqQyU7HvwLrVKZ26rEhjbbp1T4d\n\tqlmj3kSLjXdMe163DSFr9trpUkM+TI5zXlCeYWbvRcHFGf7vRuflemgEiTvSTZrZwpOL\n\tUR5w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ht9q83sP\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686072444; x=1688664444;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=BG9q/dmJ064Z8PpyIKXtdVk8ywA+Y8sIBs3S0GtmZ2w=;\n\tb=bUPAk8j8Ae5AIpmlVstZezkWM5nIvkHxDTET7KghMzlD5CGbf4FzeT7SbirdSzD5Sq\n\thiYoZtgPe15lttHaJxGcNUJDcqJvUJUi/b4WdidgPG4lMmRJCwIomJlY5wFNnZqK9H0v\n\tw5gwbdy/xys/jTdwZSCE8oqyOhbzZPtP4+JOvxcyYnrBuxAAqb/Mvmtsd2tGZVFWkFnd\n\tLHL/YP3MEboBMkKgDMI2M2Y6+1oc6Mz6uEnqXBaOUlt6fUhY+AcoRYkirhXexkLZyDEx\n\t1mYDaWYgnm6v1mxoPLlsKWYkDFOcXLAKIrH/xADlpRAngzN0/DoWhjm0dtqWs6rEm7pk\n\tpGMw==","X-Gm-Message-State":"AC+VfDxa456iXd1DXOKBa7u978Lo/CIzxOlZ022+iFqE/ubFiZ8BuKq0\n\t21eWpzMymMl6hCJhltq9w6NqDnQiQPA1p3mXSHA=","X-Google-Smtp-Source":"ACHHUZ4GqViuaZExUaHl41Dn8dI3BtmKjY3PWnyEcjmjea6W7lRZPaq7M3lvyQj0opPQqO1OcbvR0RcSAtC6sWK9mUs=","X-Received":"by 2002:a17:907:944c:b0:967:a127:7e79 with SMTP id\n\tdl12-20020a170907944c00b00967a1277e79mr3270241ejc.28.1686072444088;\n\tTue, 06 Jun 2023 10:27:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","In-Reply-To":"<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","Date":"Tue, 6 Jun 2023 19:27:12 +0200","Message-ID":"<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Benjamin Bara via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Benjamin Bara <bbara93@gmail.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27295,"web_url":"https://patchwork.libcamera.org/comment/27295/","msgid":"<bhxrbmhx4hgtvynr7orapx23dkzrtyijyfapl4uox43yzjlbz6@lauabfuqhfuh>","date":"2023-06-07T08:45:25","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Benjamin\n\nOn Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> Hi Jacopo,\n>\n> thanks for the feedback and sorry for the late response.\n>\n> On Wed, 24 May 2023 at 11:04, Jacopo Mondi\n> <jacopo.mondi@ideasonboard.com> wrote:\n> >    I wonder if we shouldn't instead remove the call to setControls(0)\n> >    in IPA::start() and return a list of v4l2 controls from\n> >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> >    and GAIN are computed all on the IPA side by re-using\n> >    updateControls() which re-computes the limits for the\n> >    Camera::controls as well.\n> >\n> >     If I'm not mistaken, with your current implementation the\n> >     Camera::controls limits are not updated after start(), right ?\n>\n> Exactly, they aren't.\n> As I am fairly new to libcamera and so far only used libcamera in\n> combination with gst-launch: Is it possible to change the frame duration\n> after start() is called? Because IMHO, vblank is static, as long as the\n\nFrame duration is certainly controllable during streaming yes.\n\ncontrols::FrameDurationLimits is a regular control, and like all other\ncontrols, it is meant to be set by the application to change the\nstreaming behaviour. It of course needs to be handled in the pipeline\nand the IPA (something which is missing in RkISP1, in example).\n\nNow, if I read it right, the gst element only allows you to control\nthe frame duration at startup, I'm not sure this is a shortcoming of\nthe current implementation or is there anything in gst which prevents\nto set those control while the \"pipeline is rolling\", but when it\ncomes to libcamera itself, the intended behaviour is for application\nto be able to set controls during streaming.\n\nIf you want to experiment with that, I would start by hacking out the\n'cam' test application and set a FrameDurationLimits[] = {x, x} in a\nRequest after the streaming has started. From there the control is\nreceived by the pipeline handler and then handed to the IPA. The IPA\nshould set the agc limits (we even have a todo)\n\n        /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n\nand compute a new vblank value to be sent back to the pipeline (in the\nsame way as it now passed back V4L2_CID_EXPOSURE and\nV4L2_CID_ANALOGUE_GAIN).\n\nIt's quite some work, but there shouldn't be anything too complex. If\nyou're interested in giving this a go I would be glad to help.\n\n> frame duration is static. Obviously, if the frame duration limit is\n> dynamic after start() is called, then it would make sense to also have\n> vblank recalculated afterwards. Under my assumption of a static frame\n> duration, I guess it would even make sense to put it \"before\" or outside\n> of the IPA-specific ph::start(), as it is just related to the camera\n> sensor, and independent of the IPA - but I guess start() is the first\n> call to libcamera where the frame durations are actually known.\n>\n> >     The only drawback I see with my proposal is that the\n> >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> >     manually in the IPA instead of relaying on it being done by the\n> >     driver when setting a new VBLANK as per your current\n> >     implementation.\n>\n> Yes, I think so too. This needs to be implemented per-sensor in the\n> helper I guess. I skimmed a little bit through the camera sensor drivers\n> and it looks like most of the drivers adapt the v4l2 exposure limits as\n> soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> least it seems to be quite reliable.\n>\n> So IMHO, for the \"given frame duration limit\" case, I guess it just\n> boils down to the question if the limits can change after calling\n> start(). For other use cases, like reducing the frame rate to increase\n> exposure when in saturation (or similar), your suggestion might fit\n> better. What do you think?\n>\n> Thanks and regards,\n> Benjamin","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 7C041C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Jun 2023 08:45:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2B8A62893;\n\tWed,  7 Jun 2023 10:45:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E1AC62728\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Jun 2023 10:45:29 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 209FFF7C;\n\tWed,  7 Jun 2023 10:45:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686127530;\n\tbh=yqYg0WXv4Woe1obaWQ9pBy9hFSPV59JjHYNiJSgVEf8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=l00HLwMZx9IuCkOYXUtgcMSP1udmh9xB5DmZXJwzKZTfVOkwp3hiB6bokUHjUIkYn\n\tZmuKcq2WABAnmuO2GB0GWS/wbp403aJs2Iq1Zui7XHFj0JHl9Jse7OHL4v7UPP14QB\n\tozq3vjlPYwusK3meWQB5JTCie99M3fQ49qp55eLcT3V1Ky+WxlI6aufCrlPdjuoVPL\n\tGGzA2u0lyvQKEtcEAVkuaj/y0pOQQAMNm7iRgdHAn9ucKCezHdtB7KfJrl9P3LYan0\n\tz4icxJVr2oI+EDIYJ0ekduaNrDzwIDJPKVGeICGtBR50GZsPRVwUuPJWiIkRNWXATp\n\t0PH37y1ANUf3g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686127503;\n\tbh=yqYg0WXv4Woe1obaWQ9pBy9hFSPV59JjHYNiJSgVEf8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZEVmARzvBbfyJ4FjtLbrOsygf8OWQJUL0zJ3U1SvUjTTzAKDCOqNDZRzjdrUFfJGC\n\t3ysEDnhp3aFhpv49jbBivd2g8clS+tjWlXbKa4XcAulw9hj+EZ6/qAGkooWecOG8q2\n\tbQEpT/UyIlp1Ph6yDCldhHz90LMdjrS8s8Ahpk1k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZEVmARzv\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 10:45:25 +0200","To":"Benjamin Bara <bbara93@gmail.com>","Message-ID":"<bhxrbmhx4hgtvynr7orapx23dkzrtyijyfapl4uox43yzjlbz6@lauabfuqhfuh>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27306,"web_url":"https://patchwork.libcamera.org/comment/27306/","msgid":"<20230607172336.GD5058@pendragon.ideasonboard.com>","date":"2023-06-07T17:23:36","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> Hi Jacopo,\n> \n> thanks for the feedback and sorry for the late response.\n\nNo need to apologize, or I'll have to do so too :-)\n\nWould you mind not stripping the context in e-mail replies ? Doing so\nmakes it more difficult to jump in the middle of the conversation. If\nthere are very large parts that are clearly not relevant for any further\ndiscussion on the topic then they can be trimmed, but in general\navoiding trimming would be nice.\n\nThis is a workflow issue, and I don't want to push everybody to comply\nwith my personal preferences, so if it causes any issue for you, please\nfollow the ancient wisdom from [1] and ignore this whole comment.\n\nhttps://objects-us-east-1.dream.io/secrettoeverybody/images/paynoattention.png\n\n> On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> >\n> >    I wonder if we shouldn't instead remove the call to setControls(0)\n> >    in IPA::start() and return a list of v4l2 controls from\n> >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> >    and GAIN are computed all on the IPA side by re-using\n> >    updateControls() which re-computes the limits for the\n> >    Camera::controls as well.\n> >\n> >     If I'm not mistaken, with your current implementation the\n> >     Camera::controls limits are not updated after start(), right ?\n> \n> Exactly, they aren't.\n> As I am fairly new to libcamera and so far only used libcamera in\n> combination with gst-launch: Is it possible to change the frame duration\n> after start() is called? Because IMHO, vblank is static, as long as the\n> frame duration is static. Obviously, if the frame duration limit is\n> dynamic after start() is called, then it would make sense to also have\n> vblank recalculated afterwards. Under my assumption of a static frame\n> duration, I guess it would even make sense to put it \"before\" or outside\n> of the IPA-specific ph::start(), as it is just related to the camera\n> sensor, and independent of the IPA - but I guess start() is the first\n> call to libcamera where the frame durations are actually known.\n\nThe FrameDurationLimits control is meant to be dynamic. That's how it is\nimplemented for Raspberry Pi, and I think we should do the same on other\nplatforms.\n\n> >     The only drawback I see with my proposal is that the\n> >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> >     manually in the IPA instead of relaying on it being done by the\n> >     driver when setting a new VBLANK as per your current\n> >     implementation.\n> \n> Yes, I think so too. This needs to be implemented per-sensor in the\n> helper I guess. I skimmed a little bit through the camera sensor drivers\n> and it looks like most of the drivers adapt the v4l2 exposure limits as\n> soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> least it seems to be quite reliable.\n\nI'm *really* annoyed by the V4L2 API here.\n\nThe fact that we need to modify VBLANK first before changing the\nexposure, in two separate ioctls, is bad. I can live with it for the\ntime being.\n\nThe fact that the VBLANK limits change when the sensor output format is\nchanged is really bad. The fact that most sensors have a fixed\nconstraint on the vertical total size, not on vertical blank itself,\nupsets me. Our troubles don't come from hardware constraints, but from a\nreally bad API design decision. If we exposed the vertical total size\ninstead of the vertical blanking, the maximum value would be fixed, and\nuserspace would be so much simpler.\n\nI don't want to live with this anymore, we should really fix it on the\nkernel side. I'd be happy to treat whoever makes my life less miserable\nby fixing this issue to dinner at whatever conference we would attend\nnext :-)\n\n> So IMHO, for the \"given frame duration limit\" case, I guess it just\n> boils down to the question if the limits can change after calling\n> start(). For other use cases, like reducing the frame rate to increase\n> exposure when in saturation (or similar), your suggestion might fit\n> better. What do you think?","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 1A67FC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Jun 2023 17:23:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6DB5B60A16;\n\tWed,  7 Jun 2023 19:23:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 778F8609FF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Jun 2023 19:23:40 +0200 (CEST)","from pendragon.ideasonboard.com (om126233170111.36.openmobile.ne.jp\n\t[126.233.170.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0BB0F74C;\n\tWed,  7 Jun 2023 19:23:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686158622;\n\tbh=2S2hEMNXfj/5ACllkogWryCrtFYr+k00y65VMhfAuP0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=l9OEWEy0qqOXYv9TgI329lOc8NnZVrlEO5ki/M5oi1pJ6UXdJMgCsnnvRJaSc8cn2\n\tHWcEeyZAVbDt6CSI0yrQafOKQCLbyk2ro9XMCLY181Aj3CEymBbAPcBx3h3wFy98+o\n\t6dCBzFe2UvufyYIiO4J+yoou9oODnmzef1VRiSchsxJN8hOiE//RRdP2/8QqcH+P9C\n\tgNcQdcMmi/8OYeKQ97/oH8285rDlMXI/R2ztE7uPsCkdiqmymkz/UiOhjV7bv1FSxy\n\t3xBpOzeQZp08JgA9JOIgZBEzGA75Qb4vc51PF6GEMZyiEcLOnBil2SnYxYXORU6h1c\n\t2G03PTKtKa+ng==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686158594;\n\tbh=2S2hEMNXfj/5ACllkogWryCrtFYr+k00y65VMhfAuP0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cN3lY5L+6YsiqRSxBaADfjgiAFmovZ+axkjYKjzs/AVlDg8Bo8KmsqJciU0Ezjj4b\n\t+zbWB4/276Ok0dCM/q+8shQ6JDiml+YZVif+I7ruO7kK742/Z9V5OdD8CIuZroamfz\n\tP2VNY88uy2Biq1ELnUfDzzWjgD9nLt/8Iskhqhbw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cN3lY5L+\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 20:23:36 +0300","To":"Benjamin Bara <bbara93@gmail.com>","Message-ID":"<20230607172336.GD5058@pendragon.ideasonboard.com>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27307,"web_url":"https://patchwork.libcamera.org/comment/27307/","msgid":"<20230607173024.GF5058@pendragon.ideasonboard.com>","date":"2023-06-07T17:30:24","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Jun 07, 2023 at 10:45:25AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> On Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> > On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> > >    I wonder if we shouldn't instead remove the call to setControls(0)\n> > >    in IPA::start() and return a list of v4l2 controls from\n> > >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> > >    and GAIN are computed all on the IPA side by re-using\n> > >    updateControls() which re-computes the limits for the\n> > >    Camera::controls as well.\n> > >\n> > >     If I'm not mistaken, with your current implementation the\n> > >     Camera::controls limits are not updated after start(), right ?\n> >\n> > Exactly, they aren't.\n> > As I am fairly new to libcamera and so far only used libcamera in\n> > combination with gst-launch: Is it possible to change the frame duration\n> > after start() is called? Because IMHO, vblank is static, as long as the\n> \n> Frame duration is certainly controllable during streaming yes.\n> \n> controls::FrameDurationLimits is a regular control, and like all other\n> controls, it is meant to be set by the application to change the\n> streaming behaviour. It of course needs to be handled in the pipeline\n> and the IPA (something which is missing in RkISP1, in example).\n> \n> Now, if I read it right, the gst element only allows you to control\n> the frame duration at startup, I'm not sure this is a shortcoming of\n> the current implementation or is there anything in gst which prevents\n> to set those control while the \"pipeline is rolling\", but when it\n> comes to libcamera itself, the intended behaviour is for application\n> to be able to set controls during streaming.\n> \n> If you want to experiment with that, I would start by hacking out the\n> 'cam' test application and set a FrameDurationLimits[] = {x, x} in a\n> Request after the streaming has started.\n\nNo need to hack the application, you can use a capture script ;-) See\n`cam -h` and src/apps/cam/capture-script.yaml.\n\n> From there the control is\n> received by the pipeline handler and then handed to the IPA. The IPA\n> should set the agc limits (we even have a todo)\n> \n>         /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> \n> and compute a new vblank value to be sent back to the pipeline (in the\n> same way as it now passed back V4L2_CID_EXPOSURE and\n> V4L2_CID_ANALOGUE_GAIN).\n> \n> It's quite some work, but there shouldn't be anything too complex. If\n> you're interested in giving this a go I would be glad to help.\n> \n> > frame duration is static. Obviously, if the frame duration limit is\n> > dynamic after start() is called, then it would make sense to also have\n> > vblank recalculated afterwards. Under my assumption of a static frame\n> > duration, I guess it would even make sense to put it \"before\" or outside\n> > of the IPA-specific ph::start(), as it is just related to the camera\n> > sensor, and independent of the IPA - but I guess start() is the first\n> > call to libcamera where the frame durations are actually known.\n> >\n> > >     The only drawback I see with my proposal is that the\n> > >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> > >     manually in the IPA instead of relaying on it being done by the\n> > >     driver when setting a new VBLANK as per your current\n> > >     implementation.\n> >\n> > Yes, I think so too. This needs to be implemented per-sensor in the\n> > helper I guess. I skimmed a little bit through the camera sensor drivers\n> > and it looks like most of the drivers adapt the v4l2 exposure limits as\n> > soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> > least it seems to be quite reliable.\n> >\n> > So IMHO, for the \"given frame duration limit\" case, I guess it just\n> > boils down to the question if the limits can change after calling\n> > start(). For other use cases, like reducing the frame rate to increase\n> > exposure when in saturation (or similar), your suggestion might fit\n> > better. What do you think?","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 3636CC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Jun 2023 17:30:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82E0C60A71;\n\tWed,  7 Jun 2023 19:30:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A24E60A00\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Jun 2023 19:30:29 +0200 (CEST)","from pendragon.ideasonboard.com (om126233170111.36.openmobile.ne.jp\n\t[126.233.170.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 824D074C;\n\tWed,  7 Jun 2023 19:30:01 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686159030;\n\tbh=1L0OAMc7ebahYyG9CsWl0sLg5MPqyFIixjKqrGixRXA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QuvWJK+U+AOYCX0PfJklwFwBp4HVcsVgFRRRzQe+Asi+kF/nGVQTIl7O7TdGvvr0k\n\tKNE89xHksP+C1YDe83vueopJxzcQ12AQneuCUFICWYqDJiQOy3SeFPRS1PLf1kymMF\n\txco67v019nUkNiJe8qeHUek0dX2zYC5VuK3rCdm0eJs5XLj6spDRterDiVeMHsdEIR\n\tKraGu6ejcgWdzcLRIqZUlYKPXilHKesmP1IUMtKd7pnzgQiabnEFHXBSDQT4St95A9\n\t9rCXTAVCuWCwWphtSheLdzZahxqkBWul44cTiqnTzfEL2bWdfTq9mByYieEuBovf9F\n\t7bOzy0jPmm5YA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686159002;\n\tbh=1L0OAMc7ebahYyG9CsWl0sLg5MPqyFIixjKqrGixRXA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HFaVUxjtM51Qi9ei5X4+0kKDeV8iOpd95R84VQ5cp2Kz07TtzhZsmhaC3cou29QmL\n\t+ApKSJPE41IxamLgQ800OI5aNHsw6y8GzYwiTj7/JuZdB0+lelqukuADjQTXPRhuRW\n\tzK3GhRyXDfCJVc4ZTFYHVfnVC7oBQeI+WPTlDV2A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HFaVUxjt\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 20:30:24 +0300","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Message-ID":"<20230607173024.GF5058@pendragon.ideasonboard.com>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>\n\t<bhxrbmhx4hgtvynr7orapx23dkzrtyijyfapl4uox43yzjlbz6@lauabfuqhfuh>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<bhxrbmhx4hgtvynr7orapx23dkzrtyijyfapl4uox43yzjlbz6@lauabfuqhfuh>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Benjamin Bara <bbara93@gmail.com>, libcamera-devel@lists.libcamera.org, \n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28325,"web_url":"https://patchwork.libcamera.org/comment/28325/","msgid":"<87v89092ri.fsf@gmail.com>","date":"2023-12-14T16:00:50","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":146,"url":"https://patchwork.libcamera.org/api/people/146/","name":"Mikhail Rudenko","email":"mike.rudenko@gmail.com"},"content":"Hi Jacopo, Benjamin,\n\nI'd like to work on this to get it merged. I'm rather new to libcamera\ncode base, so have a couple of questions (see below).\n\nOn 2023-05-24 at 11:04 +02, Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote:\n\n> Hi Benjamin\n>\n> On Tue, May 23, 2023 at 07:04:43PM +0200, Benjamin Bara via libcamera-devel wrote:\n>> From: Benjamin Bara <benjamin.bara@skidata.com>\n>>\n>> Set the vertical blanking of a sensor accordingly to a targeted\n>> framerate.\n>>\n>> E.g. gst_libcamera_clamp_and_set_frameduration() sets\n>> FrameDurationLimits to the initControls_, which are then passed to\n>> Camera::start() (and from there to PipelineHandler::start()).\n>>\n>> Example (imx327: 1080p@25; default/minimum hBlank: 280):\n>> vBlank = 0.04 * 148500000 / 2200 - 1080 = 1620\n>> which results to:\n>> 16390.268661 (25.00 fps) cam0-stream0 seq: 000001 bytesused: 2073600/1036800\n>> 16390.308661 (25.00 fps) cam0-stream0 seq: 000002 bytesused: 2073600/1036800\n>>\n>> When doing this on sensors where the allowed exposure range depends on\n>> vblank (e.g. on the imx327), the upper exposure limit is increased (\n>> from 1123 to 2698 in the example above).\n>>\n>> As the sensor controls are \"cached\" in the IPA context, they must be\n>> updated. This is done by passing the updated values via the start()\n>> method.\n>>\n>> In general, this could be done independently of the IPA.\n>>\n>> Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>\n>> ---\n>> Some checks are still missing in this version, but I wanted to check\n>> first if my approach fits and if this is something worth to look further\n>> into.\n>\n> I think the approach is right. So far frame durations set at start()\n> time are ignored on RkISP1 and this changeset is certainly welcome.\n>\n> By pieces:\n> 1) Adding sensor controls to IPA::start() I think it's correct\n>\n> 2) Updating vblank in ph before calling IPA::start()\n>\n>    Here I would consider a different approach\n>\n>    The call stack you have implemented is\n>\n>    ph::start() {\n>       compute VBLANK\n>       set VBLANK on sensor\n>\n>       ipa->start(v4l2_controls)\n>                                 IPA::start(v4l2_controls) {\n>                                         update exposure limits\n>\n>                                         setControls(0)\n>                                                 ctrls(EXPOSURE, ...)\n>                                                 ctrls(GAIN)\n>                                                 setSensorControls.emit()\n>                                 }\n>\n>       setSensorControls()\n>                 delayedCtrl_->push(ctrls);\n>    }\n>\n>    This seems convenient, as applying VBLANK on the sensor before\n>    calling the IPA guarantees EXPOSURE is up-to-date with the newly\n>    computed vertical blanking.\n>\n>    I wonder if we shouldn't instead remove the call to setControls(0)\n>    in IPA::start() and return a list of v4l2 controls from\n>    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n>    and GAIN are computed all on the IPA side by re-using\n>    updateControls() which re-computes the limits for the\n>    Camera::controls as well.\n>\n>    Your new call stack would be like\n>\n>    ph::start(ctrls)\n>         ipa->start(ctrls, data->controlInfo_)\n>\n>                                         IPA::start(ctrls, cameraCtrs)\n>                                                 recompute VBLANK\n>                                                 recompute EXPOSURE\n>                                                 recompute GAIN\n>\n>                                                 setSensorCtrl.emit(sensorCtrls)\n>\n>         setSensorControls()\n>                 delayedCtrl_->push()\n>\n>                                                 updateControlInfo(cameraCtrls)\n>\n>     If I'm not mistaken, with your current implementation the\n>     Camera::controls limits are not updated after start(), right ?\n>\n>     Assume you set a very short FrameDurationsLimit and that\n>     controls::Exposure needs to be shrink because of that. Am I wrong\n>     that after start() this will not be refelected in\n>     Camera::controls::Exposure ? Could you quickly test it, it\n>     shouldn't be too hard, just give gst a very high 'framerate' which\n>     requires shrinking the exposure time..\n>\n>     The only drawback I see with my proposal is that the\n>     re-computation of the EXPOSURE v4l2 control limits has to be done\n>     manually in the IPA instead of relaying on it being done by the\n>     driver when setting a new VBLANK as per your current\n>     implementation.\n\nI've got a couple questions regarding your proposal:\n\n1. Is it okay to start with setting frame duration in start() only (and\nthus scratching the gstreamer itch), or is it necessary to handle it on\nper-frame basis too?\n\n2. How could we recompute EXPOSURE limits in ipa, without actually\nsetting VBLANK on sensor subdev and querying EXPOSURE limits from\nkernel? Looks like rpi ipa tries to duplicate kernel logic in its\nCamHelper (frameIntegrationDiff constant). Should we do something like\nthat in CameraSensorHelper too, or is there a better way?\n\n>\n>     What do you think ?\n>\n> A few minor style issues below\n>\n>>\n>> I also saw that the getBlanking() method inside of the rpi IPA is doing\n>> something similar, but I guess this can be done independent of the\n>> sensor and IPA.\n>>\n>> Many thanks and best regards,\n>> Benjamin\n>> ---\n>>  include/libcamera/ipa/rkisp1.mojom       |  2 +-\n>>  src/ipa/rkisp1/rkisp1.cpp                | 18 ++++++++++++++--\n>>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 37 +++++++++++++++++++++++++++++++-\n>>  3 files changed, 53 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n>> index 1009e970..0e0df9e8 100644\n>> --- a/include/libcamera/ipa/rkisp1.mojom\n>> +++ b/include/libcamera/ipa/rkisp1.mojom\n>> @@ -19,7 +19,7 @@ interface IPARkISP1Interface {\n>>  \t     libcamera.IPACameraSensorInfo sensorInfo,\n>>  \t     libcamera.ControlInfoMap sensorControls)\n>>  \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n>> -\tstart() => (int32 ret);\n>> +\tstart(libcamera.ControlInfoMap sensorControls) => (int32 ret);\n>>  \tstop();\n>>\n>>  \tconfigure(IPAConfigInfo configInfo,\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 6544c925..2c092efa 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -53,7 +53,7 @@ public:\n>>  \t\t const IPACameraSensorInfo &sensorInfo,\n>>  \t\t const ControlInfoMap &sensorControls,\n>>  \t\t ControlInfoMap *ipaControls) override;\n>> -\tint start() override;\n>> +\tint start(const ControlInfoMap &sensorControls) override;\n>>  \tvoid stop() override;\n>>\n>>  \tint configure(const IPAConfigInfo &ipaConfig,\n>> @@ -197,8 +197,22 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n>>  \treturn 0;\n>>  }\n>>\n>> -int IPARkISP1::start()\n>> +int IPARkISP1::start(const ControlInfoMap &sensorControls)\n>>  {\n>> +\t/*\n>> +\t * it might be possible that VBLANK has been changed and that this has\n>> +\t * an impact on the exposure limits. therefore re-set them here.\n>> +\t */\n>> +\tconst auto itExp = sensorControls.find(V4L2_CID_EXPOSURE);\n>> +\tint32_t minExposure = itExp->second.min().get<int32_t>();\n>> +\tint32_t maxExposure = itExp->second.max().get<int32_t>();\n>> +\tcontext_.configuration.sensor.minShutterSpeed =\n>> +\t\tminExposure * context_.configuration.sensor.lineDuration;\n>> +\tcontext_.configuration.sensor.maxShutterSpeed =\n>> +\t\tmaxExposure * context_.configuration.sensor.lineDuration;\n>> +\tLOG(IPARkISP1, Debug)\n>> +\t\t<< \"Exposure: [\" << minExposure << \", \" << maxExposure << \"]\";\n>> +\n>>  \tsetControls(0);\n>>\n>>  \treturn 0;\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 8a30fe06..f9b3a3f7 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -914,12 +914,47 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>>  \tRkISP1CameraData *data = cameraData(camera);\n>>  \tint ret;\n>>\n>> +\t/*\n>> +\t * \\todo Move this to a IPA-indepent location where a camera sensor\n>> +\t * instance is available and the targeted frame duration is known.\n>> +\t * Additionally, the IPA's sensor controls must be set accordingly.\n>> +\t */\n>> +\tauto frameDurations = controls->get(controls::FrameDurationLimits);\n>> +\tif (frameDurations && frameDurations->size() == 2) {\n>> +\t\tControlList sensorControls = data->sensor_->controls();\n>> +\t\tControlList ctrls;\n>> +\t\tIPACameraSensorInfo sensorInfo;\n>> +\t\tif (data->sensor_->sensorInfo(&sensorInfo)) {\n>> +\t\t\tLOG(RkISP1, Error) << \"couldn't fetch sensor info\";\n>> +\t\t}\n>\n> No braces for single line if statements. Also this should probably be\n> a fatal error\n>\n>> +\n>> +\t\t/*\n>> +\t\t * setup vertical blanking for target frame rate:\n>> +\t\t * frameWidth = width + hBlank\n>> +\t\t * frameHeight = height + vBlank\n>> +\t\t * frameDuration = frameWidth * frameHeight / pixelRate\n>> +\t\t * =>\n>> +\t\t * vBlank = frameDuration [us] * pixelRate / frameWidth - height\n>> +\t\t */\n>> +\t\tuint32_t frameWidth = sensorInfo.minLineLength;\n>> +\t\tuint32_t height = sensorInfo.outputSize.height;\n>> +\t\tuint64_t pixelRate = sensorInfo.pixelRate;\n>> +\t\tuint32_t maxFrameDuration = (*frameDurations)[1];\n>> +\t\tint32_t vBlank = maxFrameDuration * pixelRate / (frameWidth * 1000000) - height;\n>> +\t\tLOG(RkISP1, Debug) << \"Setting VBLANK to \" << vBlank;\n>> +\t\tctrls.set(V4L2_CID_VBLANK, vBlank);\n>> +\t\tdata->sensor_->setControls(&ctrls);\n>> +\t\tdata->sensor_->updateControlInfo();\n>> +\t} else {\n>> +\t\tLOG(RkISP1, Debug) << \"Skip setting VBLANK\";\n>> +\t}\n>\n> This could be skipped. Maybe we should make sure\n> frameDurations->size() == 2 and fail if that's not the case. But if\n> !frameDurations I don't think we need a debug printout\n>\n> Thanks\n>    j\n>\n>> +\n>>  \t/* Allocate buffers for internal pipeline usage. */\n>>  \tret = allocateBuffers(camera);\n>>  \tif (ret)\n>>  \t\treturn ret;\n>>\n>> -\tret = data->ipa_->start();\n>> +\tret = data->ipa_->start(data->sensor_->controls());\n>>  \tif (ret) {\n>>  \t\tfreeBuffers(camera);\n>>  \t\tLOG(RkISP1, Error)\n>>\n>> ---\n>> base-commit: e8fccaea46b9e545282cd37d54b1acb168608a46\n>> change-id: 20230523-rkisp1-vblank-3ad862689e4a\n>>\n>> Best regards,\n>> --\n>> Benjamin Bara <benjamin.bara@skidata.com>\n>>\n\n\n--\nBest regards,\nMikhail Rudenko","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 D26B0BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Dec 2023 16:18:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E3BD62B32;\n\tThu, 14 Dec 2023 17:18:14 +0100 (CET)","from mail-lf1-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1890661D97\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Dec 2023 17:18:13 +0100 (CET)","by mail-lf1-x129.google.com with SMTP id\n\t2adb3069b0e04-50bee606265so8907327e87.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Dec 2023 08:18:13 -0800 (PST)","from razdolb ([83.149.246.185]) by smtp.gmail.com with ESMTPSA id\n\tp20-20020ac246d4000000b0050e0e3f1f43sm518927lfo.45.2023.12.14.08.18.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 14 Dec 2023 08:18:10 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1702570694;\n\tbh=h4uYW1/RWs5ChPiN/zwqc7hiWsaxXB1JpkMAmq4PYrc=;\n\th=References:To:Date:In-reply-to:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=apVL8BG/7Yu7XHKTk8iBci1h1D6IiYq5hxOMkBSkuGmiA7C+gIsAO/qvC+fWrehAI\n\tyDBx5zXl0pRFUvRQqhAUxLGhED1ugjlGa4AkqWJ+6w8TqyHmZHp2iTBdkF3S3bZqCy\n\tX35xdaIZHP71XJ0bF+NebjjxIpEBU9hQv4P4+c2FVIGQ7++9fl92e4o1ZuQGWzykHl\n\tllBV+R2jqP0496YW/jEezxlLvfeQKJQMVt9lEqDorEHAc40JNHgB7X9Njtv/W+IrlD\n\t6an2c+ZLBVzX4livh4j80wcLN73wVlmFCKmFpVaxgBOLNndTOt1KBh6bndwvQRXO2s\n\tlV0eFcJY2jQcw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1702570691; x=1703175491;\n\tdarn=lists.libcamera.org; \n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:from:to:cc:subject:date:message-id:reply-to;\n\tbh=jkrwYtRhy7OI5ofaDU7IIf8/HlxXgdu42jq2MgNJmS4=;\n\tb=V+hb9YQrNUUiZVfLMoQUyD/nOB1tzVvnatbbas0UyQNklXbtFtBppcYLQit3xQHQV+\n\tEIqjYi3ctxCe2M28/4n/fhsB4HGwu1I3V3PiPQqFayE2DriuaRDCKhv8ZKqf44DkIy8j\n\trmN2yigJA/jp5FA8IaMkAxiqvqAFM7RnV+zF51RmcoZsxAzt9KfMP4Bn/hFnsJxkUWo2\n\tNKOYpnhO+hveo7+HKzUXiVXb8H4IWWtWKF048lbCVW1RmgAsP8mLYWd4StDTXUW28B+Y\n\t4QlaGQ/losBMLxbe8zYQRsrcMzYS+JpRMrgGEbe20+roCEgyzfmbpB9wj2sZlT2hPVFE\n\t6ouw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"V+hb9YQr\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1702570691; x=1703175491;\n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=jkrwYtRhy7OI5ofaDU7IIf8/HlxXgdu42jq2MgNJmS4=;\n\tb=OVgsehiuSxwxUiZcU5oWiIuROY+mjKXWk60fD2SoVPkLj/jaQJQPbXEXySK7V6A4kW\n\tfkM8ftfYTc+71SISOPXDExfkVjfbxAb+8MqbPpISuVAtQFQuqNSELDA5leQ9DZK4tqey\n\tX4Tq0sT4MuZdq//nUOJm64DtBHYBACFi9j4y1/aRO3IX4Fo4Z8sRvC/96NnekoTCeaOv\n\tfN2p3FR6ljbwviKbPilmtWxQkjtXYhEMU2MFPGzNuZIS3WopVHjrlEBbPJyXN+1u4Cvt\n\t0f8QTq5/O/khXhJHI3+G+g23IJFlSV8BhtKWkjNJ8J8ZqYNVWPW0puh7RbJq9CJT2pl0\n\t5L5w==","X-Gm-Message-State":"AOJu0Yw2Wqvf9OQl9wxH0Dpbwjw0MlPa1yXUOwhOl1hZvKG5HURq+CfP\n\tYtRDQu31Wr92Tlea9mq6f27/5istX/k=","X-Google-Smtp-Source":"AGHT+IEomICwOMsQ+zrJlzRw2SHAi6sTyYob/M4NlwMxbimAD05t4sRXM5vR5QeLXzJMGREWbZT05A==","X-Received":"by 2002:ac2:4a74:0:b0:50e:d7c:da6 with SMTP id\n\tq20-20020ac24a74000000b0050e0d7c0da6mr1555105lfp.40.1702570691114; \n\tThu, 14 Dec 2023 08:18:11 -0800 (PST)","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","User-agent":"mu4e 1.10.7; emacs 29.1","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Thu, 14 Dec 2023 19:00:50 +0300","In-reply-to":"<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>","Message-ID":"<87v89092ri.fsf@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","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>","From":"Mikhail Rudenko via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Mikhail Rudenko <mike.rudenko@gmail.com>","Cc":"Benjamin Bara <bbara93@gmail.com>, libcamera-devel@lists.libcamera.org, \n\tBenjamin Bara <benjamin.bara@skidata.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35572,"web_url":"https://patchwork.libcamera.org/comment/35572/","msgid":"<20250826111032.GA186653@ragnatech.se>","date":"2025-08-26T11:10:32","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello,\n\nOn 2023-06-07 20:23:36 +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hello,\n> \n> On Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> > Hi Jacopo,\n> > \n> > thanks for the feedback and sorry for the late response.\n> \n> No need to apologize, or I'll have to do so too :-)\n> \n> Would you mind not stripping the context in e-mail replies ? Doing so\n> makes it more difficult to jump in the middle of the conversation. If\n> there are very large parts that are clearly not relevant for any further\n> discussion on the topic then they can be trimmed, but in general\n> avoiding trimming would be nice.\n> \n> This is a workflow issue, and I don't want to push everybody to comply\n> with my personal preferences, so if it causes any issue for you, please\n> follow the ancient wisdom from [1] and ignore this whole comment.\n> \n> https://objects-us-east-1.dream.io/secrettoeverybody/images/paynoattention.png\n> \n> > On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> > >\n> > >    I wonder if we shouldn't instead remove the call to setControls(0)\n> > >    in IPA::start() and return a list of v4l2 controls from\n> > >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> > >    and GAIN are computed all on the IPA side by re-using\n> > >    updateControls() which re-computes the limits for the\n> > >    Camera::controls as well.\n> > >\n> > >     If I'm not mistaken, with your current implementation the\n> > >     Camera::controls limits are not updated after start(), right ?\n> > \n> > Exactly, they aren't.\n> > As I am fairly new to libcamera and so far only used libcamera in\n> > combination with gst-launch: Is it possible to change the frame duration\n> > after start() is called? Because IMHO, vblank is static, as long as the\n> > frame duration is static. Obviously, if the frame duration limit is\n> > dynamic after start() is called, then it would make sense to also have\n> > vblank recalculated afterwards. Under my assumption of a static frame\n> > duration, I guess it would even make sense to put it \"before\" or outside\n> > of the IPA-specific ph::start(), as it is just related to the camera\n> > sensor, and independent of the IPA - but I guess start() is the first\n> > call to libcamera where the frame durations are actually known.\n> \n> The FrameDurationLimits control is meant to be dynamic. That's how it is\n> implemented for Raspberry Pi, and I think we should do the same on other\n> platforms.\n> \n> > >     The only drawback I see with my proposal is that the\n> > >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> > >     manually in the IPA instead of relaying on it being done by the\n> > >     driver when setting a new VBLANK as per your current\n> > >     implementation.\n> > \n> > Yes, I think so too. This needs to be implemented per-sensor in the\n> > helper I guess. I skimmed a little bit through the camera sensor drivers\n> > and it looks like most of the drivers adapt the v4l2 exposure limits as\n> > soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> > least it seems to be quite reliable.\n> \n> I'm *really* annoyed by the V4L2 API here.\n> \n> The fact that we need to modify VBLANK first before changing the\n> exposure, in two separate ioctls, is bad. I can live with it for the\n> time being.\n> \n> The fact that the VBLANK limits change when the sensor output format is\n> changed is really bad. The fact that most sensors have a fixed\n> constraint on the vertical total size, not on vertical blank itself,\n> upsets me. Our troubles don't come from hardware constraints, but from a\n> really bad API design decision. If we exposed the vertical total size\n> instead of the vertical blanking, the maximum value would be fixed, and\n> userspace would be so much simpler.\n> \n> I don't want to live with this anymore, we should really fix it on the\n> kernel side. I'd be happy to treat whoever makes my life less miserable\n> by fixing this issue to dinner at whatever conference we would attend\n> next :-)\n\nSorry to necro-bump, just wanted to check if there was any follow up \nwork done on this topic? I can't find any in the tree.\n\nI hit this issue recently, and worse, using the RkISP1 IPA. The reason I \nsay it's worse is that the range for the EXPOSURE control depends on the \nsetting of the VBLANK control for the IMX219 Linux sensor driver. So the \nIPA will need to update its view of EXPOSURE limits every time it \nchanges the VBLANK setting, not just as start() time but potentially \nevery frame.\n\nWithout this feedback loop I get an dark image using IMX219 sensor as my \nEXPOSURE is set way to low as it uses the initial maxim value which is \ntoo low from what is now possible with an adjusted VBLANK.\n\nI created a hack where all V4L2 controls where refreshed and sent to the \nIPA for every frame. This works and the AGC algorithm give me a much \nbetter image as it can now operate in the full \"active\" EXPOSURE range.\n\nBut the design is bonkers :-) It invalidates the all control caches, \nrecreates the control info, send it to the IPA which calculate new \nlimits for the algorithms. And it does this every time a control is set \non the sensor, as we can't assume that VBLANK is the only control which \ncan have effects on other control ranges...\n\nAnd to make it worse the IPA call is synchronous as the IPA controls \nexposed to the application might need to be updated too if the basic \nsettings of the V4L2 control changes and are recalculated by the IPA.\n\nSo this is likely not a good way forward. But I'm not sure what is, so \nI'm hoping somebody have done / is already working on V4L2 to make this \nwhole mess go away ;-)\n\n> \n> > So IMHO, for the \"given frame duration limit\" case, I guess it just\n> > boils down to the question if the limits can change after calling\n> > start(). For other use cases, like reducing the frame rate to increase\n> > exposure when in saturation (or similar), your suggestion might fit\n> > better. What do you think?\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 87769BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Aug 2025 11:10:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0324692E9;\n\tTue, 26 Aug 2025 13:10:39 +0200 (CEST)","from fout-b8-smtp.messagingengine.com\n\t(fout-b8-smtp.messagingengine.com [202.12.124.151])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06CA4692D1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Aug 2025 13:10:36 +0200 (CEST)","from phl-compute-06.internal (phl-compute-06.internal\n\t[10.202.2.46])\n\tby mailfout.stl.internal (Postfix) with ESMTP id 804F81D0011D;\n\tTue, 26 Aug 2025 07:10:35 -0400 (EDT)","from phl-mailfrontend-01 ([10.202.2.162])\n\tby phl-compute-06.internal (MEProxy); Tue, 26 Aug 2025 07:10:35 -0400","by mail.messagingengine.com (Postfix) with ESMTPA; Tue,\n\t26 Aug 2025 07:10:34 -0400 (EDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ragnatech.se header.i=@ragnatech.se\n\theader.b=\"KLgkCe9u\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"KHXCU5Bq\"; \n\tdkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=ragnatech.se; h=\n\tcc:cc:content-transfer-encoding:content-type:content-type:date\n\t:date:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to; s=fm3; t=1756206635;\n\tx=1756293035; bh=UZN5cQ9HZQL9VZpStyjPd/TR5RIoR6HkzH3RzZDWx44=; b=\n\tKLgkCe9ue5qSfzM9CQh/wdxEZNt82fKzUyW1MQTuw0Yitk0uhw+40Y8KgPrxBgLI\n\tYNaA0kSVIbKYVsSU2m6dT5zQO4Wa12u1u1AwEbwxl/npD6JnxReXUqbCXBtukJbf\n\trtY1/ZCieabi3kILUrwIXPu14oDcx8RvKmHA8qTU6VApAX8b66wIE5D/C9MfVYfz\n\t1aJG3NmOmauTLqVJFpMail7PDUQNomilmhpUy6RHzIXBzMNw9S8ztMAWBV0F/Erc\n\tqxPqQWcl10Wu/zn1ddld0LrR/rbUapih/fN8q+HWLUBe20b/9d5O/+ffBh+hwMpr\n\tPo+CD2cUdwJOK3YqQLuRfg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:cc:content-transfer-encoding\n\t:content-type:content-type:date:date:feedback-id:feedback-id\n\t:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to:x-me-proxy\n\t:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1756206635; x=\n\t1756293035; bh=UZN5cQ9HZQL9VZpStyjPd/TR5RIoR6HkzH3RzZDWx44=; b=K\n\tHXCU5BqWQQy9Pj8wr6DIIIprBvUklIb7NvyejLVqXZS1+nUTBfHdF3tIUlcwm/4d\n\tEJLOF4wWVH/7hRSzgfcW4rYhTuQcKOEMwmOxwKpSnXsayfhKXpoD+Rdpi1C3Hd5m\n\tjgnbi2KJZlzcIaoqonMnecEfOxqIcbv2NVU/00wSE7YYhZm3t9lB8s9Us505BzPz\n\tsunkUissKVuWYtP25oZs7njqnaZnOEORzBHpvPV+bgZbgTe+HybbBvJlOrtgn6Xb\n\t7r+/FCO8Cgid1PcKbLNvFkZ+JH6QW9Ex0vfXW5VABRNOdX576FdsTceb1gF/oMwo\n\tLQc0+ITDFXRHWKXvT3mtQ=="],"X-ME-Sender":"<xms:KpataGhMVUo5CgiDkwm5O4b2olZCJvEky9-AMkb9GW9Ilp_zg_8spg>\n\t<xme:KpataCiL6YtoiYiOJ2cgHJObaLzwNMkrxUyAgnx_ASPKZj4TiP3NluXo0roME4uvR\n\tl0vIUSaE3roGphYOws>","X-ME-Received":"<xmr:KpataEha9-MRPJ5gMs7TpmBWYqTwsPljfoVj8yL110czpScuRr6q3amEUoVqAB4BSSqIdbYqqPlroslQzTypJQyhpw>","X-ME-Proxy-Cause":"gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgddujeehuddvucetufdoteggodetrf\n\tdotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu\n\trghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf\n\tgurhepfffhvfevuffkfhggtggugfgjsehtkeertddttdejnecuhfhrohhmpefpihhklhgr\n\tshcuufpnuggvrhhluhhnugcuoehnihhklhgrshdrshhouggvrhhluhhnugesrhgrghhnrg\n\thtvggthhdrshgvqeenucggtffrrghtthgvrhhnpeeigffhhfetffehgffhgfdtleektedv\n\tudeiveegueevveehudeggefhiedvhfegudenucffohhmrghinhepughrvggrmhdrihhone\n\tcuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepnhhikhhl\n\trghsrdhsohguvghrlhhunhgusehrrghgnhgrthgvtghhrdhsvgdpnhgspghrtghpthhtoh\n\tephedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhgruhhrvghnthdrphhinhgt\n\thhgrrhhtsehiuggvrghsohhnsghorghrugdrtghomhdprhgtphhtthhopegssggrrhgrle\n\tefsehgmhgrihhlrdgtohhmpdhrtghpthhtohepjhgrtghophhordhmohhnughisehiuggv\n\trghsohhnsghorghrugdrtghomhdprhgtphhtthhopehlihgstggrmhgvrhgrqdguvghvvg\n\thlsehlihhsthhsrdhlihgstggrmhgvrhgrrdhorhhgpdhrtghpthhtohepsggvnhhjrghm\n\tihhnrdgsrghrrgesshhkihgurghtrgdrtghomh","X-ME-Proxy":"<xmx:KpataKLd4yqvkXjAaR493mkt0Fo9eGUG-9TwPk3P6ueL5cji6Mqx3A>\n\t<xmx:KpataLFDec1nbIQ9GD0FzifsEOMlDfaRW7LRAsbExiBcu3cG8MBiSg>\n\t<xmx:KpataGQg2KooOKoJS1R2Be3uVasmiiWy7ak4hZVAb39XfLcygM7G4Q>\n\t<xmx:KpataPfjwHbNi7INXTbngd5tik-yfTHHJkVkps3lwSfRJcSXczgdiQ>\n\t<xmx:K5ataHzWonbzfDML2hzUbea18Fwrwmwtn0-zetTzeSj_LUNAPMovpK3_>","Feedback-ID":"i80c9496c:Fastmail","Date":"Tue, 26 Aug 2025 13:10:32 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Benjamin Bara <bbara93@gmail.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","Message-ID":"<20250826111032.GA186653@ragnatech.se>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>\n\t<20230607172336.GD5058@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20230607172336.GD5058@pendragon.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":35584,"web_url":"https://patchwork.libcamera.org/comment/35584/","msgid":"<175626673976.2457228.18164207011650398896@neptunite.rasen.tech>","date":"2025-08-27T03:52:19","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Niklas Söderlund (2025-08-26 20:10:32)\n> Hello,\n> \n> On 2023-06-07 20:23:36 +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Hello,\n> > \n> > On Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> > > Hi Jacopo,\n> > > \n> > > thanks for the feedback and sorry for the late response.\n> > \n> > No need to apologize, or I'll have to do so too :-)\n> > \n> > Would you mind not stripping the context in e-mail replies ? Doing so\n> > makes it more difficult to jump in the middle of the conversation. If\n> > there are very large parts that are clearly not relevant for any further\n> > discussion on the topic then they can be trimmed, but in general\n> > avoiding trimming would be nice.\n> > \n> > This is a workflow issue, and I don't want to push everybody to comply\n> > with my personal preferences, so if it causes any issue for you, please\n> > follow the ancient wisdom from [1] and ignore this whole comment.\n> > \n> > https://objects-us-east-1.dream.io/secrettoeverybody/images/paynoattention.png\n> > \n> > > On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> > > >\n> > > >    I wonder if we shouldn't instead remove the call to setControls(0)\n> > > >    in IPA::start() and return a list of v4l2 controls from\n> > > >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> > > >    and GAIN are computed all on the IPA side by re-using\n> > > >    updateControls() which re-computes the limits for the\n> > > >    Camera::controls as well.\n> > > >\n> > > >     If I'm not mistaken, with your current implementation the\n> > > >     Camera::controls limits are not updated after start(), right ?\n> > > \n> > > Exactly, they aren't.\n> > > As I am fairly new to libcamera and so far only used libcamera in\n> > > combination with gst-launch: Is it possible to change the frame duration\n> > > after start() is called? Because IMHO, vblank is static, as long as the\n> > > frame duration is static. Obviously, if the frame duration limit is\n> > > dynamic after start() is called, then it would make sense to also have\n> > > vblank recalculated afterwards. Under my assumption of a static frame\n> > > duration, I guess it would even make sense to put it \"before\" or outside\n> > > of the IPA-specific ph::start(), as it is just related to the camera\n> > > sensor, and independent of the IPA - but I guess start() is the first\n> > > call to libcamera where the frame durations are actually known.\n> > \n> > The FrameDurationLimits control is meant to be dynamic. That's how it is\n> > implemented for Raspberry Pi, and I think we should do the same on other\n> > platforms.\n> > \n> > > >     The only drawback I see with my proposal is that the\n> > > >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> > > >     manually in the IPA instead of relaying on it being done by the\n> > > >     driver when setting a new VBLANK as per your current\n> > > >     implementation.\n> > > \n> > > Yes, I think so too. This needs to be implemented per-sensor in the\n> > > helper I guess. I skimmed a little bit through the camera sensor drivers\n> > > and it looks like most of the drivers adapt the v4l2 exposure limits as\n> > > soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> > > least it seems to be quite reliable.\n> > \n> > I'm *really* annoyed by the V4L2 API here.\n> > \n> > The fact that we need to modify VBLANK first before changing the\n> > exposure, in two separate ioctls, is bad. I can live with it for the\n> > time being.\n> > \n> > The fact that the VBLANK limits change when the sensor output format is\n> > changed is really bad. The fact that most sensors have a fixed\n> > constraint on the vertical total size, not on vertical blank itself,\n> > upsets me. Our troubles don't come from hardware constraints, but from a\n> > really bad API design decision. If we exposed the vertical total size\n> > instead of the vertical blanking, the maximum value would be fixed, and\n> > userspace would be so much simpler.\n> > \n> > I don't want to live with this anymore, we should really fix it on the\n> > kernel side. I'd be happy to treat whoever makes my life less miserable\n> > by fixing this issue to dinner at whatever conference we would attend\n> > next :-)\n> \n> Sorry to necro-bump, just wanted to check if there was any follow up \n> work done on this topic? I can't find any in the tree.\n\nI'm not sure it fixes your problem but we are able to set FrameDurationLimits\n(and therefore vblank) on the rkisp1 as of \"f72c76eb6e06 rkisp1: Honor the\nFrameDurationLimits control\" (merged in v0.5.0).\n\nThough it doesn't handle it at start() time.\n\n> \n> I hit this issue recently, and worse, using the RkISP1 IPA. The reason I \n> say it's worse is that the range for the EXPOSURE control depends on the \n> setting of the VBLANK control for the IMX219 Linux sensor driver. So the \n> IPA will need to update its view of EXPOSURE limits every time it \n> changes the VBLANK setting, not just as start() time but potentially \n> every frame.\n\nDoes setting FrameDurationLimits not update the ExposureTime limits? Oh, I see,\nwe only get updated sensor ControlInfo at init() or configure()... ok...\n\n\nPaul\n\n> \n> Without this feedback loop I get an dark image using IMX219 sensor as my \n> EXPOSURE is set way to low as it uses the initial maxim value which is \n> too low from what is now possible with an adjusted VBLANK.\n> \n> I created a hack where all V4L2 controls where refreshed and sent to the \n> IPA for every frame. This works and the AGC algorithm give me a much \n> better image as it can now operate in the full \"active\" EXPOSURE range.\n> \n> But the design is bonkers :-) It invalidates the all control caches, \n> recreates the control info, send it to the IPA which calculate new \n> limits for the algorithms. And it does this every time a control is set \n> on the sensor, as we can't assume that VBLANK is the only control which \n> can have effects on other control ranges...\n> \n> And to make it worse the IPA call is synchronous as the IPA controls \n> exposed to the application might need to be updated too if the basic \n> settings of the V4L2 control changes and are recalculated by the IPA.\n> \n> So this is likely not a good way forward. But I'm not sure what is, so \n> I'm hoping somebody have done / is already working on V4L2 to make this \n> whole mess go away ;-)\n> \n> > \n> > > So IMHO, for the \"given frame duration limit\" case, I guess it just\n> > > boils down to the question if the limits can change after calling\n> > > start(). For other use cases, like reducing the frame rate to increase\n> > > exposure when in saturation (or similar), your suggestion might fit\n> > > better. What do you think?\n> > \n> > -- \n> > Regards,\n> > \n> > Laurent Pinchart\n> \n> -- \n> Kind Regards,\n> Niklas Söderlund","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 B97D7BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Aug 2025 03:52:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3BDE3692EE;\n\tWed, 27 Aug 2025 05:52:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F959613B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Aug 2025 05:52:26 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:5e76:92d2:3747:378e])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 75A7369C7; \n\tWed, 27 Aug 2025 05:51:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XSylnw27\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756266683;\n\tbh=D2mtF+zBRlBIAQlcmADyarTFSomNJqrZuxWDYsgRmTY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=XSylnw27KOie9+QvbDUXl8Z4qMN0xPfZ/MZrblKRNzmTxN5NXVwVkfxs4HHR1V4BR\n\tfStAbS3cxaLNR4yfWXCOHJb+bJPBLrQ0r7Qgzo3p7509pLPQLleR84KJmTxqyyKo3t\n\tRApmCfx5igvlJql+Xg2kS/HkkLLHjxm2Cy7xM3yM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250826111032.GA186653@ragnatech.se>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>\n\t<20230607172336.GD5058@pendragon.ideasonboard.com>\n\t<20250826111032.GA186653@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Benjamin Bara <bbara93@gmail.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Niklas\n\t=?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Date":"Wed, 27 Aug 2025 12:52:19 +0900","Message-ID":"<175626673976.2457228.18164207011650398896@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>"}},{"id":35585,"web_url":"https://patchwork.libcamera.org/comment/35585/","msgid":"<20250827061350.GA2533009@ragnatech.se>","date":"2025-08-27T06:13:50","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nOn 2025-08-27 12:52:19 +0900, Paul Elder wrote:\n> Quoting Niklas Söderlund (2025-08-26 20:10:32)\n> > Hello,\n> > \n> > On 2023-06-07 20:23:36 +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > Hello,\n> > > \n> > > On Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> > > > Hi Jacopo,\n> > > > \n> > > > thanks for the feedback and sorry for the late response.\n> > > \n> > > No need to apologize, or I'll have to do so too :-)\n> > > \n> > > Would you mind not stripping the context in e-mail replies ? Doing so\n> > > makes it more difficult to jump in the middle of the conversation. If\n> > > there are very large parts that are clearly not relevant for any further\n> > > discussion on the topic then they can be trimmed, but in general\n> > > avoiding trimming would be nice.\n> > > \n> > > This is a workflow issue, and I don't want to push everybody to comply\n> > > with my personal preferences, so if it causes any issue for you, please\n> > > follow the ancient wisdom from [1] and ignore this whole comment.\n> > > \n> > > https://objects-us-east-1.dream.io/secrettoeverybody/images/paynoattention.png\n> > > \n> > > > On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> > > > >\n> > > > >    I wonder if we shouldn't instead remove the call to setControls(0)\n> > > > >    in IPA::start() and return a list of v4l2 controls from\n> > > > >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> > > > >    and GAIN are computed all on the IPA side by re-using\n> > > > >    updateControls() which re-computes the limits for the\n> > > > >    Camera::controls as well.\n> > > > >\n> > > > >     If I'm not mistaken, with your current implementation the\n> > > > >     Camera::controls limits are not updated after start(), right ?\n> > > > \n> > > > Exactly, they aren't.\n> > > > As I am fairly new to libcamera and so far only used libcamera in\n> > > > combination with gst-launch: Is it possible to change the frame duration\n> > > > after start() is called? Because IMHO, vblank is static, as long as the\n> > > > frame duration is static. Obviously, if the frame duration limit is\n> > > > dynamic after start() is called, then it would make sense to also have\n> > > > vblank recalculated afterwards. Under my assumption of a static frame\n> > > > duration, I guess it would even make sense to put it \"before\" or outside\n> > > > of the IPA-specific ph::start(), as it is just related to the camera\n> > > > sensor, and independent of the IPA - but I guess start() is the first\n> > > > call to libcamera where the frame durations are actually known.\n> > > \n> > > The FrameDurationLimits control is meant to be dynamic. That's how it is\n> > > implemented for Raspberry Pi, and I think we should do the same on other\n> > > platforms.\n> > > \n> > > > >     The only drawback I see with my proposal is that the\n> > > > >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> > > > >     manually in the IPA instead of relaying on it being done by the\n> > > > >     driver when setting a new VBLANK as per your current\n> > > > >     implementation.\n> > > > \n> > > > Yes, I think so too. This needs to be implemented per-sensor in the\n> > > > helper I guess. I skimmed a little bit through the camera sensor drivers\n> > > > and it looks like most of the drivers adapt the v4l2 exposure limits as\n> > > > soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> > > > least it seems to be quite reliable.\n> > > \n> > > I'm *really* annoyed by the V4L2 API here.\n> > > \n> > > The fact that we need to modify VBLANK first before changing the\n> > > exposure, in two separate ioctls, is bad. I can live with it for the\n> > > time being.\n> > > \n> > > The fact that the VBLANK limits change when the sensor output format is\n> > > changed is really bad. The fact that most sensors have a fixed\n> > > constraint on the vertical total size, not on vertical blank itself,\n> > > upsets me. Our troubles don't come from hardware constraints, but from a\n> > > really bad API design decision. If we exposed the vertical total size\n> > > instead of the vertical blanking, the maximum value would be fixed, and\n> > > userspace would be so much simpler.\n> > > \n> > > I don't want to live with this anymore, we should really fix it on the\n> > > kernel side. I'd be happy to treat whoever makes my life less miserable\n> > > by fixing this issue to dinner at whatever conference we would attend\n> > > next :-)\n> > \n> > Sorry to necro-bump, just wanted to check if there was any follow up \n> > work done on this topic? I can't find any in the tree.\n> \n> I'm not sure it fixes your problem but we are able to set FrameDurationLimits\n> (and therefore vblank) on the rkisp1 as of \"f72c76eb6e06 rkisp1: Honor the\n> FrameDurationLimits control\" (merged in v0.5.0).\n> \n> Though it doesn't handle it at start() time.\n\nI can set FrameDurationLimits...\n\n> \n> > \n> > I hit this issue recently, and worse, using the RkISP1 IPA. The reason I \n> > say it's worse is that the range for the EXPOSURE control depends on the \n> > setting of the VBLANK control for the IMX219 Linux sensor driver. So the \n> > IPA will need to update its view of EXPOSURE limits every time it \n> > changes the VBLANK setting, not just as start() time but potentially \n> > every frame.\n> \n> Does setting FrameDurationLimits not update the ExposureTime limits? Oh, I see,\n> we only get updated sensor ControlInfo at init() or configure()... ok...\n\n... but as you discover here the ExposureTime limits are not updated \ninside the IPA. So it will operate within the constraints of the old \nVBLANK setting. So If my reason to increase the FrameDurationLimits is \nto allow for a longer ExposureTime, I'm still at a loss ;-)\n\nI solved my use-case of under exposed images by adding support for \ndigital gain using the IPA (patches will be posted momentarily). So I'm \nno longer perusing this issue. I guess it should be fixed at some point, \npreferably by better V4L2 control API ;-)\n\n> \n> \n> Paul\n> \n> > \n> > Without this feedback loop I get an dark image using IMX219 sensor as my \n> > EXPOSURE is set way to low as it uses the initial maxim value which is \n> > too low from what is now possible with an adjusted VBLANK.\n> > \n> > I created a hack where all V4L2 controls where refreshed and sent to the \n> > IPA for every frame. This works and the AGC algorithm give me a much \n> > better image as it can now operate in the full \"active\" EXPOSURE range.\n> > \n> > But the design is bonkers :-) It invalidates the all control caches, \n> > recreates the control info, send it to the IPA which calculate new \n> > limits for the algorithms. And it does this every time a control is set \n> > on the sensor, as we can't assume that VBLANK is the only control which \n> > can have effects on other control ranges...\n> > \n> > And to make it worse the IPA call is synchronous as the IPA controls \n> > exposed to the application might need to be updated too if the basic \n> > settings of the V4L2 control changes and are recalculated by the IPA.\n> > \n> > So this is likely not a good way forward. But I'm not sure what is, so \n> > I'm hoping somebody have done / is already working on V4L2 to make this \n> > whole mess go away ;-)\n> > \n> > > \n> > > > So IMHO, for the \"given frame duration limit\" case, I guess it just\n> > > > boils down to the question if the limits can change after calling\n> > > > start(). For other use cases, like reducing the frame rate to increase\n> > > > exposure when in saturation (or similar), your suggestion might fit\n> > > > better. What do you think?\n> > > \n> > > -- \n> > > Regards,\n> > > \n> > > Laurent Pinchart\n> > \n> > -- \n> > Kind Regards,\n> > Niklas Söderlund","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 66867BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Aug 2025 06:13:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9025692EE;\n\tWed, 27 Aug 2025 08:13:56 +0200 (CEST)","from fhigh-a2-smtp.messagingengine.com\n\t(fhigh-a2-smtp.messagingengine.com [103.168.172.153])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0E8F613B8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Aug 2025 08:13:54 +0200 (CEST)","from phl-compute-06.internal (phl-compute-06.internal\n\t[10.202.2.46])\n\tby mailfhigh.phl.internal (Postfix) with ESMTP id B79F9140010C;\n\tWed, 27 Aug 2025 02:13:53 -0400 (EDT)","from phl-mailfrontend-01 ([10.202.2.162])\n\tby phl-compute-06.internal (MEProxy); Wed, 27 Aug 2025 02:13:53 -0400","by mail.messagingengine.com (Postfix) with ESMTPA; Wed,\n\t27 Aug 2025 02:13:52 -0400 (EDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ragnatech.se header.i=@ragnatech.se\n\theader.b=\"dkeSHbE7\"; dkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"k28YHNFK\"; \n\tdkim-atps=neutral","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=ragnatech.se; h=\n\tcc:cc:content-transfer-encoding:content-type:content-type:date\n\t:date:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to; s=fm3; t=1756275233;\n\tx=1756361633; bh=ELhMbxXvhkPr61p5sjDRfhIYyGB2aB/S0JUTa6Owe6s=; b=\n\tdkeSHbE7k9ieq/p1xas29kpsSiKab+cEeyE4jdzalxTKzZgEKWn6wDbUCyjdS6p2\n\tnDEYYJsr2uKreitnr02pjf6xoCylXw47i63X7Cx28DkEroE/DxmHuZlONelI3Qsr\n\tP0FGCkA0YQgZn61CpQjVo7vaXebRpTE0El5hrN50xC0vXgiUKggwyVkRJFMJyXTS\n\to4p00W2P2goJyFYaBpC6fBad9aAClzybuS7LKBwr1cRCXF2vomLi0Q3dGP8DmELG\n\txXnGsowNmASVy9FW5t4PevzrN187ztfRF2Sg4d4+mZH3awmWv+n2Hix+lOI9sRra\n\tFA3BPJzZKyD5lYUbdOzTow==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:cc:content-transfer-encoding\n\t:content-type:content-type:date:date:feedback-id:feedback-id\n\t:from:from:in-reply-to:in-reply-to:message-id:mime-version\n\t:references:reply-to:subject:subject:to:to:x-me-proxy\n\t:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1756275233; x=\n\t1756361633; bh=ELhMbxXvhkPr61p5sjDRfhIYyGB2aB/S0JUTa6Owe6s=; b=k\n\t28YHNFKg5nP1STmF6Fe6hCIj3UM/EccOtkOZgVC8Q5AtF2KcMer2a4rC5OH3W3Mh\n\tGCpEm62TVQdKXP+FMyXIEDx3fJIkfwijoQOn378gLq4iHVfX5YbgsxnFmmPxk5Cz\n\tho9Bl2VhngTkUq4UbyoQzSSKH1YZ1EHmmwwBO/JOkF6rt+lMVOvuHjfTReZk/4g+\n\tSZjnZVQJA8idcIHzJEaE+7XOpxXi/65O30G1Lseca6pMtz004EEOr3C8E6nU/XbS\n\tb1H8muTZrO+W0zII4KqBw8TLK6PGnsZe1IkzOZSk73I+5t9K3XG56VzXaH3TjdOz\n\tpEMCcrtanodHRjrsXp7xQ=="],"X-ME-Sender":"<xms:IaKuaIu7QdwHLItLkV0XaAMOImZG5hB_tD_N6QUzroxhkz2tc4BEpA>\n\t<xme:IaKuaN4WY430G4PSo6gf_fqXQFvmmA9lO8bWg-3-oklWd7EPH9lF0L8fzVFoJCIiT\n\t1mzMpiuNCmwemwdnZo>","X-ME-Received":"<xmr:IaKuaEOgUbaUOPj8SFd7IPAONgmhFl8P0SZM8so7du-V--WM6FxKPojsCpwZfmhtzMDzH_gOb711COwgCUp3BQfL0Ob6KEdosw>","X-ME-Proxy-Cause":"gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgddujeejgedtucetufdoteggodetrf\n\tdotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu\n\trghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf\n\tgurhepfffhvfevuffkfhggtggugfgjsehtkeertddttdejnecuhfhrohhmpefpihhklhgr\n\tshcuufpnuggvrhhluhhnugcuoehnihhklhgrshdrshhouggvrhhluhhnugesrhgrghhnrg\n\thtvggthhdrshgvqeenucggtffrrghtthgvrhhnpeeigffhhfetffehgffhgfdtleektedv\n\tudeiveegueevveehudeggefhiedvhfegudenucffohhmrghinhepughrvggrmhdrihhone\n\tcuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepnhhikhhl\n\trghsrdhsohguvghrlhhunhgusehrrghgnhgrthgvtghhrdhsvgdpnhgspghrtghpthhtoh\n\tepiedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepphgruhhlrdgvlhguvghrsehi\n\tuggvrghsohhnsghorghrugdrtghomhdprhgtphhtthhopehlrghurhgvnhhtrdhpihhntg\n\thhrghrthesihguvggrshhonhgsohgrrhgurdgtohhmpdhrtghpthhtohepsggsrghrrgel\n\tfeesghhmrghilhdrtghomhdprhgtphhtthhopehjrggtohhpohdrmhhonhguihesihguvg\n\tgrshhonhgsohgrrhgurdgtohhmpdhrtghpthhtoheplhhisggtrghmvghrrgdquggvvhgv\n\tlheslhhishhtshdrlhhisggtrghmvghrrgdrohhrghdprhgtphhtthhopegsvghnjhgrmh\n\thinhdrsggrrhgrsehskhhiuggrthgrrdgtohhm","X-ME-Proxy":"<xmx:IaKuaLs99zQV7FxnKaQFmAVxt8rlp5sH2zPoCaYAWTqvmExvWhZ7sw>\n\t<xmx:IaKuaHZpT1CPg2NPN41dQQKLXM1QH7cBTajxcaFcEvD-E5wtd-Arfw>\n\t<xmx:IaKuaNxhYCVCgl3MAUtbEof-wLnXyIO1teU3WM_8mnKonUGMzXEhwA>\n\t<xmx:IaKuaPhWk39X1lke9VXtcZZRMwfmV0onoT0mYUXGdnXmGhYpqnOgHg>\n\t<xmx:IaKuaLMiHh8xONGd1IXURDOAXpFdeRot2Eb0GHrnDcyJEOBNNksdEAfJ>","Feedback-ID":"i80c9496c:Fastmail","Date":"Wed, 27 Aug 2025 08:13:50 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tBenjamin Bara <bbara93@gmail.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tBenjamin Bara <benjamin.bara@skidata.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","Message-ID":"<20250827061350.GA2533009@ragnatech.se>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>\n\t<20230607172336.GD5058@pendragon.ideasonboard.com>\n\t<20250826111032.GA186653@ragnatech.se>\n\t<175626673976.2457228.18164207011650398896@neptunite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<175626673976.2457228.18164207011650398896@neptunite.rasen.tech>","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":36321,"web_url":"https://patchwork.libcamera.org/comment/36321/","msgid":"<um6ogdv3qfwlytgboshnapwgdk5slziyl3py3rxbquhd3rm7tr@ybb4qevk4cuz>","date":"2025-10-16T15:58:31","subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Niklas, Paul\n\nOn Wed, Aug 27, 2025 at 08:13:50AM +0200, Niklas Söderlund wrote:\n> Hi Paul,\n>\n> On 2025-08-27 12:52:19 +0900, Paul Elder wrote:\n> > Quoting Niklas Söderlund (2025-08-26 20:10:32)\n> > > Hello,\n> > >\n> > > On 2023-06-07 20:23:36 +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > Hello,\n> > > >\n> > > > On Tue, Jun 06, 2023 at 07:27:12PM +0200, Benjamin Bara via libcamera-devel wrote:\n> > > > > Hi Jacopo,\n> > > > >\n> > > > > thanks for the feedback and sorry for the late response.\n> > > >\n> > > > No need to apologize, or I'll have to do so too :-)\n> > > >\n> > > > Would you mind not stripping the context in e-mail replies ? Doing so\n> > > > makes it more difficult to jump in the middle of the conversation. If\n> > > > there are very large parts that are clearly not relevant for any further\n> > > > discussion on the topic then they can be trimmed, but in general\n> > > > avoiding trimming would be nice.\n> > > >\n> > > > This is a workflow issue, and I don't want to push everybody to comply\n> > > > with my personal preferences, so if it causes any issue for you, please\n> > > > follow the ancient wisdom from [1] and ignore this whole comment.\n> > > >\n> > > > https://objects-us-east-1.dream.io/secrettoeverybody/images/paynoattention.png\n> > > >\n> > > > > On Wed, 24 May 2023 at 11:04, Jacopo Mondi wrote:\n> > > > > >\n> > > > > >    I wonder if we shouldn't instead remove the call to setControls(0)\n> > > > > >    in IPA::start() and return a list of v4l2 controls from\n> > > > > >    IPA::start() as raspberrypi does so that the new VBLANK EXPOSURE\n> > > > > >    and GAIN are computed all on the IPA side by re-using\n> > > > > >    updateControls() which re-computes the limits for the\n> > > > > >    Camera::controls as well.\n> > > > > >\n> > > > > >     If I'm not mistaken, with your current implementation the\n> > > > > >     Camera::controls limits are not updated after start(), right ?\n> > > > >\n> > > > > Exactly, they aren't.\n> > > > > As I am fairly new to libcamera and so far only used libcamera in\n> > > > > combination with gst-launch: Is it possible to change the frame duration\n> > > > > after start() is called? Because IMHO, vblank is static, as long as the\n> > > > > frame duration is static. Obviously, if the frame duration limit is\n> > > > > dynamic after start() is called, then it would make sense to also have\n> > > > > vblank recalculated afterwards. Under my assumption of a static frame\n> > > > > duration, I guess it would even make sense to put it \"before\" or outside\n> > > > > of the IPA-specific ph::start(), as it is just related to the camera\n> > > > > sensor, and independent of the IPA - but I guess start() is the first\n> > > > > call to libcamera where the frame durations are actually known.\n> > > >\n> > > > The FrameDurationLimits control is meant to be dynamic. That's how it is\n> > > > implemented for Raspberry Pi, and I think we should do the same on other\n> > > > platforms.\n> > > >\n> > > > > >     The only drawback I see with my proposal is that the\n> > > > > >     re-computation of the EXPOSURE v4l2 control limits has to be done\n> > > > > >     manually in the IPA instead of relaying on it being done by the\n> > > > > >     driver when setting a new VBLANK as per your current\n> > > > > >     implementation.\n> > > > >\n> > > > > Yes, I think so too. This needs to be implemented per-sensor in the\n> > > > > helper I guess. I skimmed a little bit through the camera sensor drivers\n> > > > > and it looks like most of the drivers adapt the v4l2 exposure limits as\n> > > > > soon as vblank is changed (except e.g. imx258 or imx415). So I guess at\n> > > > > least it seems to be quite reliable.\n> > > >\n> > > > I'm *really* annoyed by the V4L2 API here.\n> > > >\n> > > > The fact that we need to modify VBLANK first before changing the\n> > > > exposure, in two separate ioctls, is bad. I can live with it for the\n> > > > time being.\n> > > >\n> > > > The fact that the VBLANK limits change when the sensor output format is\n> > > > changed is really bad. The fact that most sensors have a fixed\n> > > > constraint on the vertical total size, not on vertical blank itself,\n> > > > upsets me. Our troubles don't come from hardware constraints, but from a\n> > > > really bad API design decision. If we exposed the vertical total size\n> > > > instead of the vertical blanking, the maximum value would be fixed, and\n> > > > userspace would be so much simpler.\n> > > >\n> > > > I don't want to live with this anymore, we should really fix it on the\n> > > > kernel side. I'd be happy to treat whoever makes my life less miserable\n> > > > by fixing this issue to dinner at whatever conference we would attend\n> > > > next :-)\n> > >\n> > > Sorry to necro-bump, just wanted to check if there was any follow up\n> > > work done on this topic? I can't find any in the tree.\n> >\n> > I'm not sure it fixes your problem but we are able to set FrameDurationLimits\n> > (and therefore vblank) on the rkisp1 as of \"f72c76eb6e06 rkisp1: Honor the\n> > FrameDurationLimits control\" (merged in v0.5.0).\n> >\n> > Though it doesn't handle it at start() time.\n>\n> I can set FrameDurationLimits...\n>\n> >\n> > >\n> > > I hit this issue recently, and worse, using the RkISP1 IPA. The reason I\n> > > say it's worse is that the range for the EXPOSURE control depends on the\n> > > setting of the VBLANK control for the IMX219 Linux sensor driver. So the\n> > > IPA will need to update its view of EXPOSURE limits every time it\n> > > changes the VBLANK setting, not just as start() time but potentially\n> > > every frame.\n> >\n> > Does setting FrameDurationLimits not update the ExposureTime limits? Oh, I see,\n> > we only get updated sensor ControlInfo at init() or configure()... ok...\n>\n> ... but as you discover here the ExposureTime limits are not updated\n> inside the IPA. So it will operate within the constraints of the old\n> VBLANK setting. So If my reason to increase the FrameDurationLimits is\n> to allow for a longer ExposureTime, I'm still at a loss ;-)\n\nConfirmed :(\n\n>\n> I solved my use-case of under exposed images by adding support for\n> digital gain using the IPA (patches will be posted momentarily). So I'm\n> no longer perusing this issue. I guess it should be fixed at some point,\n> preferably by better V4L2 control API ;-)\n>\n> >\n> >\n> > Paul\n> >\n> > >\n> > > Without this feedback loop I get an dark image using IMX219 sensor as my\n> > > EXPOSURE is set way to low as it uses the initial maxim value which is\n> > > too low from what is now possible with an adjusted VBLANK.\n> > >\n> > > I created a hack where all V4L2 controls where refreshed and sent to the\n> > > IPA for every frame. This works and the AGC algorithm give me a much\n> > > better image as it can now operate in the full \"active\" EXPOSURE range.\n> > >\n> > > But the design is bonkers :-) It invalidates the all control caches,\n> > > recreates the control info, send it to the IPA which calculate new\n> > > limits for the algorithms. And it does this every time a control is set\n> > > on the sensor, as we can't assume that VBLANK is the only control which\n> > > can have effects on other control ranges...\n\n\nmmm, however I think it's the most common case where a change on a\ncontrol value implies changes on the limits of another one (when only\nconsidering image sensors at least) and I think it's worth handling\nit, even with a stop-gap measure.\n\nYes, ideally we should program in the sensor the whole frame length\nand not vblank separately from the frame height, so that the it\nautomatically serves as a limit for the exposure time, but for the\ntime being I think a fix in the RkISP1 IPA is required.\n\nI'll post patches soon.\n\n> > >\n> > > And to make it worse the IPA call is synchronous as the IPA controls\n> > > exposed to the application might need to be updated too if the basic\n> > > settings of the V4L2 control changes and are recalculated by the IPA.\n> > >\n> > > So this is likely not a good way forward. But I'm not sure what is, so\n> > > I'm hoping somebody have done / is already working on V4L2 to make this\n> > > whole mess go away ;-)\n> > >\n> > > >\n> > > > > So IMHO, for the \"given frame duration limit\" case, I guess it just\n> > > > > boils down to the question if the limits can change after calling\n> > > > > start(). For other use cases, like reducing the frame rate to increase\n> > > > > exposure when in saturation (or similar), your suggestion might fit\n> > > > > better. What do you think?\n> > > >\n> > > > --\n> > > > Regards,\n> > > >\n> > > > Laurent Pinchart\n> > >\n> > > --\n> > > Kind Regards,\n> > > Niklas Söderlund\n>\n> --\n> Kind Regards,\n> Niklas Söderlund","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 85B5EBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 16 Oct 2025 15:58:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D5E346068B;\n\tThu, 16 Oct 2025 17:58:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 909E6605D7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Oct 2025 17:58:34 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D15B10D8;\n\tThu, 16 Oct 2025 17:56:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VktFui/o\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1760630214;\n\tbh=B0/3qrnxTcM7nhm8sPC5Wt/Sjb9a+Ji6fs8MLSf3PlQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VktFui/oWBIO3k/ODkxFhyW53FvU1aR/lTDw3ONI7ay2HrztPURsUXiulOdRT+FPo\n\tOAva474d8ixHmFaS8Ot4lBOFbvwalSX84QGAq+SfULxxLRRzAlCkLk5+p06fwlWYlz\n\tJYp/exeGzU1Q61MtDamJpwP8BZ+Aa4R7uCBJDHlo=","Date":"Thu, 16 Oct 2025 17:58:31 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Paul Elder <paul.elder@ideasonboard.com>, \n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tBenjamin Bara <bbara93@gmail.com>, \n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tBenjamin Bara <benjamin.bara@skidata.com>","Subject":"Re: [libcamera-devel] [PATCH RFC] rkisp1: adjust vblank to target\n\tframerate","Message-ID":"<um6ogdv3qfwlytgboshnapwgdk5slziyl3py3rxbquhd3rm7tr@ybb4qevk4cuz>","References":"<20230523-rkisp1-vblank-v1-1-381c6f025ff6@skidata.com>\n\t<thedug77jyk5ph66dj4inlzjc564cvqcep4gkvpojbjul5srbu@5qf7zrayiku2>\n\t<CAJpcXm6_id5_imqYYtYjwtHX2a3_RN6PqLhh57LDBNAL+iEn0w@mail.gmail.com>\n\t<20230607172336.GD5058@pendragon.ideasonboard.com>\n\t<20250826111032.GA186653@ragnatech.se>\n\t<175626673976.2457228.18164207011650398896@neptunite.rasen.tech>\n\t<20250827061350.GA2533009@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250827061350.GA2533009@ragnatech.se>","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>"}}]