[libcamera-devel] [PATCH] ipa: ipu3: Use libcamera::utils::Duration helper class for durations

[Naushir Patuck]

Hi Umang,


On Wed, 23 Jun 2021 at 12:22, Umang Jain <umang.jain at ideasonboard.com>
wrote:

> std::chrono::Duration is provided quite conveniently by
> libcamera::utils::Duration wrapper. Port IPAIPU3 to use that
> for duration-type entities (such as exposure time), such that
> it becomes consistent with rest of the codebase.
>
> The commit doesn't introduce any functional changes.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3_agc.cpp | 21 ++++++++++++---------
>  src/ipa/ipu3/ipu3_agc.h   | 14 ++++++++------
>  2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> index 8ca95013..101ef288 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/ipu3_agc.cpp
> @@ -19,6 +19,8 @@
>
>  namespace libcamera {
>
> +using namespace std::literals::chrono_literals;
> +
>  namespace ipa::ipu3 {
>
>  LOG_DEFINE_CATEGORY(IPU3Agc)
> @@ -51,9 +53,9 @@ static constexpr uint8_t kCellSize = 8;
>  IPU3Agc::IPU3Agc()
>         : frameCount_(0), lastFrame_(0), converged_(false),
>           updateControls_(false), iqMean_(0.0), gamma_(1.0),
> -         lineDuration_(0.0), maxExposureTime_(0.0),
> -         prevExposure_(0.0), prevExposureNoDg_(0.0),
> -         currentExposure_(0.0), currentExposureNoDg_(0.0)
> +         lineDuration_(0s), maxExposureTime_(0s),
> +         prevExposure_(0s), prevExposureNoDg_(0s),
> +         currentExposure_(0s), currentExposureNoDg_(0s)
>  {
>  }
>
> @@ -61,8 +63,9 @@ void IPU3Agc::initialise(struct ipu3_uapi_grid_config
> &bdsGrid, const IPACameraS
>  {
>         aeGrid_ = bdsGrid;
>
> -       /* line duration in microseconds */
> -       lineDuration_ = sensorInfo.lineLength * 1000000ULL /
> static_cast<double>(sensorInfo.pixelRate);
> +       double ld = sensorInfo.lineLength * 1000000ULL /
> static_cast<double>(sensorInfo.pixelRate);
> +       std::chrono::duration<double, std::micro> lineDuration(ld);
> +       lineDuration_ = lineDuration;
>

You could write this as:
lineDuration_ = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate

and let the compiler worry about any and all conversions.

Naush



>         maxExposureTime_ = kMaxExposure * lineDuration_;
>  }
>
> @@ -113,7 +116,7 @@ void IPU3Agc::processBrightness(const
> ipu3_uapi_stats_3a *stats)
>  void IPU3Agc::filterExposure()
>  {
>         double speed = 0.2;
> -       if (prevExposure_ == 0.0) {
> +       if (prevExposure_ == 0s) {
>                 /* DG stands for digital gain.*/
>                 prevExposure_ = currentExposure_;
>                 prevExposureNoDg_ = currentExposureNoDg_;
> @@ -162,20 +165,20 @@ void IPU3Agc::lockExposureGain(uint32_t &exposure,
> uint32_t &gain)
>                 double newGain = kEvGainTarget * knumHistogramBins /
> iqMean_;
>
>                 /* extracted from Rpi::Agc::computeTargetExposure */
> -               double currentShutter = exposure * lineDuration_;
> +               libcamera::utils::Duration currentShutter = exposure *
> lineDuration_;
>                 currentExposureNoDg_ = currentShutter * gain;
>                 LOG(IPU3Agc, Debug) << "Actual total exposure " <<
> currentExposureNoDg_
>                                     << " Shutter speed " << currentShutter
>                                     << " Gain " << gain;
>                 currentExposure_ = currentExposureNoDg_ * newGain;
> -               double maxTotalExposure = maxExposureTime_ * kMaxGain;
> +               libcamera::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();
>
> -               double newExposure = 0.0;
> +               libcamera::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/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> index f3d40557..de3e2dbf 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -14,6 +14,8 @@
>
>  #include <libcamera/geometry.h>
>
> +#include "libcamera/internal/utils.h"
> +
>  #include "libipa/algorithm.h"
>
>  namespace libcamera {
> @@ -51,13 +53,13 @@ private:
>         double iqMean_;
>         double gamma_;
>
> -       double lineDuration_;
> -       double maxExposureTime_;
> +       libcamera::utils::Duration lineDuration_;
> +       libcamera::utils::Duration maxExposureTime_;
>
> -       double prevExposure_;
> -       double prevExposureNoDg_;
> -       double currentExposure_;
> -       double currentExposureNoDg_;
> +       libcamera::utils::Duration prevExposure_;
> +       libcamera::utils::Duration prevExposureNoDg_;
> +       libcamera::utils::Duration currentExposure_;
> +       libcamera::utils::Duration currentExposureNoDg_;
>  };
>
>  } /* namespace ipa::ipu3 */
> --
> 2.31.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210623/387a44e3/attachment.htm>