[libcamera-devel] [PATCH v3 4/4] ipa: raspberrypi: Switch the AGC/Lux code to use utils::Duration
Jacopo Mondi
jacopo at jmondi.org
Fri May 21 15:09:54 CEST 2021
Hi Naush,
On Fri, May 21, 2021 at 01:49:30PM +0100, Naushir Patuck wrote:
> On Fri, 21 May 2021 at 13:37, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> > Hello,
> >
> > On Fri, May 21, 2021 at 12:18:55PM +0100, David Plowman wrote:
> > > Hi Naush
> > >
> > > Thanks for all this work!
> > >
> > > On Fri, 21 May 2021 at 09:05, Naushir Patuck <naush at raspberrypi.com>
> > wrote:
> > > >
> > > > Convert the core AGC and Lux controller code to use
> > > > utils::Duration for all exposure time related variables and
> > > > calculations.
> > > >
> > > > Convert the exposure/shutter time fields in AgcStatus and DeviceStatus
> > > > to use utils::Duration.
> > > >
> > > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > > ---
> > > > src/ipa/raspberrypi/cam_helper.cpp | 2 +-
> > > > src/ipa/raspberrypi/controller/agc_status.h | 14 +--
> > > > .../raspberrypi/controller/device_status.h | 6 +-
> > > > src/ipa/raspberrypi/controller/rpi/agc.cpp | 89 +++++++++++--------
> > > > src/ipa/raspberrypi/controller/rpi/agc.hpp | 28 +++---
> > > > src/ipa/raspberrypi/controller/rpi/lux.cpp | 17 ++--
> > > > src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +-
> > > > src/ipa/raspberrypi/raspberrypi.cpp | 13 +--
> > > > src/libcamera/utils.cpp | 9 +-
> > > > 9 files changed, 105 insertions(+), 77 deletions(-)
> > > >
> > > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> > b/src/ipa/raspberrypi/cam_helper.cpp
> > > > index feb02d1806b2..968fae13bd5e 100644
> > > > --- a/src/ipa/raspberrypi/cam_helper.cpp
> > > > +++ b/src/ipa/raspberrypi/cam_helper.cpp
> > > > @@ -183,7 +183,7 @@ void CamHelper::parseEmbeddedData(Span<const
> > uint8_t> buffer,
> > > > return;
> > > > }
> > > >
> > > > - deviceStatus.shutter_speed =
> > Exposure(exposureLines).get<std::micro>();
> > > > + deviceStatus.shutter_speed = Exposure(exposureLines);
> > > > deviceStatus.analogue_gain = Gain(gainCode);
> > > >
> > > > LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
> > > > diff --git a/src/ipa/raspberrypi/controller/agc_status.h
> > b/src/ipa/raspberrypi/controller/agc_status.h
> > > > index 10381c90a313..c893715af588 100644
> > > > --- a/src/ipa/raspberrypi/controller/agc_status.h
> > > > +++ b/src/ipa/raspberrypi/controller/agc_status.h
> > > > @@ -6,6 +6,8 @@
> > > > */
> > > > #pragma once
> > > >
> > > > +#include "libcamera/internal/utils.h"
> > > > +
> > > > // The AGC algorithm should post the following structure into the
> > image's
> > > > // "agc.status" metadata.
> > > >
> > > > @@ -17,18 +19,20 @@ extern "C" {
> > > > // seen statistics and calculated meaningful values. The contents
> > should be
> > > > // ignored until then.
> > > >
> > > > +using libcamera::utils::Duration;
> > > > +
> > > > struct AgcStatus {
> > > > - double total_exposure_value; // value for all exposure and
> > gain for this image
> > > > - double target_exposure_value; // (unfiltered) target total
> > exposure AGC is aiming for
> > > > - double shutter_time;
> > > > + Duration total_exposure_value; // value for all exposure and
> > gain for this image
> > > > + Duration target_exposure_value; // (unfiltered) target total
> > exposure AGC is aiming for
> > > > + Duration shutter_time;
> > > > double analogue_gain;
> > > > char exposure_mode[32];
> > > > char constraint_mode[32];
> > > > char metering_mode[32];
> > > > double ev;
> > > > - double flicker_period;
> > > > + Duration flicker_period;
> > > > int floating_region_enable;
> > > > - double fixed_shutter;
> > > > + Duration fixed_shutter;
> > > > double fixed_analogue_gain;
> > > > double digital_gain;
> > > > int locked;
> > > > diff --git a/src/ipa/raspberrypi/controller/device_status.h
> > b/src/ipa/raspberrypi/controller/device_status.h
> > > > index aa08608b5d40..131b4cd344ee 100644
> > > > --- a/src/ipa/raspberrypi/controller/device_status.h
> > > > +++ b/src/ipa/raspberrypi/controller/device_status.h
> > > > @@ -6,6 +6,8 @@
> > > > */
> > > > #pragma once
> > > >
> > > > +#include "libcamera/internal/utils.h"
> > > > +
> > > > // Definition of "device metadata" which stores things like shutter
> > time and
> > > > // analogue gain that downstream control algorithms will want to know.
> > > >
> > > > @@ -14,8 +16,8 @@ extern "C" {
> > > > #endif
> > > >
> > > > struct DeviceStatus {
> > > > - // time shutter is open, in microseconds
> > > > - double shutter_speed;
> > > > + // time shutter is open
> > > > + libcamera::utils::Duration shutter_speed;
> > > > double analogue_gain;
> > > > // 1.0/distance-in-metres, or 0 if unknown
> > > > double lens_position;
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > > index 482eb3ef962d..46742d845034 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > > > @@ -21,6 +21,7 @@
> > > >
> > > > using namespace RPiController;
> > > > using namespace libcamera;
> > > > +using utils::operator<<;
> > >
> > > Still can't say that I like this... just because someone wants to use
> > > Durations, it seems like an awkward thing to make them remember. But
> > > unless anyone has anything better....
> >
> > I got the same feeling while reading the rest of the patches.
> > I can't say toString() is that better though, but probably is less
> > implicit that this using statment, which if read in isolation does not
> > immediately relates to the Duration class ?
> >
>
> I do agree with these points, and I don't like this as well. I can see two
> alternatives without
> using toString():
>
> 1) Move the Duration class into its own namespace under utils, so then the
> above statement
> will look a bit more informative:
>
> using utils::Duration::operator<<;
>
> 2) Move the operator() out of the utils namespace. This way we eliminate
> the need for the
> statement completely.
I vote for 2 if it can be done nicely!
>
> Regards,
> Naush
>
>
> >
> > >
> > > >
> > > > LOG_DEFINE_CATEGORY(RPiAgc)
> > > >
> > > > @@ -55,19 +56,27 @@ read_metering_modes(std::map<std::string,
> > AgcMeteringMode> &metering_modes,
> > > > return first;
> > > > }
> > > >
> > > > -static int read_double_list(std::vector<double> &list,
> > > > - boost::property_tree::ptree const ¶ms)
> > > > +static int read_list(std::vector<double> &list,
> > > > + boost::property_tree::ptree const ¶ms)
> > > > {
> > > > for (auto &p : params)
> > > > list.push_back(p.second.get_value<double>());
> > > > return list.size();
> > > > }
> > > >
> > > > +static int read_list(std::vector<Duration> &list,
> > > > + boost::property_tree::ptree const ¶ms)
> > > > +{
> > > > + for (auto &p : params)
> > > > + list.push_back(p.second.get_value<double>() * 1us);
> > > > + return list.size();
> > > > +}
> > > > +
> > > > void AgcExposureMode::Read(boost::property_tree::ptree const ¶ms)
> > > > {
> > > > int num_shutters =
> > > > - read_double_list(shutter, params.get_child("shutter"));
> > > > - int num_ags = read_double_list(gain, params.get_child("gain"));
> > > > + read_list(shutter, params.get_child("shutter"));
> > > > + int num_ags = read_list(gain, params.get_child("gain"));
> > > > if (num_shutters < 2 || num_ags < 2)
> > > > throw std::runtime_error(
> > > > "AgcConfig: must have at least two entries in
> > exposure profile");
> > > > @@ -147,7 +156,7 @@ void AgcConfig::Read(boost::property_tree::ptree
> > const ¶ms)
> > > > params.get<double>("fast_reduce_threshold", 0.4);
> > > > base_ev = params.get<double>("base_ev", 1.0);
> > > > // Start with quite a low value as ramping up is easier than
> > ramping down.
> > > > - default_exposure_time =
> > params.get<double>("default_exposure_time", 1000);
> > > > + default_exposure_time =
> > params.get<double>("default_exposure_time", 1000) * 1us;
> > > > default_analogue_gain =
> > params.get<double>("default_analogue_gain", 1.0);
> > > > }
> > > >
> > > > @@ -155,9 +164,9 @@ Agc::Agc(Controller *controller)
> > > > : AgcAlgorithm(controller), metering_mode_(nullptr),
> > > > exposure_mode_(nullptr), constraint_mode_(nullptr),
> > > > frame_count_(0), lock_count_(0),
> > > > - last_target_exposure_(0.0),
> > > > - ev_(1.0), flicker_period_(0.0),
> > > > - max_shutter_(0), fixed_shutter_(0), fixed_analogue_gain_(0.0)
> > > > + last_target_exposure_(0s),
> > > > + ev_(1.0), flicker_period_(0s),
> > > > + max_shutter_(0s), fixed_shutter_(0s),
> > fixed_analogue_gain_(0.0)
> > > > {
> > > > memset(&awb_, 0, sizeof(awb_));
> > > > // Setting status_.total_exposure_value_ to zero initially
> > tells us
> > > > @@ -203,7 +212,7 @@ void Agc::Pause()
> > > >
> > > > void Agc::Resume()
> > > > {
> > > > - fixed_shutter_ = 0;
> > > > + fixed_shutter_ = 0s;
> > > > fixed_analogue_gain_ = 0;
> > > > }
> > > >
> > > > @@ -224,17 +233,17 @@ void Agc::SetEv(double ev)
> > > >
> > > > void Agc::SetFlickerPeriod(const Duration &flicker_period)
> > > > {
> > > > - flicker_period_ = flicker_period.get<std::micro>();
> > > > + flicker_period_ = flicker_period;
> > > > }
> > > >
> > > > void Agc::SetMaxShutter(const Duration &max_shutter)
> > > > {
> > > > - max_shutter_ = max_shutter.get<std::micro>();
> > > > + max_shutter_ = max_shutter;
> > > > }
> > > >
> > > > void Agc::SetFixedShutter(const Duration &fixed_shutter)
> > > > {
> > > > - fixed_shutter_ = fixed_shutter.get<std::micro>();
> > > > + fixed_shutter_ = fixed_shutter;
> > > > // Set this in case someone calls Pause() straight after.
> > > > status_.shutter_time = clipShutter(fixed_shutter_);
> > > > }
> > > > @@ -266,8 +275,8 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode
> > const &camera_mode,
> > > > {
> > > > housekeepConfig();
> > > >
> > > > - double fixed_shutter = clipShutter(fixed_shutter_);
> > > > - if (fixed_shutter != 0.0 && fixed_analogue_gain_ != 0.0) {
> > > > + Duration fixed_shutter = clipShutter(fixed_shutter_);
> > > > + if (fixed_shutter && fixed_analogue_gain_) {
> > > > // We're going to reset the algorithm here with these
> > fixed values.
> > > >
> > > > fetchAwbStatus(metadata);
> > > > @@ -312,8 +321,8 @@ void Agc::Prepare(Metadata *image_metadata)
> > > > // Process has run, so we have meaningful values.
> > > > DeviceStatus device_status;
> > > > if (image_metadata->Get("device.status",
> > device_status) == 0) {
> > > > - double actual_exposure =
> > device_status.shutter_speed *
> > > > -
> > device_status.analogue_gain;
> > > > + Duration actual_exposure =
> > device_status.shutter_speed *
> > > > +
> > device_status.analogue_gain;
> > > > if (actual_exposure) {
> > > > status_.digital_gain =
> > > > status_.total_exposure_value /
> > > > @@ -326,7 +335,8 @@ void Agc::Prepare(Metadata *image_metadata)
> > > > std::min(status_.digital_gain,
> > 4.0));
> > > > LOG(RPiAgc, Debug) << "Actual exposure
> > " << actual_exposure;
> > > > LOG(RPiAgc, Debug) << "Use
> > digital_gain " << status_.digital_gain;
> > > > - LOG(RPiAgc, Debug) << "Effective
> > exposure " << actual_exposure * status_.digital_gain;
> > > > + LOG(RPiAgc, Debug) << "Effective
> > exposure "
> > > > + << actual_exposure
> > * status_.digital_gain;
> > > > // Decide whether AEC/AGC has
> > converged.
> > > > updateLockStatus(device_status);
> > > > }
> > > > @@ -370,9 +380,9 @@ void Agc::updateLockStatus(DeviceStatus const
> > &device_status)
> > > > const double RESET_MARGIN = 1.5;
> > > >
> > > > // Add 200us to the exposure time error to allow for line
> > quantisation.
> > > > - double exposure_error = last_device_status_.shutter_speed *
> > ERROR_FACTOR + 200;
> > > > + Duration exposure_error = last_device_status_.shutter_speed *
> > ERROR_FACTOR + 200us;
> > > > double gain_error = last_device_status_.analogue_gain *
> > ERROR_FACTOR;
> > > > - double target_error = last_target_exposure_ * ERROR_FACTOR;
> > > > + Duration target_error = last_target_exposure_ * ERROR_FACTOR;
> > > >
> > > > // Note that we don't know the exposure/gain limits of the
> > sensor, so
> > > > // the values we keep requesting may be unachievable. For this
> > reason
> > > > @@ -462,7 +472,7 @@ void Agc::fetchCurrentExposure(Metadata
> > *image_metadata)
> > > > current_.analogue_gain = device_status->analogue_gain;
> > > > AgcStatus *agc_status =
> > > > image_metadata->GetLocked<AgcStatus>("agc.status");
> > > > - current_.total_exposure = agc_status ?
> > agc_status->total_exposure_value : 0;
> > > > + current_.total_exposure = agc_status ?
> > agc_status->total_exposure_value : 0s;
> > > > current_.total_exposure_no_dg = current_.shutter *
> > current_.analogue_gain;
> > > > }
> > > >
> > > > @@ -573,7 +583,7 @@ void Agc::computeGain(bcm2835_isp_stats
> > *statistics, Metadata *image_metadata,
> > > >
> > > > void Agc::computeTargetExposure(double gain)
> > > > {
> > > > - if (status_.fixed_shutter != 0.0 &&
> > status_.fixed_analogue_gain != 0.0) {
> > > > + if (status_.fixed_shutter && status_.fixed_analogue_gain) {
> > > > // When ag and shutter are both fixed, we need to
> > drive the
> > > > // total exposure so that we end up with a digital
> > gain of at least
> > > > // 1/min_colour_gain. Otherwise we'd desaturate
> > channels causing
> > > > @@ -588,11 +598,11 @@ void Agc::computeTargetExposure(double gain)
> > > > target_.total_exposure = current_.total_exposure_no_dg
> > * gain;
> > > > // The final target exposure is also limited to what
> > the exposure
> > > > // mode allows.
> > > > - double max_shutter = status_.fixed_shutter != 0.0
> > > > + Duration max_shutter = status_.fixed_shutter
> > > > ? status_.fixed_shutter
> > > > : exposure_mode_->shutter.back();
> > > > max_shutter = clipShutter(max_shutter);
> > > > - double max_total_exposure =
> > > > + Duration max_total_exposure =
> > > > max_shutter *
> > > > (status_.fixed_analogue_gain != 0.0
> > > > ? status_.fixed_analogue_gain
> > > > @@ -637,7 +647,7 @@ void Agc::filterExposure(bool desaturate)
> > > > if ((status_.fixed_shutter && status_.fixed_analogue_gain) ||
> > > > frame_count_ <= config_.startup_frames)
> > > > speed = 1.0;
> > > > - if (filtered_.total_exposure == 0.0) {
> > > > + if (!filtered_.total_exposure) {
> > > > filtered_.total_exposure = target_.total_exposure;
> > > > filtered_.total_exposure_no_dg =
> > target_.total_exposure_no_dg;
> > > > } else {
> > > > @@ -674,9 +684,10 @@ void Agc::divideUpExposure()
> > > > // Sending the fixed shutter/gain cases through the same code
> > may seem
> > > > // unnecessary, but it will make more sense when extend this
> > to cover
> > > > // variable aperture.
> > > > - double exposure_value = filtered_.total_exposure_no_dg;
> > > > - double shutter_time, analogue_gain;
> > > > - shutter_time = status_.fixed_shutter != 0.0
> > > > + Duration exposure_value = filtered_.total_exposure_no_dg;
> > > > + Duration shutter_time;
> > > > + double analogue_gain;
> > > > + shutter_time = status_.fixed_shutter
> > > > ? status_.fixed_shutter
> > > > : exposure_mode_->shutter[0];
> > > > shutter_time = clipShutter(shutter_time);
> > > > @@ -686,8 +697,8 @@ void Agc::divideUpExposure()
> > > > if (shutter_time * analogue_gain < exposure_value) {
> > > > for (unsigned int stage = 1;
> > > > stage < exposure_mode_->gain.size(); stage++) {
> > > > - if (status_.fixed_shutter == 0.0) {
> > > > - double stage_shutter =
> > > > + if (!status_.fixed_shutter) {
> > > > + Duration stage_shutter =
> > > >
> > clipShutter(exposure_mode_->shutter[stage]);
> > > > if (stage_shutter * analogue_gain >=
> > > > exposure_value) {
> > > > @@ -713,12 +724,11 @@ void Agc::divideUpExposure()
> > > > << analogue_gain;
> > > > // Finally adjust shutter time for flicker avoidance (require
> > both
> > > > // shutter and gain not to be fixed).
> > > > - if (status_.fixed_shutter == 0.0 &&
> > > > - status_.fixed_analogue_gain == 0.0 &&
> > > > - status_.flicker_period != 0.0) {
> > > > - int flicker_periods = shutter_time /
> > status_.flicker_period;
> > > > - if (flicker_periods > 0) {
> > > > - double new_shutter_time = flicker_periods *
> > status_.flicker_period;
> > > > + if (!status_.fixed_shutter && !status_.fixed_analogue_gain &&
> > > > + status_.flicker_period) {
> > > > + double flicker_periods = shutter_time /
> > status_.flicker_period;
> > > > + if (flicker_periods > 0.0) {
> > > > + Duration new_shutter_time = flicker_periods *
> > status_.flicker_period;
> > >
> > > Just wondering if this will behave the same now that flicker_periods
> > > is a double. Won't it now hold fractional values with the result that
> > > new_shutter_time isn't an exact multiple of the period?
> > >
> > > > analogue_gain *= shutter_time /
> > new_shutter_time;
> > > > // We should still not allow the ag to go over
> > the
> > > > // largest value in the exposure mode. Note
> > that this
> > > > @@ -738,7 +748,7 @@ void Agc::divideUpExposure()
> > > > void Agc::writeAndFinish(Metadata *image_metadata, bool desaturate)
> > > > {
> > > > status_.total_exposure_value = filtered_.total_exposure;
> > > > - status_.target_exposure_value = desaturate ? 0 :
> > target_.total_exposure_no_dg;
> > > > + status_.target_exposure_value = desaturate ? 0s :
> > target_.total_exposure_no_dg;
> > > > status_.shutter_time = filtered_.shutter;
> > > > status_.analogue_gain = filtered_.analogue_gain;
> > > > // Write to metadata as well, in case anyone wants to update
> > the camera
> > > > @@ -750,11 +760,12 @@ void Agc::writeAndFinish(Metadata
> > *image_metadata, bool desaturate)
> > > > << " analogue gain " <<
> > filtered_.analogue_gain;
> > > > }
> > > >
> > > > -double Agc::clipShutter(double shutter)
> > > > +Duration Agc::clipShutter(const Duration &shutter)
> > > > {
> > > > + Duration clip_shutter = shutter;
> > >
> > > Given that we copy it anyway I wonder if we might have left this as
> > > call-by-value, resulting in fewer changes. But it's only the teeniest
> > > of nit-picks!
> > >
> > > If you can put my mind at rest over the flicker period thing:
> > >
> > > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > >
> > > Thanks!
> > > David
> > >
> > > > if (max_shutter_)
> > > > - shutter = std::min(shutter, max_shutter_);
> > > > - return shutter;
> > > > + clip_shutter = std::min(clip_shutter, max_shutter_);
> > > > + return clip_shutter;
> > > > }
> > > >
> > > > // Register algorithm with the system.
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > > index 16a65959d90e..24a6610096c2 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > > > @@ -9,6 +9,8 @@
> > > > #include <vector>
> > > > #include <mutex>
> > > >
> > > > +#include "libcamera/internal/utils.h"
> > > > +
> > > > #include "../agc_algorithm.hpp"
> > > > #include "../agc_status.h"
> > > > #include "../pwl.hpp"
> > > > @@ -22,13 +24,15 @@
> > > >
> > > > namespace RPiController {
> > > >
> > > > +using namespace std::literals::chrono_literals;
> > > > +
> > > > struct AgcMeteringMode {
> > > > double weights[AGC_STATS_SIZE];
> > > > void Read(boost::property_tree::ptree const ¶ms);
> > > > };
> > > >
> > > > struct AgcExposureMode {
> > > > - std::vector<double> shutter;
> > > > + std::vector<Duration> shutter;
> > > > std::vector<double> gain;
> > > > void Read(boost::property_tree::ptree const ¶ms);
> > > > };
> > > > @@ -61,7 +65,7 @@ struct AgcConfig {
> > > > std::string default_exposure_mode;
> > > > std::string default_constraint_mode;
> > > > double base_ev;
> > > > - double default_exposure_time;
> > > > + Duration default_exposure_time;
> > > > double default_analogue_gain;
> > > > };
> > > >
> > > > @@ -101,19 +105,19 @@ private:
> > > > void filterExposure(bool desaturate);
> > > > void divideUpExposure();
> > > > void writeAndFinish(Metadata *image_metadata, bool desaturate);
> > > > - double clipShutter(double shutter);
> > > > + Duration clipShutter(const Duration &shutter);
> > > > AgcMeteringMode *metering_mode_;
> > > > AgcExposureMode *exposure_mode_;
> > > > AgcConstraintMode *constraint_mode_;
> > > > uint64_t frame_count_;
> > > > AwbStatus awb_;
> > > > struct ExposureValues {
> > > > - ExposureValues() : shutter(0), analogue_gain(0),
> > > > - total_exposure(0),
> > total_exposure_no_dg(0) {}
> > > > - double shutter;
> > > > + ExposureValues() : shutter(0s), analogue_gain(0),
> > > > + total_exposure(0s),
> > total_exposure_no_dg(0s) {}
> > > > + Duration shutter;
> > > > double analogue_gain;
> > > > - double total_exposure;
> > > > - double total_exposure_no_dg; // without digital gain
> > > > + Duration total_exposure;
> > > > + Duration total_exposure_no_dg; // without digital gain
> > > > };
> > > > ExposureValues current_; // values for the current frame
> > > > ExposureValues target_; // calculate the values we want here
> > > > @@ -121,15 +125,15 @@ private:
> > > > AgcStatus status_;
> > > > int lock_count_;
> > > > DeviceStatus last_device_status_;
> > > > - double last_target_exposure_;
> > > > + Duration last_target_exposure_;
> > > > // Below here the "settings" that applications can change.
> > > > std::string metering_mode_name_;
> > > > std::string exposure_mode_name_;
> > > > std::string constraint_mode_name_;
> > > > double ev_;
> > > > - double flicker_period_;
> > > > - double max_shutter_;
> > > > - double fixed_shutter_;
> > > > + Duration flicker_period_;
> > > > + Duration max_shutter_;
> > > > + Duration fixed_shutter_;
> > > > double fixed_analogue_gain_;
> > > > };
> > > >
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > > > index f74381cab2b4..0fa6457c1376 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > > > @@ -16,6 +16,7 @@
> > > >
> > > > using namespace RPiController;
> > > > using namespace libcamera;
> > > > +using namespace std::literals::chrono_literals;
> > > >
> > > > LOG_DEFINE_CATEGORY(RPiLux)
> > > >
> > > > @@ -38,7 +39,7 @@ char const *Lux::Name() const
> > > > void Lux::Read(boost::property_tree::ptree const ¶ms)
> > > > {
> > > > reference_shutter_speed_ =
> > > > - params.get<double>("reference_shutter_speed");
> > > > + params.get<double>("reference_shutter_speed") * 1.0us;
> > > > reference_gain_ = params.get<double>("reference_gain");
> > > > reference_aperture_ = params.get<double>("reference_aperture",
> > 1.0);
> > > > reference_Y_ = params.get<double>("reference_Y");
> > > > @@ -60,15 +61,13 @@ void Lux::Prepare(Metadata *image_metadata)
> > > > void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)
> > > > {
> > > > // set some initial values to shut the compiler up
> > > > - DeviceStatus device_status =
> > > > - { .shutter_speed = 1.0,
> > > > - .analogue_gain = 1.0,
> > > > - .lens_position = 0.0,
> > > > - .aperture = 0.0,
> > > > - .flash_intensity = 0.0 };
> > > > + DeviceStatus device_status = { .shutter_speed = 1.0ms,
> > > > + .analogue_gain = 1.0,
> > > > + .lens_position = 0.0,
> > > > + .aperture = 0.0,
> > > > + .flash_intensity = 0.0 };
> > > > if (image_metadata->Get("device.status", device_status) == 0) {
> > > > double current_gain = device_status.analogue_gain;
> > > > - double current_shutter_speed =
> > device_status.shutter_speed;
> > > > double current_aperture = device_status.aperture;
> > > > if (current_aperture == 0)
> > > > current_aperture = current_aperture_;
> > > > @@ -83,7 +82,7 @@ void Lux::Process(StatisticsPtr &stats, Metadata
> > *image_metadata)
> > > > double current_Y = sum / (double)num + .5;
> > > > double gain_ratio = reference_gain_ / current_gain;
> > > > double shutter_speed_ratio =
> > > > - reference_shutter_speed_ /
> > current_shutter_speed;
> > > > + reference_shutter_speed_ /
> > device_status.shutter_speed;
> > > > double aperture_ratio = reference_aperture_ /
> > current_aperture;
> > > > double Y_ratio = current_Y * (65536 / num_bins) /
> > reference_Y_;
> > > > double estimated_lux = shutter_speed_ratio *
> > gain_ratio *
> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > b/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > > > index f9090484a136..45c844393e62 100644
> > > > --- a/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > > > +++ b/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > > > @@ -8,6 +8,8 @@
> > > >
> > > > #include <mutex>
> > > >
> > > > +#include "libcamera/internal/utils.h"
> > > > +
> > > > #include "../lux_status.h"
> > > > #include "../algorithm.hpp"
> > > >
> > > > @@ -28,7 +30,7 @@ public:
> > > > private:
> > > > // These values define the conditions of the reference image,
> > against
> > > > // which we compare the new image.
> > > > - double reference_shutter_speed_; // in micro-seconds
> > > > + libcamera::utils::Duration reference_shutter_speed_;
> > > > double reference_gain_;
> > > > double reference_aperture_; // units of 1/f
> > > > double reference_Y_; // out of 65536
> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> > b/src/ipa/raspberrypi/raspberrypi.cpp
> > > > index f8f36420b076..b77230ded591 100644
> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > > > @@ -225,11 +225,11 @@ void IPARPi::start(const ControlList &controls,
> > ipa::RPi::StartConfig *startConf
> > > >
> > > > /* SwitchMode may supply updated exposure/gain values to use.
> > */
> > > > AgcStatus agcStatus;
> > > > - agcStatus.shutter_time = 0.0;
> > > > + agcStatus.shutter_time = 0.0s;
> > > > agcStatus.analogue_gain = 0.0;
> > > >
> > > > metadata.Get("agc.status", agcStatus);
> > > > - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain
> > != 0.0) {
> > > > + if (agcStatus.shutter_time && agcStatus.analogue_gain) {
> > > > ControlList ctrls(sensorCtrls_);
> > > > applyAGC(&agcStatus, ctrls);
> > > > startConfig->controls = std::move(ctrls);
> > > > @@ -392,7 +392,7 @@ int IPARPi::configure(const CameraSensorInfo
> > &sensorInfo,
> > > > /* Supply initial values for gain and exposure. */
> > > > ControlList ctrls(sensorCtrls_);
> > > > AgcStatus agcStatus;
> > > > - agcStatus.shutter_time =
> > DefaultExposureTime.get<std::micro>();
> > > > + agcStatus.shutter_time = DefaultExposureTime;
> > > > agcStatus.analogue_gain = DefaultAnalogueGain;
> > > > applyAGC(&agcStatus, ctrls);
> > > >
> > > > @@ -464,7 +464,8 @@ void IPARPi::reportMetadata()
> > > > */
> > > > DeviceStatus *deviceStatus =
> > rpiMetadata_.GetLocked<DeviceStatus>("device.status");
> > > > if (deviceStatus) {
> > > > - libcameraMetadata_.set(controls::ExposureTime,
> > deviceStatus->shutter_speed);
> > > > + libcameraMetadata_.set(controls::ExposureTime,
> > > > +
> > deviceStatus->shutter_speed.get<std::micro>());
> > > > libcameraMetadata_.set(controls::AnalogueGain,
> > deviceStatus->analogue_gain);
> > > > }
> > > >
> > > > @@ -1017,7 +1018,7 @@ void IPARPi::fillDeviceStatus(const ControlList
> > &sensorControls)
> > > > int32_t exposureLines =
> > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > > > int32_t gainCode =
> > sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
> > > >
> > > > - deviceStatus.shutter_speed =
> > helper_->Exposure(exposureLines).get<std::micro>();
> > > > + deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
> > > > deviceStatus.analogue_gain = helper_->Gain(gainCode);
> > > >
> > > > LOG(IPARPI, Debug) << "Metadata - Exposure : "
> > > > @@ -1103,7 +1104,7 @@ void IPARPi::applyAGC(const struct AgcStatus
> > *agcStatus, ControlList &ctrls)
> > > > int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);
> > > >
> > > > /* GetVBlanking might clip exposure time to the fps limits. */
> > > > - Duration exposure = agcStatus->shutter_time * 1.0us;
> > > > + Duration exposure = agcStatus->shutter_time;
> > > > int32_t vblanking = helper_->GetVBlanking(exposure,
> > minFrameDuration_, maxFrameDuration_);
> > > > int32_t exposureLines = helper_->ExposureLines(exposure);
> > > >
> > > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> > > > index 3131aa2d6666..5fa6f8366f3d 100644
> > > > --- a/src/libcamera/utils.cpp
> > > > +++ b/src/libcamera/utils.cpp
> > > > @@ -506,9 +506,14 @@ std::string libcameraSourcePath()
> > > > * loop, iterates over an indexed view of the \a iterable
> > > > */
> > > >
> > > > +/**
> > > > + * \typedef BaseDuration
> > > > + * \brief Base struct for the \a Duration helper class.
> > > > + */
> > > > +
> > > > /**
> > > > * \class Duration
> > > > - * \brief Helper for std::chrono::duration types. Derived from \a
> > BaseDuration
> > > > + * \brief Helper class for std::chrono::duration types.
> > > > */
> > > >
> > > > /**
> > > > @@ -517,7 +522,7 @@ std::string libcameraSourcePath()
> > > > * \param[in] d The std::chrono::duration object to convert from.
> > > > *
> > > > * Constructs a \a Duration object from a \a std::chrono::duration
> > object,
> > > > - * converting the representation to \a BaseDuration type.
> > > > + * internally converting the representation to \a BaseDuration type.
> > > > */
> > > >
> > > > /**
> > > > --
> > > > 2.25.1
> > > >
> >
More information about the libcamera-devel
mailing list