<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 19 May 2021 at 16:28, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush<br>
<br>
Thanks for all this work!<br>
<br>
On Tue, 18 May 2021 at 11:09, Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>> wrote:<br>
><br>
> Convert the core AGC and Lux controller code to use<br>
> RPiController::Duration for all exposure time related variables and<br>
> calculations.<br>
<br>
I wonder if I might have tried to keep the AGC and Lux patches<br>
separate? Having said that, they both *have* to change as soon as you<br>
touch the DeviceStatus, so maybe that would be difficult. Hmm, I think<br>
I'll leave that up to you!<br>
><br>
> Convert the exposure/shutter time fields in AgcStatus and DeviceStatus<br>
> to use RPiController::Duration.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> src/ipa/raspberrypi/cam_helper.cpp | 2 +-<br>
> src/ipa/raspberrypi/controller/agc_status.h | 12 +--<br>
> .../raspberrypi/controller/device_status.h | 6 +-<br>
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 89 ++++++++++---------<br>
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 26 +++---<br>
> src/ipa/raspberrypi/controller/rpi/lux.cpp | 17 ++--<br>
> src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 13 +--<br>
> 8 files changed, 91 insertions(+), 76 deletions(-)<br>
><br>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp<br>
> index e2b6c8eb8e03..c399987e47bf 100644<br>
> --- a/src/ipa/raspberrypi/cam_helper.cpp<br>
> +++ b/src/ipa/raspberrypi/cam_helper.cpp<br>
> @@ -183,7 +183,7 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,<br>
> return;<br>
> }<br>
><br>
> - deviceStatus.shutter_speed = DurationValue<std::micro>(Exposure(exposureLines));<br>
> + deviceStatus.shutter_speed = Exposure(exposureLines);<br>
<br>
Much nicer again!<br>
<br>
> deviceStatus.analogue_gain = Gain(gainCode);<br>
><br>
> LOG(IPARPI, Debug) << "Metadata updated - Exposure : "<br>
> diff --git a/src/ipa/raspberrypi/controller/agc_status.h b/src/ipa/raspberrypi/controller/agc_status.h<br>
> index 10381c90a313..b2a64ce562fa 100644<br>
> --- a/src/ipa/raspberrypi/controller/agc_status.h<br>
> +++ b/src/ipa/raspberrypi/controller/agc_status.h<br>
> @@ -6,6 +6,8 @@<br>
> */<br>
> #pragma once<br>
><br>
> +#include "duration.hpp"<br>
> +<br>
> // The AGC algorithm should post the following structure into the image's<br>
> // "agc.status" metadata.<br>
><br>
> @@ -18,17 +20,17 @@ extern "C" {<br>
> // ignored until then.<br>
><br>
> struct AgcStatus {<br>
> - double total_exposure_value; // value for all exposure and gain for this image<br>
> - double target_exposure_value; // (unfiltered) target total exposure AGC is aiming for<br>
> - double shutter_time;<br>
> + RPiController::Duration total_exposure_value; // value for all exposure and gain for this image<br>
> + RPiController::Duration target_exposure_value; // (unfiltered) target total exposure AGC is aiming for<br>
> + RPiController::Duration shutter_time;<br>
> double analogue_gain;<br>
> char exposure_mode[32];<br>
> char constraint_mode[32];<br>
> char metering_mode[32];<br>
> double ev;<br>
> - double flicker_period;<br>
> + RPiController::Duration flicker_period;<br>
> int floating_region_enable;<br>
> - double fixed_shutter;<br>
> + RPiController::Duration fixed_shutter;<br>
> double fixed_analogue_gain;<br>
> double digital_gain;<br>
> int locked;<br>
> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h<br>
> index aa08608b5d40..a8496176eb92 100644<br>
> --- a/src/ipa/raspberrypi/controller/device_status.h<br>
> +++ b/src/ipa/raspberrypi/controller/device_status.h<br>
> @@ -6,6 +6,8 @@<br>
> */<br>
> #pragma once<br>
><br>
> +#include "duration.hpp"<br>
> +<br>
> // Definition of "device metadata" which stores things like shutter time and<br>
> // analogue gain that downstream control algorithms will want to know.<br>
><br>
> @@ -14,8 +16,8 @@ extern "C" {<br>
> #endif<br>
><br>
> struct DeviceStatus {<br>
> - // time shutter is open, in microseconds<br>
> - double shutter_speed;<br>
> + // time shutter is open<br>
> + RPiController::Duration shutter_speed;<br>
> double analogue_gain;<br>
> // 1.0/distance-in-metres, or 0 if unknown<br>
> double lens_position;<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> index 1cb4472b2691..3af2ef3cf6ed 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> @@ -21,6 +21,7 @@<br>
><br>
> using namespace RPiController;<br>
> using namespace libcamera;<br>
> +using namespace std::literals::chrono_literals;<br>
><br>
> LOG_DEFINE_CATEGORY(RPiAgc)<br>
><br>
> @@ -55,19 +56,27 @@ read_metering_modes(std::map<std::string, AgcMeteringMode> &metering_modes,<br>
> return first;<br>
> }<br>
><br>
> -static int read_double_list(std::vector<double> &list,<br>
> - boost::property_tree::ptree const ¶ms)<br>
> +static int read_list(std::vector<double> &list,<br>
> + boost::property_tree::ptree const ¶ms)<br>
> {<br>
> for (auto &p : params)<br>
> list.push_back(p.second.get_value<double>());<br>
> return list.size();<br>
> }<br>
><br>
> +static int read_list(std::vector<Duration> &list,<br>
> + boost::property_tree::ptree const ¶ms)<br>
> +{<br>
> + for (auto &p : params)<br>
> + list.push_back(p.second.get_value<double>() * 1us);<br>
> + return list.size();<br>
> +}<br>
> +<br>
<br>
I wonder if there's a template-y way to do these in one go? (sorry!)<br></blockquote><div><br></div><div>I did consider that, but I do not know how to translate the "* 1us" with a simple</div><div>template. Any ideas?</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> void AgcExposureMode::Read(boost::property_tree::ptree const ¶ms)<br>
> {<br>
> int num_shutters =<br>
> - read_double_list(shutter, params.get_child("shutter"));<br>
> - int num_ags = read_double_list(gain, params.get_child("gain"));<br>
> + read_list(shutter, params.get_child("shutter"));<br>
> + int num_ags = read_list(gain, params.get_child("gain"));<br>
> if (num_shutters < 2 || num_ags < 2)<br>
> throw std::runtime_error(<br>
> "AgcConfig: must have at least two entries in exposure profile");<br>
> @@ -147,7 +156,7 @@ void AgcConfig::Read(boost::property_tree::ptree const ¶ms)<br>
> params.get<double>("fast_reduce_threshold", 0.4);<br>
> base_ev = params.get<double>("base_ev", 1.0);<br>
> // Start with quite a low value as ramping up is easier than ramping down.<br>
> - default_exposure_time = params.get<double>("default_exposure_time", 1000);<br>
> + default_exposure_time = params.get<double>("default_exposure_time", 1000) * 1us;<br>
<br>
Mostly I notice that you write, for example, "1.0us" rather than "1us"<br>
as you have here. I take it that it makes no difference, right?<br></blockquote><div><br></div><div>No difference. But I will try to be consistent :-)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> default_analogue_gain = params.get<double>("default_analogue_gain", 1.0);<br>
> }<br>
><br>
> @@ -157,7 +166,7 @@ Agc::Agc(Controller *controller)<br>
> frame_count_(0), lock_count_(0),<br>
> last_target_exposure_(0.0),<br>
> ev_(1.0), flicker_period_(0.0),<br>
> - max_shutter_(0), fixed_shutter_(0), fixed_analogue_gain_(0.0)<br>
> + max_shutter_(0.0s), fixed_shutter_(0.0s), fixed_analogue_gain_(0.0)<br>
> {<br>
> memset(&awb_, 0, sizeof(awb_));<br>
> // Setting status_.total_exposure_value_ to zero initially tells us<br>
> @@ -203,7 +212,7 @@ void Agc::Pause()<br>
><br>
> void Agc::Resume()<br>
> {<br>
> - fixed_shutter_ = 0;<br>
> + fixed_shutter_ = 0.0s;<br>
> fixed_analogue_gain_ = 0;<br>
> }<br>
><br>
> @@ -211,7 +220,7 @@ unsigned int Agc::GetConvergenceFrames() const<br>
> {<br>
> // If shutter and gain have been explicitly set, there is no<br>
> // convergence to happen, so no need to drop any frames - return zero.<br>
> - if (fixed_shutter_ && fixed_analogue_gain_)<br>
> + if (fixed_shutter_ > 0.0s && fixed_analogue_gain_)<br>
<br>
No implicit bool conversion for Durations, I take it. I notice that<br>
throughout this file we sometimes use "> 0.0s", sometimes "!= 0.0s". I<br>
assume that they're effectively interchangeable for us (and that the<br>
units don't matter when we compare for zero!), so should we be<br>
consistent?<br></blockquote><div><br></div><div>My next revision with the formal class definition for Duration will have</div><div>an operator bool() so this (and other) statements can now be simplified and</div><div>be consistent!</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Apart from that nothing much to add, so:<br>
<br>
Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
<br>
Thanks!<br>
David<br>
<br>
> return 0;<br>
> else<br>
> return config_.convergence_frames;<br>
> @@ -224,17 +233,17 @@ void Agc::SetEv(double ev)<br>
><br>
> void Agc::SetFlickerPeriod(Duration flicker_period)<br>
> {<br>
> - flicker_period_ = DurationValue<std::micro>(flicker_period);<br>
> + flicker_period_ = flicker_period;<br>
> }<br>
><br>
> void Agc::SetMaxShutter(Duration max_shutter)<br>
> {<br>
> - max_shutter_ = DurationValue<std::micro>(max_shutter);<br>
> + max_shutter_ = max_shutter;<br>
> }<br>
><br>
> void Agc::SetFixedShutter(Duration fixed_shutter)<br>
> {<br>
> - fixed_shutter_ = DurationValue<std::micro>(fixed_shutter);<br>
> + fixed_shutter_ = fixed_shutter;<br>
> // Set this in case someone calls Pause() straight after.<br>
> status_.shutter_time = clipShutter(fixed_shutter_);<br>
> }<br>
> @@ -266,8 +275,8 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,<br>
> {<br>
> housekeepConfig();<br>
><br>
> - double fixed_shutter = clipShutter(fixed_shutter_);<br>
> - if (fixed_shutter != 0.0 && fixed_analogue_gain_ != 0.0) {<br>
> + Duration fixed_shutter = clipShutter(fixed_shutter_);<br>
> + if (fixed_shutter != 0.0s && fixed_analogue_gain_ != 0.0) {<br>
> // We're going to reset the algorithm here with these fixed values.<br>
><br>
> fetchAwbStatus(metadata);<br>
> @@ -284,7 +293,7 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,<br>
> // Equivalent of divideUpExposure.<br>
> filtered_.shutter = fixed_shutter;<br>
> filtered_.analogue_gain = fixed_analogue_gain_;<br>
> - } else if (status_.total_exposure_value) {<br>
> + } else if (status_.total_exposure_value > 0.0s) {<br>
> // On a mode switch, it's possible the exposure profile could change,<br>
> // or a fixed exposure/gain might be set so we divide up the exposure/<br>
> // gain again, but we don't change any target values.<br>
> @@ -296,7 +305,7 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,<br>
> // for any that weren't set.<br>
><br>
> // Equivalent of divideUpExposure.<br>
> - filtered_.shutter = fixed_shutter ? fixed_shutter : config_.default_exposure_time;<br>
> + filtered_.shutter = fixed_shutter > 0.0s ? fixed_shutter : config_.default_exposure_time;<br>
> filtered_.analogue_gain = fixed_analogue_gain_ ? fixed_analogue_gain_ : config_.default_analogue_gain;<br>
> }<br>
><br>
> @@ -308,13 +317,12 @@ void Agc::Prepare(Metadata *image_metadata)<br>
> status_.digital_gain = 1.0;<br>
> fetchAwbStatus(image_metadata); // always fetch it so that Process knows it's been done<br>
><br>
> - if (status_.total_exposure_value) {<br>
> + if (status_.total_exposure_value > 0.0s) {<br>
> // Process has run, so we have meaningful values.<br>
> DeviceStatus device_status;<br>
> if (image_metadata->Get("device.status", device_status) == 0) {<br>
> - double actual_exposure = device_status.shutter_speed *<br>
> - device_status.analogue_gain;<br>
> - if (actual_exposure) {<br>
> + Duration actual_exposure = device_status.shutter_speed * device_status.analogue_gain;<br>
> + if (actual_exposure > 0.0s) {<br>
> status_.digital_gain =<br>
> status_.total_exposure_value /<br>
> actual_exposure;<br>
> @@ -370,9 +378,9 @@ void Agc::updateLockStatus(DeviceStatus const &device_status)<br>
> const double RESET_MARGIN = 1.5;<br>
><br>
> // Add 200us to the exposure time error to allow for line quantisation.<br>
> - double exposure_error = last_device_status_.shutter_speed * ERROR_FACTOR + 200;<br>
> + Duration exposure_error = last_device_status_.shutter_speed * ERROR_FACTOR + 200us;<br>
> double gain_error = last_device_status_.analogue_gain * ERROR_FACTOR;<br>
> - double target_error = last_target_exposure_ * ERROR_FACTOR;<br>
> + Duration target_error = last_target_exposure_ * ERROR_FACTOR;<br>
><br>
> // Note that we don't know the exposure/gain limits of the sensor, so<br>
> // the values we keep requesting may be unachievable. For this reason<br>
> @@ -462,7 +470,7 @@ void Agc::fetchCurrentExposure(Metadata *image_metadata)<br>
> current_.analogue_gain = device_status->analogue_gain;<br>
> AgcStatus *agc_status =<br>
> image_metadata->GetLocked<AgcStatus>("agc.status");<br>
> - current_.total_exposure = agc_status ? agc_status->total_exposure_value : 0;<br>
> + current_.total_exposure = agc_status ? agc_status->total_exposure_value : 0s;<br>
> current_.total_exposure_no_dg = current_.shutter * current_.analogue_gain;<br>
> }<br>
><br>
> @@ -573,7 +581,7 @@ void Agc::computeGain(bcm2835_isp_stats *statistics, Metadata *image_metadata,<br>
><br>
> void Agc::computeTargetExposure(double gain)<br>
> {<br>
> - if (status_.fixed_shutter != 0.0 && status_.fixed_analogue_gain != 0.0) {<br>
> + if (status_.fixed_shutter != 0.0s && status_.fixed_analogue_gain != 0.0) {<br>
> // When ag and shutter are both fixed, we need to drive the<br>
> // total exposure so that we end up with a digital gain of at least<br>
> // 1/min_colour_gain. Otherwise we'd desaturate channels causing<br>
> @@ -588,11 +596,11 @@ void Agc::computeTargetExposure(double gain)<br>
> target_.total_exposure = current_.total_exposure_no_dg * gain;<br>
> // The final target exposure is also limited to what the exposure<br>
> // mode allows.<br>
> - double max_shutter = status_.fixed_shutter != 0.0<br>
> + Duration max_shutter = status_.fixed_shutter != 0.0s<br>
> ? status_.fixed_shutter<br>
> : exposure_mode_->shutter.back();<br>
> max_shutter = clipShutter(max_shutter);<br>
> - double max_total_exposure =<br>
> + Duration max_total_exposure =<br>
> max_shutter *<br>
> (status_.fixed_analogue_gain != 0.0<br>
> ? status_.fixed_analogue_gain<br>
> @@ -634,10 +642,10 @@ void Agc::filterExposure(bool desaturate)<br>
> double speed = config_.speed;<br>
> // AGC adapts instantly if both shutter and gain are directly specified<br>
> // or we're in the startup phase.<br>
> - if ((status_.fixed_shutter && status_.fixed_analogue_gain) ||<br>
> + if ((status_.fixed_shutter > 0.0s && status_.fixed_analogue_gain) ||<br>
> frame_count_ <= config_.startup_frames)<br>
> speed = 1.0;<br>
> - if (filtered_.total_exposure == 0.0) {<br>
> + if (filtered_.total_exposure == 0.0s) {<br>
> filtered_.total_exposure = target_.total_exposure;<br>
> filtered_.total_exposure_no_dg = target_.total_exposure_no_dg;<br>
> } else {<br>
> @@ -674,9 +682,10 @@ void Agc::divideUpExposure()<br>
> // Sending the fixed shutter/gain cases through the same code may seem<br>
> // unnecessary, but it will make more sense when extend this to cover<br>
> // variable aperture.<br>
> - double exposure_value = filtered_.total_exposure_no_dg;<br>
> - double shutter_time, analogue_gain;<br>
> - shutter_time = status_.fixed_shutter != 0.0<br>
> + Duration exposure_value = filtered_.total_exposure_no_dg;<br>
> + Duration shutter_time;<br>
> + double analogue_gain;<br>
> + shutter_time = status_.fixed_shutter != 0.0s<br>
> ? status_.fixed_shutter<br>
> : exposure_mode_->shutter[0];<br>
> shutter_time = clipShutter(shutter_time);<br>
> @@ -686,8 +695,8 @@ void Agc::divideUpExposure()<br>
> if (shutter_time * analogue_gain < exposure_value) {<br>
> for (unsigned int stage = 1;<br>
> stage < exposure_mode_->gain.size(); stage++) {<br>
> - if (status_.fixed_shutter == 0.0) {<br>
> - double stage_shutter =<br>
> + if (status_.fixed_shutter == 0.0s) {<br>
> + Duration stage_shutter =<br>
> clipShutter(exposure_mode_->shutter[stage]);<br>
> if (stage_shutter * analogue_gain >=<br>
> exposure_value) {<br>
> @@ -713,12 +722,12 @@ void Agc::divideUpExposure()<br>
> << analogue_gain;<br>
> // Finally adjust shutter time for flicker avoidance (require both<br>
> // shutter and gain not to be fixed).<br>
> - if (status_.fixed_shutter == 0.0 &&<br>
> + if (status_.fixed_shutter == 0.0s &&<br>
> status_.fixed_analogue_gain == 0.0 &&<br>
> - status_.flicker_period != 0.0) {<br>
> - int flicker_periods = shutter_time / status_.flicker_period;<br>
> - if (flicker_periods > 0) {<br>
> - double new_shutter_time = flicker_periods * status_.flicker_period;<br>
> + status_.flicker_period != 0.0s) {<br>
> + double flicker_periods = shutter_time / status_.flicker_period;<br>
> + if (flicker_periods > 0.0) {<br>
> + Duration new_shutter_time = flicker_periods * status_.flicker_period;<br>
> analogue_gain *= shutter_time / new_shutter_time;<br>
> // We should still not allow the ag to go over the<br>
> // largest value in the exposure mode. Note that this<br>
> @@ -738,7 +747,7 @@ void Agc::divideUpExposure()<br>
> void Agc::writeAndFinish(Metadata *image_metadata, bool desaturate)<br>
> {<br>
> status_.total_exposure_value = filtered_.total_exposure;<br>
> - status_.target_exposure_value = desaturate ? 0 : target_.total_exposure_no_dg;<br>
> + status_.target_exposure_value = desaturate ? 0s : target_.total_exposure_no_dg;<br>
> status_.shutter_time = filtered_.shutter;<br>
> status_.analogue_gain = filtered_.analogue_gain;<br>
> // Write to metadata as well, in case anyone wants to update the camera<br>
> @@ -750,9 +759,9 @@ void Agc::writeAndFinish(Metadata *image_metadata, bool desaturate)<br>
> << " analogue gain " << filtered_.analogue_gain;<br>
> }<br>
><br>
> -double Agc::clipShutter(double shutter)<br>
> +Duration Agc::clipShutter(Duration shutter)<br>
> {<br>
> - if (max_shutter_)<br>
> + if (max_shutter_ > 0.0s)<br>
> shutter = std::min(shutter, max_shutter_);<br>
> return shutter;<br>
> }<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
> index cb79bf61ba42..68b97ce91c99 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp<br>
> @@ -22,13 +22,15 @@<br>
><br>
> namespace RPiController {<br>
><br>
> +using namespace std::literals::chrono_literals;<br>
> +<br>
> struct AgcMeteringMode {<br>
> double weights[AGC_STATS_SIZE];<br>
> void Read(boost::property_tree::ptree const ¶ms);<br>
> };<br>
><br>
> struct AgcExposureMode {<br>
> - std::vector<double> shutter;<br>
> + std::vector<Duration> shutter;<br>
> std::vector<double> gain;<br>
> void Read(boost::property_tree::ptree const ¶ms);<br>
> };<br>
> @@ -61,7 +63,7 @@ struct AgcConfig {<br>
> std::string default_exposure_mode;<br>
> std::string default_constraint_mode;<br>
> double base_ev;<br>
> - double default_exposure_time;<br>
> + Duration default_exposure_time;<br>
> double default_analogue_gain;<br>
> };<br>
><br>
> @@ -101,19 +103,19 @@ private:<br>
> void filterExposure(bool desaturate);<br>
> void divideUpExposure();<br>
> void writeAndFinish(Metadata *image_metadata, bool desaturate);<br>
> - double clipShutter(double shutter);<br>
> + Duration clipShutter(Duration shutter);<br>
> AgcMeteringMode *metering_mode_;<br>
> AgcExposureMode *exposure_mode_;<br>
> AgcConstraintMode *constraint_mode_;<br>
> uint64_t frame_count_;<br>
> AwbStatus awb_;<br>
> struct ExposureValues {<br>
> - ExposureValues() : shutter(0), analogue_gain(0),<br>
> - total_exposure(0), total_exposure_no_dg(0) {}<br>
> - double shutter;<br>
> + ExposureValues() : shutter(0.0s), analogue_gain(0),<br>
> + total_exposure(0.0s), total_exposure_no_dg(0.0s) {}<br>
> + Duration shutter;<br>
> double analogue_gain;<br>
> - double total_exposure;<br>
> - double total_exposure_no_dg; // without digital gain<br>
> + Duration total_exposure;<br>
> + Duration total_exposure_no_dg; // without digital gain<br>
> };<br>
> ExposureValues current_; // values for the current frame<br>
> ExposureValues target_; // calculate the values we want here<br>
> @@ -121,15 +123,15 @@ private:<br>
> AgcStatus status_;<br>
> int lock_count_;<br>
> DeviceStatus last_device_status_;<br>
> - double last_target_exposure_;<br>
> + Duration last_target_exposure_;<br>
> // Below here the "settings" that applications can change.<br>
> std::string metering_mode_name_;<br>
> std::string exposure_mode_name_;<br>
> std::string constraint_mode_name_;<br>
> double ev_;<br>
> - double flicker_period_;<br>
> - double max_shutter_;<br>
> - double fixed_shutter_;<br>
> + Duration flicker_period_;<br>
> + Duration max_shutter_;<br>
> + Duration fixed_shutter_;<br>
> double fixed_analogue_gain_;<br>
> };<br>
><br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> index f74381cab2b4..46d3f3fab2c6 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp<br>
> @@ -16,6 +16,7 @@<br>
><br>
> using namespace RPiController;<br>
> using namespace libcamera;<br>
> +using namespace std::literals::chrono_literals;<br>
><br>
> LOG_DEFINE_CATEGORY(RPiLux)<br>
><br>
> @@ -38,7 +39,7 @@ char const *Lux::Name() const<br>
> void Lux::Read(boost::property_tree::ptree const ¶ms)<br>
> {<br>
> reference_shutter_speed_ =<br>
> - params.get<double>("reference_shutter_speed");<br>
> + params.get<double>("reference_shutter_speed") * 1us;<br>
> reference_gain_ = params.get<double>("reference_gain");<br>
> reference_aperture_ = params.get<double>("reference_aperture", 1.0);<br>
> reference_Y_ = params.get<double>("reference_Y");<br>
> @@ -60,15 +61,13 @@ void Lux::Prepare(Metadata *image_metadata)<br>
> void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)<br>
> {<br>
> // set some initial values to shut the compiler up<br>
> - DeviceStatus device_status =<br>
> - { .shutter_speed = 1.0,<br>
> - .analogue_gain = 1.0,<br>
> - .lens_position = 0.0,<br>
> - .aperture = 0.0,<br>
> - .flash_intensity = 0.0 };<br>
> + DeviceStatus device_status = { .shutter_speed = 1.0ms,<br>
> + .analogue_gain = 1.0,<br>
> + .lens_position = 0.0,<br>
> + .aperture = 0.0,<br>
> + .flash_intensity = 0.0 };<br>
> if (image_metadata->Get("device.status", device_status) == 0) {<br>
> double current_gain = device_status.analogue_gain;<br>
> - double current_shutter_speed = device_status.shutter_speed;<br>
> double current_aperture = device_status.aperture;<br>
> if (current_aperture == 0)<br>
> current_aperture = current_aperture_;<br>
> @@ -83,7 +82,7 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)<br>
> double current_Y = sum / (double)num + .5;<br>
> double gain_ratio = reference_gain_ / current_gain;<br>
> double shutter_speed_ratio =<br>
> - reference_shutter_speed_ / current_shutter_speed;<br>
> + reference_shutter_speed_ / device_status.shutter_speed;<br>
> double aperture_ratio = reference_aperture_ / current_aperture;<br>
> double Y_ratio = current_Y * (65536 / num_bins) / reference_Y_;<br>
> double estimated_lux = shutter_speed_ratio * gain_ratio *<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.hpp b/src/ipa/raspberrypi/controller/rpi/lux.hpp<br>
> index f9090484a136..726a7f7ca627 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/lux.hpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.hpp<br>
> @@ -28,7 +28,7 @@ public:<br>
> private:<br>
> // These values define the conditions of the reference image, against<br>
> // which we compare the new image.<br>
> - double reference_shutter_speed_; // in micro-seconds<br>
> + Duration reference_shutter_speed_;<br>
> double reference_gain_;<br>
> double reference_aperture_; // units of 1/f<br>
> double reference_Y_; // out of 65536<br>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> index f080f2e53bac..15f51162afec 100644<br>
> --- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> @@ -227,11 +227,11 @@ void IPARPi::start(const ControlList &controls, ipa::RPi::StartConfig *startConf<br>
><br>
> /* SwitchMode may supply updated exposure/gain values to use. */<br>
> AgcStatus agcStatus;<br>
> - agcStatus.shutter_time = 0.0;<br>
> + agcStatus.shutter_time = 0.0s;<br>
> agcStatus.analogue_gain = 0.0;<br>
><br>
> metadata.Get("agc.status", agcStatus);<br>
> - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {<br>
> + if (agcStatus.shutter_time != 0.0s && agcStatus.analogue_gain != 0.0) {<br>
> ControlList ctrls(sensorCtrls_);<br>
> applyAGC(&agcStatus, ctrls);<br>
> startConfig->controls = std::move(ctrls);<br>
> @@ -394,7 +394,7 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,<br>
> /* Supply initial values for gain and exposure. */<br>
> ControlList ctrls(sensorCtrls_);<br>
> AgcStatus agcStatus;<br>
> - agcStatus.shutter_time = DurationValue<std::micro>(DefaultExposureTime);<br>
> + agcStatus.shutter_time = DefaultExposureTime;<br>
> agcStatus.analogue_gain = DefaultAnalogueGain;<br>
> applyAGC(&agcStatus, ctrls);<br>
><br>
> @@ -466,7 +466,8 @@ void IPARPi::reportMetadata()<br>
> */<br>
> DeviceStatus *deviceStatus = rpiMetadata_.GetLocked<DeviceStatus>("device.status");<br>
> if (deviceStatus) {<br>
> - libcameraMetadata_.set(controls::ExposureTime, deviceStatus->shutter_speed);<br>
> + libcameraMetadata_.set(controls::ExposureTime,<br>
> + DurationValue<std::micro>(deviceStatus->shutter_speed));<br>
> libcameraMetadata_.set(controls::AnalogueGain, deviceStatus->analogue_gain);<br>
> }<br>
><br>
> @@ -1019,7 +1020,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)<br>
> int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();<br>
> int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();<br>
><br>
> - deviceStatus.shutter_speed = DurationValue<std::micro>(helper_->Exposure(exposureLines));<br>
> + deviceStatus.shutter_speed = helper_->Exposure(exposureLines);<br>
> deviceStatus.analogue_gain = helper_->Gain(gainCode);<br>
><br>
> LOG(IPARPI, Debug) << "Metadata - Exposure : "<br>
> @@ -1105,7 +1106,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)<br>
> int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);<br>
><br>
> /* GetVBlanking might clip exposure time to the fps limits. */<br>
> - Duration exposure = agcStatus->shutter_time * 1.0us;<br>
> + Duration exposure = agcStatus->shutter_time;<br>
> int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);<br>
> int32_t exposureLines = helper_->ExposureLines(exposure);<br>
><br>
> --<br>
> 2.25.1<br>
><br>
</blockquote></div></div>