[{"id":27959,"web_url":"https://patchwork.libcamera.org/comment/27959/","msgid":"<aqxtaymcpd5cml7ri4oo6lprvwei6eveo2uw2gvzgx5ryygxbf@pcoilitikq5g>","date":"2023-10-12T10:47:17","subject":"Re: [libcamera-devel] [PATCH 16/20] ipa: rpi: agc: Add an AGC\n\tstable region","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Oct 06, 2023 at 02:19:56PM +0100, Naushir Patuck via libcamera-devel wrote:\n> From: David Plowman <david.plowman@raspberrypi.com>\n>\n> Add a small \"stable region\" parameter (defaulting to 2%) within which\n> the AGC will not adjust the exposure it requests. It allows\n> applications to configure the AGC to avoid continual micro-adjustments\n> of exposure values if they are somehow sensitive to it.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/ipa/rpi/controller/rpi/agc_channel.cpp | 7 +++++++\n>  src/ipa/rpi/controller/rpi/agc_channel.h   | 1 +\n>  2 files changed, 8 insertions(+)\n>\n> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> index 3957dbc3fa32..1e7eae06d425 100644\n> --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> @@ -251,6 +251,8 @@ int AgcConfig::read(const libcamera::YamlObject &params)\n>  \tdefaultExposureTime = params[\"default_exposure_time\"].get<double>(1000) * 1us;\n>  \tdefaultAnalogueGain = params[\"default_analogue_gain\"].get<double>(1.0);\n>\n> +\tstableRegion = params[\"stable_region\"].get<double>(0.02);\n> +\n>  \treturn 0;\n>  }\n>\n> @@ -871,6 +873,8 @@ bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound\n>  void AgcChannel::filterExposure()\n>  {\n>  \tdouble speed = config_.speed;\n> +\tdouble stableRegion = config_.stableRegion;\n> +\n>  \t/*\n>  \t * AGC adapts instantly if both shutter and gain are directly specified\n>  \t * or we're in the startup phase.\n> @@ -880,6 +884,9 @@ void AgcChannel::filterExposure()\n>  \t\tspeed = 1.0;\n>  \tif (!filtered_.totalExposure) {\n>  \t\tfiltered_.totalExposure = target_.totalExposure;\n> +\t} else if (filtered_.totalExposure * (1.0 - stableRegion) < target_.totalExposure &&\n> +\t\t   filtered_.totalExposure * (1.0 + stableRegion) > target_.totalExposure) {\n> +\t\t/* Total exposure must change by more than this or we leave it alone. */\n\nI was a bit confused as just here below we have:\n\n                /*\n                 * If close to the result go faster, to save making so many\n                 * micro-adjustments on the way. (Make this customisable?)\n                 */\n                if (filtered_.totalExposure < 1.2 * target_.totalExposure &&\n                    filtered_.totalExposure > 0.8 * target_.totalExposure)\n\nWhich does more or less the same thing but with a larger 20% interval\n\n        if (0.8 * target < filtered < 1.2 * target)\n\nWhile I read your above check, assuming stableRegion = 0.02 as\n\n        if (filtered * 0.98 < target < filtered * 1.02)\n\nI guess both ways of expressing this work\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n\n\n>  \t} else {\n>  \t\t/*\n>  \t\t * If close to the result go faster, to save making so many\n> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h\n> index ae826fa8097f..c1808422498a 100644\n> --- a/src/ipa/rpi/controller/rpi/agc_channel.h\n> +++ b/src/ipa/rpi/controller/rpi/agc_channel.h\n> @@ -75,6 +75,7 @@ struct AgcConfig {\n>  \tdouble baseEv;\n>  \tlibcamera::utils::Duration defaultExposureTime;\n>  \tdouble defaultAnalogueGain;\n> +\tdouble stableRegion;\n>  };\n>\n>  class AgcChannel\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7BF62BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 10:47:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 318846297C;\n\tThu, 12 Oct 2023 12:47:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 903B66296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 12:47:20 +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 665357FC;\n\tThu, 12 Oct 2023 12:47:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697107642;\n\tbh=7IEQq6HQREsWn09jnTp87FgCVzcq9tJGbF0QEI02l/U=;\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=ocdoVPTLzkpN9MShXvNMUtpPkQI03ExU7ejsQu18jKKig31kvSbsPDYh/XbbSsyOX\n\tvFnlS0F+p7eZcylUaF3e2aLIKVBsX4Mei3s1jVkwv/9M8N/EOP3qsvrvS8N16cRVJl\n\tR3lt8z46tAc0nawOP6aCy9DAZ0bOfDp7/TYJ6/HpMAYI+5NiUkOkA/rWwecQmhFENU\n\tvP9pMgrF6P9Z2qhcts7J7Eqq8Kcb0V3CeyU6iZU4goQPaD8OQqIcsWmZKF9lI6mQfV\n\t5rquTixnrWPYExrHsKZxd2H5uRx/8Zh+iUoADzH8hksi5q8v6wEO/ccV0Ey3c1gThM\n\t/xGGxVHA7p6rA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697107637;\n\tbh=7IEQq6HQREsWn09jnTp87FgCVzcq9tJGbF0QEI02l/U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RekthBH3fqrDH41BUP/k92BgjUf7Did2GvpRtlscAH9XXzTmxZO9JLRUOx1S7RW+0\n\tL+3bz9G/7BnCpe9tKP5y52YSBOqaAoVUo/Bpu37vGhxzSrTpRJf4TcDUcPH/T2nt8s\n\t0zvthFQ35NK5R/mqp5rLKXf3I/dlVYm5/v6g18m8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RekthBH3\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 12:47:17 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<aqxtaymcpd5cml7ri4oo6lprvwei6eveo2uw2gvzgx5ryygxbf@pcoilitikq5g>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-17-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231006132000.23504-17-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 16/20] ipa: rpi: agc: Add an AGC\n\tstable region","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]