[{"id":35346,"web_url":"https://patchwork.libcamera.org/comment/35346/","msgid":"<2a9385aa-1050-428a-bba0-4909e4198f59@ideasonboard.com>","date":"2025-08-11T15:05:37","subject":"Re: [PATCH v2 06/16] libipa: exposure_mode_helper: Remove\n\tunnecessary clamp calls","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 08/08/2025 15:12, Stefan Klug wrote:\n> Except for the first iteration of the loop and in the case of an empty\n> gains_ vector, the values were run through clamp two times which is\n> unnecessary. Remove that by clamping the initial value.\n> \n> Signed-off-by: Stefan Klug \n> ---\n\nNot sure why I got so clamp-happy...as far as I can see this is fine:\n\nReviewed-by: Daniel Scally \n\n> src/ipa/libipa/exposure_mode_helper.cpp | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp\n> index 3c758851679f..7d470ebe487c 100644\n> --- a/src/ipa/libipa/exposure_mode_helper.cpp\n> +++ b/src/ipa/libipa/exposure_mode_helper.cpp\n> @@ -197,8 +197,8 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const\n> \t\treturn { minExposureTime_, minGain_, exposure / (minExposureTime_ * minGain_) };\n> \n> \tutils::Duration exposureTime;\n> -\tdouble stageGain = 1.0;\n> -\tdouble lastStageGain = 1.0;\n> +\tdouble stageGain = clampGain(1.0);\n> +\tdouble lastStageGain = stageGain;\n> \tdouble gain;\n> \n> \tfor (unsigned int stage = 0; stage < gains_.size(); stage++) {\n> @@ -215,7 +215,7 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const\n> \n> \t\t/* Clamp the gain to lastStageGain and regulate exposureTime. */\n> \t\tif (stageExposureTime * lastStageGain >= exposure) {\n> -\t\t\texposureTime = clampExposureTime(exposure / clampGain(lastStageGain));\n> +\t\t\texposureTime = clampExposureTime(exposure / lastStageGain);\n> \t\t\tgain = clampGain(exposure / exposureTime);\n> \n> \t\t\treturn { exposureTime, gain, exposure / (exposureTime * gain) };\n> @@ -223,7 +223,7 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const\n> \n> \t\t/* Clamp the exposureTime to stageExposureTime and regulate gain. */\n> \t\tif (stageExposureTime * stageGain >= exposure) {\n> -\t\t\texposureTime = clampExposureTime(stageExposureTime);\n> +\t\t\texposureTime = stageExposureTime;\n> \t\t\tgain = clampGain(exposure / exposureTime);\n> \n> \t\t\treturn { exposureTime, gain, exposure / (exposureTime * gain) };\n> @@ -239,7 +239,7 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const\n> \t * stages to use then the default stageGain of 1.0 is used so that\n> \t * exposure time is maxed before gain is touched at all.\n> \t */\n> -\texposureTime = clampExposureTime(exposure / clampGain(stageGain));\n> +\texposureTime = clampExposureTime(exposure / stageGain);\n> \tgain = clampGain(exposure / exposureTime);\n> \n> \treturn { exposureTime, gain, exposure / (exposureTime * gain) };","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 D8559BEFBE\n\tfor ;\n\tMon, 11 Aug 2025 15:05:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9276969236;\n\tMon, 11 Aug 2025 17:05:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0995369233\n\tfor ;\n\tMon, 11 Aug 2025 17:05:41 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70A1D4A4\n\tfor ;\n\tMon, 11 Aug 2025 17:04:48 +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=\"ePDvg05z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754924688;\n\tbh=uroQ553aq3MahJ4uzn8mVslO6HHW5L9bdxm9i07jR3I=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ePDvg05zJBVaa02fpUgUZuX3k4k1eUO42niJiV7wt/XjCK4/9WG5qnADdtRhtA7Dv\n\t3QC6lTzzjSV+e6+jisrbj7vmlY+U+m0IFJtjNiZEH/yoRyMJjRGTi+mff8HbCp+UJq\n\t32E+Epu9fv/YsVe2jsjw/hcWeM9vXIdy/7pjneZI=","Message-ID":"<2a9385aa-1050-428a-bba0-4909e4198f59@ideasonboard.com>","Date":"Mon, 11 Aug 2025 16:05:37 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 06/16] libipa: exposure_mode_helper: Remove\n\tunnecessary clamp calls","To":"libcamera-devel@lists.libcamera.org","References":"<20250808141315.413839-1-stefan.klug@ideasonboard.com>\n\t<20250808141315.413839-7-stefan.klug@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"<20250808141315.413839-7-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}}]