[{"id":20368,"web_url":"https://patchwork.libcamera.org/comment/20368/","msgid":"<163485084447.4046931.2560270441869430334@Monstersaurus>","date":"2021-10-21T21:14:04","subject":"Re: [libcamera-devel] [PATCH v3 14/14] ipa: ipu3: Use sensor limits\n\tfor analogue gain","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-10-21 17:44:01)\n> Instead of using constants for the analogue gains limits, use the\n> minimum and maximum from the configured sensor.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Reviewed-by: Laurent Pinchart \n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 19 ++++++++++---------\n> src/ipa/ipu3/algorithms/agc.h | 3 +++\n> 2 files changed, 13 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index be59d0b0..2981abc7 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -30,10 +30,9 @@ static constexpr uint32_t kInitialFrameMinAECount = 4;\n> /* Number of frames to wait between new gain/exposure estimations */\n> static constexpr uint32_t kFrameSkipCount = 6;\n> \n> -/* Maximum analogue gain value\n> - * \\todo grab it from a camera helper */\n> -static constexpr double kMinGain = 1.0;\n> -static constexpr double kMaxGain = 8.0;\n> +/* Limits for analogue gain values */\n> +static constexpr double kMinAnalogueGain = 1.0;\n> +static constexpr double kMaxAnalogueGain = 8.0;\n> \n> /* Histogram constants */\n> static constexpr uint32_t knumHistogramBins = 256;\n> @@ -57,9 +56,11 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;\n> maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_;\n> \n> + minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);\n> + maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);\n> +\n> /* Configure the default exposure and gain. */\n> - context.frameContext.agc.gain =\n> - context.configuration.agc.minAnalogueGain;\n> + context.frameContext.agc.gain = minAnalogueGain_;\n> context.frameContext.agc.exposure = minExposureLines_;\n> \n> prevExposureValue_ = context.frameContext.agc.gain\n> @@ -142,7 +143,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)\n> utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;\n> utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;\n> \n> - utils::Duration maxTotalExposure = maxShutterSpeed * kMaxGain;\n> + utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;\n> currentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> LOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_\n> << \", maximum is \" << maxTotalExposure;\n> @@ -157,10 +158,10 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)\n> * Push the shutter time up to the maximum first, and only then\n> * increase the gain.\n> */\n> - shutterTime = std::clamp(exposureValue / kMinGain,\n> + shutterTime = std::clamp(exposureValue / minAnalogueGain_,\n> minShutterSpeed, maxShutterSpeed);\n> double stepGain = std::clamp(exposureValue / shutterTime,\n> - kMinGain, kMaxGain);\n> + minAnalogueGain_, maxAnalogueGain_);\n> LOG(IPU3Agc, Debug) << \"Divided up shutter and gain are \"\n> << shutterTime << \" and \"\n> << stepGain;\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index ad133b98..1840205b 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -45,6 +45,9 @@ private:\n> uint32_t minExposureLines_;\n> uint32_t maxExposureLines_;\n> \n> + double minAnalogueGain_;\n> + double maxAnalogueGain_;\n> +\n> utils::Duration filteredExposure_;\n> utils::Duration currentExposure_;\n> utils::Duration prevExposureValue_;\n> -- \n> 2.32.0\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 79EA8BDB1C\n\tfor ;\n\tThu, 21 Oct 2021 21:14:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D633A68F58;\n\tThu, 21 Oct 2021 23:14:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67704604FE\n\tfor ;\n\tThu, 21 Oct 2021 23:14:07 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F9608B6;\n\tThu, 21 Oct 2021 23:14:07 +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=\"AMyzN7rL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634850847;\n\tbh=quu33pWbqf0azqOr1HDpfWx6xfJYFvLHLpp8ICPGl58=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=AMyzN7rLnTaMfoSxR9ZzoZ7Tscksmtkvk6wlUjfjqQ5vKhB0oD4BrpzemjG76YGL2\n\tAfG0FvX3LWyDtH5tLiTdxfQNL+LNXMY2rCvJSqK0QcUrAtMZ2SUzn0vTqowmS8IGQM\n\tGXT0XCvwhu+hemSHgeto5vyaUl7jtaVwvwH1A3bw=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211021164401.110033-15-jeanmichel.hautbois@ideasonboard.com>","References":"<20211021164401.110033-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211021164401.110033-15-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 21 Oct 2021 22:14:04 +0100","Message-ID":"<163485084447.4046931.2560270441869430334@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 14/14] ipa: ipu3: Use sensor limits\n\tfor analogue gain","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]