[{"id":17058,"web_url":"https://patchwork.libcamera.org/comment/17058/","msgid":"","date":"2021-05-21T08:00:59","subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Switch the\n\tAGC/Lux code to use utils::Duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 20 May 2021 at 16:17, Naushir Patuck wrote:\n\n> Convert the core AGC and Lux controller code to use\n> utils::Duration for all exposure time related variables and\n> calculations.\n>\n> Convert the exposure/shutter time fields in AgcStatus and DeviceStatus\n> to use utils::Duration.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/cam_helper.cpp | 2 +-\n> src/ipa/raspberrypi/controller/agc_status.h | 14 +--\n> .../raspberrypi/controller/device_status.h | 6 +-\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 88 +++++++++++--------\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 28 +++---\n> src/ipa/raspberrypi/controller/rpi/lux.cpp | 17 ++--\n> src/ipa/raspberrypi/controller/rpi/lux.hpp | 4 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 13 +--\n> src/libcamera/utils.cpp | 9 +-\n> 9 files changed, 104 insertions(+), 77 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp\n> b/src/ipa/raspberrypi/cam_helper.cpp\n> index feb02d1806b2..968fae13bd5e 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -183,7 +183,7 @@ void CamHelper::parseEmbeddedData(Span\n> buffer,\n> return;\n> }\n>\n> - deviceStatus.shutter_speed =\n> Exposure(exposureLines).get();\n> + deviceStatus.shutter_speed = Exposure(exposureLines);\n> deviceStatus.analogue_gain = Gain(gainCode);\n>\n> LOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\n> diff --git a/src/ipa/raspberrypi/controller/agc_status.h\n> b/src/ipa/raspberrypi/controller/agc_status.h\n> index 10381c90a313..c893715af588 100644\n> --- a/src/ipa/raspberrypi/controller/agc_status.h\n> +++ b/src/ipa/raspberrypi/controller/agc_status.h\n> @@ -6,6 +6,8 @@\n> */\n> #pragma once\n>\n> +#include \"libcamera/internal/utils.h\"\n> +\n> // The AGC algorithm should post the following structure into the image's\n> // \"agc.status\" metadata.\n>\n> @@ -17,18 +19,20 @@ extern \"C\" {\n> // seen statistics and calculated meaningful values. The contents should\n> be\n> // ignored until then.\n>\n> +using libcamera::utils::Duration;\n> +\n> struct AgcStatus {\n> - double total_exposure_value; // value for all exposure and gain\n> for this image\n> - double target_exposure_value; // (unfiltered) target total\n> exposure AGC is aiming for\n> - double shutter_time;\n> + Duration total_exposure_value; // value for all exposure and gain\n> for this image\n> + Duration target_exposure_value; // (unfiltered) target total\n> exposure AGC is aiming for\n> + Duration shutter_time;\n> double analogue_gain;\n> char exposure_mode[32];\n> char constraint_mode[32];\n> char metering_mode[32];\n> double ev;\n> - double flicker_period;\n> + Duration flicker_period;\n> int floating_region_enable;\n> - double fixed_shutter;\n> + Duration fixed_shutter;\n> double fixed_analogue_gain;\n> double digital_gain;\n> int locked;\n> diff --git a/src/ipa/raspberrypi/controller/device_status.h\n> b/src/ipa/raspberrypi/controller/device_status.h\n> index aa08608b5d40..131b4cd344ee 100644\n> --- a/src/ipa/raspberrypi/controller/device_status.h\n> +++ b/src/ipa/raspberrypi/controller/device_status.h\n> @@ -6,6 +6,8 @@\n> */\n> #pragma once\n>\n> +#include \"libcamera/internal/utils.h\"\n> +\n> // Definition of \"device metadata\" which stores things like shutter time\n> and\n> // analogue gain that downstream control algorithms will want to know.\n>\n> @@ -14,8 +16,8 @@ extern \"C\" {\n> #endif\n>\n> struct DeviceStatus {\n> - // time shutter is open, in microseconds\n> - double shutter_speed;\n> + // time shutter is open\n> + libcamera::utils::Duration shutter_speed;\n> double analogue_gain;\n> // 1.0/distance-in-metres, or 0 if unknown\n> double lens_position;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index 482eb3ef962d..2db5156d396e 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -55,19 +55,27 @@ read_metering_modes(std::map AgcMeteringMode> &metering_modes,\n> return first;\n> }\n>\n> -static int read_double_list(std::vector &list,\n> - boost::property_tree::ptree const ¶ms)\n> +static int read_list(std::vector &list,\n> + boost::property_tree::ptree const ¶ms)\n> {\n> for (auto &p : params)\n> list.push_back(p.second.get_value());\n> return list.size();\n> }\n>\n> +static int read_list(std::vector &list,\n> + boost::property_tree::ptree const ¶ms)\n> +{\n> + for (auto &p : params)\n> + list.push_back(p.second.get_value() * 1us);\n> + return list.size();\n> +}\n> +\n> void AgcExposureMode::Read(boost::property_tree::ptree const ¶ms)\n> {\n> int num_shutters =\n> - read_double_list(shutter, params.get_child(\"shutter\"));\n> - int num_ags = read_double_list(gain, params.get_child(\"gain\"));\n> + read_list(shutter, params.get_child(\"shutter\"));\n> + int num_ags = read_list(gain, params.get_child(\"gain\"));\n> if (num_shutters < 2 || num_ags < 2)\n> throw std::runtime_error(\n> \"AgcConfig: must have at least two entries in\n> exposure profile\");\n> @@ -147,7 +155,7 @@ void AgcConfig::Read(boost::property_tree::ptree const\n> ¶ms)\n> params.get(\"fast_reduce_threshold\", 0.4);\n> base_ev = params.get(\"base_ev\", 1.0);\n> // Start with quite a low value as ramping up is easier than\n> ramping down.\n> - default_exposure_time =\n> params.get(\"default_exposure_time\", 1000);\n> + default_exposure_time =\n> params.get(\"default_exposure_time\", 1000) * 1us;\n> default_analogue_gain =\n> params.get(\"default_analogue_gain\", 1.0);\n> }\n>\n> @@ -155,9 +163,9 @@ Agc::Agc(Controller *controller)\n> : AgcAlgorithm(controller), metering_mode_(nullptr),\n> exposure_mode_(nullptr), constraint_mode_(nullptr),\n> frame_count_(0), lock_count_(0),\n> - last_target_exposure_(0.0),\n> - ev_(1.0), flicker_period_(0.0),\n> - max_shutter_(0), fixed_shutter_(0), fixed_analogue_gain_(0.0)\n> + last_target_exposure_(0s),\n> + ev_(1.0), flicker_period_(0s),\n> + max_shutter_(0s), fixed_shutter_(0s), fixed_analogue_gain_(0.0)\n> {\n> memset(&awb_, 0, sizeof(awb_));\n> // Setting status_.total_exposure_value_ to zero initially tells us\n> @@ -203,7 +211,7 @@ void Agc::Pause()\n>\n> void Agc::Resume()\n> {\n> - fixed_shutter_ = 0;\n> + fixed_shutter_ = 0s;\n> fixed_analogue_gain_ = 0;\n> }\n>\n> @@ -224,17 +232,17 @@ void Agc::SetEv(double ev)\n>\n> void Agc::SetFlickerPeriod(const Duration &flicker_period)\n> {\n> - flicker_period_ = flicker_period.get();\n> + flicker_period_ = flicker_period;\n> }\n>\n> void Agc::SetMaxShutter(const Duration &max_shutter)\n> {\n> - max_shutter_ = max_shutter.get();\n> + max_shutter_ = max_shutter;\n> }\n>\n> void Agc::SetFixedShutter(const Duration &fixed_shutter)\n> {\n> - fixed_shutter_ = fixed_shutter.get();\n> + fixed_shutter_ = fixed_shutter;\n> // Set this in case someone calls Pause() straight after.\n> status_.shutter_time = clipShutter(fixed_shutter_);\n> }\n> @@ -266,8 +274,8 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode const\n> &camera_mode,\n> {\n> housekeepConfig();\n>\n> - double fixed_shutter = clipShutter(fixed_shutter_);\n> - if (fixed_shutter != 0.0 && fixed_analogue_gain_ != 0.0) {\n> + Duration fixed_shutter = clipShutter(fixed_shutter_);\n> + if (fixed_shutter && fixed_analogue_gain_) {\n> // We're going to reset the algorithm here with these\n> fixed values.\n>\n> fetchAwbStatus(metadata);\n> @@ -312,8 +320,8 @@ void Agc::Prepare(Metadata *image_metadata)\n> // Process has run, so we have meaningful values.\n> DeviceStatus device_status;\n> if (image_metadata->Get(\"device.status\", device_status) ==\n> 0) {\n> - double actual_exposure =\n> device_status.shutter_speed *\n> -\n> device_status.analogue_gain;\n> + Duration actual_exposure =\n> device_status.shutter_speed *\n> +\n> device_status.analogue_gain;\n> if (actual_exposure) {\n> status_.digital_gain =\n> status_.total_exposure_value /\n> @@ -326,7 +334,8 @@ void Agc::Prepare(Metadata *image_metadata)\n> std::min(status_.digital_gain,\n> 4.0));\n> LOG(RPiAgc, Debug) << \"Actual exposure \"\n> << actual_exposure;\n> LOG(RPiAgc, Debug) << \"Use digital_gain \"\n> << status_.digital_gain;\n> - LOG(RPiAgc, Debug) << \"Effective exposure\n> \" << actual_exposure * status_.digital_gain;\n> + LOG(RPiAgc, Debug) << \"Effective exposure \"\n> + <<\n> Duration(actual_exposure * status_.digital_gain);\n>\n\nThis last logging line has been bugging me. The compiler did not seem to\nwant to do an implicit conversion from\nstd::chrono::duration into Duration, so I had to do it manually. Other\nsimilar logging lines in other source files\ndid work fine.\n\nTurns out, I need to add a \"using utils::operator<<\" statement on the top\nof this file to get the compile to recognise\nthe implicit conversion is possible (thanks David). I am not entirely sure\nwhy this is the case, as the\noperator<<(Duration &) on the other log messages do still work without the\nusing directive.\n\nI will fix this up and post an updated patch shortly.\n\nThanks,\nNaush\n\n\n // Decide whether AEC/AGC has converged.\n> updateLockStatus(device_status);\n> }\n> @@ -370,9 +379,9 @@ void Agc::updateLockStatus(DeviceStatus const\n> &device_status)\n> const double RESET_MARGIN = 1.5;\n>\n> // Add 200us to the exposure time error to allow for line\n> quantisation.\n> - double exposure_error = last_device_status_.shutter_speed *\n> ERROR_FACTOR + 200;\n> + Duration exposure_error = last_device_status_.shutter_speed *\n> ERROR_FACTOR + 200us;\n> double gain_error = last_device_status_.analogue_gain *\n> ERROR_FACTOR;\n> - double target_error = last_target_exposure_ * ERROR_FACTOR;\n> + Duration target_error = last_target_exposure_ * ERROR_FACTOR;\n>\n> // Note that we don't know the exposure/gain limits of the sensor,\n> so\n> // the values we keep requesting may be unachievable. For this\n> reason\n> @@ -462,7 +471,7 @@ void Agc::fetchCurrentExposure(Metadata\n> *image_metadata)\n> current_.analogue_gain = device_status->analogue_gain;\n> AgcStatus *agc_status =\n> image_metadata->GetLocked(\"agc.status\");\n> - current_.total_exposure = agc_status ?\n> agc_status->total_exposure_value : 0;\n> + current_.total_exposure = agc_status ?\n> agc_status->total_exposure_value : 0s;\n> current_.total_exposure_no_dg = current_.shutter *\n> current_.analogue_gain;\n> }\n>\n> @@ -573,7 +582,7 @@ void Agc::computeGain(bcm2835_isp_stats *statistics,\n> Metadata *image_metadata,\n>\n> void Agc::computeTargetExposure(double gain)\n> {\n> - if (status_.fixed_shutter != 0.0 && status_.fixed_analogue_gain !=\n> 0.0) {\n> + if (status_.fixed_shutter && status_.fixed_analogue_gain) {\n> // When ag and shutter are both fixed, we need to drive the\n> // total exposure so that we end up with a digital gain of\n> at least\n> // 1/min_colour_gain. Otherwise we'd desaturate channels\n> causing\n> @@ -588,11 +597,11 @@ void Agc::computeTargetExposure(double gain)\n> target_.total_exposure = current_.total_exposure_no_dg *\n> gain;\n> // The final target exposure is also limited to what the\n> exposure\n> // mode allows.\n> - double max_shutter = status_.fixed_shutter != 0.0\n> + Duration max_shutter = status_.fixed_shutter\n> ? status_.fixed_shutter\n> : exposure_mode_->shutter.back();\n> max_shutter = clipShutter(max_shutter);\n> - double max_total_exposure =\n> + Duration max_total_exposure =\n> max_shutter *\n> (status_.fixed_analogue_gain != 0.0\n> ? status_.fixed_analogue_gain\n> @@ -637,7 +646,7 @@ void Agc::filterExposure(bool desaturate)\n> if ((status_.fixed_shutter && status_.fixed_analogue_gain) ||\n> frame_count_ <= config_.startup_frames)\n> speed = 1.0;\n> - if (filtered_.total_exposure == 0.0) {\n> + if (!filtered_.total_exposure) {\n> filtered_.total_exposure = target_.total_exposure;\n> filtered_.total_exposure_no_dg =\n> target_.total_exposure_no_dg;\n> } else {\n> @@ -674,9 +683,10 @@ void Agc::divideUpExposure()\n> // Sending the fixed shutter/gain cases through the same code may\n> seem\n> // unnecessary, but it will make more sense when extend this to\n> cover\n> // variable aperture.\n> - double exposure_value = filtered_.total_exposure_no_dg;\n> - double shutter_time, analogue_gain;\n> - shutter_time = status_.fixed_shutter != 0.0\n> + Duration exposure_value = filtered_.total_exposure_no_dg;\n> + Duration shutter_time;\n> + double analogue_gain;\n> + shutter_time = status_.fixed_shutter\n> ? status_.fixed_shutter\n> : exposure_mode_->shutter[0];\n> shutter_time = clipShutter(shutter_time);\n> @@ -686,8 +696,8 @@ void Agc::divideUpExposure()\n> if (shutter_time * analogue_gain < exposure_value) {\n> for (unsigned int stage = 1;\n> stage < exposure_mode_->gain.size(); stage++) {\n> - if (status_.fixed_shutter == 0.0) {\n> - double stage_shutter =\n> + if (!status_.fixed_shutter) {\n> + Duration stage_shutter =\n>\n> clipShutter(exposure_mode_->shutter[stage]);\n> if (stage_shutter * analogue_gain >=\n> exposure_value) {\n> @@ -713,12 +723,11 @@ void Agc::divideUpExposure()\n> << analogue_gain;\n> // Finally adjust shutter time for flicker avoidance (require both\n> // shutter and gain not to be fixed).\n> - if (status_.fixed_shutter == 0.0 &&\n> - status_.fixed_analogue_gain == 0.0 &&\n> - status_.flicker_period != 0.0) {\n> - int flicker_periods = shutter_time /\n> status_.flicker_period;\n> - if (flicker_periods > 0) {\n> - double new_shutter_time = flicker_periods *\n> status_.flicker_period;\n> + if (!status_.fixed_shutter && !status_.fixed_analogue_gain &&\n> + status_.flicker_period) {\n> + double flicker_periods = shutter_time /\n> status_.flicker_period;\n> + if (flicker_periods > 0.0) {\n> + Duration new_shutter_time = flicker_periods *\n> status_.flicker_period;\n> analogue_gain *= shutter_time / new_shutter_time;\n> // We should still not allow the ag to go over the\n> // largest value in the exposure mode. Note that\n> this\n> @@ -738,7 +747,7 @@ void Agc::divideUpExposure()\n> void Agc::writeAndFinish(Metadata *image_metadata, bool desaturate)\n> {\n> status_.total_exposure_value = filtered_.total_exposure;\n> - status_.target_exposure_value = desaturate ? 0 :\n> target_.total_exposure_no_dg;\n> + status_.target_exposure_value = desaturate ? 0s :\n> target_.total_exposure_no_dg;\n> status_.shutter_time = filtered_.shutter;\n> status_.analogue_gain = filtered_.analogue_gain;\n> // Write to metadata as well, in case anyone wants to update the\n> camera\n> @@ -750,11 +759,12 @@ void Agc::writeAndFinish(Metadata *image_metadata,\n> bool desaturate)\n> << \" analogue gain \" << filtered_.analogue_gain;\n> }\n>\n> -double Agc::clipShutter(double shutter)\n> +Duration Agc::clipShutter(const Duration &shutter)\n> {\n> + Duration clip_shutter = shutter;\n> if (max_shutter_)\n> - shutter = std::min(shutter, max_shutter_);\n> - return shutter;\n> + clip_shutter = std::min(clip_shutter, max_shutter_);\n> + return clip_shutter;\n> }\n>\n> // Register algorithm with the system.\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> index 16a65959d90e..24a6610096c2 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp\n> @@ -9,6 +9,8 @@\n> #include \n> #include \n>\n> +#include \"libcamera/internal/utils.h\"\n> +\n> #include \"../agc_algorithm.hpp\"\n> #include \"../agc_status.h\"\n> #include \"../pwl.hpp\"\n> @@ -22,13 +24,15 @@\n>\n> namespace RPiController {\n>\n> +using namespace std::literals::chrono_literals;\n> +\n> struct AgcMeteringMode {\n> double weights[AGC_STATS_SIZE];\n> void Read(boost::property_tree::ptree const ¶ms);\n> };\n>\n> struct AgcExposureMode {\n> - std::vector shutter;\n> + std::vector shutter;\n> std::vector gain;\n> void Read(boost::property_tree::ptree const ¶ms);\n> };\n> @@ -61,7 +65,7 @@ struct AgcConfig {\n> std::string default_exposure_mode;\n> std::string default_constraint_mode;\n> double base_ev;\n> - double default_exposure_time;\n> + Duration default_exposure_time;\n> double default_analogue_gain;\n> };\n>\n> @@ -101,19 +105,19 @@ private:\n> void filterExposure(bool desaturate);\n> void divideUpExposure();\n> void writeAndFinish(Metadata *image_metadata, bool desaturate);\n> - double clipShutter(double shutter);\n> + Duration clipShutter(const Duration &shutter);\n> AgcMeteringMode *metering_mode_;\n> AgcExposureMode *exposure_mode_;\n> AgcConstraintMode *constraint_mode_;\n> uint64_t frame_count_;\n> AwbStatus awb_;\n> struct ExposureValues {\n> - ExposureValues() : shutter(0), analogue_gain(0),\n> - total_exposure(0),\n> total_exposure_no_dg(0) {}\n> - double shutter;\n> + ExposureValues() : shutter(0s), analogue_gain(0),\n> + total_exposure(0s),\n> total_exposure_no_dg(0s) {}\n> + Duration shutter;\n> double analogue_gain;\n> - double total_exposure;\n> - double total_exposure_no_dg; // without digital gain\n> + Duration total_exposure;\n> + Duration total_exposure_no_dg; // without digital gain\n> };\n> ExposureValues current_; // values for the current frame\n> ExposureValues target_; // calculate the values we want here\n> @@ -121,15 +125,15 @@ private:\n> AgcStatus status_;\n> int lock_count_;\n> DeviceStatus last_device_status_;\n> - double last_target_exposure_;\n> + Duration last_target_exposure_;\n> // Below here the \"settings\" that applications can change.\n> std::string metering_mode_name_;\n> std::string exposure_mode_name_;\n> std::string constraint_mode_name_;\n> double ev_;\n> - double flicker_period_;\n> - double max_shutter_;\n> - double fixed_shutter_;\n> + Duration flicker_period_;\n> + Duration max_shutter_;\n> + Duration fixed_shutter_;\n> double fixed_analogue_gain_;\n> };\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> index f74381cab2b4..0fa6457c1376 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> @@ -16,6 +16,7 @@\n>\n> using namespace RPiController;\n> using namespace libcamera;\n> +using namespace std::literals::chrono_literals;\n>\n> LOG_DEFINE_CATEGORY(RPiLux)\n>\n> @@ -38,7 +39,7 @@ char const *Lux::Name() const\n> void Lux::Read(boost::property_tree::ptree const ¶ms)\n> {\n> reference_shutter_speed_ =\n> - params.get(\"reference_shutter_speed\");\n> + params.get(\"reference_shutter_speed\") * 1.0us;\n> reference_gain_ = params.get(\"reference_gain\");\n> reference_aperture_ = params.get(\"reference_aperture\",\n> 1.0);\n> reference_Y_ = params.get(\"reference_Y\");\n> @@ -60,15 +61,13 @@ void Lux::Prepare(Metadata *image_metadata)\n> void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> {\n> // set some initial values to shut the compiler up\n> - DeviceStatus device_status =\n> - { .shutter_speed = 1.0,\n> - .analogue_gain = 1.0,\n> - .lens_position = 0.0,\n> - .aperture = 0.0,\n> - .flash_intensity = 0.0 };\n> + DeviceStatus device_status = { .shutter_speed = 1.0ms,\n> + .analogue_gain = 1.0,\n> + .lens_position = 0.0,\n> + .aperture = 0.0,\n> + .flash_intensity = 0.0 };\n> if (image_metadata->Get(\"device.status\", device_status) == 0) {\n> double current_gain = device_status.analogue_gain;\n> - double current_shutter_speed = device_status.shutter_speed;\n> double current_aperture = device_status.aperture;\n> if (current_aperture == 0)\n> current_aperture = current_aperture_;\n> @@ -83,7 +82,7 @@ void Lux::Process(StatisticsPtr &stats, Metadata\n> *image_metadata)\n> double current_Y = sum / (double)num + .5;\n> double gain_ratio = reference_gain_ / current_gain;\n> double shutter_speed_ratio =\n> - reference_shutter_speed_ / current_shutter_speed;\n> + reference_shutter_speed_ /\n> device_status.shutter_speed;\n> double aperture_ratio = reference_aperture_ /\n> current_aperture;\n> double Y_ratio = current_Y * (65536 / num_bins) /\n> reference_Y_;\n> double estimated_lux = shutter_speed_ratio * gain_ratio *\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.hpp\n> b/src/ipa/raspberrypi/controller/rpi/lux.hpp\n> index f9090484a136..45c844393e62 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.hpp\n> @@ -8,6 +8,8 @@\n>\n> #include \n>\n> +#include \"libcamera/internal/utils.h\"\n> +\n> #include \"../lux_status.h\"\n> #include \"../algorithm.hpp\"\n>\n> @@ -28,7 +30,7 @@ public:\n> private:\n> // These values define the conditions of the reference image,\n> against\n> // which we compare the new image.\n> - double reference_shutter_speed_; // in micro-seconds\n> + libcamera::utils::Duration reference_shutter_speed_;\n> double reference_gain_;\n> double reference_aperture_; // units of 1/f\n> double reference_Y_; // out of 65536\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index e083f6c254cc..1c77549b618d 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -224,11 +224,11 @@ void IPARPi::start(const ControlList &controls,\n> ipa::RPi::StartConfig *startConf\n>\n> /* SwitchMode may supply updated exposure/gain values to use. */\n> AgcStatus agcStatus;\n> - agcStatus.shutter_time = 0.0;\n> + agcStatus.shutter_time = 0.0s;\n> agcStatus.analogue_gain = 0.0;\n>\n> metadata.Get(\"agc.status\", agcStatus);\n> - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain !=\n> 0.0) {\n> + if (agcStatus.shutter_time && agcStatus.analogue_gain) {\n> ControlList ctrls(sensorCtrls_);\n> applyAGC(&agcStatus, ctrls);\n> startConfig->controls = std::move(ctrls);\n> @@ -391,7 +391,7 @@ int IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> /* Supply initial values for gain and exposure. */\n> ControlList ctrls(sensorCtrls_);\n> AgcStatus agcStatus;\n> - agcStatus.shutter_time =\n> DefaultExposureTime.get();\n> + agcStatus.shutter_time = DefaultExposureTime;\n> agcStatus.analogue_gain = DefaultAnalogueGain;\n> applyAGC(&agcStatus, ctrls);\n>\n> @@ -463,7 +463,8 @@ void IPARPi::reportMetadata()\n> */\n> DeviceStatus *deviceStatus =\n> rpiMetadata_.GetLocked(\"device.status\");\n> if (deviceStatus) {\n> - libcameraMetadata_.set(controls::ExposureTime,\n> deviceStatus->shutter_speed);\n> + libcameraMetadata_.set(controls::ExposureTime,\n> +\n> deviceStatus->shutter_speed.get());\n> libcameraMetadata_.set(controls::AnalogueGain,\n> deviceStatus->analogue_gain);\n> }\n>\n> @@ -1016,7 +1017,7 @@ void IPARPi::fillDeviceStatus(const ControlList\n> &sensorControls)\n> int32_t exposureLines =\n> sensorControls.get(V4L2_CID_EXPOSURE).get();\n> int32_t gainCode =\n> sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n>\n> - deviceStatus.shutter_speed =\n> helper_->Exposure(exposureLines).get();\n> + deviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n> deviceStatus.analogue_gain = helper_->Gain(gainCode);\n>\n> LOG(IPARPI, Debug) << \"Metadata - Exposure : \"\n> @@ -1102,7 +1103,7 @@ void IPARPi::applyAGC(const struct AgcStatus\n> *agcStatus, ControlList &ctrls)\n> int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n>\n> /* GetVBlanking might clip exposure time to the fps limits. */\n> - Duration exposure = agcStatus->shutter_time * 1.0us;\n> + Duration exposure = agcStatus->shutter_time;\n> int32_t vblanking = helper_->GetVBlanking(exposure,\n> minFrameDuration_, maxFrameDuration_);\n> int32_t exposureLines = helper_->ExposureLines(exposure);\n>\n> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp\n> index 3131aa2d6666..5fa6f8366f3d 100644\n> --- a/src/libcamera/utils.cpp\n> +++ b/src/libcamera/utils.cpp\n> @@ -506,9 +506,14 @@ std::string libcameraSourcePath()\n> * loop, iterates over an indexed view of the \\a iterable\n> */\n>\n> +/**\n> + * \\typedef BaseDuration\n> + * \\brief Base struct for the \\a Duration helper class.\n> + */\n> +\n> /**\n> * \\class Duration\n> - * \\brief Helper for std::chrono::duration types. Derived from \\a\n> BaseDuration\n> + * \\brief Helper class for std::chrono::duration types.\n> */\n>\n> /**\n> @@ -517,7 +522,7 @@ std::string libcameraSourcePath()\n> * \\param[in] d The std::chrono::duration object to convert from.\n> *\n> * Constructs a \\a Duration object from a \\a std::chrono::duration object,\n> - * converting the representation to \\a BaseDuration type.\n> + * internally converting the representation to \\a BaseDuration type.\n> */\n>\n> /**\n> --\n> 2.25.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A8100C31FF\n\tfor ;\n\tFri, 21 May 2021 08:01:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D75EC6891D;\n\tFri, 21 May 2021 10:01:18 +0200 (CEST)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD7A768918\n\tfor ;\n\tFri, 21 May 2021 10:01:16 +0200 (CEST)","by mail-lf1-x12f.google.com with SMTP id z13so28383757lft.1\n\tfor ;\n\tFri, 21 May 2021 01:01:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"fK2z2UzP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=JXaBZNheQ3Kdcagi02nF6r64d818Cy3mzgbi5X8rulA=;\n\tb=fK2z2UzPNr6d16opvibVYuuYJRHR79IGAjMgBGG5MWjP4RHg3mVq5x7QO3hQLp1XbQ\n\tJYBkK28880WIuwiKIKKvYdjpPY5aHyAD+pJjY3Yb8aj+Na5dMF/uX2Xqen8H/OVIiBIl\n\tcPY9UGmlbK3E2A+bNzBcJPJq51SHsKW7tuSERBuMVBhTqmgI2tcSkiYpKj4YppRLMCi+\n\tVsTXxFGCeB9yF/lqfe4uThoMtgDZ6tRHe77/mIrsFI54R6UhMfsMB2F0SPvc0B7ESvlL\n\tNhsmD/1ngIh39UjUdNPv7qy8TWxkHSrB58re/dPIuhZH3vDfmWBIwjpG6VJHIPv9DT1j\n\tj33Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=JXaBZNheQ3Kdcagi02nF6r64d818Cy3mzgbi5X8rulA=;\n\tb=UhRVjipZKsUVlB6on0PQDOSi9MA4sMmZMwF8WTGH8C5TmCTtRD7dm3rig8HYY97tyu\n\tt8eAl1b7k8n86CKXCcPlWea1iR6+dNQQ3o3GlhHItZUXPRH13HZkaX9VFFDo42Wk3EJQ\n\tnW40ytEOutgP5Ih47xI1xmT4Q9rA7XvLVNl1PQFihJTqGVycXsZuHXnlyE2JPSwwMR2n\n\tuwPyAwaDq34A2jDNwXPc2h/XkdZU55jD7BH/90thR7a+07PLvIzNpz2hI+Sx2yvteoW6\n\t94G2Isv782yIqWhDcJdxubnU8179oZZz8omAgIHJc9U1SgSn1Frixf2FsJEI8FyURGB0\n\tmOJg==","X-Gm-Message-State":"AOAM532VBaZLrxk2K97o/G3qd+9Xye043gBakGVgz6kzDROth0H5l+TJ\n\t8aHFIJos7O7sWsa3dRJAoe3KaG9lPUGr9iWlOzVXeFG5AZw=","X-Google-Smtp-Source":"ABdhPJwNLgkcB3kSiu/RIZLX6+u87FjhXscca8qryoqqZMgkTK/fCvXZpMEasyVlkJvKN/Wig7V/GZbAHmsUndm6+RY=","X-Received":"by 2002:a05:6512:3f87:: with SMTP id\n\tx7mr1359845lfa.617.1621584075366; \n\tFri, 21 May 2021 01:01:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20210520151714.3366445-1-naush@raspberrypi.com>\n\t<20210520151714.3366445-5-naush@raspberrypi.com>","In-Reply-To":"<20210520151714.3366445-5-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Fri, 21 May 2021 09:00:59 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"multipart/alternative; boundary=\"000000000000c9358e05c2d27664\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] ipa: raspberrypi: Switch the\n\tAGC/Lux code to use utils::Duration","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]