[{"id":20061,"web_url":"https://patchwork.libcamera.org/comment/20061/","msgid":"","date":"2021-10-05T09:53:40","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\n(CC'ing David and Naush)\n\nThank you for the patch.\n\nOn Tue, Oct 05, 2021 at 04:31:09PM +0900, Hirokazu Honda wrote:\n> \"using namespace\" in a header file propagates the namespace to\n> the files including the header file. So it should be avoided.\n> This removes \"using namespace\" in agc.hpp.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 7 +++++++\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 ++----\n> 2 files changed, 9 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index 289c1fce..f6a9cb0a 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -22,6 +22,7 @@\n> using namespace RPiController;\n> using namespace libcamera;\n> using libcamera::utils::Duration;\n> +using namespace std::literals::chrono_literals;\n> \n> LOG_DEFINE_CATEGORY(RPiAgc)\n> \n> @@ -159,6 +160,12 @@ void AgcConfig::Read(boost::property_tree::ptree const ¶ms)\n> \tdefault_analogue_gain = params.get(\"default_analogue_gain\", 1.0);\n> }\n> \n> +Agc::ExposureValues::ExposureValues()\n> +\t: shutter(0s), analogue_gain(0),\n> +\t total_exposure(0s), total_exposure_no_dg(0s)\n> +{\n> +}\n> +\n> Agc::Agc(Controller *controller)\n> \t: AgcAlgorithm(controller), metering_mode_(nullptr),\n> \t exposure_mode_(nullptr), constraint_mode_(nullptr),\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> index 82063636..c100d312 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> @@ -24,8 +24,6 @@\n> \n> namespace RPiController {\n> \n> -using namespace std::literals::chrono_literals;\n> -\n> struct AgcMeteringMode {\n> \tdouble weights[AGC_STATS_SIZE];\n> \tvoid Read(boost::property_tree::ptree const ¶ms);\n> @@ -112,8 +110,8 @@ private:\n> \tuint64_t frame_count_;\n> \tAwbStatus awb_;\n> \tstruct ExposureValues {\n> -\t\tExposureValues() : shutter(0s), analogue_gain(0),\n> -\t\t\t\t total_exposure(0s), total_exposure_no_dg(0s) {}\n> +\t\tExposureValues();\n> +\n\nI'm a bit more annoyed by this patch than the other ones in the series,\nas there's no way to use namespaces explicitly for literals, which leads\nto the constructor having to be de-inlined.\n\nAs this header is used internally in the Raspberry Pi IPA I don't think\nthe using directive is a big deal. I don't object to the change though.\nI'll let David and Naush decide.\n\n> \t\tlibcamera::utils::Duration shutter;\n> \t\tdouble analogue_gain;\n> \t\tlibcamera::utils::Duration total_exposure;","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 78300BDC71\n\tfor ;\n\tTue, 5 Oct 2021 09:53:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2701691B6;\n\tTue, 5 Oct 2021 11:53:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB4F7684C6\n\tfor ;\n\tTue, 5 Oct 2021 11:53:47 +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 2E4A9B91;\n\tTue, 5 Oct 2021 11:53:47 +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=\"mFvd+2i9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1633427627;\n\tbh=qfprFM0UbCGmFFpvxMTVE6Ix3xoiaNYjWFMW5biieFI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mFvd+2i9AyuGEbOlTy0pkdpaYFmZqn3kfXtcdQBB35YhIumywjFFQvMfflUuICvx2\n\ta4dsRnw+BXNjn7ezoSrDWCOHmtybj+Kf9JL4VZWkH93EMlVvNofH2O2rfXm9QaWwgc\n\tamFD5GGXpKIiGCf6HlnsFhWpCV1AFNuRFUZ2mgkg=","Date":"Tue, 5 Oct 2021 12:53:40 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211005073114.3997303-1-hiroh@chromium.org>\n\t<20211005073114.3997303-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211005073114.3997303-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","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\" "}},{"id":20064,"web_url":"https://patchwork.libcamera.org/comment/20064/","msgid":"","date":"2021-10-05T10:20:19","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Hiro, Laurent\n\nThanks for this change.\n\nOn Tue, 5 Oct 2021 at 10:53, Laurent Pinchart\n wrote:\n>\n> Hi Hiro,\n>\n> (CC'ing David and Naush)\n>\n> Thank you for the patch.\n>\n> On Tue, Oct 05, 2021 at 04:31:09PM +0900, Hirokazu Honda wrote:\n> > \"using namespace\" in a header file propagates the namespace to\n> > the files including the header file. So it should be avoided.\n> > This removes \"using namespace\" in agc.hpp.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 7 +++++++\n> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 ++----\n> > 2 files changed, 9 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > index 289c1fce..f6a9cb0a 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > @@ -22,6 +22,7 @@\n> > using namespace RPiController;\n> > using namespace libcamera;\n> > using libcamera::utils::Duration;\n> > +using namespace std::literals::chrono_literals;\n> >\n> > LOG_DEFINE_CATEGORY(RPiAgc)\n> >\n> > @@ -159,6 +160,12 @@ void AgcConfig::Read(boost::property_tree::ptree const ¶ms)\n> > default_analogue_gain = params.get(\"default_analogue_gain\", 1.0);\n> > }\n> >\n> > +Agc::ExposureValues::ExposureValues()\n> > + : shutter(0s), analogue_gain(0),\n> > + total_exposure(0s), total_exposure_no_dg(0s)\n> > +{\n> > +}\n> > +\n> > Agc::Agc(Controller *controller)\n> > : AgcAlgorithm(controller), metering_mode_(nullptr),\n> > exposure_mode_(nullptr), constraint_mode_(nullptr),\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > index 82063636..c100d312 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > @@ -24,8 +24,6 @@\n> >\n> > namespace RPiController {\n> >\n> > -using namespace std::literals::chrono_literals;\n> > -\n> > struct AgcMeteringMode {\n> > double weights[AGC_STATS_SIZE];\n> > void Read(boost::property_tree::ptree const ¶ms);\n> > @@ -112,8 +110,8 @@ private:\n> > uint64_t frame_count_;\n> > AwbStatus awb_;\n> > struct ExposureValues {\n> > - ExposureValues() : shutter(0s), analogue_gain(0),\n> > - total_exposure(0s), total_exposure_no_dg(0s) {}\n> > + ExposureValues();\n> > +\n>\n> I'm a bit more annoyed by this patch than the other ones in the series,\n> as there's no way to use namespaces explicitly for literals, which leads\n> to the constructor having to be de-inlined.\n>\n> As this header is used internally in the Raspberry Pi IPA I don't think\n> the using directive is a big deal. I don't object to the change though.\n> I'll let David and Naush decide.\n\nI'm fine with this.\n\nActually, one could argue that we should get rid of agc.hpp and put\nthe class definition at the top of agc.cpp. Any external access to the\nclass is meant to be through the AgcAlgorithm base class (and the same\nis true of most of our other algorithms). But that's probably one for\nanother day!\n\nBest regards\nDavid\n\n>\n> > libcamera::utils::Duration shutter;\n> > double analogue_gain;\n> > libcamera::utils::Duration total_exposure;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 ABB72BDC71\n\tfor ;\n\tTue, 5 Oct 2021 10:20:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 371A4684C6;\n\tTue, 5 Oct 2021 12:20:32 +0200 (CEST)","from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com\n\t[IPv6:2a00:1450:4864:20::32f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B04A7684C6\n\tfor ;\n\tTue, 5 Oct 2021 12:20:30 +0200 (CEST)","by mail-wm1-x32f.google.com with SMTP id\n\ta11-20020a7bc1cb000000b0030d6aae48b5so2478665wmj.4\n\tfor ;\n\tTue, 05 Oct 2021 03:20:30 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"DpDLz2k2\"; 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=yyAaokkZieAl3PaRTrYNkCk3LkSWkUuE0kjLdCfksgk=;\n\tb=DpDLz2k2dfZ+HAWe1g/gGt+mPwauPB8wawJ6sh80b8KX8caLA8UpsGSPZboVDLBb4h\n\tCoskmo34STeTa3paylfr/e1COW2FMKMgGbD9Ycdcr6urW6czS+E7Z+FX8kzjAyPdru9G\n\tVRTZMMQARhuiPYIcff3eH4OITgVK34k8JeAIaQhurX5aFxxc9c5jHloMnokqP7tkE+Zu\n\tQnma2ASJAFyutmv+At/mDZLrXUX6BSgo0VyKqLlGl3BbcHRW2orWorGeOCa50h5tEXSs\n\tvPI4xo+xFsU2yksSYjTEzv6JplaBP2gfg/l2XdKu8hpC4AO0uMOSqZmg94NdWBTUSPYj\n\tAuaA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=yyAaokkZieAl3PaRTrYNkCk3LkSWkUuE0kjLdCfksgk=;\n\tb=pO1/fG/nZUVvYCjie7+57oVvZ9yatlWKRMIwm4AjHTI8OV2+SOuLIx58VwbXtEGIvx\n\tXVIeDtujEKwwTKlFtNMnGQyl4aNriGzGOcB9XTEeDk3OFDWMU4Q2RvMj1WD0/C7iFWj+\n\t3II6EYJlv09O9dZkaf08cCvloO8mnJ0y3fNaJ/rmD2M0KJO/D8IQgeedHNWTJDW8+G1j\n\tEXfFpPDnQFzp3jvNeAc5TJE71bBWT/lwevY0PG4A03D1z4HJHZdHQK9v3YXMqDBMpgyM\n\tJBli1fF+YRyRPn3QDxd3/gc+mun+jv8VNPWV2rotV/KlkS0K/pX5nVkgEDpHKxM0ct9r\n\tXyrA==","X-Gm-Message-State":"AOAM530zlGmTAO0HzAqRWT4Et3Ed8xt0vO3hPAr2q/JUTXxagytbbGfp\n\tyRhODzHZis2G4/iYd2YZJuY2EiSBP1KvwAfnwJ1MPQ==","X-Google-Smtp-Source":"ABdhPJzeHGEnUgHiIbCDY1d0IRjAgmDFZKCsY0T2b6Ua+P5gJ/5KpUj9r3h3TUw3Lq0W699w15mxiQl2zbwRA7K9cWw=","X-Received":"by 2002:a05:600c:350a:: with SMTP id\n\th10mr2386546wmq.163.1633429230312; \n\tTue, 05 Oct 2021 03:20:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20211005073114.3997303-1-hiroh@chromium.org>\n\t<20211005073114.3997303-2-hiroh@chromium.org>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 5 Oct 2021 11:20:19 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","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":20068,"web_url":"https://patchwork.libcamera.org/comment/20068/","msgid":"","date":"2021-10-05T12:07:28","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Tue, Oct 05, 2021 at 11:20:19AM +0100, David Plowman wrote:\n> On Tue, 5 Oct 2021 at 10:53, Laurent Pinchart wrote:\n> > On Tue, Oct 05, 2021 at 04:31:09PM +0900, Hirokazu Honda wrote:\n> > > \"using namespace\" in a header file propagates the namespace to\n> > > the files including the header file. So it should be avoided.\n> > > This removes \"using namespace\" in agc.hpp.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > src/ipa/raspberrypi/controller/rpi/agc.cpp | 7 +++++++\n> > > src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 ++----\n> > > 2 files changed, 9 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > index 289c1fce..f6a9cb0a 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > @@ -22,6 +22,7 @@\n> > > using namespace RPiController;\n> > > using namespace libcamera;\n> > > using libcamera::utils::Duration;\n> > > +using namespace std::literals::chrono_literals;\n> > >\n> > > LOG_DEFINE_CATEGORY(RPiAgc)\n> > >\n> > > @@ -159,6 +160,12 @@ void AgcConfig::Read(boost::property_tree::ptree const ¶ms)\n> > > default_analogue_gain = params.get(\"default_analogue_gain\", 1.0);\n> > > }\n> > >\n> > > +Agc::ExposureValues::ExposureValues()\n> > > + : shutter(0s), analogue_gain(0),\n> > > + total_exposure(0s), total_exposure_no_dg(0s)\n> > > +{\n> > > +}\n> > > +\n> > > Agc::Agc(Controller *controller)\n> > > : AgcAlgorithm(controller), metering_mode_(nullptr),\n> > > exposure_mode_(nullptr), constraint_mode_(nullptr),\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > index 82063636..c100d312 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > @@ -24,8 +24,6 @@\n> > >\n> > > namespace RPiController {\n> > >\n> > > -using namespace std::literals::chrono_literals;\n> > > -\n> > > struct AgcMeteringMode {\n> > > double weights[AGC_STATS_SIZE];\n> > > void Read(boost::property_tree::ptree const ¶ms);\n> > > @@ -112,8 +110,8 @@ private:\n> > > uint64_t frame_count_;\n> > > AwbStatus awb_;\n> > > struct ExposureValues {\n> > > - ExposureValues() : shutter(0s), analogue_gain(0),\n> > > - total_exposure(0s), total_exposure_no_dg(0s) {}\n> > > + ExposureValues();\n> > > +\n> >\n> > I'm a bit more annoyed by this patch than the other ones in the series,\n> > as there's no way to use namespaces explicitly for literals, which leads\n> > to the constructor having to be de-inlined.\n> >\n> > As this header is used internally in the Raspberry Pi IPA I don't think\n> > the using directive is a big deal. I don't object to the change though.\n> > I'll let David and Naush decide.\n> \n> I'm fine with this.\n> \n> Actually, one could argue that we should get rid of agc.hpp and put\n> the class definition at the top of agc.cpp. Any external access to the\n> class is meant to be through the AgcAlgorithm base class (and the same\n> is true of most of our other algorithms). But that's probably one for\n> another day!\n\nCan I Have your reviewed-by ?\n\n> > > libcamera::utils::Duration shutter;\n> > > double analogue_gain;\n> > > libcamera::utils::Duration total_exposure;","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 3561FC3243\n\tfor ;\n\tTue, 5 Oct 2021 12:07:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9EBE7684C6;\n\tTue, 5 Oct 2021 14:07:37 +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 15BF6684C6\n\tfor ;\n\tTue, 5 Oct 2021 14:07:36 +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 7C1C425B;\n\tTue, 5 Oct 2021 14:07:35 +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=\"OJCAmPIB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1633435655;\n\tbh=5NQBLPwiCieS7Dta+PYjVoV6d9yZHSnf+wZuvd3DvGA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OJCAmPIB8Ed6lx8nW9I4oZnq9YIWKV3EyRJzQgIQz0KoNZCNSxqWLQ04PecZIHfQV\n\tpEiqEiYMt4eOr4xv9Au56FzFFhCEUh4C09fID7M18rPIKArdOki14hYGlbpbAzjn5F\n\tRpdQ8qnFF0uusUNcMtPu/leiLBykirmFqSkG4aIc=","Date":"Tue, 5 Oct 2021 15:07:28 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20211005073114.3997303-1-hiroh@chromium.org>\n\t<20211005073114.3997303-2-hiroh@chromium.org>\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] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","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":20069,"web_url":"https://patchwork.libcamera.org/comment/20069/","msgid":"","date":"2021-10-05T12:20:20","subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nOn Tue, 5 Oct 2021 at 13:07, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> On Tue, Oct 05, 2021 at 11:20:19AM +0100, David Plowman wrote:\n> > On Tue, 5 Oct 2021 at 10:53, Laurent Pinchart wrote:\n> > > On Tue, Oct 05, 2021 at 04:31:09PM +0900, Hirokazu Honda wrote:\n> > > > \"using namespace\" in a header file propagates the namespace to\n> > > > the files including the header file. So it should be avoided.\n> > > > This removes \"using namespace\" in agc.hpp.\n> > > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > src/ipa/raspberrypi/controller/rpi/agc.cpp | 7 +++++++\n> > > > src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 ++----\n> > > > 2 files changed, 9 insertions(+), 4 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > > index 289c1fce..f6a9cb0a 100644\n> > > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > > > @@ -22,6 +22,7 @@\n> > > > using namespace RPiController;\n> > > > using namespace libcamera;\n> > > > using libcamera::utils::Duration;\n> > > > +using namespace std::literals::chrono_literals;\n> > > >\n> > > > LOG_DEFINE_CATEGORY(RPiAgc)\n> > > >\n> > > > @@ -159,6 +160,12 @@ void AgcConfig::Read(boost::property_tree::ptree const ¶ms)\n> > > > default_analogue_gain = params.get(\"default_analogue_gain\", 1.0);\n> > > > }\n> > > >\n> > > > +Agc::ExposureValues::ExposureValues()\n> > > > + : shutter(0s), analogue_gain(0),\n> > > > + total_exposure(0s), total_exposure_no_dg(0s)\n> > > > +{\n> > > > +}\n> > > > +\n> > > > Agc::Agc(Controller *controller)\n> > > > : AgcAlgorithm(controller), metering_mode_(nullptr),\n> > > > exposure_mode_(nullptr), constraint_mode_(nullptr),\n> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > > index 82063636..c100d312 100644\n> > > > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> > > > @@ -24,8 +24,6 @@\n> > > >\n> > > > namespace RPiController {\n> > > >\n> > > > -using namespace std::literals::chrono_literals;\n> > > > -\n> > > > struct AgcMeteringMode {\n> > > > double weights[AGC_STATS_SIZE];\n> > > > void Read(boost::property_tree::ptree const ¶ms);\n> > > > @@ -112,8 +110,8 @@ private:\n> > > > uint64_t frame_count_;\n> > > > AwbStatus awb_;\n> > > > struct ExposureValues {\n> > > > - ExposureValues() : shutter(0s), analogue_gain(0),\n> > > > - total_exposure(0s), total_exposure_no_dg(0s) {}\n> > > > + ExposureValues();\n> > > > +\n> > >\n> > > I'm a bit more annoyed by this patch than the other ones in the series,\n> > > as there's no way to use namespaces explicitly for literals, which leads\n> > > to the constructor having to be de-inlined.\n> > >\n> > > As this header is used internally in the Raspberry Pi IPA I don't think\n> > > the using directive is a big deal. I don't object to the change though.\n> > > I'll let David and Naush decide.\n> >\n> > I'm fine with this.\n> >\n> > Actually, one could argue that we should get rid of agc.hpp and put\n> > the class definition at the top of agc.cpp. Any external access to the\n> > class is meant to be through the AgcAlgorithm base class (and the same\n> > is true of most of our other algorithms). But that's probably one for\n> > another day!\n>\n> Can I Have your reviewed-by ?\n\nOops, sorry!!\n\nReview-by: David Plowman \n\nDavid\n\n>\n> > > > libcamera::utils::Duration shutter;\n> > > > double analogue_gain;\n> > > > libcamera::utils::Duration total_exposure;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 E57B5BDC71\n\tfor ;\n\tTue, 5 Oct 2021 12:20:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39650691B7;\n\tTue, 5 Oct 2021 14:20:33 +0200 (CEST)","from mail-wm1-x334.google.com (mail-wm1-x334.google.com\n\t[IPv6:2a00:1450:4864:20::334])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 32254684C6\n\tfor ;\n\tTue, 5 Oct 2021 14:20:31 +0200 (CEST)","by mail-wm1-x334.google.com with SMTP id\n\tm14-20020a05600c3b0e00b0030d4dffd04fso2738477wms.3\n\tfor ;\n\tTue, 05 Oct 2021 05:20:31 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"TugEKXCH\"; 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=gTWM+4coX444mArUKVnVuHhz2l4js96A6Q5kocqmnIY=;\n\tb=TugEKXCH65jMjDvO6ruP+qoj2zASll7+F6N2RWkVn1I/oUt13T1BDeulHXnHH3/d1l\n\tsCNE0RsLP4PLqaGklnZ8iqIhGb80SrLYIcc5qZ6JxysH1MAk+gWWfAsuanzbqrW6mz87\n\twJIdGlsHTpu62tuEq+MIlcAkJlnQhkimuHAWOfAQHQ59oWK7TO/G6aDLrEfvIAFELZ/2\n\tWM4KOd4TBVJVSFQiYilJDPqnGqUOl2wdEE8i5IX+aWyNuXenA/QKOlU4a9HPDeIK4XsN\n\tFZ3LigUpneAWXjAGe40h0kzOHws/wKTLFVAYNOJnJpSBy1dipPNN1VdccJXxElsWMKPT\n\tTYaQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=gTWM+4coX444mArUKVnVuHhz2l4js96A6Q5kocqmnIY=;\n\tb=j36tvQ7KScqM2lNT3syV91QAD815NFIke92crtrp702b7SzWHjy1vWuJO6wOrJ+wAd\n\tLEeWNyaaBueJ+yabZBHW5kdd6OewjZ+Oq91MJsfTNbi8LAWblYEv0XiK/ADDeeYDMH+G\n\trmp7CBSiyYdooY8IocoHHarQ383Kudrxa3PLlM0VU12tT1ZShdd7m/h59LXQ+wNmVd4J\n\tJtrlPNuqfjpxs0H31zn9hf8JP0S2GW7S+6G3MghxckPiMI18lLoT7Tp9EQeA20rBZCVs\n\tZIelC4CEYaQ8QY4x4ifd0w15s29EPD48xW+gfPoZdLGBcPe6gUb01OFDr7odNuoZGRC9\n\t0G5w==","X-Gm-Message-State":"AOAM530GE/3dc3KUgfas5NFzL8HsLYbQJ4/r+GWKXH9LXF8720LyFEEg\n\tcFeODVpvYarzzdg3mIaXJCAyFkES8HEhGurA4IY7+g==","X-Google-Smtp-Source":"ABdhPJwQEe5tZeBpoh9XN3/Wezz4EApJrEgToEjXm5E5H2/Un2gXu9tPHcH2gCznQNybZ5vjSwX+GfJXlX+5FWH7/BM=","X-Received":"by 2002:a05:600c:3b22:: with SMTP id\n\tm34mr2998680wms.130.1633436430845; \n\tTue, 05 Oct 2021 05:20:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20211005073114.3997303-1-hiroh@chromium.org>\n\t<20211005073114.3997303-2-hiroh@chromium.org>\n\t\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 5 Oct 2021 13:20:20 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove\n\tusing namespace in agc.hpp","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\" "}}]