[{"id":17725,"web_url":"https://patchwork.libcamera.org/comment/17725/","msgid":"<31c9395c-0ef3-4a99-a28e-e358b14baacf@ideasonboard.com>","date":"2021-06-23T11:31:40","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Umang,\n\nThanks for the patch !\n\nOn 23/06/2021 13:22, Umang Jain wrote:\n> std::chrono::Duration is provided quite conveniently by\n> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n> for duration-type entities (such as exposure time), such that\n> it becomes consistent with rest of the codebase.\n> \n> The commit doesn't introduce any functional changes.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n> 2 files changed, 20 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> index 8ca95013..101ef288 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> @@ -19,6 +19,8 @@\n> \n> namespace libcamera {\n> \n> +using namespace std::literals::chrono_literals;\nYou could also add using libcamera::utils::Duration which would then\nallow to simplify the reading ?\n\n> +\n> namespace ipa::ipu3 {\n> \n> LOG_DEFINE_CATEGORY(IPU3Agc)\n> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n> IPU3Agc::IPU3Agc()\n> \t: frameCount_(0), lastFrame_(0), converged_(false),\n> \t updateControls_(false), iqMean_(0.0), gamma_(1.0),\n> -\t lineDuration_(0.0), maxExposureTime_(0.0),\n> -\t prevExposure_(0.0), prevExposureNoDg_(0.0),\n> -\t currentExposure_(0.0), currentExposureNoDg_(0.0)\n> +\t lineDuration_(0s), maxExposureTime_(0s),\n> +\t prevExposure_(0s), prevExposureNoDg_(0s),\n> +\t currentExposure_(0s), currentExposureNoDg_(0s)\n> {\n> }\n> \n> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraS\n> {\n> \taeGrid_ = bdsGrid;\n> \n> -\t/* line duration in microseconds */\n> -\tlineDuration_ = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n> +\tdouble ld = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n> +\tstd::chrono::duration lineDuration(ld);\n> +\tlineDuration_ = lineDuration;\n> \tmaxExposureTime_ = kMaxExposure * lineDuration_;\n> }\n> \n> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> void IPU3Agc::filterExposure()\n> {\n> \tdouble speed = 0.2;\n> -\tif (prevExposure_ == 0.0) {\n> +\tif (prevExposure_ == 0s) {\n> \t\t/* DG stands for digital gain.*/\n> \t\tprevExposure_ = currentExposure_;\n> \t\tprevExposureNoDg_ = currentExposureNoDg_;\n> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n> \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tdouble currentShutter = exposure * lineDuration_;\n> +\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> \t\tcurrentExposureNoDg_ = currentShutter * gain;\n> \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n> \t\t\t\t << \" Shutter speed \" << currentShutter\n> \t\t\t\t << \" Gain \" << gain;\n> \t\tcurrentExposure_ = currentExposureNoDg_ * newGain;\n> -\t\tdouble maxTotalExposure = maxExposureTime_ * kMaxGain;\n> +\t\tlibcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n> \t\tcurrentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> \t\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n> \n> \t\t/* \\todo: estimate if we need to desaturate */\n> \t\tfilterExposure();\n> \n> -\t\tdouble newExposure = 0.0;\n> +\t\tlibcamera::utils::Duration newExposure = 0.0s;\n> \t\tif (currentShutter < maxExposureTime_) {\n> \t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n> \t\t\tnewExposure = currentExposure_ / exposure;\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n> index f3d40557..de3e2dbf 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/ipu3_agc.h\n> @@ -14,6 +14,8 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/utils.h\"\n> +\n> #include \"libipa/algorithm.h\"\n> \n> namespace libcamera {\n> @@ -51,13 +53,13 @@ private:\n> \tdouble iqMean_;\n> \tdouble gamma_;\n> \n> -\tdouble lineDuration_;\n> -\tdouble maxExposureTime_;\n> +\tlibcamera::utils::Duration lineDuration_;\n> +\tlibcamera::utils::Duration maxExposureTime_;\n> \n> -\tdouble prevExposure_;\n> -\tdouble prevExposureNoDg_;\n> -\tdouble currentExposure_;\n> -\tdouble currentExposureNoDg_;\n> +\tlibcamera::utils::Duration prevExposure_;\n> +\tlibcamera::utils::Duration prevExposureNoDg_;\n> +\tlibcamera::utils::Duration currentExposure_;\n> +\tlibcamera::utils::Duration currentExposureNoDg_;\n> };\n> \n> } /* namespace ipa::ipu3 */\n> \n\nThis code will be removed in a short notice, but in the meantime (which\nwould let me rebase with this Duration usage ;-)):\nReviewed-by: Jean-Michel Hautbois ","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 7D8C6C321B\n\tfor ;\n\tWed, 23 Jun 2021 11:31:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F363B68937;\n\tWed, 23 Jun 2021 13:31:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2888268932\n\tfor ;\n\tWed, 23 Jun 2021 13:31:43 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:795d:5d24:f85f:c2e])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB7419AA;\n\tWed, 23 Jun 2021 13:31:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jd1tLfV8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624447902;\n\tbh=idkshPPxoS44FM6my19DEw3T2ZdJZOVKLq5RqP/4vWQ=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=jd1tLfV8HVhMf7KF3Pmx89fP9Lv1GhsPHB9kuYzuOptcQo5KEzQaefObvU2x6D6ZY\n\tG215iaZepBsmKsB7qUxghOdaK2L14NA1p2j09/YDUhw/7NVZVFzp/w/rX49xAdhJpQ\n\tFYi8E1AFZJvLrxaZpZiraf+BlxZ9OQ2islh2jjJo=","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"<31c9395c-0ef3-4a99-a28e-e358b14baacf@ideasonboard.com>","Date":"Wed, 23 Jun 2021 13:31:40 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210623112203.33561-1-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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":17726,"web_url":"https://patchwork.libcamera.org/comment/17726/","msgid":"<607e8714-7b80-e36f-97b1-753badf9a3e5@ideasonboard.com>","date":"2021-06-23T11:49:53","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 23/06/2021 12:31, Jean-Michel Hautbois wrote:\n> Hi Umang,\n> \n> Thanks for the patch !\n> \n> On 23/06/2021 13:22, Umang Jain wrote:\n>> std::chrono::Duration is provided quite conveniently by\n>> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n>> for duration-type entities (such as exposure time), such that\n>> it becomes consistent with rest of the codebase.\n>>\n>> The commit doesn't introduce any functional changes.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n>> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n>> 2 files changed, 20 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n>> index 8ca95013..101ef288 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.cpp\n>> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n>> @@ -19,6 +19,8 @@\n>> \n>> namespace libcamera {\n>> \n>> +using namespace std::literals::chrono_literals;\n> You could also add using libcamera::utils::Duration which would then\n> allow to simplify the reading ?\n> \n>> +\n>> namespace ipa::ipu3 {\n>> \n>> LOG_DEFINE_CATEGORY(IPU3Agc)\n>> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n>> IPU3Agc::IPU3Agc()\n>> \t: frameCount_(0), lastFrame_(0), converged_(false),\n>> \t updateControls_(false), iqMean_(0.0), gamma_(1.0),\n>> -\t lineDuration_(0.0), maxExposureTime_(0.0),\n>> -\t prevExposure_(0.0), prevExposureNoDg_(0.0),\n>> -\t currentExposure_(0.0), currentExposureNoDg_(0.0)\n>> +\t lineDuration_(0s), maxExposureTime_(0s),\n>> +\t prevExposure_(0s), prevExposureNoDg_(0s),\n>> +\t currentExposure_(0s), currentExposureNoDg_(0s)\n>> {\n>> }\n>> \n>> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraS\n>> {\n>> \taeGrid_ = bdsGrid;\n>> \n>> -\t/* line duration in microseconds */\n>> -\tlineDuration_ = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n>> +\tdouble ld = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n>> +\tstd::chrono::duration lineDuration(ld);\n>> +\tlineDuration_ = lineDuration;\n>> \tmaxExposureTime_ = kMaxExposure * lineDuration_;\n>> }\n>> \n>> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>> void IPU3Agc::filterExposure()\n>> {\n>> \tdouble speed = 0.2;\n>> -\tif (prevExposure_ == 0.0) {\n>> +\tif (prevExposure_ == 0s) {\n>> \t\t/* DG stands for digital gain.*/\n>> \t\tprevExposure_ = currentExposure_;\n>> \t\tprevExposureNoDg_ = currentExposureNoDg_;\n>> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n>> \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n>> \n>> \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n>> -\t\tdouble currentShutter = exposure * lineDuration_;\n>> +\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n>> \t\tcurrentExposureNoDg_ = currentShutter * gain;\n>> \t\tLOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n>> \t\t\t\t << \" Shutter speed \" << currentShutter\n>> \t\t\t\t << \" Gain \" << gain;\n>> \t\tcurrentExposure_ = currentExposureNoDg_ * newGain;\n>> -\t\tdouble maxTotalExposure = maxExposureTime_ * kMaxGain;\n>> +\t\tlibcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n>> \t\tcurrentExposure_ = std::min(currentExposure_, maxTotalExposure);\n>> \t\tLOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n>> \n>> \t\t/* \\todo: estimate if we need to desaturate */\n>> \t\tfilterExposure();\n>> \n>> -\t\tdouble newExposure = 0.0;\n>> +\t\tlibcamera::utils::Duration newExposure = 0.0s;\n>> \t\tif (currentShutter < maxExposureTime_) {\n>> \t\t\texposure = std::clamp(static_cast(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n>> \t\t\tnewExposure = currentExposure_ / exposure;\n>> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n>> index f3d40557..de3e2dbf 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.h\n>> +++ b/src/ipa/ipu3/ipu3_agc.h\n>> @@ -14,6 +14,8 @@\n>> \n>> #include \n>> \n>> +#include \"libcamera/internal/utils.h\"\n>> +\n>> #include \"libipa/algorithm.h\"\n>> \n>> namespace libcamera {\n>> @@ -51,13 +53,13 @@ private:\n>> \tdouble iqMean_;\n>> \tdouble gamma_;\n>> \n>> -\tdouble lineDuration_;\n>> -\tdouble maxExposureTime_;\n>> +\tlibcamera::utils::Duration lineDuration_;\n>> +\tlibcamera::utils::Duration maxExposureTime_;\n\nNo need to prefix with libcamera:: when you're in the libcamera\nnamespace already.\n\n\n>> \n>> -\tdouble prevExposure_;\n>> -\tdouble prevExposureNoDg_;\n>> -\tdouble currentExposure_;\n>> -\tdouble currentExposureNoDg_;\n>> +\tlibcamera::utils::Duration prevExposure_;\n>> +\tlibcamera::utils::Duration prevExposureNoDg_;\n>> +\tlibcamera::utils::Duration currentExposure_;\n>> +\tlibcamera::utils::Duration currentExposureNoDg_;\n\nAnd same of course.\n\nBut as JM suggested, perhaps this is a case where\n'using libcamera::utils::Duration'\n\nwould let us express these lines concisely as:\n\n\tDuration currentExposureNoDg_;\n\n>> };\n>> \n>> } /* namespace ipa::ipu3 */\n>>\n> \n> This code will be removed in a short notice, but in the meantime (which\n> would let me rebase with this Duration usage ;-)):\n> Reviewed-by: Jean-Michel Hautbois \n\n\nSounds like this is better in sooner than later to ease the races.\n\nI bet I'm also going to be racing here with the library splits.\n\nReviewed-by: Kieran Bingham ","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 B071DC321A\n\tfor ;\n\tWed, 23 Jun 2021 11:49:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E95E468937;\n\tWed, 23 Jun 2021 13:49:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2A5F68932\n\tfor ;\n\tWed, 23 Jun 2021 13:49:56 +0200 (CEST)","from [192.168.0.20]\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 6101EA66;\n\tWed, 23 Jun 2021 13:49:56 +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=\"uVHNA1ag\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624448996;\n\tbh=BYkhf/PZOwQamcuycVbi5n1jGTqwUeZ/6XUlRTrv7rE=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=uVHNA1agZugCGI6NjDtIg4MoArHrL5dUywx1jRzAd0FkbsNiD5478hqlFyoqpxCzh\n\tSSkdUdupl83sQcUP2ZDs4Z8KyYId7dQkDWYaXYpwqUCGZA8G8TvnqBM8dPUmmivDVZ\n\tP567yP96CrJWPvB0Efin7ugplHMoBMYoNbFXHRwI=","To":"Jean-Michel Hautbois ,\n\tUmang Jain ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>\n\t<31c9395c-0ef3-4a99-a28e-e358b14baacf@ideasonboard.com>","From":"Kieran Bingham ","Message-ID":"<607e8714-7b80-e36f-97b1-753badf9a3e5@ideasonboard.com>","Date":"Wed, 23 Jun 2021 12:49:53 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.8.1","MIME-Version":"1.0","In-Reply-To":"<31c9395c-0ef3-4a99-a28e-e358b14baacf@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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":17727,"web_url":"https://patchwork.libcamera.org/comment/17727/","msgid":"","date":"2021-06-23T11:57:32","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Umang,\n\n\nOn Wed, 23 Jun 2021 at 12:22, Umang Jain \nwrote:\n\n> std::chrono::Duration is provided quite conveniently by\n> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n> for duration-type entities (such as exposure time), such that\n> it becomes consistent with rest of the codebase.\n>\n> The commit doesn't introduce any functional changes.\n>\n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n> 2 files changed, 20 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> index 8ca95013..101ef288 100644\n> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> @@ -19,6 +19,8 @@\n>\n> namespace libcamera {\n>\n> +using namespace std::literals::chrono_literals;\n> +\n> namespace ipa::ipu3 {\n>\n> LOG_DEFINE_CATEGORY(IPU3Agc)\n> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n> IPU3Agc::IPU3Agc()\n> : frameCount_(0), lastFrame_(0), converged_(false),\n> updateControls_(false), iqMean_(0.0), gamma_(1.0),\n> - lineDuration_(0.0), maxExposureTime_(0.0),\n> - prevExposure_(0.0), prevExposureNoDg_(0.0),\n> - currentExposure_(0.0), currentExposureNoDg_(0.0)\n> + lineDuration_(0s), maxExposureTime_(0s),\n> + prevExposure_(0s), prevExposureNoDg_(0s),\n> + currentExposure_(0s), currentExposureNoDg_(0s)\n> {\n> }\n>\n> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config\n> &bdsGrid, const IPACameraS\n> {\n> aeGrid_ = bdsGrid;\n>\n> - /* line duration in microseconds */\n> - lineDuration_ = sensorInfo.lineLength * 1000000ULL /\n> static_cast(sensorInfo.pixelRate);\n> + double ld = sensorInfo.lineLength * 1000000ULL /\n> static_cast(sensorInfo.pixelRate);\n> + std::chrono::duration lineDuration(ld);\n> + lineDuration_ = lineDuration;\n>\n\nYou could write this as:\nlineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate\n\nand let the compiler worry about any and all conversions.\n\nNaush\n\n\n\n> maxExposureTime_ = kMaxExposure * lineDuration_;\n> }\n>\n> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const\n> ipu3_uapi_stats_3a *stats)\n> void IPU3Agc::filterExposure()\n> {\n> double speed = 0.2;\n> - if (prevExposure_ == 0.0) {\n> + if (prevExposure_ == 0s) {\n> /* DG stands for digital gain.*/\n> prevExposure_ = currentExposure_;\n> prevExposureNoDg_ = currentExposureNoDg_;\n> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure,\n> uint32_t &gain)\n> double newGain = kEvGainTarget * knumHistogramBins /\n> iqMean_;\n>\n> /* extracted from Rpi::Agc::computeTargetExposure */\n> - double currentShutter = exposure * lineDuration_;\n> + libcamera::utils::Duration currentShutter = exposure *\n> lineDuration_;\n> currentExposureNoDg_ = currentShutter * gain;\n> LOG(IPU3Agc, Debug) << \"Actual total exposure \" <<\n> currentExposureNoDg_\n> << \" Shutter speed \" << currentShutter\n> << \" Gain \" << gain;\n> currentExposure_ = currentExposureNoDg_ * newGain;\n> - double maxTotalExposure = maxExposureTime_ * kMaxGain;\n> + libcamera::utils::Duration maxTotalExposure =\n> maxExposureTime_ * kMaxGain;\n> currentExposure_ = std::min(currentExposure_,\n> maxTotalExposure);\n> LOG(IPU3Agc, Debug) << \"Target total exposure \" <<\n> currentExposure_;\n>\n> /* \\todo: estimate if we need to desaturate */\n> filterExposure();\n>\n> - double newExposure = 0.0;\n> + libcamera::utils::Duration newExposure = 0.0s;\n> if (currentShutter < maxExposureTime_) {\n> exposure =\n> std::clamp(static_cast(exposure * currentExposure_ /\n> currentExposureNoDg_), kMinExposure, kMaxExposure);\n> newExposure = currentExposure_ / exposure;\n> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n> index f3d40557..de3e2dbf 100644\n> --- a/src/ipa/ipu3/ipu3_agc.h\n> +++ b/src/ipa/ipu3/ipu3_agc.h\n> @@ -14,6 +14,8 @@\n>\n> #include \n>\n> +#include \"libcamera/internal/utils.h\"\n> +\n> #include \"libipa/algorithm.h\"\n>\n> namespace libcamera {\n> @@ -51,13 +53,13 @@ private:\n> double iqMean_;\n> double gamma_;\n>\n> - double lineDuration_;\n> - double maxExposureTime_;\n> + libcamera::utils::Duration lineDuration_;\n> + libcamera::utils::Duration maxExposureTime_;\n>\n> - double prevExposure_;\n> - double prevExposureNoDg_;\n> - double currentExposure_;\n> - double currentExposureNoDg_;\n> + libcamera::utils::Duration prevExposure_;\n> + libcamera::utils::Duration prevExposureNoDg_;\n> + libcamera::utils::Duration currentExposure_;\n> + libcamera::utils::Duration currentExposureNoDg_;\n> };\n>\n> } /* namespace ipa::ipu3 */\n> --\n> 2.31.1\n>\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 64FB6C321B\n\tfor ;\n\tWed, 23 Jun 2021 11:57:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DC34768933;\n\tWed, 23 Jun 2021 13:57:50 +0200 (CEST)","from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com\n\t[IPv6:2a00:1450:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BE9E68932\n\tfor ;\n\tWed, 23 Jun 2021 13:57:49 +0200 (CEST)","by mail-lj1-x22f.google.com with SMTP id d13so2584182ljg.12\n\tfor ;\n\tWed, 23 Jun 2021 04:57:49 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"AjLmvAXn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=UL2XKzWF+np/c7aZQtKP67O4XbYRlaiZ+gZTwcqgwjc=;\n\tb=AjLmvAXnJwbYjVF0pyO/nE8h6Yy+fGgR40WdzDmmI3DukTQsKIzTVXsIQKhcPL9IrE\n\tM6AWdST71kdLpEqlUCE6VftiejWQ2PXC589J8FSBn7wWYQVjXI8nVRiWmzbMwc55KUqj\n\tG4Ab/PHwWuNbpOLYSNObX1G1AIxzlxL4dSCScEd5PYrP134e9eiEcKAWMzPYr07uwj8N\n\tX7dy1b+RT+IJ+nuBGl94h/rMpZblr53Ii0tSOxss1Mt+n7ZD/AMwaVHEzNwesyE5Yey6\n\tRYELpBWPBwkXrAjmAvIuZ5nGPA6atzPAfQLKbRxcOrEI162sgxcPyZQOgrLAGUHcfDka\n\tF8yw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=UL2XKzWF+np/c7aZQtKP67O4XbYRlaiZ+gZTwcqgwjc=;\n\tb=e/CzstOHOCt3SFwqEfucpgmvN3ANj3zHGJwdP+N5xAWAIpovYsPF4OX7ijrhSwHS7r\n\tSYOSyFekLUvVj7VSaxKQrbcA9LKwo5zGbhq/IPRwKEAuV9H8bP2HQxpJVgRobjZSaURb\n\tl+8Hfc19HS0q5U4MGdt4yJo3lxsUrvK+QjeobMeb+izxPTEwruotF/eHi4PlP4b0RSCj\n\t34MwKFjDFQXsoLe/MO2emMKEWPJ19Pgka7lC9UvyVU2vQHQyTYE5qtgsNoQPb6wwnnY4\n\t9imn9H85+tTe5TM3KgQEJxQB581Egg2MUnnRdMkk22mx6InfRJAkc7flK3qxoAyzxAf9\n\tpcHQ==","X-Gm-Message-State":"AOAM530BusVABezRniWjrEUiNhM+h5sQo9eC3FQoKZkfDgB/wAyk64tV\n\trMRwwQhh6s7aNs+XFEWrGgvmz7KeO3Qnofur7k0hgjYcV3z9MA==","X-Google-Smtp-Source":"ABdhPJydL1HZNIlUuGiActflbumbEr6Kbl3e0kKuROJZkiVY1+YMGQW+2G1u0lnijRaMFSSp2XATgFBwGIIT52c2hqw=","X-Received":"by 2002:a05:651c:542:: with SMTP id\n\tq2mr7295403ljp.400.1624449468765; \n\tWed, 23 Jun 2021 04:57:48 -0700 (PDT)","MIME-Version":"1.0","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>","In-Reply-To":"<20210623112203.33561-1-umang.jain@ideasonboard.com>","From":"Naushir Patuck ","Date":"Wed, 23 Jun 2021 12:57:32 +0100","Message-ID":"","To":"Umang Jain ","Content-Type":"multipart/alternative; boundary=\"0000000000008aacba05c56d9d13\"","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17728,"web_url":"https://patchwork.libcamera.org/comment/17728/","msgid":"","date":"2021-06-23T12:28:40","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Naushir,\n\nOn 6/23/21 5:27 PM, Naushir Patuck wrote:\n> Hi Umang,\n>\n>\n> On Wed, 23 Jun 2021 at 12:22, Umang Jain \n> wrote:\n>\n>> std::chrono::Duration is provided quite conveniently by\n>> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n>> for duration-type entities (such as exposure time), such that\n>> it becomes consistent with rest of the codebase.\n>>\n>> The commit doesn't introduce any functional changes.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n>> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n>> 2 files changed, 20 insertions(+), 15 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n>> index 8ca95013..101ef288 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.cpp\n>> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n>> @@ -19,6 +19,8 @@\n>>\n>> namespace libcamera {\n>>\n>> +using namespace std::literals::chrono_literals;\n>> +\n>> namespace ipa::ipu3 {\n>>\n>> LOG_DEFINE_CATEGORY(IPU3Agc)\n>> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n>> IPU3Agc::IPU3Agc()\n>> : frameCount_(0), lastFrame_(0), converged_(false),\n>> updateControls_(false), iqMean_(0.0), gamma_(1.0),\n>> - lineDuration_(0.0), maxExposureTime_(0.0),\n>> - prevExposure_(0.0), prevExposureNoDg_(0.0),\n>> - currentExposure_(0.0), currentExposureNoDg_(0.0)\n>> + lineDuration_(0s), maxExposureTime_(0s),\n>> + prevExposure_(0s), prevExposureNoDg_(0s),\n>> + currentExposure_(0s), currentExposureNoDg_(0s)\n>> {\n>> }\n>>\n>> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config\n>> &bdsGrid, const IPACameraS\n>> {\n>> aeGrid_ = bdsGrid;\n>>\n>> - /* line duration in microseconds */\n>> - lineDuration_ = sensorInfo.lineLength * 1000000ULL /\n>> static_cast(sensorInfo.pixelRate);\n>> + double ld = sensorInfo.lineLength * 1000000ULL /\n>> static_cast(sensorInfo.pixelRate);\n>> + std::chrono::duration lineDuration(ld);\n>> + lineDuration_ = lineDuration;\n>>\n> You could write this as:\n> lineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate\n>\n> and let the compiler worry about any and all conversions.\n\n\nthis is neat - I think I should have read more rather than assuming I \nneed to convert it manually at first!\n\nThanks!\n\n>\n> Naush\n>\n>\n>\n>> maxExposureTime_ = kMaxExposure * lineDuration_;\n>> }\n>>\n>> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const\n>> ipu3_uapi_stats_3a *stats)\n>> void IPU3Agc::filterExposure()\n>> {\n>> double speed = 0.2;\n>> - if (prevExposure_ == 0.0) {\n>> + if (prevExposure_ == 0s) {\n>> /* DG stands for digital gain.*/\n>> prevExposure_ = currentExposure_;\n>> prevExposureNoDg_ = currentExposureNoDg_;\n>> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure,\n>> uint32_t &gain)\n>> double newGain = kEvGainTarget * knumHistogramBins /\n>> iqMean_;\n>>\n>> /* extracted from Rpi::Agc::computeTargetExposure */\n>> - double currentShutter = exposure * lineDuration_;\n>> + libcamera::utils::Duration currentShutter = exposure *\n>> lineDuration_;\n>> currentExposureNoDg_ = currentShutter * gain;\n>> LOG(IPU3Agc, Debug) << \"Actual total exposure \" <<\n>> currentExposureNoDg_\n>> << \" Shutter speed \" << currentShutter\n>> << \" Gain \" << gain;\n>> currentExposure_ = currentExposureNoDg_ * newGain;\n>> - double maxTotalExposure = maxExposureTime_ * kMaxGain;\n>> + libcamera::utils::Duration maxTotalExposure =\n>> maxExposureTime_ * kMaxGain;\n>> currentExposure_ = std::min(currentExposure_,\n>> maxTotalExposure);\n>> LOG(IPU3Agc, Debug) << \"Target total exposure \" <<\n>> currentExposure_;\n>>\n>> /* \\todo: estimate if we need to desaturate */\n>> filterExposure();\n>>\n>> - double newExposure = 0.0;\n>> + libcamera::utils::Duration newExposure = 0.0s;\n>> if (currentShutter < maxExposureTime_) {\n>> exposure =\n>> std::clamp(static_cast(exposure * currentExposure_ /\n>> currentExposureNoDg_), kMinExposure, kMaxExposure);\n>> newExposure = currentExposure_ / exposure;\n>> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n>> index f3d40557..de3e2dbf 100644\n>> --- a/src/ipa/ipu3/ipu3_agc.h\n>> +++ b/src/ipa/ipu3/ipu3_agc.h\n>> @@ -14,6 +14,8 @@\n>>\n>> #include \n>>\n>> +#include \"libcamera/internal/utils.h\"\n>> +\n>> #include \"libipa/algorithm.h\"\n>>\n>> namespace libcamera {\n>> @@ -51,13 +53,13 @@ private:\n>> double iqMean_;\n>> double gamma_;\n>>\n>> - double lineDuration_;\n>> - double maxExposureTime_;\n>> + libcamera::utils::Duration lineDuration_;\n>> + libcamera::utils::Duration maxExposureTime_;\n>>\n>> - double prevExposure_;\n>> - double prevExposureNoDg_;\n>> - double currentExposure_;\n>> - double currentExposureNoDg_;\n>> + libcamera::utils::Duration prevExposure_;\n>> + libcamera::utils::Duration prevExposureNoDg_;\n>> + libcamera::utils::Duration currentExposure_;\n>> + libcamera::utils::Duration currentExposureNoDg_;\n>> };\n>>\n>> } /* namespace ipa::ipu3 */\n>> --\n>> 2.31.1\n>>\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 00280C321A\n\tfor ;\n\tWed, 23 Jun 2021 12:28:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73CFE68939;\n\tWed, 23 Jun 2021 14:28:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E9B7368932\n\tfor ;\n\tWed, 23 Jun 2021 14:28:45 +0200 (CEST)","from [192.168.0.104] (unknown [103.251.226.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F36A6A66;\n\tWed, 23 Jun 2021 14:28:44 +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=\"JeAwnaHX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624451325;\n\tbh=d4clo2woaSJ8SGhUPQ/FtB4FrQH9CteBY5V2ZpN5tMs=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=JeAwnaHXoEQ6khHGrM3YlHJILnGKaaIdlV4ZOfMSP3IcE3IDYf8czJBd928zpwjks\n\tQr1/ZF92MEqgZciK6VOa60EZf+SJ3SRhBpqF3gwLCc7NLQJIstyadHbkysBzZbBXok\n\tQJhRjjdiaWS7aFUd67Fs6KSQTqtBgaqf4NWFn1JU=","To":"Naushir Patuck ","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>\n\t","From":"Umang Jain ","Message-ID":"","Date":"Wed, 23 Jun 2021 17:58:40 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17729,"web_url":"https://patchwork.libcamera.org/comment/17729/","msgid":"","date":"2021-06-23T12:36:46","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Wed, Jun 23, 2021 at 05:58:40PM +0530, Umang Jain wrote:\n> On 6/23/21 5:27 PM, Naushir Patuck wrote:\n> > On Wed, 23 Jun 2021 at 12:22, Umang Jain wrote:\n> >\n> >> std::chrono::Duration is provided quite conveniently by\n> >> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n> >> for duration-type entities (such as exposure time), such that\n> >> it becomes consistent with rest of the codebase.\n> >>\n> >> The commit doesn't introduce any functional changes.\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n> >> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n> >> 2 files changed, 20 insertions(+), 15 deletions(-)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n> >> index 8ca95013..101ef288 100644\n> >> --- a/src/ipa/ipu3/ipu3_agc.cpp\n> >> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n> >> @@ -19,6 +19,8 @@\n> >>\n> >> namespace libcamera {\n> >>\n> >> +using namespace std::literals::chrono_literals;\n> >> +\n> >> namespace ipa::ipu3 {\n> >>\n> >> LOG_DEFINE_CATEGORY(IPU3Agc)\n> >> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n> >> IPU3Agc::IPU3Agc()\n> >> : frameCount_(0), lastFrame_(0), converged_(false),\n> >> updateControls_(false), iqMean_(0.0), gamma_(1.0),\n> >> - lineDuration_(0.0), maxExposureTime_(0.0),\n> >> - prevExposure_(0.0), prevExposureNoDg_(0.0),\n> >> - currentExposure_(0.0), currentExposureNoDg_(0.0)\n> >> + lineDuration_(0s), maxExposureTime_(0s),\n> >> + prevExposure_(0s), prevExposureNoDg_(0s),\n> >> + currentExposure_(0s), currentExposureNoDg_(0s)\n> >> {\n> >> }\n> >>\n> >> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraS\n> >> {\n> >> aeGrid_ = bdsGrid;\n> >>\n> >> - /* line duration in microseconds */\n> >> - lineDuration_ = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n> >> + double ld = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n> >> + std::chrono::duration lineDuration(ld);\n> >> + lineDuration_ = lineDuration;\n> >>\n> >\n> > You could write this as:\n> > lineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate\n> >\n> > and let the compiler worry about any and all conversions.\n> \n> this is neat - I think I should have read more rather than assuming I \n> need to convert it manually at first!\n\nIt's kind of the whole point of std::chrono :-) If you have to do the\nconversions explicitly, there's a high chance the code isn't using\nstd::chrono correctly (there are of course exceptions).\n\n> >> maxExposureTime_ = kMaxExposure * lineDuration_;\n> >> }\n> >>\n> >> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n> >> void IPU3Agc::filterExposure()\n> >> {\n> >> double speed = 0.2;\n> >> - if (prevExposure_ == 0.0) {\n> >> + if (prevExposure_ == 0s) {\n> >> /* DG stands for digital gain.*/\n> >> prevExposure_ = currentExposure_;\n> >> prevExposureNoDg_ = currentExposureNoDg_;\n> >> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n> >> double newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> >>\n> >> /* extracted from Rpi::Agc::computeTargetExposure */\n> >> - double currentShutter = exposure * lineDuration_;\n> >> + libcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> >> currentExposureNoDg_ = currentShutter * gain;\n> >> LOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n> >> << \" Shutter speed \" << currentShutter\n> >> << \" Gain \" << gain;\n> >> currentExposure_ = currentExposureNoDg_ * newGain;\n> >> - double maxTotalExposure = maxExposureTime_ * kMaxGain;\n> >> + libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n> >> currentExposure_ = std::min(currentExposure_, maxTotalExposure);\n> >> LOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n> >>\n> >> /* \\todo: estimate if we need to desaturate */\n> >> filterExposure();\n> >>\n> >> - double newExposure = 0.0;\n> >> + libcamera::utils::Duration newExposure = 0.0s;\n> >> if (currentShutter < maxExposureTime_) {\n> >> exposure = std::clamp(static_cast(exposure * currentExposure_ /\n> >> currentExposureNoDg_), kMinExposure, kMaxExposure);\n> >> newExposure = currentExposure_ / exposure;\n> >> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n> >> index f3d40557..de3e2dbf 100644\n> >> --- a/src/ipa/ipu3/ipu3_agc.h\n> >> +++ b/src/ipa/ipu3/ipu3_agc.h\n> >> @@ -14,6 +14,8 @@\n> >>\n> >> #include \n> >>\n> >> +#include \"libcamera/internal/utils.h\"\n> >> +\n> >> #include \"libipa/algorithm.h\"\n> >>\n> >> namespace libcamera {\n> >> @@ -51,13 +53,13 @@ private:\n> >> double iqMean_;\n> >> double gamma_;\n> >>\n> >> - double lineDuration_;\n> >> - double maxExposureTime_;\n> >> + libcamera::utils::Duration lineDuration_;\n> >> + libcamera::utils::Duration maxExposureTime_;\n> >>\n> >> - double prevExposure_;\n> >> - double prevExposureNoDg_;\n> >> - double currentExposure_;\n> >> - double currentExposureNoDg_;\n> >> + libcamera::utils::Duration prevExposure_;\n> >> + libcamera::utils::Duration prevExposureNoDg_;\n> >> + libcamera::utils::Duration currentExposure_;\n> >> + libcamera::utils::Duration currentExposureNoDg_;\n> >> };\n> >>\n> >> } /* namespace ipa::ipu3 */","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 5E2AEC321B\n\tfor ;\n\tWed, 23 Jun 2021 12:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0ECA068934;\n\tWed, 23 Jun 2021 14:37:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E10868932\n\tfor ;\n\tWed, 23 Jun 2021 14:37:17 +0200 (CEST)","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 3471D9AA;\n\tWed, 23 Jun 2021 14:37:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"T73P6wQX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624451837;\n\tbh=uCVEXOhsZFhJwE2TWWz+DFoHUcLQmxXk5MMw49X210Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T73P6wQXJI3Dr9KdHpGK3FSCO1wIAxWlqOj1KDxf0TuFSlhgpllHBlVT9CkVi/URZ\n\tRLEUOIGBYDnlA6XtiwLl664BXFTgEaV9HVtkfPi3Bjqsgjxv63HfC1RalV1gmXapFa\n\tYYfJeZ3yYU2GJQOqt1jhLRkHvMYQKocg1a8TTKiA=","Date":"Wed, 23 Jun 2021 15:36:46 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17730,"web_url":"https://patchwork.libcamera.org/comment/17730/","msgid":"<96d555cd-a154-c58d-b552-ef6c6676497a@ideasonboard.com>","date":"2021-06-23T12:40:50","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 6/23/21 6:06 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> On Wed, Jun 23, 2021 at 05:58:40PM +0530, Umang Jain wrote:\n>> On 6/23/21 5:27 PM, Naushir Patuck wrote:\n>>> On Wed, 23 Jun 2021 at 12:22, Umang Jain wrote:\n>>>\n>>>> std::chrono::Duration is provided quite conveniently by\n>>>> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that\n>>>> for duration-type entities (such as exposure time), such that\n>>>> it becomes consistent with rest of the codebase.\n>>>>\n>>>> The commit doesn't introduce any functional changes.\n>>>>\n>>>> Signed-off-by: Umang Jain \n>>>> ---\n>>>> src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------\n>>>> src/ipa/ipu3/ipu3_agc.h | 14 ++++++++------\n>>>> 2 files changed, 20 insertions(+), 15 deletions(-)\n>>>>\n>>>> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp\n>>>> index 8ca95013..101ef288 100644\n>>>> --- a/src/ipa/ipu3/ipu3_agc.cpp\n>>>> +++ b/src/ipa/ipu3/ipu3_agc.cpp\n>>>> @@ -19,6 +19,8 @@\n>>>>\n>>>> namespace libcamera {\n>>>>\n>>>> +using namespace std::literals::chrono_literals;\n>>>> +\n>>>> namespace ipa::ipu3 {\n>>>>\n>>>> LOG_DEFINE_CATEGORY(IPU3Agc)\n>>>> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;\n>>>> IPU3Agc::IPU3Agc()\n>>>> : frameCount_(0), lastFrame_(0), converged_(false),\n>>>> updateControls_(false), iqMean_(0.0), gamma_(1.0),\n>>>> - lineDuration_(0.0), maxExposureTime_(0.0),\n>>>> - prevExposure_(0.0), prevExposureNoDg_(0.0),\n>>>> - currentExposure_(0.0), currentExposureNoDg_(0.0)\n>>>> + lineDuration_(0s), maxExposureTime_(0s),\n>>>> + prevExposure_(0s), prevExposureNoDg_(0s),\n>>>> + currentExposure_(0s), currentExposureNoDg_(0s)\n>>>> {\n>>>> }\n>>>>\n>>>> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config &bdsGrid, const IPACameraS\n>>>> {\n>>>> aeGrid_ = bdsGrid;\n>>>>\n>>>> - /* line duration in microseconds */\n>>>> - lineDuration_ = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n>>>> + double ld = sensorInfo.lineLength * 1000000ULL / static_cast(sensorInfo.pixelRate);\n>>>> + std::chrono::duration lineDuration(ld);\n>>>> + lineDuration_ = lineDuration;\n>>>>\n>>> You could write this as:\n>>> lineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate\n>>>\n>>> and let the compiler worry about any and all conversions.\n>> this is neat - I think I should have read more rather than assuming I\n>> need to convert it manually at first!\n> It's kind of the whole point of std::chrono :-) If you have to do the\n> conversions explicitly, there's a high chance the code isn't using\n> std::chrono correctly (there are of course exceptions).\n\nYes, I got that. In my mind, I was initially thinking, once we have \natleast one std::chrono , we can derive/operate others with that \nstd::chrono entity. In this case, lineDuration_ is the base variable \nfrom which further calculations or entities follow.  And I just assumed, \nthat the first one, we need to go from a scaler-value -> std::chrono, \ndoing manual conversion.\n\nNaushir's introduction of ( * 1.0s) is just converts everything nicely!\n\n>>>> maxExposureTime_ = kMaxExposure * lineDuration_;\n>>>> }\n>>>>\n>>>> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const ipu3_uapi_stats_3a *stats)\n>>>> void IPU3Agc::filterExposure()\n>>>> {\n>>>> double speed = 0.2;\n>>>> - if (prevExposure_ == 0.0) {\n>>>> + if (prevExposure_ == 0s) {\n>>>> /* DG stands for digital gain.*/\n>>>> prevExposure_ = currentExposure_;\n>>>> prevExposureNoDg_ = currentExposureNoDg_;\n>>>> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure, uint32_t &gain)\n>>>> double newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n>>>>\n>>>> /* extracted from Rpi::Agc::computeTargetExposure */\n>>>> - double currentShutter = exposure * lineDuration_;\n>>>> + libcamera::utils::Duration currentShutter = exposure * lineDuration_;\n>>>> currentExposureNoDg_ = currentShutter * gain;\n>>>> LOG(IPU3Agc, Debug) << \"Actual total exposure \" << currentExposureNoDg_\n>>>> << \" Shutter speed \" << currentShutter\n>>>> << \" Gain \" << gain;\n>>>> currentExposure_ = currentExposureNoDg_ * newGain;\n>>>> - double maxTotalExposure = maxExposureTime_ * kMaxGain;\n>>>> + libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n>>>> currentExposure_ = std::min(currentExposure_, maxTotalExposure);\n>>>> LOG(IPU3Agc, Debug) << \"Target total exposure \" << currentExposure_;\n>>>>\n>>>> /* \\todo: estimate if we need to desaturate */\n>>>> filterExposure();\n>>>>\n>>>> - double newExposure = 0.0;\n>>>> + libcamera::utils::Duration newExposure = 0.0s;\n>>>> if (currentShutter < maxExposureTime_) {\n>>>> exposure = std::clamp(static_cast(exposure * currentExposure_ /\n>>>> currentExposureNoDg_), kMinExposure, kMaxExposure);\n>>>> newExposure = currentExposure_ / exposure;\n>>>> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h\n>>>> index f3d40557..de3e2dbf 100644\n>>>> --- a/src/ipa/ipu3/ipu3_agc.h\n>>>> +++ b/src/ipa/ipu3/ipu3_agc.h\n>>>> @@ -14,6 +14,8 @@\n>>>>\n>>>> #include \n>>>>\n>>>> +#include \"libcamera/internal/utils.h\"\n>>>> +\n>>>> #include \"libipa/algorithm.h\"\n>>>>\n>>>> namespace libcamera {\n>>>> @@ -51,13 +53,13 @@ private:\n>>>> double iqMean_;\n>>>> double gamma_;\n>>>>\n>>>> - double lineDuration_;\n>>>> - double maxExposureTime_;\n>>>> + libcamera::utils::Duration lineDuration_;\n>>>> + libcamera::utils::Duration maxExposureTime_;\n>>>>\n>>>> - double prevExposure_;\n>>>> - double prevExposureNoDg_;\n>>>> - double currentExposure_;\n>>>> - double currentExposureNoDg_;\n>>>> + libcamera::utils::Duration prevExposure_;\n>>>> + libcamera::utils::Duration prevExposureNoDg_;\n>>>> + libcamera::utils::Duration currentExposure_;\n>>>> + libcamera::utils::Duration currentExposureNoDg_;\n>>>> };\n>>>>\n>>>> } /* namespace ipa::ipu3 */","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 32383C321A\n\tfor ;\n\tWed, 23 Jun 2021 12:40:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA88B68938;\n\tWed, 23 Jun 2021 14:40:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9007468932\n\tfor ;\n\tWed, 23 Jun 2021 14:40:55 +0200 (CEST)","from [192.168.0.104] (unknown [103.251.226.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 332ED9AA;\n\tWed, 23 Jun 2021 14:40:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QObGQRe7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624452055;\n\tbh=EAi1m8DaQ6gE2XrRssVPkvvGWim0MYH/D2Nux5mYCOY=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=QObGQRe71FAz89Y0MmCyN6m5G1Ryuol5AfwN+kanslnD8TA2+4PRj9LWuJjAprHYD\n\tiIepyGJ/2v2sB0GZK8oReAhLJbJ8t/tIAI9keTBBIiCsHGRLHqkyQ+agetSwYW2u9H\n\tqpetzLLjczOZ6xim/0/3lXObHZFh+oxAwMeVdBg8=","To":"Laurent Pinchart ","References":"<20210623112203.33561-1-umang.jain@ideasonboard.com>\n\t\n\t\n\t","From":"Umang Jain ","Message-ID":"<96d555cd-a154-c58d-b552-ef6c6676497a@ideasonboard.com>","Date":"Wed, 23 Jun 2021 18:10:50 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Use\n\tlibcamera::utils::Duration helper class for durations","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]