[libcamera-devel] [PATCH v3 4/4] ipa: raspberrypi: Switch the AGC/Lux code to use utils::Duration
Naushir Patuck
naush at raspberrypi.com
Fri May 21 13:58:57 CEST 2021
On Fri, 21 May 2021 at 12:36, Naushir Patuck <naush at raspberrypi.com> wrote:
> On Fri, 21 May 2021 at 12:19, David Plowman <david.plowman at raspberrypi.com>
> 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....
>>
>
Sorry, I forgot to add this comment in the last email.
One option to make things cleaner is to move the operator<< into the
libcamera namespace,
avoiding the need to add the above using directive everywhere.
>
>> >
>> > 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?
>>
>
> Quite right, it can be fractional! I should just keep flicker_periods as
> an int to avoid
> this. Thanks for pointing that out.
>
>
>> > 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
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210521/71c51132/attachment-0001.htm>
More information about the libcamera-devel
mailing list