Message ID | 20211015015804.11758-2-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Commit | 3b07397f0e7d93d54535d9c87e7b9c19d04c97f0 |
Headers | show |
Series |
|
Related | show |
Hi Laurent, On 15/10/2021 03:58, Laurent Pinchart wrote: > "using" directives are harmful in headers, as they propagate the > namespace short-circuit to all files that include the header, directly > or indirectly. Drop the directive from agc.h, and use utils::Duration > explicitly. While at it, shorten the namespace qualifier from > libcamera::utils:: to utils:: in agc.cpp for Duration. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Indeed, it is better that way, thanks ! Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 6 +++--- > src/ipa/ipu3/algorithms/agc.h | 14 ++++++-------- > 2 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5eafe82ca404..970ec42419cc 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) > double newGain = kEvGainTarget * knumHistogramBins / iqMean_; > > /* extracted from Rpi::Agc::computeTargetExposure */ > - libcamera::utils::Duration currentShutter = exposure * lineDuration_; > + utils::Duration currentShutter = exposure * lineDuration_; > currentExposureNoDg_ = currentShutter * gain; > LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ > << " Shutter speed " << currentShutter > << " Gain " << gain; > currentExposure_ = currentExposureNoDg_ * newGain; > - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > + utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > currentExposure_ = std::min(currentExposure_, maxTotalExposure); > LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; > > /* \todo: estimate if we need to desaturate */ > filterExposure(); > > - libcamera::utils::Duration newExposure = 0.0s; > + utils::Duration newExposure = 0.0s; > if (currentShutter < maxExposureTime_) { > exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); > newExposure = currentExposure_ / exposure; > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 64b71c65ef00..e0c315fc126c 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -21,8 +21,6 @@ struct IPACameraSensorInfo; > > namespace ipa::ipu3::algorithms { > > -using utils::Duration; > - > class Agc : public Algorithm > { > public: > @@ -43,13 +41,13 @@ private: > > double iqMean_; > > - Duration lineDuration_; > - Duration maxExposureTime_; > + utils::Duration lineDuration_; > + utils::Duration maxExposureTime_; > > - Duration prevExposure_; > - Duration prevExposureNoDg_; > - Duration currentExposure_; > - Duration currentExposureNoDg_; > + utils::Duration prevExposure_; > + utils::Duration prevExposureNoDg_; > + utils::Duration currentExposure_; > + utils::Duration currentExposureNoDg_; > > uint32_t stride_; > }; >
Quoting Laurent Pinchart (2021-10-15 02:58:04) > "using" directives are harmful in headers, as they propagate the > namespace short-circuit to all files that include the header, directly > or indirectly. Drop the directive from agc.h, and use utils::Duration > explicitly. While at it, shorten the namespace qualifier from > libcamera::utils:: to utils:: in agc.cpp for Duration. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 6 +++--- > src/ipa/ipu3/algorithms/agc.h | 14 ++++++-------- > 2 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5eafe82ca404..970ec42419cc 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) > double newGain = kEvGainTarget * knumHistogramBins / iqMean_; > > /* extracted from Rpi::Agc::computeTargetExposure */ > - libcamera::utils::Duration currentShutter = exposure * lineDuration_; > + utils::Duration currentShutter = exposure * lineDuration_; > currentExposureNoDg_ = currentShutter * gain; > LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ > << " Shutter speed " << currentShutter > << " Gain " << gain; > currentExposure_ = currentExposureNoDg_ * newGain; > - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > + utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > currentExposure_ = std::min(currentExposure_, maxTotalExposure); > LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; > > /* \todo: estimate if we need to desaturate */ > filterExposure(); > > - libcamera::utils::Duration newExposure = 0.0s; > + utils::Duration newExposure = 0.0s; > if (currentShutter < maxExposureTime_) { > exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); > newExposure = currentExposure_ / exposure; > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 64b71c65ef00..e0c315fc126c 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -21,8 +21,6 @@ struct IPACameraSensorInfo; > > namespace ipa::ipu3::algorithms { > > -using utils::Duration; > - > class Agc : public Algorithm > { > public: > @@ -43,13 +41,13 @@ private: > > double iqMean_; > > - Duration lineDuration_; > - Duration maxExposureTime_; > + utils::Duration lineDuration_; > + utils::Duration maxExposureTime_; > > - Duration prevExposure_; > - Duration prevExposureNoDg_; > - Duration currentExposure_; > - Duration currentExposureNoDg_; > + utils::Duration prevExposure_; > + utils::Duration prevExposureNoDg_; > + utils::Duration currentExposure_; > + utils::Duration currentExposureNoDg_; > > uint32_t stride_; > }; > -- > Regards, > > Laurent Pinchart >
Hi Laurent, On Fri, Oct 15, 2021 at 04:58:04AM +0300, Laurent Pinchart wrote: > "using" directives are harmful in headers, as they propagate the > namespace short-circuit to all files that include the header, directly > or indirectly. Drop the directive from agc.h, and use utils::Duration > explicitly. While at it, shorten the namespace qualifier from > libcamera::utils:: to utils:: in agc.cpp for Duration. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 6 +++--- > src/ipa/ipu3/algorithms/agc.h | 14 ++++++-------- > 2 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5eafe82ca404..970ec42419cc 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) > double newGain = kEvGainTarget * knumHistogramBins / iqMean_; > > /* extracted from Rpi::Agc::computeTargetExposure */ > - libcamera::utils::Duration currentShutter = exposure * lineDuration_; > + utils::Duration currentShutter = exposure * lineDuration_; > currentExposureNoDg_ = currentShutter * gain; > LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ > << " Shutter speed " << currentShutter > << " Gain " << gain; > currentExposure_ = currentExposureNoDg_ * newGain; > - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > + utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > currentExposure_ = std::min(currentExposure_, maxTotalExposure); > LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; > > /* \todo: estimate if we need to desaturate */ > filterExposure(); > > - libcamera::utils::Duration newExposure = 0.0s; > + utils::Duration newExposure = 0.0s; > if (currentShutter < maxExposureTime_) { > exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); > newExposure = currentExposure_ / exposure; > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index 64b71c65ef00..e0c315fc126c 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -21,8 +21,6 @@ struct IPACameraSensorInfo; > > namespace ipa::ipu3::algorithms { > > -using utils::Duration; > - > class Agc : public Algorithm > { > public: > @@ -43,13 +41,13 @@ private: > > double iqMean_; > > - Duration lineDuration_; > - Duration maxExposureTime_; > + utils::Duration lineDuration_; > + utils::Duration maxExposureTime_; > > - Duration prevExposure_; > - Duration prevExposureNoDg_; > - Duration currentExposure_; > - Duration currentExposureNoDg_; > + utils::Duration prevExposure_; > + utils::Duration prevExposureNoDg_; > + utils::Duration currentExposure_; > + utils::Duration currentExposureNoDg_; > > uint32_t stride_; > }; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 5eafe82ca404..970ec42419cc 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -140,20 +140,20 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) double newGain = kEvGainTarget * knumHistogramBins / iqMean_; /* extracted from Rpi::Agc::computeTargetExposure */ - libcamera::utils::Duration currentShutter = exposure * lineDuration_; + utils::Duration currentShutter = exposure * lineDuration_; currentExposureNoDg_ = currentShutter * gain; LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ << " Shutter speed " << currentShutter << " Gain " << gain; currentExposure_ = currentExposureNoDg_ * newGain; - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; + utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; currentExposure_ = std::min(currentExposure_, maxTotalExposure); LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; /* \todo: estimate if we need to desaturate */ filterExposure(); - libcamera::utils::Duration newExposure = 0.0s; + utils::Duration newExposure = 0.0s; if (currentShutter < maxExposureTime_) { exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); newExposure = currentExposure_ / exposure; diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 64b71c65ef00..e0c315fc126c 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -21,8 +21,6 @@ struct IPACameraSensorInfo; namespace ipa::ipu3::algorithms { -using utils::Duration; - class Agc : public Algorithm { public: @@ -43,13 +41,13 @@ private: double iqMean_; - Duration lineDuration_; - Duration maxExposureTime_; + utils::Duration lineDuration_; + utils::Duration maxExposureTime_; - Duration prevExposure_; - Duration prevExposureNoDg_; - Duration currentExposure_; - Duration currentExposureNoDg_; + utils::Duration prevExposure_; + utils::Duration prevExposureNoDg_; + utils::Duration currentExposure_; + utils::Duration currentExposureNoDg_; uint32_t stride_; };
"using" directives are harmful in headers, as they propagate the namespace short-circuit to all files that include the header, directly or indirectly. Drop the directive from agc.h, and use utils::Duration explicitly. While at it, shorten the namespace qualifier from libcamera::utils:: to utils:: in agc.cpp for Duration. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 6 +++--- src/ipa/ipu3/algorithms/agc.h | 14 ++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-)