[libcamera-devel] [PATCH 10/15] DNI: ipa: raspberrypi: Code refactoring to match style guidelines
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 25 22:58:25 CEST 2022
Hi Naush,
Thank you for the patch.
On Mon, Jul 25, 2022 at 02:46:34PM +0100, Naushir Patuck via libcamera-devel wrote:
> Refactor the source files src/ipa/raspberrypi/controller/rps/[n|s]* 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/noise.cpp | 34 ++++++++---------
> src/ipa/raspberrypi/controller/rpi/noise.hpp | 14 +++----
> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 32 ++++++++--------
> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 10 ++---
> .../raspberrypi/controller/rpi/sharpen.cpp | 38 +++++++++----------
> .../raspberrypi/controller/rpi/sharpen.hpp | 14 +++----
> 6 files changed, 72 insertions(+), 70 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> index 63cad639f313..d6e4df4192f2 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> @@ -22,45 +22,45 @@ LOG_DEFINE_CATEGORY(RPiNoise)
> #define NAME "rpi.noise"
>
> Noise::Noise(Controller *controller)
> - : Algorithm(controller), mode_factor_(1.0)
> + : Algorithm(controller), modeFactor_(1.0)
> {
> }
>
> -char const *Noise::Name() const
> +char const *Noise::name() const
> {
> return NAME;
> }
>
> -void Noise::SwitchMode(CameraMode const &camera_mode,
> +void Noise::switchMode(CameraMode const &cameraMode,
> [[maybe_unused]] Metadata *metadata)
> {
> // For example, we would expect a 2x2 binned mode to have a "noise
> // factor" of sqrt(2x2) = 2. (can't be less than one, right?)
> - mode_factor_ = std::max(1.0, camera_mode.noise_factor);
> + modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
> }
>
> -void Noise::Read(boost::property_tree::ptree const ¶ms)
> +void Noise::read(boost::property_tree::ptree const ¶ms)
> {
> - reference_constant_ = params.get<double>("reference_constant");
> - reference_slope_ = params.get<double>("reference_slope");
> + referenceConstant_ = params.get<double>("reference_constant");
> + referenceSlope_ = params.get<double>("reference_slope");
> }
>
> -void Noise::Prepare(Metadata *image_metadata)
> +void Noise::prepare(Metadata *imageMetadata)
> {
> - struct DeviceStatus device_status;
> - device_status.analogue_gain = 1.0; // keep compiler calm
> - if (image_metadata->Get("device.status", device_status) == 0) {
> + struct DeviceStatus deviceStatus;
> + deviceStatus.analogueGain = 1.0; // keep compiler calm
> + if (imageMetadata->get("device.status", deviceStatus) == 0) {
> // There is a slight question as to exactly how the noise
> // profile, specifically the constant part of it, scales. For
> // now we assume it all scales the same, and we'll revisit this
> // if it proves substantially wrong. NOTE: we may also want to
> // make some adjustments based on the camera mode (such as
> // binning), if we knew how to discover it...
> - double factor = sqrt(device_status.analogue_gain) / mode_factor_;
> + double factor = sqrt(deviceStatus.analogueGain) / modeFactor_;
> struct NoiseStatus status;
> - status.noise_constant = reference_constant_ * factor;
> - status.noise_slope = reference_slope_ * factor;
> - image_metadata->Set("noise.status", status);
> + status.noise_constant = referenceConstant_ * factor;
> + status.noise_slope = referenceSlope_ * factor;
> + imageMetadata->set("noise.status", status);
> LOG(RPiNoise, Debug)
> << "constant " << status.noise_constant
> << " slope " << status.noise_slope;
> @@ -69,8 +69,8 @@ void Noise::Prepare(Metadata *image_metadata)
> }
>
> // Register algorithm with the system.
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return new Noise(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> index 1c9de5c87d08..ed6ffe910e27 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> @@ -17,16 +17,16 @@ class Noise : public Algorithm
> {
> public:
> Noise(Controller *controller);
> - char const *Name() const override;
> - void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> - void Read(boost::property_tree::ptree const ¶ms) override;
> - void Prepare(Metadata *image_metadata) override;
> + char const *name() const override;
> + void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> + void read(boost::property_tree::ptree const ¶ms) override;
> + void prepare(Metadata *imageMetadata) override;
>
> private:
> // the noise profile for analogue gain of 1.0
> - double reference_constant_;
> - double reference_slope_;
> - double mode_factor_;
> + double referenceConstant_;
> + double referenceSlope_;
> + double modeFactor_;
> };
>
> } // namespace RPiController
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> index 9384550983e7..8707b6d9cd9e 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -27,49 +27,51 @@ Sdn::Sdn(Controller *controller)
> {
> }
>
> -char const *Sdn::Name() const
> +char const *Sdn::name() const
> {
> return NAME;
> }
>
> -void Sdn::Read(boost::property_tree::ptree const ¶ms)
> +void Sdn::read(boost::property_tree::ptree const ¶ms)
> {
> deviation_ = params.get<double>("deviation", 3.2);
> strength_ = params.get<double>("strength", 0.75);
> }
>
> -void Sdn::Initialise() {}
> +void Sdn::initialise()
> +{
> +}
>
> -void Sdn::Prepare(Metadata *image_metadata)
> +void Sdn::prepare(Metadata *imageMetadata)
> {
> - struct NoiseStatus noise_status = {};
> - noise_status.noise_slope = 3.0; // in case no metadata
> - if (image_metadata->Get("noise.status", noise_status) != 0)
> + struct NoiseStatus noiseStatus = {};
> + noiseStatus.noise_slope = 3.0; // in case no metadata
> + if (imageMetadata->get("noise.status", noiseStatus) != 0)
> LOG(RPiSdn, Warning) << "no noise profile found";
> LOG(RPiSdn, Debug)
> - << "Noise profile: constant " << noise_status.noise_constant
> - << " slope " << noise_status.noise_slope;
> + << "Noise profile: constant " << noiseStatus.noise_constant
> + << " slope " << noiseStatus.noise_slope;
> struct DenoiseStatus status;
> - status.noise_constant = noise_status.noise_constant * deviation_;
> - status.noise_slope = noise_status.noise_slope * deviation_;
> + status.noise_constant = noiseStatus.noise_constant * deviation_;
> + status.noise_slope = noiseStatus.noise_slope * deviation_;
> status.strength = strength_;
> status.mode = static_cast<std::underlying_type_t<DenoiseMode>>(mode_);
> - image_metadata->Set("denoise.status", status);
> + imageMetadata->set("denoise.status", status);
> LOG(RPiSdn, Debug)
> << "programmed constant " << status.noise_constant
> << " slope " << status.noise_slope
> << " strength " << status.strength;
> }
>
> -void Sdn::SetMode(DenoiseMode mode)
> +void Sdn::setMode(DenoiseMode mode)
> {
> // We only distinguish between off and all other modes.
> mode_ = mode;
> }
>
> // Register algorithm with the system.
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return (Algorithm *)new Sdn(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.hpp b/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> index 2371ce04163f..d9b18f296635 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.hpp
> @@ -17,11 +17,11 @@ class Sdn : public DenoiseAlgorithm
> {
> public:
> Sdn(Controller *controller = NULL);
> - char const *Name() const override;
> - void Read(boost::property_tree::ptree const ¶ms) override;
> - void Initialise() override;
> - void Prepare(Metadata *image_metadata) override;
> - void SetMode(DenoiseMode mode) override;
> + char const *name() const override;
> + void read(boost::property_tree::ptree const ¶ms) override;
> + void initialise() override;
> + void prepare(Metadata *imageMetadata) override;
> + void setMode(DenoiseMode mode) override;
>
> private:
> double deviation_;
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> index 18825a43867b..775ed0fd2c46 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> @@ -21,23 +21,23 @@ LOG_DEFINE_CATEGORY(RPiSharpen)
> #define NAME "rpi.sharpen"
>
> Sharpen::Sharpen(Controller *controller)
> - : SharpenAlgorithm(controller), user_strength_(1.0)
> + : SharpenAlgorithm(controller), userStrength_(1.0)
> {
> }
>
> -char const *Sharpen::Name() const
> +char const *Sharpen::name() const
> {
> return NAME;
> }
>
> -void Sharpen::SwitchMode(CameraMode const &camera_mode,
> +void Sharpen::switchMode(CameraMode const &cameraMode,
> [[maybe_unused]] Metadata *metadata)
> {
> // can't be less than one, right?
> - mode_factor_ = std::max(1.0, camera_mode.noise_factor);
> + modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
> }
>
> -void Sharpen::Read(boost::property_tree::ptree const ¶ms)
> +void Sharpen::read(boost::property_tree::ptree const ¶ms)
> {
> threshold_ = params.get<double>("threshold", 1.0);
> strength_ = params.get<double>("strength", 1.0);
> @@ -48,38 +48,38 @@ void Sharpen::Read(boost::property_tree::ptree const ¶ms)
> << " limit " << limit_;
> }
>
> -void Sharpen::SetStrength(double strength)
> +void Sharpen::setStrength(double strength)
> {
> // Note that this function is how an application sets the overall
> // sharpening "strength". We call this the "user strength" field
> // as there already is a strength_ field - being an internal gain
> // parameter that gets passed to the ISP control code. Negative
> // values are not allowed - coerce them to zero (no sharpening).
> - user_strength_ = std::max(0.0, strength);
> + userStrength_ = std::max(0.0, strength);
> }
>
> -void Sharpen::Prepare(Metadata *image_metadata)
> +void Sharpen::prepare(Metadata *imageMetadata)
> {
> // The user_strength_ affects the algorithm's internal gain directly, but
s/user_strength_/userStrength_/
> // we adjust the limit and threshold less aggressively. Using a sqrt
> // function is an arbitrary but gentle way of accomplishing this.
> - double user_strength_sqrt = sqrt(user_strength_);
> + double userStrengthSqrt = sqrt(userStrength_);
> struct SharpenStatus status;
> // Binned modes seem to need the sharpening toned down with this
> // pipeline, thus we use the mode_factor here. Also avoid
s/mode_factor/modeFactor/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> - // divide-by-zero with the user_strength_sqrt.
> - status.threshold = threshold_ * mode_factor_ /
> - std::max(0.01, user_strength_sqrt);
> - status.strength = strength_ / mode_factor_ * user_strength_;
> - status.limit = limit_ / mode_factor_ * user_strength_sqrt;
> - // Finally, report any application-supplied parameters that were used.
> - status.user_strength = user_strength_;
> - image_metadata->Set("sharpen.status", status);
> + // divide-by-zero with the userStrengthSqrt.
> + status.threshold = threshold_ * modeFactor_ /
> + std::max(0.01, userStrengthSqrt);
> + status.strength = strength_ / modeFactor_ * userStrength_;
> + status.limit = limit_ / modeFactor_ * userStrengthSqrt;
> + /* Finally, report any application-supplied parameters that were used. */
> + status.userStrength = userStrength_;
> + imageMetadata->set("sharpen.status", status);
> }
>
> // Register algorithm with the system.
> -static Algorithm *Create(Controller *controller)
> +static Algorithm *create(Controller *controller)
> {
> return new Sharpen(controller);
> }
> -static RegisterAlgorithm reg(NAME, &Create);
> +static RegisterAlgorithm reg(NAME, &create);
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> index 13a076a86895..ced917f3c42b 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> @@ -17,18 +17,18 @@ class Sharpen : public SharpenAlgorithm
> {
> public:
> Sharpen(Controller *controller);
> - char const *Name() const override;
> - void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> - void Read(boost::property_tree::ptree const ¶ms) override;
> - void SetStrength(double strength) override;
> - void Prepare(Metadata *image_metadata) override;
> + char const *name() const override;
> + void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> + void read(boost::property_tree::ptree const ¶ms) override;
> + void setStrength(double strength) override;
> + void prepare(Metadata *imageMetadata) override;
>
> private:
> double threshold_;
> double strength_;
> double limit_;
> - double mode_factor_;
> - double user_strength_;
> + double modeFactor_;
> + double userStrength_;
> };
>
> } // namespace RPiController
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list