[{"id":27156,"web_url":"https://patchwork.libcamera.org/comment/27156/","msgid":"<20230529114009.GR25984@pendragon.ideasonboard.com>","date":"2023-05-29T11:40:09","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: drop hard-codec\n\tanalogue gain range","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, May 19, 2023 at 06:22:15PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> As the sensor's analogue gain range is known drop the arbitrary\n> limits for the sensor analogue gain.\n\nAs written in the review of the corresponding rkisp1 patch, I think\nlimits should be handled in the IPA module. While I don't expect many\ncamera sensors to support gains smaller than 1.0, some may, and the\nCameraSensor class should expose the real capabilities of the sensor.\nThe IPA module is responsible for setting the policies of the\nalgorithms.\n\nFor the minimum gain, we shouldn't go below 1.0 as I really don't see\nany major use case.\n\nFor the maximum gain, there's a policy decision to consider, but that\nshould come from the tuning data, not be hardcoded here. I'm thus OK\nwith the change for the maximum gain. There's a risk it will cause\nregressions for sensors that have a very high maximum gain value, so we\nmay need to add AGC tuning data sooner than later.\n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 10 +++-------\n> 1 file changed, 3 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index b5309bdbea25..74a14675fca0 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -47,10 +47,6 @@ namespace ipa::ipu3::algorithms {\n> \n> LOG_DEFINE_CATEGORY(IPU3Agc)\n> \n> -/* Limits for analogue gain values */\n> -static constexpr double kMinAnalogueGain = 1.0;\n> -static constexpr double kMaxAnalogueGain = 8.0;\n> -\n> /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> \n> @@ -96,11 +92,11 @@ int Agc::configure(IPAContext &context,\n> \tmaxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,\n> \t\t\t\t kMaxShutterSpeed);\n> \n> -\tminAnalogueGain_ = std::max(configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> -\tmaxAnalogueGain_ = std::min(configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n> +\tminAnalogueGain_ = configuration.agc.minAnalogueGain;\n> +\tmaxAnalogueGain_ = configuration.agc.maxAnalogueGain;\n> \n> \t/* Configure the default exposure and gain. */\n> -\tactiveState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);\n> +\tactiveState.agc.gain = minAnalogueGain_;\n> \tactiveState.agc.exposure = 10ms / configuration.sensor.lineDuration;\n> \n> \tframeCount_ = 0;","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 5980FC3213\n\tfor ;\n\tMon, 29 May 2023 11:40:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F7F2626F9;\n\tMon, 29 May 2023 13:40:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7961D6038E\n\tfor ;\n\tMon, 29 May 2023 13:40:08 +0200 (CEST)","from pendragon.ideasonboard.com (om126255106133.24.openmobile.ne.jp\n\t[126.255.106.133])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 92F60836;\n\tMon, 29 May 2023 13:39:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685360409;\n\tbh=0NWQBCMZxsva+YAyqY4PGvepU/WgIeL5QHyo8o8lvh0=;\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=cAr/wWFSKY8P9s/LOV+Dk8Kp7iq9Q0OpbL2UiDWrh/qaz9Ex+GsEOVDZs9VJGiOjG\n\tEmulLjrogjOZ8vhlwBem+qWfrDDwfB4KV/B3Y4ywytkaGKL5EC6+Vnz2Rfef2YwIhs\n\tP9rcrISjZ77L71PB7MiTaBG+Z8TszCjNq6XeN1WPvb4+Wo6+dFzw8yS9XdrAU07UPl\n\tmG9S3yFV6TEjhPkqWMN9I9XXAgZAL4MDOdZOX+2k2vRBLfx9fukVBLbQrl0dcxvtFv\n\tKOVD89V0m8847D9DeSBAfIuQYPMChSjwpMO2Ir3lj5Yt+nPDeFF+YIc92GKBRKNksc\n\tx/X+EQ3pO0qqg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685360388;\n\tbh=0NWQBCMZxsva+YAyqY4PGvepU/WgIeL5QHyo8o8lvh0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HSP7h8ElOwJO0ka8ilS2tA2Pl9vRGoJzxVkJCAQZIiKuqS3IP+OT7gy0DDwE2rwPY\n\t+1Us7RWuofTr6G+ccN0Owq7tRlsm4kPfSW0ZElVA4KfeAR0cxC46EOo5n9Xy9k3AKH\n\tdYLjqzMX8rcxpeZ/ZJ4Mb8N272KsDWznneG//rLM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HSP7h8El\"; dkim-atps=neutral","Date":"Mon, 29 May 2023 14:40:09 +0300","To":"Jacopo Mondi ","Message-ID":"<20230529114009.GR25984@pendragon.ideasonboard.com>","References":"<20230519162215.413831-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230519162215.413831-1-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: agc: drop hard-codec\n\tanalogue gain range","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]