[{"id":23891,"web_url":"https://patchwork.libcamera.org/comment/23891/","msgid":"<20220714200525.eub5iki4idhsy5su@uno.localdomain>","date":"2022-07-14T20:05:25","subject":"Re: [libcamera-devel] [PATCH v2 09/11] ipa: af_hill_climb: Skip the\n\tfirst frame after triggering auto focus","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Daniel,\n +cc rpi which might find the series interesting\n\nOn Wed, Jul 13, 2022 at 10:43:15AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> When AFTrigger is received, AF algorithm internal state is reset to\n> initial values. If reset happens between frames, then first contrast\n> value that arrives after this, is interpreted as contrast for initial\n> position. This is wrong, because it was measured for lens position\n\nI still don't get why the focus position should be reset...\n\nBut the control description is under-specified in this case, so we\nshould probably decide what to do and also specify that.\n\nI sounds natural to me to assume that when the mode moves from Auto to\nManual the last computed value is kept until the application doesn't\nprovide a LensPosition. When switching from manual to auto the last\napplied value is kept until an AF scan is request with AfTriggerStart.\n\nDoes this sound reasonable ?\n\n> before the reset. Skip this first frame to allow correct contrast\n> measure for the initial lens position.\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> .../libipa/algorithms/af_hill_climbing.cpp | 8 +++++++\n> src/ipa/libipa/algorithms/af_hill_climbing.h | 23 ++++++++++++++++++-\n> 2 files changed, 30 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp\n> index f666c6c2..28d09176 100644\n> --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp\n> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp\n> @@ -84,6 +84,14 @@ namespace ipa::common::algorithms {\n> * \\return New lens position calculated by AF algorithm\n> */\n>\n> +/**\n> + * \\fn AfHillClimbing::setFramesToSkip()\n> + * \\brief Request AF to skip n frames\n> + * \\param[in] n Number of frames to be skipped\n> + *\n> + * Requested number of frames will not be used for AF calculation.\n> + */\n> +\n> } /* namespace ipa::common::algorithms */\n>\n> } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.h b/src/ipa/libipa/algorithms/af_hill_climbing.h\n> index db9fc058..e251f3eb 100644\n> --- a/src/ipa/libipa/algorithms/af_hill_climbing.h\n> +++ b/src/ipa/libipa/algorithms/af_hill_climbing.h\n> @@ -29,7 +29,7 @@ public:\n> \t\t lensPosition_(0), bestPosition_(0), currentContrast_(0.0),\n> \t\t previousContrast_(0.0), maxContrast_(0.0), maxStep_(0),\n> \t\t coarseCompleted_(false), fineCompleted_(false),\n> -\t\t lowStep_(0), highStep_(kMaxFocusSteps)\n> +\t\t lowStep_(0), highStep_(kMaxFocusSteps), framesToSkip_(0)\n> \t{\n> \t}\n>\n> @@ -92,6 +92,9 @@ protected:\n> \t{\n> \t\tcurrentContrast_ = currentContrast;\n>\n> +\t\tif (shouldSkipFrame())\n> +\t\t\treturn lensPosition_;\n> +\n> \t\t/* If we are in a paused state, we won't process the stats */\n> \t\tif (pauseState_ == libcamera::controls::AfPauseStatePaused)\n> \t\t\treturn lensPosition_;\n> @@ -113,6 +116,12 @@ protected:\n> \t\treturn lensPosition_;\n> \t}\n>\n> +\tvoid setFramesToSkip(uint32_t n)\n> +\t{\n> +\t\tif (n > framesToSkip_)\n> +\t\t\tframesToSkip_ = n;\n> +\t}\n> +\n> private:\n> \tvoid afCoarseScan()\n> \t{\n> @@ -191,6 +200,7 @@ private:\n> \t\tcoarseCompleted_ = false;\n> \t\tfineCompleted_ = false;\n> \t\tmaxContrast_ = 0.0;\n> +\t\tsetFramesToSkip(1);\n> \t}\n>\n> \tbool afIsOutOfFocus()\n> @@ -206,6 +216,16 @@ private:\n> \t\t\treturn false;\n> \t}\n>\n> +\tbool shouldSkipFrame()\n> +\t{\n> +\t\tif (framesToSkip_ > 0) {\n> +\t\t\tframesToSkip_--;\n> +\t\t\treturn true;\n> +\t\t}\n> +\n> +\t\treturn false;\n> +\t}\n> +\n> \tcontrols::AfModeEnum mode_;\n> \tcontrols::AfStateEnum state_;\n> \tcontrols::AfPauseStateEnum pauseState_;\n> @@ -229,6 +249,7 @@ private:\n>\n> \tuint32_t lowStep_;\n> \tuint32_t highStep_;\n> +\tuint32_t framesToSkip_;\n>\n> \t/*\n> \t* Maximum focus steps of the VCM control\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6AB12BD1F1\n\tfor ;\n\tThu, 14 Jul 2022 20:05:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6B116330D;\n\tThu, 14 Jul 2022 22:05:29 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 731766330D\n\tfor ;\n\tThu, 14 Jul 2022 22:05:28 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 4AE5A240007;\n\tThu, 14 Jul 2022 20:05:26 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657829129;\n\tbh=otvpw5SMKyeURKNkmjMA7EY0+Vf4lgrWYpIjuYsV5LU=;\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=rPSbCFoe3+gGmoRTgllL1iwwBbxJBP4cJhmu/YEQ4PnqxO4VYgm+oI6/HdsdvYbNO\n\tlGNNMxwVl7WSVuV3dXxdpWjHQGC53u9LC29tdLedVgB8422OuczqLo7nzzdusH0Yx+\n\tFLjXUIgUyWMZCXVv8uSfngAWqSoJV0M5PwA3MrOeL+OLzgcOX/3FLVFeMNHx4mE4H5\n\tYL33X+9HB+sDKcHB/J6mAvhyd6xMLoNYkUPXY4GiajMkYaGsz5VWelFpe9Ka0to/lM\n\tEz6FT8+fKzraDL4rwTgEDn88gH0dmJAtNJWdDoo8LsGtJaBsK3TK8pmZFdtgfm197J\n\t2mrAVZsgh6ong==","Date":"Thu, 14 Jul 2022 22:05:25 +0200","To":"Daniel Semkowicz ","Message-ID":"<20220714200525.eub5iki4idhsy5su@uno.localdomain>","References":"<20220713084317.24268-1-dse@thaumatec.com>\n\t<20220713084317.24268-10-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220713084317.24268-10-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v2 09/11] ipa: af_hill_climb: Skip the\n\tfirst frame after triggering auto focus","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]