[{"id":22273,"web_url":"https://patchwork.libcamera.org/comment/22273/","msgid":"","date":"2022-03-15T08:29:54","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Drop private\n\texposure and gain limits","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Tue, Mar 08, 2022 at 02:45:19PM +0530, Umang Jain wrote:\n> The private members of exposure and gain limits can be dropped\n> from IPARkISP1 since they are not used class-wide and can be easily\n> replaced by local counter-parts.\n> \n> In case they are required to be widely available, these should then\n> sit in IPASessionConfiguration.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 24 ++++++++++--------------\n> 1 file changed, 10 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 2d79f15f..129afddd 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -69,10 +69,6 @@ private:\n> \n> \t/* Camera sensor controls. */\n> \tbool autoExposure_;\n> -\tuint32_t minExposure_;\n> -\tuint32_t maxExposure_;\n> -\tuint32_t minGain_;\n> -\tuint32_t maxGain_;\n> \n> \t/* revision-specific data */\n> \trkisp1_cif_isp_version hwRevision_;\n> @@ -166,15 +162,15 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \n> \tautoExposure_ = true;\n> \n> -\tminExposure_ = itExp->second.min().get();\n> -\tmaxExposure_ = itExp->second.max().get();\n> +\tint32_t minExposure = itExp->second.min().get();\n> +\tint32_t maxExposure = itExp->second.max().get();\n> \n> -\tminGain_ = itGain->second.min().get();\n> -\tmaxGain_ = itGain->second.max().get();\n> +\tint32_t minGain = itGain->second.min().get();\n> +\tint32_t maxGain = itGain->second.max().get();\n> \n> \tLOG(IPARkISP1, Info)\n> -\t\t<< \"Exposure: \" << minExposure_ << \"-\" << maxExposure_\n> -\t\t<< \" Gain: \" << minGain_ << \"-\" << maxGain_;\n> +\t\t<< \"Exposure: \" << minExposure << \"-\" << maxExposure\n> +\t\t<< \" Gain: \" << minGain << \"-\" << maxGain;\n> \n> \t/* Clean context at configuration */\n> \tcontext_ = {};\n> @@ -191,10 +187,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \t *\n> \t * \\todo take VBLANK into account for maximum shutter speed\n> \t */\n> -\tcontext_.configuration.agc.minShutterSpeed = minExposure_ * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.agc.maxShutterSpeed = maxExposure_ * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);\n> -\tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);\n> +\tcontext_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;\n> +\tcontext_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;\n> +\tcontext_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n> +\tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> \n> \tfor (auto const &algo : algorithms_) {\n> \t\tint ret = algo->configure(context_, info);","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 72FA5BF415\n\tfor ;\n\tTue, 15 Mar 2022 08:30:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D852D604EA;\n\tTue, 15 Mar 2022 09:30:12 +0100 (CET)","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 452F4601F8\n\tfor ;\n\tTue, 15 Mar 2022 09:30:11 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AE99751C;\n\tTue, 15 Mar 2022 09:30:10 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647333012;\n\tbh=QuxFU17/rgJxm7tkFrEoFKMg0gWysl+IpOdWMMvLKBQ=;\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=BRF4I0l7beZePZ7YXoiWBcRvBt3IzbQJSknkbxg4mrM73Vfb9Iaau3riXwMPFugoX\n\tA/qFs4bBPFwWn7ZSSzTmMbMla+WZtgTu9bpm1RkzxZuD8o2uOimOWeLI5ukv4t/d2w\n\tE2voz5N41Nxwj5i6m189rz19DDfdbhE3asCWEfwSOrhnnYt9TBbi5T7tTEQb+FVhj8\n\tB/cdATZy39xhdo9GdQ5e4CLjZVyekOwFCWMpH3b5TXnaFWbWfY3Amz4GHlGUW4/Ujk\n\tcaPxqQKJ3LSZdfDmLak/qyLaXpEq7g2ZE83KRYf7B7md5WznOWeyx8IlhPOLvnZlUi\n\touH9UhLwTx5eQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647333010;\n\tbh=QuxFU17/rgJxm7tkFrEoFKMg0gWysl+IpOdWMMvLKBQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nKxjq1Y75AkCWUkLOPbUWTAim69ddIQ3OaG9//oHIsJWnoE5T4BTM1PAOrg9Rvb6E\n\tBxXftDm88/JRbju9I/9zf5fExf52Aa0ai/LfB/EvCtAV7mA9cadpmDczpvNkUuaRqF\n\tuc8aIjM/S24YuKJh39QR9ZrJp/rXG+vucXqIdTAo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nKxjq1Y7\"; dkim-atps=neutral","Date":"Tue, 15 Mar 2022 10:29:54 +0200","To":"Umang Jain ","Message-ID":"","References":"<20220308091520.34607-1-umang.jain@ideasonboard.com>\n\t<20220308091520.34607-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220308091520.34607-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Drop private\n\texposure and gain limits","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\" "}},{"id":22404,"web_url":"https://patchwork.libcamera.org/comment/22404/","msgid":"<20220323114612.GD3212506@pyrite.rasen.tech>","date":"2022-03-23T11:46:12","subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Drop private\n\texposure and gain limits","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nOn Tue, Mar 08, 2022 at 02:45:19PM +0530, Umang Jain via libcamera-devel wrote:\n> The private members of exposure and gain limits can be dropped\n> from IPARkISP1 since they are not used class-wide and can be easily\n> replaced by local counter-parts.\n> \n> In case they are required to be widely available, these should then\n> sit in IPASessionConfiguration.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 24 ++++++++++--------------\n> 1 file changed, 10 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 2d79f15f..129afddd 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -69,10 +69,6 @@ private:\n> \n> \t/* Camera sensor controls. */\n> \tbool autoExposure_;\n> -\tuint32_t minExposure_;\n> -\tuint32_t maxExposure_;\n> -\tuint32_t minGain_;\n> -\tuint32_t maxGain_;\n> \n> \t/* revision-specific data */\n> \trkisp1_cif_isp_version hwRevision_;\n> @@ -166,15 +162,15 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \n> \tautoExposure_ = true;\n> \n> -\tminExposure_ = itExp->second.min().get();\n> -\tmaxExposure_ = itExp->second.max().get();\n> +\tint32_t minExposure = itExp->second.min().get();\n> +\tint32_t maxExposure = itExp->second.max().get();\n> \n> -\tminGain_ = itGain->second.min().get();\n> -\tmaxGain_ = itGain->second.max().get();\n> +\tint32_t minGain = itGain->second.min().get();\n> +\tint32_t maxGain = itGain->second.max().get();\n> \n> \tLOG(IPARkISP1, Info)\n> -\t\t<< \"Exposure: \" << minExposure_ << \"-\" << maxExposure_\n> -\t\t<< \" Gain: \" << minGain_ << \"-\" << maxGain_;\n> +\t\t<< \"Exposure: \" << minExposure << \"-\" << maxExposure\n> +\t\t<< \" Gain: \" << minGain << \"-\" << maxGain;\n> \n> \t/* Clean context at configuration */\n> \tcontext_ = {};\n> @@ -191,10 +187,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \t *\n> \t * \\todo take VBLANK into account for maximum shutter speed\n> \t */\n> -\tcontext_.configuration.agc.minShutterSpeed = minExposure_ * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.agc.maxShutterSpeed = maxExposure_ * context_.configuration.sensor.lineDuration;\n> -\tcontext_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);\n> -\tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);\n> +\tcontext_.configuration.agc.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration;\n> +\tcontext_.configuration.agc.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration;\n> +\tcontext_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);\n> +\tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n> \n> \tfor (auto const &algo : algorithms_) {\n> \t\tint ret = algo->configure(context_, info);\n> -- \n> 2.31.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 B152CBD80A\n\tfor ;\n\tWed, 23 Mar 2022 11:46:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1EDA5604DA;\n\tWed, 23 Mar 2022 12:46:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A952A604C4\n\tfor ;\n\tWed, 23 Mar 2022 12:46:19 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 426E09DE;\n\tWed, 23 Mar 2022 12:46:17 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648035981;\n\tbh=uyp5u+8FBTHt/AwsiWWIbAikx+JftWT9AN+F4rF12Ls=;\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=TmnlBD7N0gz9z49GnrbA0RHbMrpDAi99WNV+4+yMtJuga1lb+UkkxvSzMtZd4yuEA\n\tpKUxjkQTjXU21F08GRqyNO6wr/TFLeXRSgoNJ5zMEwGk5vmfvvc+ZuxXzm2BtOvY4w\n\teiz7fEL4Q/pMjWtVCqX7bs4tE4lEESlNqiz6Lre+reDDiKGVbvrhMTlTD0AzNeyfwZ\n\toFYIU4nCpw90uMJHba81jvG/y42jpM75xUzBeW3nr1mL9ZGj1yEvYJolX+X8bIkVdl\n\tqgbYxHAfTT/M/9e43vxsp2+pOjpnI6oCBX+/5kfcWVV8ohYVhhPXNsSO+hO6B4rCGn\n\t2XxcqKn1Nq/Jw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648035979;\n\tbh=uyp5u+8FBTHt/AwsiWWIbAikx+JftWT9AN+F4rF12Ls=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eGM6BWk1k8Dt0fihXbit7x68i2Cfuo4OIv9rg8/y2t/9Bx3gUBo782T83wC7nma9W\n\tX+LQtLnX5fsJf6xD0+7gAUAxLPIP6Mo1dv5jgau8MoiO73o6xF/lHyG2FLpWRT4QtQ\n\tQvw8Exv8HrJXnI1kdI98ti7AEWDOFgIiY3LupfzY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"eGM6BWk1\"; dkim-atps=neutral","Date":"Wed, 23 Mar 2022 20:46:12 +0900","To":"Umang Jain ","Message-ID":"<20220323114612.GD3212506@pyrite.rasen.tech>","References":"<20220308091520.34607-1-umang.jain@ideasonboard.com>\n\t<20220308091520.34607-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220308091520.34607-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] ipa: rkisp1: Drop private\n\texposure and gain limits","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]