[{"id":29268,"web_url":"https://patchwork.libcamera.org/comment/29268/","msgid":"","date":"2024-04-19T03:00:01","subject":"Re: [PATCH 2/3] ipa: rkisp1: Refer to integration time rather than\n\tshutter speed","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Apr 18, 2024 at 01:46:31PM +0100, Daniel Scally wrote:\n> Multiple variables referencing shutter speed in the RkISP1 IPA module\n> are in fact calculated and used as integration time. The discrepancy\n> is problematic given the minimum shutter speed would produce the max\n> integration time.\n> \n> Replace references to shutter speed with integration time.\n> \n> Signed-off-by: Daniel Scally \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> src/ipa/rkisp1/ipa_context.cpp | 8 ++++----\n> src/ipa/rkisp1/ipa_context.h | 4 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 8 ++++----\n> 4 files changed, 12 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 27b6f2c1..13f54398 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -95,8 +95,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> \tcontext.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> \n> \t/* \\todo Run this again when FrameDurationLimits is passed in */\n> -\tconfigureExposureModeHelpers(context.configuration.sensor.minShutterSpeed,\n> -\t\t\t\t context.configuration.sensor.maxShutterSpeed,\n> +\tconfigureExposureModeHelpers(context.configuration.sensor.minIntegrationTime,\n> +\t\t\t\t context.configuration.sensor.maxIntegrationTime,\n> \t\t\t\t context.configuration.sensor.minAnalogueGain,\n> \t\t\t\t context.configuration.sensor.maxAnalogueGain);\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 070834fa..991ca1c2 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -78,11 +78,11 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPASessionConfiguration::sensor\n> * \\brief Sensor-specific configuration of the IPA\n> *\n> - * \\var IPASessionConfiguration::sensor.minShutterSpeed\n> - * \\brief Minimum shutter speed supported with the sensor\n> + * \\var IPASessionConfiguration::sensor.minIntegrationTime\n> + * \\brief Minimum integration time supported with the sensor\n> *\n> - * \\var IPASessionConfiguration::sensor.maxShutterSpeed\n> - * \\brief Maximum shutter speed supported with the sensor\n> + * \\var IPASessionConfiguration::sensor.maxIntegrationTime\n> + * \\brief Maximum integration time supported with the sensor\n> *\n> * \\var IPASessionConfiguration::sensor.minAnalogueGain\n> * \\brief Minimum analogue gain supported with the sensor\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 256b75eb..3405a260 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -43,8 +43,8 @@ struct IPASessionConfiguration {\n> \t} lsc;\n> \n> \tstruct {\n> -\t\tutils::Duration minShutterSpeed;\n> -\t\tutils::Duration maxShutterSpeed;\n> +\t\tutils::Duration minIntegrationTime;\n> +\t\tutils::Duration maxIntegrationTime;\n> \t\tdouble minAnalogueGain;\n> \t\tdouble maxAnalogueGain;\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index d8610095..15919d3f 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -245,14 +245,14 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> \n> \t/*\n> \t * When the AGC computes the new exposure values for a frame, it needs\n> -\t * to know the limits for shutter speed and analogue gain.\n> +\t * to know the limits for integration time and analogue gain.\n> \t * As it depends on the sensor, update it with the controls.\n> \t *\n> -\t * \\todo take VBLANK into account for maximum shutter speed\n> +\t * \\todo take VBLANK into account for maximum integration time\n> \t */\n> -\tcontext_.configuration.sensor.minShutterSpeed =\n> +\tcontext_.configuration.sensor.minIntegrationTime =\n> \t\tminExposure * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.sensor.maxShutterSpeed =\n> +\tcontext_.configuration.sensor.maxIntegrationTime =\n> \t\tmaxExposure * context_.configuration.sensor.lineDuration;\n> \tcontext_.configuration.sensor.minAnalogueGain = camHelper_->gain(minGain);\n> \tcontext_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);\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 A5BC1BE08B\n\tfor ;\n\tFri, 19 Apr 2024 03:00:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BE600633F5;\n\tFri, 19 Apr 2024 05:00:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B00B633ED\n\tfor ;\n\tFri, 19 Apr 2024 05:00:09 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C44C63B;\n\tFri, 19 Apr 2024 04:59:19 +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=\"cYhK4lMC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713495561;\n\tbh=OF2mCY+xGYYfV7IQ+hPnHWfC+9gIK2UXVTuGNTW1IaI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cYhK4lMC8kxyqnQt6kD+AiEZcoBT+H/VGry/D8pZAbl4WCFAXXUnyZqmXryiiuu4H\n\txoWguhNmBQ3A7hXSb+a0KQPjR/apE1LbInE1gXJIBQxOMqvqfqA3ojBoF9+lVxGw2S\n\ttq5PtazPGcgCQrFrlf6eVR2D86fPyD+mmVwFRFf0=","Date":"Fri, 19 Apr 2024 12:00:01 +0900","From":"Paul Elder ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Refer to integration time rather than\n\tshutter speed","Message-ID":"","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-3-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240418124632.652163-3-dan.scally@ideasonboard.com>","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\" "}},{"id":29449,"web_url":"https://patchwork.libcamera.org/comment/29449/","msgid":"<20240508133400.GL2012@pendragon.ideasonboard.com>","date":"2024-05-08T13:34:00","subject":"Re: [PATCH 2/3] ipa: rkisp1: Refer to integration time rather than\n\tshutter speed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nOn Thu, Apr 18, 2024 at 01:46:31PM +0100, Daniel Scally wrote:\n> Multiple variables referencing shutter speed in the RkISP1 IPA module\n> are in fact calculated and used as integration time. The discrepancy\n> is problematic given the minimum shutter speed would produce the max\n> integration time.\n> \n> Replace references to shutter speed with integration time.\n> \n> Signed-off-by: Daniel Scally \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 4 ++--\n> src/ipa/rkisp1/ipa_context.cpp | 8 ++++----\n> src/ipa/rkisp1/ipa_context.h | 4 ++--\n> src/ipa/rkisp1/rkisp1.cpp | 8 ++++----\n> 4 files changed, 12 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 27b6f2c1..13f54398 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -95,8 +95,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> \tcontext.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;\n> \n> \t/* \\todo Run this again when FrameDurationLimits is passed in */\n> -\tconfigureExposureModeHelpers(context.configuration.sensor.minShutterSpeed,\n> -\t\t\t\t context.configuration.sensor.maxShutterSpeed,\n> +\tconfigureExposureModeHelpers(context.configuration.sensor.minIntegrationTime,\n> +\t\t\t\t context.configuration.sensor.maxIntegrationTime,\n> \t\t\t\t context.configuration.sensor.minAnalogueGain,\n> \t\t\t\t context.configuration.sensor.maxAnalogueGain);\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 070834fa..991ca1c2 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -78,11 +78,11 @@ namespace libcamera::ipa::rkisp1 {\n> * \\var IPASessionConfiguration::sensor\n> * \\brief Sensor-specific configuration of the IPA\n> *\n> - * \\var IPASessionConfiguration::sensor.minShutterSpeed\n> - * \\brief Minimum shutter speed supported with the sensor\n> + * \\var IPASessionConfiguration::sensor.minIntegrationTime\n> + * \\brief Minimum integration time supported with the sensor\n> *\n> - * \\var IPASessionConfiguration::sensor.maxShutterSpeed\n> - * \\brief Maximum shutter speed supported with the sensor\n> + * \\var IPASessionConfiguration::sensor.maxIntegrationTime\n> + * \\brief Maximum integration time supported with the sensor\n> *\n> * \\var IPASessionConfiguration::sensor.minAnalogueGain\n> * \\brief Minimum analogue gain supported with the sensor\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 256b75eb..3405a260 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -43,8 +43,8 @@ struct IPASessionConfiguration {\n> \t} lsc;\n> \n> \tstruct {\n> -\t\tutils::Duration minShutterSpeed;\n> -\t\tutils::Duration maxShutterSpeed;\n> +\t\tutils::Duration minIntegrationTime;\n> +\t\tutils::Duration maxIntegrationTime;\n> \t\tdouble minAnalogueGain;\n> \t\tdouble maxAnalogueGain;\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index d8610095..15919d3f 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -245,14 +245,14 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> \n> \t/*\n> \t * When the AGC computes the new exposure values for a frame, it needs\n> -\t * to know the limits for shutter speed and analogue gain.\n> +\t * to know the limits for integration time and analogue gain.\n> \t * As it depends on the sensor, update it with the controls.\n> \t *\n> -\t * \\todo take VBLANK into account for maximum shutter speed\n> +\t * \\todo take VBLANK into account for maximum integration time\n> \t */\n> -\tcontext_.configuration.sensor.minShutterSpeed =\n> +\tcontext_.configuration.sensor.minIntegrationTime =\n> \t\tminExposure * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.sensor.maxShutterSpeed =\n> +\tcontext_.configuration.sensor.maxIntegrationTime =\n> \t\tmaxExposure * context_.configuration.sensor.lineDuration;\n> \tcontext_.configuration.sensor.minAnalogueGain = camHelper_->gain(minGain);\n> \tcontext_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);","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 9674AC3226\n\tfor ;\n\tWed, 8 May 2024 13:34:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E69E62C9F;\n\tWed, 8 May 2024 15:34:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93AAE62C9F\n\tfor ;\n\tWed, 8 May 2024 15:34:08 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9EE083D5;\n\tWed, 8 May 2024 15:34:05 +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=\"jKNg/hd1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715175245;\n\tbh=RleEAGccEVlDKFhUikwXo0EfRxgVzTXokaqvO6dD8UE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jKNg/hd1pIWxV7fmxfZxYHTS1Lzj4uB3EwHYs+R5W/nEgeNNS0KWqLj/RMjnpBkoj\n\tUzduEWEGUmMppjcGocqQ4A0urLa/3DaBN3vzwbN1PRJo0cPhhAMHZht9v/noxDdD+E\n\t0TqJGXtuQNTAAWta0nY6+m1WmwypRHUNG1OyusZs=","Date":"Wed, 8 May 2024 16:34:00 +0300","From":"Laurent Pinchart ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Refer to integration time rather than\n\tshutter speed","Message-ID":"<20240508133400.GL2012@pendragon.ideasonboard.com>","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-3-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240418124632.652163-3-dan.scally@ideasonboard.com>","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\" "}}]