[{"id":20236,"web_url":"https://patchwork.libcamera.org/comment/20236/","msgid":"<06625e44-ee04-8f6c-f9e7-68239ac9c5f5@ideasonboard.com>","date":"2021-10-15T07:04:49","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 15/10/2021 03:58, Laurent Pinchart wrote:\n> \"using\" directives are harmful in headers, as they propagate the\n> namespace short-circuit to all files that include the header, directly\n> or indirectly. Drop the directive from agc.h, and use utils::Duration\n> explicitly. While at it, shorten the namespace qualifier from\n> libcamera::utils:: to utils:: in agc.cpp for Duration.\n> \n> Signed-off-by: Laurent Pinchart \n\nIndeed, it is better that way, thanks !\n\nReviewed-by: Jean-Michel Hautbois \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 6 +++---\n> src/ipa/ipu3/algorithms/agc.h | 14 ++++++--------\n> 2 files changed, 9 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 5eafe82ca404..970ec42419cc 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> +\t\tutils::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\tlibcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n> +\t\tutils::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\tlibcamera::utils::Duration newExposure = 0.0s;\n> +\t\tutils::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/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 64b71c65ef00..e0c315fc126c 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;\n> \n> namespace ipa::ipu3::algorithms {\n> \n> -using utils::Duration;\n> -\n> class Agc : public Algorithm\n> {\n> public:\n> @@ -43,13 +41,13 @@ private:\n> \n> \tdouble iqMean_;\n> \n> -\tDuration lineDuration_;\n> -\tDuration maxExposureTime_;\n> +\tutils::Duration lineDuration_;\n> +\tutils::Duration maxExposureTime_;\n> \n> -\tDuration prevExposure_;\n> -\tDuration prevExposureNoDg_;\n> -\tDuration currentExposure_;\n> -\tDuration currentExposureNoDg_;\n> +\tutils::Duration prevExposure_;\n> +\tutils::Duration prevExposureNoDg_;\n> +\tutils::Duration currentExposure_;\n> +\tutils::Duration currentExposureNoDg_;\n> \n> \tuint32_t stride_;\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 78285C324C\n\tfor ;\n\tFri, 15 Oct 2021 07:04:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4142F6023B;\n\tFri, 15 Oct 2021 09:04:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 188D46023B\n\tfor ;\n\tFri, 15 Oct 2021 09:04:52 +0200 (CEST)","from tatooine.ideasonboard.com (unknown\n\t[IPv6:2a01:e0a:169:7140:ae2a:a11b:484b:c825])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4A602E3;\n\tFri, 15 Oct 2021 09:04:51 +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=\"qm+LJlac\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634281491;\n\tbh=oQv0vnCwsvkaD/+ygAgsQkqQroX8d6JZBRRDj+tjBhw=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=qm+LJlacCWXWmcKgAawNy6hFw4Z51DTkGre/POsW8Biyvn/Is7h2OUAvTIGSAPcXm\n\tn2FQ/n9IZgN//p/nlhoYHxNrbv5QzkkWyFHEcO8D1/t/7wUkakL7EXkX53KCKR0A9W\n\tXtVOJccqkgIOLZY0UBl8CrcaIuAfkNJr5Xdfjyyo=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>\n\t<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"<06625e44-ee04-8f6c-f9e7-68239ac9c5f5@ideasonboard.com>","Date":"Fri, 15 Oct 2021 09:04:49 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.13.0","MIME-Version":"1.0","In-Reply-To":"<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","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":20244,"web_url":"https://patchwork.libcamera.org/comment/20244/","msgid":"<163433199039.456562.16281084935262397668@Monstersaurus>","date":"2021-10-15T21:06:30","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-10-15 02:58:04)\n> \"using\" directives are harmful in headers, as they propagate the\n> namespace short-circuit to all files that include the header, directly\n> or indirectly. Drop the directive from agc.h, and use utils::Duration\n> explicitly. While at it, shorten the namespace qualifier from\n> libcamera::utils:: to utils:: in agc.cpp for Duration.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 6 +++---\n> src/ipa/ipu3/algorithms/agc.h | 14 ++++++--------\n> 2 files changed, 9 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 5eafe82ca404..970ec42419cc 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> double newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> /* extracted from Rpi::Agc::computeTargetExposure */\n> - libcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> + 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> - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n> + 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> - libcamera::utils::Duration newExposure = 0.0s;\n> + utils::Duration newExposure = 0.0s;\n> if (currentShutter < maxExposureTime_) {\n> exposure = std::clamp(static_cast(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure);\n> newExposure = currentExposure_ / exposure;\n> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 64b71c65ef00..e0c315fc126c 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;\n> \n> namespace ipa::ipu3::algorithms {\n> \n> -using utils::Duration;\n> -\n> class Agc : public Algorithm\n> {\n> public:\n> @@ -43,13 +41,13 @@ private:\n> \n> double iqMean_;\n> \n> - Duration lineDuration_;\n> - Duration maxExposureTime_;\n> + utils::Duration lineDuration_;\n> + utils::Duration maxExposureTime_;\n> \n> - Duration prevExposure_;\n> - Duration prevExposureNoDg_;\n> - Duration currentExposure_;\n> - Duration currentExposureNoDg_;\n> + utils::Duration prevExposure_;\n> + utils::Duration prevExposureNoDg_;\n> + utils::Duration currentExposure_;\n> + utils::Duration currentExposureNoDg_;\n> \n> uint32_t stride_;\n> };\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 1EAC4C324C\n\tfor ;\n\tFri, 15 Oct 2021 21:06:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B17F68F4A;\n\tFri, 15 Oct 2021 23:06:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34E0B68546\n\tfor ;\n\tFri, 15 Oct 2021 23:06:33 +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 BE7312E3;\n\tFri, 15 Oct 2021 23:06:32 +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=\"JDk6eYuD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634331992;\n\tbh=UAdjs6QTxfL7+unppxYVwQGuaqMfw7XI45rDybTcGpQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=JDk6eYuDgZIwniz/Qy40VEIer21AfbbrLCVQymD0c/dMeLBB3owKVOQyIJkyQ2wYO\n\teKUV+0AsPeMKliC/TnpQR23bEYde+CpzZu8MAQEbWdQXErhr97r+Q/YFSC/I5sA6fJ\n\toV8Yc5rvA1Fnn9w42N5Z+ZEtPmggt0gOCH3WjEJs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>\n\t<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 15 Oct 2021 22:06:30 +0100","Message-ID":"<163433199039.456562.16281084935262397668@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","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":20259,"web_url":"https://patchwork.libcamera.org/comment/20259/","msgid":"<20211018042315.GF2066071@pyrite.rasen.tech>","date":"2021-10-18T04:23:15","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, Oct 15, 2021 at 04:58:04AM +0300, Laurent Pinchart wrote:\n> \"using\" directives are harmful in headers, as they propagate the\n> namespace short-circuit to all files that include the header, directly\n> or indirectly. Drop the directive from agc.h, and use utils::Duration\n> explicitly. While at it, shorten the namespace qualifier from\n> libcamera::utils:: to utils:: in agc.cpp for Duration.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/ipu3/algorithms/agc.cpp | 6 +++---\n> src/ipa/ipu3/algorithms/agc.h | 14 ++++++--------\n> 2 files changed, 9 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index 5eafe82ca404..970ec42419cc 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain)\n> \t\tdouble newGain = kEvGainTarget * knumHistogramBins / iqMean_;\n> \n> \t\t/* extracted from Rpi::Agc::computeTargetExposure */\n> -\t\tlibcamera::utils::Duration currentShutter = exposure * lineDuration_;\n> +\t\tutils::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\tlibcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain;\n> +\t\tutils::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\tlibcamera::utils::Duration newExposure = 0.0s;\n> +\t\tutils::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/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h\n> index 64b71c65ef00..e0c315fc126c 100644\n> --- a/src/ipa/ipu3/algorithms/agc.h\n> +++ b/src/ipa/ipu3/algorithms/agc.h\n> @@ -21,8 +21,6 @@ struct IPACameraSensorInfo;\n> \n> namespace ipa::ipu3::algorithms {\n> \n> -using utils::Duration;\n> -\n> class Agc : public Algorithm\n> {\n> public:\n> @@ -43,13 +41,13 @@ private:\n> \n> \tdouble iqMean_;\n> \n> -\tDuration lineDuration_;\n> -\tDuration maxExposureTime_;\n> +\tutils::Duration lineDuration_;\n> +\tutils::Duration maxExposureTime_;\n> \n> -\tDuration prevExposure_;\n> -\tDuration prevExposureNoDg_;\n> -\tDuration currentExposure_;\n> -\tDuration currentExposureNoDg_;\n> +\tutils::Duration prevExposure_;\n> +\tutils::Duration prevExposureNoDg_;\n> +\tutils::Duration currentExposure_;\n> +\tutils::Duration currentExposureNoDg_;\n> \n> \tuint32_t stride_;\n> };\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 68987C323E\n\tfor ;\n\tMon, 18 Oct 2021 04:23:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C77D68F55;\n\tMon, 18 Oct 2021 06:23:24 +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 0482060128\n\tfor ;\n\tMon, 18 Oct 2021 06:23:22 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B96B012A;\n\tMon, 18 Oct 2021 06:23:20 +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=\"ncCoURHp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634531001;\n\tbh=Ry6AHAZN1kDOmlQhJEIMEiqAWMqDz9tU2WpCc/vUUaI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ncCoURHpqQcpFh0eeDR1aRMHG6b7BWx6kQfnVdTypNrHiPHlxKQx3MgtyLBnwk/7Q\n\tqNPPf//kW1/f08GmdjqOAhs5E/QLdJRn2pDxoDgBgg5YG3xpfkWBtMr+OfqB+20deC\n\tg/a6t/kLpvQMAzirV7L+4nmsKM2JST0k7M5RmQ+o=","Date":"Mon, 18 Oct 2021 13:23:15 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20211018042315.GF2066071@pyrite.rasen.tech>","References":"<20211015015804.11758-1-laurent.pinchart@ideasonboard.com>\n\t<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20211015015804.11758-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: ipu3: agc: Remove \"using\"\n\tdirective from agc.h header","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]