[libcamera-devel] [PATCH 4/4] ipa: raspberrypi: Switch the AGC/Lux code to use RPiController::Duration
Naushir Patuck
naush at raspberrypi.com
Wed May 19 17:32:44 CEST 2021
Hi David,
Thank you for your feedback.
On Wed, 19 May 2021 at 16:28, David Plowman <david.plowman at raspberrypi.com>
wrote:
> Hi Naush
>
> Thanks for all this work!
>
> On Tue, 18 May 2021 at 11:09, Naushir Patuck <naush at raspberrypi.com>
> wrote:
> >
> > Convert the core AGC and Lux controller code to use
> > RPiController::Duration for all exposure time related variables and
> > calculations.
>
> I wonder if I might have tried to keep the AGC and Lux patches
> separate? Having said that, they both *have* to change as soon as you
> touch the DeviceStatus, so maybe that would be difficult. Hmm, I think
> I'll leave that up to you!
> >
> > Convert the exposure/shutter time fields in AgcStatus and DeviceStatus
> > to use RPiController::Duration.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > src/ipa/raspberrypi/cam_helper.cpp | 2 +-
> > src/ipa/raspberrypi/controller/agc_status.h | 12 +--
> > .../raspberrypi/controller/device_status.h | 6 +-
> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 89 ++++++++++---------
> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 26 +++---
> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 17 ++--
> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-
> > src/ipa/raspberrypi/raspberrypi.cpp | 13 +--
> > 8 files changed, 91 insertions(+), 76 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> b/src/ipa/raspberrypi/cam_helper.cpp
> > index e2b6c8eb8e03..c399987e47bf 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 =
> DurationValue<std::micro>(Exposure(exposureLines));
> > + deviceStatus.shutter_speed = Exposure(exposureLines);
>
> Much nicer again!
>
> > 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..b2a64ce562fa 100644
> > --- a/src/ipa/raspberrypi/controller/agc_status.h
> > +++ b/src/ipa/raspberrypi/controller/agc_status.h
> > @@ -6,6 +6,8 @@
> > */
> > #pragma once
> >
> > +#include "duration.hpp"
> > +
> > // The AGC algorithm should post the following structure into the
> image's
> > // "agc.status" metadata.
> >
> > @@ -18,17 +20,17 @@ extern "C" {
> > // ignored until then.
> >
> > 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;
> > + RPiController::Duration total_exposure_value; // value for all
> exposure and gain for this image
> > + RPiController::Duration target_exposure_value; // (unfiltered)
> target total exposure AGC is aiming for
> > + RPiController::Duration shutter_time;
> > double analogue_gain;
> > char exposure_mode[32];
> > char constraint_mode[32];
> > char metering_mode[32];
> > double ev;
> > - double flicker_period;
> > + RPiController::Duration flicker_period;
> > int floating_region_enable;
> > - double fixed_shutter;
> > + RPiController::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..a8496176eb92 100644
> > --- a/src/ipa/raspberrypi/controller/device_status.h
> > +++ b/src/ipa/raspberrypi/controller/device_status.h
> > @@ -6,6 +6,8 @@
> > */
> > #pragma once
> >
> > +#include "duration.hpp"
> > +
> > // 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
> > + RPiController::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 1cb4472b2691..3af2ef3cf6ed 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 namespace std::literals::chrono_literals;
> >
> > 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();
> > +}
> > +
>
> I wonder if there's a template-y way to do these in one go? (sorry!)
>
I did consider that, but I do not know how to translate the "* 1us" with a
simple
template. Any ideas?
>
> > 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;
>
> Mostly I notice that you write, for example, "1.0us" rather than "1us"
> as you have here. I take it that it makes no difference, right?
>
No difference. But I will try to be consistent :-)
>
> > default_analogue_gain =
> params.get<double>("default_analogue_gain", 1.0);
> > }
> >
> > @@ -157,7 +166,7 @@ Agc::Agc(Controller *controller)
> > 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)
> > + max_shutter_(0.0s), fixed_shutter_(0.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_ = 0.0s;
> > fixed_analogue_gain_ = 0;
> > }
> >
> > @@ -211,7 +220,7 @@ unsigned int Agc::GetConvergenceFrames() const
> > {
> > // If shutter and gain have been explicitly set, there is no
> > // convergence to happen, so no need to drop any frames - return
> zero.
> > - if (fixed_shutter_ && fixed_analogue_gain_)
> > + if (fixed_shutter_ > 0.0s && fixed_analogue_gain_)
>
> No implicit bool conversion for Durations, I take it. I notice that
> throughout this file we sometimes use "> 0.0s", sometimes "!= 0.0s". I
> assume that they're effectively interchangeable for us (and that the
> units don't matter when we compare for zero!), so should we be
> consistent?
>
My next revision with the formal class definition for Duration will have
an operator bool() so this (and other) statements can now be simplified and
be consistent!
Regards,
Naush
>
> Apart from that nothing much to add, so:
>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
>
> Thanks!
> David
>
> > return 0;
> > else
> > return config_.convergence_frames;
> > @@ -224,17 +233,17 @@ void Agc::SetEv(double ev)
> >
> > void Agc::SetFlickerPeriod(Duration flicker_period)
> > {
> > - flicker_period_ = DurationValue<std::micro>(flicker_period);
> > + flicker_period_ = flicker_period;
> > }
> >
> > void Agc::SetMaxShutter(Duration max_shutter)
> > {
> > - max_shutter_ = DurationValue<std::micro>(max_shutter);
> > + max_shutter_ = max_shutter;
> > }
> >
> > void Agc::SetFixedShutter(Duration fixed_shutter)
> > {
> > - fixed_shutter_ = DurationValue<std::micro>(fixed_shutter);
> > + 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 != 0.0s && fixed_analogue_gain_ != 0.0) {
> > // We're going to reset the algorithm here with these
> fixed values.
> >
> > fetchAwbStatus(metadata);
> > @@ -284,7 +293,7 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode
> const &camera_mode,
> > // Equivalent of divideUpExposure.
> > filtered_.shutter = fixed_shutter;
> > filtered_.analogue_gain = fixed_analogue_gain_;
> > - } else if (status_.total_exposure_value) {
> > + } else if (status_.total_exposure_value > 0.0s) {
> > // On a mode switch, it's possible the exposure profile
> could change,
> > // or a fixed exposure/gain might be set so we divide up
> the exposure/
> > // gain again, but we don't change any target values.
> > @@ -296,7 +305,7 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode
> const &camera_mode,
> > // for any that weren't set.
> >
> > // Equivalent of divideUpExposure.
> > - filtered_.shutter = fixed_shutter ? fixed_shutter :
> config_.default_exposure_time;
> > + filtered_.shutter = fixed_shutter > 0.0s ? fixed_shutter
> : config_.default_exposure_time;
> > filtered_.analogue_gain = fixed_analogue_gain_ ?
> fixed_analogue_gain_ : config_.default_analogue_gain;
> > }
> >
> > @@ -308,13 +317,12 @@ void Agc::Prepare(Metadata *image_metadata)
> > status_.digital_gain = 1.0;
> > fetchAwbStatus(image_metadata); // always fetch it so that
> Process knows it's been done
> >
> > - if (status_.total_exposure_value) {
> > + if (status_.total_exposure_value > 0.0s) {
> > // 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;
> > - if (actual_exposure) {
> > + Duration actual_exposure =
> device_status.shutter_speed * device_status.analogue_gain;
> > + if (actual_exposure > 0.0s) {
> > status_.digital_gain =
> > status_.total_exposure_value /
> > actual_exposure;
> > @@ -370,9 +378,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 +470,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 +581,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 != 0.0s && status_.fixed_analogue_gain
> != 0.0) {
> > // 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 +596,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 != 0.0s
> > ? 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
> > @@ -634,10 +642,10 @@ void Agc::filterExposure(bool desaturate)
> > double speed = config_.speed;
> > // AGC adapts instantly if both shutter and gain are directly
> specified
> > // or we're in the startup phase.
> > - if ((status_.fixed_shutter && status_.fixed_analogue_gain) ||
> > + if ((status_.fixed_shutter > 0.0s &&
> status_.fixed_analogue_gain) ||
> > frame_count_ <= config_.startup_frames)
> > speed = 1.0;
> > - if (filtered_.total_exposure == 0.0) {
> > + if (filtered_.total_exposure == 0.0s) {
> > filtered_.total_exposure = target_.total_exposure;
> > filtered_.total_exposure_no_dg =
> target_.total_exposure_no_dg;
> > } else {
> > @@ -674,9 +682,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 != 0.0s
> > ? status_.fixed_shutter
> > : exposure_mode_->shutter[0];
> > shutter_time = clipShutter(shutter_time);
> > @@ -686,8 +695,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 == 0.0s) {
> > + Duration stage_shutter =
> >
> clipShutter(exposure_mode_->shutter[stage]);
> > if (stage_shutter * analogue_gain >=
> > exposure_value) {
> > @@ -713,12 +722,12 @@ 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 &&
> > + if (status_.fixed_shutter == 0.0s &&
> > 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;
> > + status_.flicker_period != 0.0s) {
> > + double flicker_periods = shutter_time /
> status_.flicker_period;
> > + if (flicker_periods > 0.0) {
> > + Duration new_shutter_time = flicker_periods *
> status_.flicker_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 +747,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,9 +759,9 @@ void Agc::writeAndFinish(Metadata *image_metadata,
> bool desaturate)
> > << " analogue gain " <<
> filtered_.analogue_gain;
> > }
> >
> > -double Agc::clipShutter(double shutter)
> > +Duration Agc::clipShutter(Duration shutter)
> > {
> > - if (max_shutter_)
> > + if (max_shutter_ > 0.0s)
> > shutter = std::min(shutter, max_shutter_);
> > return shutter;
> > }
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > index cb79bf61ba42..68b97ce91c99 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> > @@ -22,13 +22,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 +63,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 +103,19 @@ private:
> > void filterExposure(bool desaturate);
> > void divideUpExposure();
> > void writeAndFinish(Metadata *image_metadata, bool desaturate);
> > - double clipShutter(double shutter);
> > + Duration clipShutter(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(0.0s), analogue_gain(0),
> > + total_exposure(0.0s),
> total_exposure_no_dg(0.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 +123,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..46d3f3fab2c6 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") * 1us;
> > 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..726a7f7ca627 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.hpp
> > @@ -28,7 +28,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
> > + 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 f080f2e53bac..15f51162afec 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -227,11 +227,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 != 0.0s && agcStatus.analogue_gain !=
> 0.0) {
> > ControlList ctrls(sensorCtrls_);
> > applyAGC(&agcStatus, ctrls);
> > startConfig->controls = std::move(ctrls);
> > @@ -394,7 +394,7 @@ int IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> > /* Supply initial values for gain and exposure. */
> > ControlList ctrls(sensorCtrls_);
> > AgcStatus agcStatus;
> > - agcStatus.shutter_time =
> DurationValue<std::micro>(DefaultExposureTime);
> > + agcStatus.shutter_time = DefaultExposureTime;
> > agcStatus.analogue_gain = DefaultAnalogueGain;
> > applyAGC(&agcStatus, ctrls);
> >
> > @@ -466,7 +466,8 @@ void IPARPi::reportMetadata()
> > */
> > DeviceStatus *deviceStatus =
> rpiMetadata_.GetLocked<DeviceStatus>("device.status");
> > if (deviceStatus) {
> > - libcameraMetadata_.set(controls::ExposureTime,
> deviceStatus->shutter_speed);
> > + libcameraMetadata_.set(controls::ExposureTime,
> > +
> DurationValue<std::micro>(deviceStatus->shutter_speed));
> > libcameraMetadata_.set(controls::AnalogueGain,
> deviceStatus->analogue_gain);
> > }
> >
> > @@ -1019,7 +1020,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 =
> DurationValue<std::micro>(helper_->Exposure(exposureLines));
> > + deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
> > deviceStatus.analogue_gain = helper_->Gain(gainCode);
> >
> > LOG(IPARPI, Debug) << "Metadata - Exposure : "
> > @@ -1105,7 +1106,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);
> >
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210519/3209a566/attachment-0001.htm>
More information about the libcamera-devel
mailing list