[libcamera-devel] [PATCH 09/15] DNI: ipa: raspberrypi: Code refactoring to match style guidelines
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jul 26 00:40:17 CEST 2022
Hi Naush,
One comment actually.
On Mon, Jul 25, 2022 at 02:46:33PM +0100, Naushir Patuck via libcamera-devel wrote:
> Refactor the source files src/ipa/raspberrypi/controller/rps/[f|g|l]* to match the
> recommended formatting guidelines for the libcamera project. The vast majority
> of changes in this commit comprise of switching from snake_case to CamelCase,
> and starting class member functions with a lower case character.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/ipa/raspberrypi/controller/rpi/focus.cpp | 14 ++--
> src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +-
> src/ipa/raspberrypi/controller/rpi/geq.cpp | 48 +++++++-------
> src/ipa/raspberrypi/controller/rpi/geq.hpp | 6 +-
> src/ipa/raspberrypi/controller/rpi/lux.cpp | 70 ++++++++++----------
> src/ipa/raspberrypi/controller/rpi/lux.hpp | 22 +++---
> 6 files changed, 81 insertions(+), 83 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> index a87ec802b964..90f36e58f28c 100644
> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp
> @@ -23,28 +23,28 @@ Focus::Focus(Controller *controller)
> {
> }
>
> -char const *Focus::Name() const
> +char const *Focus::name() const
> {
> return NAME;
> }
>
> -void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)
> +void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)
> {
> FocusStatus status;
> unsigned int i;
> for (i = 0; i < FOCUS_REGIONS; i++)
> - status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;
> + status.focusMeasures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;
> status.num = i;
> - image_metadata->Set("focus.status", status);
> + imageMetadata->set("focus.status", status);
>
> LOG(RPiFocus, Debug)
> << "Focus contrast measure: "
> - << (status.focus_measures[5] + status.focus_measures[6]) / 10;
> + << (status.focusMeasures[5] + status.focusMeasures[6]) / 10;
> }
>
> /* Register algorithm with the system. */
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return new Focus(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp
> index 131b1d0f2fbf..a9207eb3cc23 100644
> --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp
> @@ -21,8 +21,8 @@ class Focus : public Algorithm
> {
> public:
> Focus(Controller *controller);
> - char const *Name() const override;
> - void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
> + char const *name() const override;
> + void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> };
>
> } /* namespace RPiController */
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> index 4530cb75792c..0da5efdf3d3d 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> @@ -28,54 +28,52 @@ Geq::Geq(Controller *controller)
> {
> }
>
> -char const *Geq::Name() const
> +char const *Geq::name() const
> {
> return NAME;
> }
>
> -void Geq::Read(boost::property_tree::ptree const ¶ms)
> +void Geq::read(boost::property_tree::ptree const ¶ms)
> {
> config_.offset = params.get<uint16_t>("offset", 0);
> config_.slope = params.get<double>("slope", 0.0);
> if (config_.slope < 0.0 || config_.slope >= 1.0)
> throw std::runtime_error("Geq: bad slope value");
> if (params.get_child_optional("strength"))
> - config_.strength.Read(params.get_child("strength"));
> + config_.strength.read(params.get_child("strength"));
> }
>
> -void Geq::Prepare(Metadata *image_metadata)
> +void Geq::prepare(Metadata *imageMetadata)
> {
> - LuxStatus lux_status = {};
> - lux_status.lux = 400;
> - if (image_metadata->Get("lux.status", lux_status))
> + LuxStatus luxStatus = {};
> + luxStatus.lux = 400;
> + if (imageMetadata->get("lux.status", luxStatus))
> LOG(RPiGeq, Warning) << "no lux data found";
> - DeviceStatus device_status;
> - device_status.analogue_gain = 1.0; // in case not found
> - if (image_metadata->Get("device.status", device_status))
> + DeviceStatus deviceStatus;
> + deviceStatus.analogueGain = 1.0; // in case not found
> + if (imageMetadata->get("device.status", deviceStatus))
> LOG(RPiGeq, Warning)
> << "no device metadata - use analogue gain of 1x";
> - GeqStatus geq_status = {};
> - double strength =
> - config_.strength.Empty()
> + GeqStatus geqStatus = {};
> + double strength = config_.strength.empty()
> ? 1.0
> - : config_.strength.Eval(config_.strength.Domain().Clip(
> - lux_status.lux));
> - strength *= device_status.analogue_gain;
> + : config_.strength.eval(config_.strength.domain().clip(luxStatus.lux));
> + strength *= deviceStatus.analogueGain;
> double offset = config_.offset * strength;
> double slope = config_.slope * strength;
> - geq_status.offset = std::min(65535.0, std::max(0.0, offset));
> - geq_status.slope = std::min(.99999, std::max(0.0, slope));
> + geqStatus.offset = std::min(65535.0, std::max(0.0, offset));
> + geqStatus.slope = std::min(.99999, std::max(0.0, slope));
> LOG(RPiGeq, Debug)
> - << "offset " << geq_status.offset << " slope "
> - << geq_status.slope << " (analogue gain "
> - << device_status.analogue_gain << " lux "
> - << lux_status.lux << ")";
> - image_metadata->Set("geq.status", geq_status);
> + << "offset " << geqStatus.offset << " slope "
> + << geqStatus.slope << " (analogue gain "
> + << deviceStatus.analogueGain << " lux "
> + << luxStatus.lux << ")";
> + imageMetadata->set("geq.status", geqStatus);
> }
>
> // Register algorithm with the system.
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return (Algorithm *)new Geq(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.hpp b/src/ipa/raspberrypi/controller/rpi/geq.hpp
> index 8ba3046b2a2b..bdbc55b2e2d9 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.hpp
> @@ -23,9 +23,9 @@ class Geq : public Algorithm
> {
> public:
> Geq(Controller *controller);
> - char const *Name() const override;
> - void Read(boost::property_tree::ptree const ¶ms) override;
> - void Prepare(Metadata *image_metadata) override;
> + char const *name() const override;
> + void read(boost::property_tree::ptree const ¶ms) override;
> + void prepare(Metadata *imageMetadata) override;
>
> private:
> GeqConfig config_;
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> index 4d145b6ff0e9..10654fbba94a 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> @@ -31,74 +31,74 @@ Lux::Lux(Controller *controller)
> status_.lux = 400;
> }
>
> -char const *Lux::Name() const
> +char const *Lux::name() const
> {
> return NAME;
> }
>
> -void Lux::Read(boost::property_tree::ptree const ¶ms)
> +void Lux::read(boost::property_tree::ptree const ¶ms)
> {
> - reference_shutter_speed_ =
> + referenceshutterSpeed_ =
> 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");
> - reference_lux_ = params.get<double>("reference_lux");
> - current_aperture_ = reference_aperture_;
> + referenceGain_ = params.get<double>("reference_gain");
> + referenceAperture_ = params.get<double>("reference_aperture", 1.0);
> + referenceY_ = params.get<double>("reference_Y");
> + referenceLux_ = params.get<double>("reference_lux");
> + currentAperture_ = referenceAperture_;
> }
>
> -void Lux::SetCurrentAperture(double aperture)
> +void Lux::setCurrentAperture(double aperture)
> {
> - current_aperture_ = aperture;
> + currentAperture_ = aperture;
> }
>
> -void Lux::Prepare(Metadata *image_metadata)
> +void Lux::prepare(Metadata *imageMetadata)
> {
> std::unique_lock<std::mutex> lock(mutex_);
> - image_metadata->Set("lux.status", status_);
> + imageMetadata->set("lux.status", status_);
> }
>
> -void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)
> +void Lux::process(StatisticsPtr &stats, Metadata *imageMetadata)
> {
> - DeviceStatus device_status;
> - if (image_metadata->Get("device.status", device_status) == 0) {
> - double current_gain = device_status.analogue_gain;
> - double current_aperture = device_status.aperture.value_or(current_aperture_);
> + DeviceStatus deviceStatus;
> + if (imageMetadata->get("device.status", deviceStatus) == 0) {
> + double currentGain = deviceStatus.analogueGain;
> + double currentAperture = deviceStatus.aperture.value_or(currentAperture_);
> uint64_t sum = 0;
> uint32_t num = 0;
> uint32_t *bin = stats->hist[0].g_hist;
> - const int num_bins = sizeof(stats->hist[0].g_hist) /
> - sizeof(stats->hist[0].g_hist[0]);
> - for (int i = 0; i < num_bins; i++)
> + const int numBins = sizeof(stats->hist[0].g_hist) /
> + sizeof(stats->hist[0].g_hist[0]);
> + for (int i = 0; i < numBins; i++)
> sum += bin[i] * (uint64_t)i, num += bin[i];
> // add .5 to reflect the mid-points of bins
> - double current_Y = sum / (double)num + .5;
> - double gain_ratio = reference_gain_ / current_gain;
> - double shutter_speed_ratio =
> - 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 *
> - aperture_ratio * aperture_ratio *
> - Y_ratio * reference_lux_;
> + double currentY = sum / (double)num + .5;
> + double gainRatio = referenceGain_ / currentGain;
> + double shutterSpeedRatio =
> + referenceshutterSpeed_ / deviceStatus.shutterSpeed;
> + double apertureRatio = referenceAperture_ / currentAperture;
> + double yRatio = currentY * (65536 / numBins) / referenceY_;
> + double estimatedLux = shutterSpeedRatio * gainRatio *
> + apertureRatio * apertureRatio *
> + yRatio * referenceLux_;
> LuxStatus status;
> - status.lux = estimated_lux;
> - status.aperture = current_aperture;
> - LOG(RPiLux, Debug) << ": estimated lux " << estimated_lux;
> + status.lux = estimatedLux;
> + status.aperture = currentAperture;
> + LOG(RPiLux, Debug) << ": estimated lux " << estimatedLux;
> {
> std::unique_lock<std::mutex> lock(mutex_);
> status_ = status;
> }
> // Overwrite the metadata here as well, so that downstream
> // algorithms get the latest value.
> - image_metadata->Set("lux.status", status);
> + imageMetadata->set("lux.status", status);
> } else
> LOG(RPiLux, Warning) << ": no device metadata";
> }
>
> // Register algorithm with the system.
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return (Algorithm *)new Lux(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.hpp b/src/ipa/raspberrypi/controller/rpi/lux.hpp
> index 3ebd35d1e382..98cfd0ac8bd0 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.hpp
> @@ -21,21 +21,21 @@ class Lux : public Algorithm
> {
> public:
> Lux(Controller *controller);
> - char const *Name() const override;
> - void Read(boost::property_tree::ptree const ¶ms) override;
> - void Prepare(Metadata *image_metadata) override;
> - void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
> - void SetCurrentAperture(double aperture);
> + char const *name() const override;
> + void read(boost::property_tree::ptree const ¶ms) override;
> + void prepare(Metadata *imageMetadata) override;
> + void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> + void setCurrentAperture(double aperture);
>
> private:
> // These values define the conditions of the reference image, against
> // which we compare the new image.
> - libcamera::utils::Duration reference_shutter_speed_;
> - double reference_gain_;
> - double reference_aperture_; // units of 1/f
> - double reference_Y_; // out of 65536
> - double reference_lux_;
> - double current_aperture_;
> + libcamera::utils::Duration referenceshutterSpeed_;
This should be referenceShutterSpeed_.
> + double referenceGain_;
> + double referenceAperture_; // units of 1/f
> + double referenceY_; // out of 65536
> + double referenceLux_;
> + double currentAperture_;
> LuxStatus status_;
> std::mutex mutex_;
> };
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list