[libcamera-devel] [RFC PATCH 1/6] ipa: raspberrypi: agc: Remove using namespace in agc.hpp

[Laurent Pinchart]

Hi David,

On Tue, Oct 05, 2021 at 11:20:19AM +0100, David Plowman wrote:
> On Tue, 5 Oct 2021 at 10:53, Laurent Pinchart wrote:
> > On Tue, Oct 05, 2021 at 04:31:09PM +0900, Hirokazu Honda wrote:
> > > "using namespace" in a header file propagates the namespace to
> > > the files including the header file. So it should be avoided.
> > > This removes "using namespace" in agc.hpp.
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > > ---
> > >  src/ipa/raspberrypi/controller/rpi/agc.cpp | 7 +++++++
> > >  src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 ++----
> > >  2 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > index 289c1fce..f6a9cb0a 100644
> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > @@ -22,6 +22,7 @@
> > >  using namespace RPiController;
> > >  using namespace libcamera;
> > >  using libcamera::utils::Duration;
> > > +using namespace std::literals::chrono_literals;
> > >
> > >  LOG_DEFINE_CATEGORY(RPiAgc)
> > >
> > > @@ -159,6 +160,12 @@ void AgcConfig::Read(boost::property_tree::ptree const &params)
> > >       default_analogue_gain = params.get<double>("default_analogue_gain", 1.0);
> > >  }
> > >
> > > +Agc::ExposureValues::ExposureValues()
> > > +     : shutter(0s), analogue_gain(0),
> > > +       total_exposure(0s), total_exposure_no_dg(0s)
> > > +{
> > > +}
> > > +
> > >  Agc::Agc(Controller *controller)
> > >       : AgcAlgorithm(controller), metering_mode_(nullptr),
> > >         exposure_mode_(nullptr), constraint_mode_(nullptr),
> > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > index 82063636..c100d312 100644
> > > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > @@ -24,8 +24,6 @@
> > >
> > >  namespace RPiController {
> > >
> > > -using namespace std::literals::chrono_literals;
> > > -
> > >  struct AgcMeteringMode {
> > >       double weights[AGC_STATS_SIZE];
> > >       void Read(boost::property_tree::ptree const &params);
> > > @@ -112,8 +110,8 @@ private:
> > >       uint64_t frame_count_;
> > >       AwbStatus awb_;
> > >       struct ExposureValues {
> > > -             ExposureValues() : shutter(0s), analogue_gain(0),
> > > -                                total_exposure(0s), total_exposure_no_dg(0s) {}
> > > +             ExposureValues();
> > > +
> >
> > I'm a bit more annoyed by this patch than the other ones in the series,
> > as there's no way to use namespaces explicitly for literals, which leads
> > to the constructor having to be de-inlined.
> >
> > As this header is used internally in the Raspberry Pi IPA I don't think
> > the using directive is a big deal. I don't object to the change though.
> > I'll let David and Naush decide.
> 
> I'm fine with this.
> 
> Actually, one could argue that we should get rid of agc.hpp and put
> the class definition at the top of agc.cpp. Any external access to the
> class is meant to be through the AgcAlgorithm base class (and the same
> is true of most of our other algorithms). But that's probably one for
> another day!

Can I Have your reviewed-by ?

> > >               libcamera::utils::Duration shutter;
> > >               double analogue_gain;
> > >               libcamera::utils::Duration total_exposure;

-- 
Regards,

Laurent Pinchart