[{"id":24127,"web_url":"https://patchwork.libcamera.org/comment/24127/","msgid":"","date":"2022-07-25T20:58:25","subject":"Re: [libcamera-devel] [PATCH 10/15] DNI: ipa: raspberrypi: Code\n\trefactoring to match style guidelines","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Jul 25, 2022 at 02:46:34PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Refactor the source files src/ipa/raspberrypi/controller/rps/[n|s]* to match the\n> recommended formatting guidelines for the libcamera project. The vast majority\n> of changes in this commit comprise of switching from snake_case to CamelCase,\n> and starting class member functions with a lower case character.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/rpi/noise.cpp | 34 ++++++++---------\n> src/ipa/raspberrypi/controller/rpi/noise.hpp | 14 +++----\n> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 32 ++++++++--------\n> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 10 ++---\n> .../raspberrypi/controller/rpi/sharpen.cpp | 38 +++++++++----------\n> .../raspberrypi/controller/rpi/sharpen.hpp | 14 +++----\n> 6 files changed, 72 insertions(+), 70 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> index 63cad639f313..d6e4df4192f2 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> @@ -22,45 +22,45 @@ LOG_DEFINE_CATEGORY(RPiNoise)\n> #define NAME \"rpi.noise\"\n> \n> Noise::Noise(Controller *controller)\n> -\t: Algorithm(controller), mode_factor_(1.0)\n> +\t: Algorithm(controller), modeFactor_(1.0)\n> {\n> }\n> \n> -char const *Noise::Name() const\n> +char const *Noise::name() const\n> {\n> \treturn NAME;\n> }\n> \n> -void Noise::SwitchMode(CameraMode const &camera_mode,\n> +void Noise::switchMode(CameraMode const &cameraMode,\n> \t\t [[maybe_unused]] Metadata *metadata)\n> {\n> \t// For example, we would expect a 2x2 binned mode to have a \"noise\n> \t// factor\" of sqrt(2x2) = 2. (can't be less than one, right?)\n> -\tmode_factor_ = std::max(1.0, camera_mode.noise_factor);\n> +\tmodeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n> }\n> \n> -void Noise::Read(boost::property_tree::ptree const ¶ms)\n> +void Noise::read(boost::property_tree::ptree const ¶ms)\n> {\n> -\treference_constant_ = params.get(\"reference_constant\");\n> -\treference_slope_ = params.get(\"reference_slope\");\n> +\treferenceConstant_ = params.get(\"reference_constant\");\n> +\treferenceSlope_ = params.get(\"reference_slope\");\n> }\n> \n> -void Noise::Prepare(Metadata *image_metadata)\n> +void Noise::prepare(Metadata *imageMetadata)\n> {\n> -\tstruct DeviceStatus device_status;\n> -\tdevice_status.analogue_gain = 1.0; // keep compiler calm\n> -\tif (image_metadata->Get(\"device.status\", device_status) == 0) {\n> +\tstruct DeviceStatus deviceStatus;\n> +\tdeviceStatus.analogueGain = 1.0; // keep compiler calm\n> +\tif (imageMetadata->get(\"device.status\", deviceStatus) == 0) {\n> \t\t// There is a slight question as to exactly how the noise\n> \t\t// profile, specifically the constant part of it, scales. For\n> \t\t// now we assume it all scales the same, and we'll revisit this\n> \t\t// if it proves substantially wrong. NOTE: we may also want to\n> \t\t// make some adjustments based on the camera mode (such as\n> \t\t// binning), if we knew how to discover it...\n> -\t\tdouble factor = sqrt(device_status.analogue_gain) / mode_factor_;\n> +\t\tdouble factor = sqrt(deviceStatus.analogueGain) / modeFactor_;\n> \t\tstruct NoiseStatus status;\n> -\t\tstatus.noise_constant = reference_constant_ * factor;\n> -\t\tstatus.noise_slope = reference_slope_ * factor;\n> -\t\timage_metadata->Set(\"noise.status\", status);\n> +\t\tstatus.noise_constant = referenceConstant_ * factor;\n> +\t\tstatus.noise_slope = referenceSlope_ * factor;\n> +\t\timageMetadata->set(\"noise.status\", status);\n> \t\tLOG(RPiNoise, Debug)\n> \t\t\t<< \"constant \" << status.noise_constant\n> \t\t\t<< \" slope \" << status.noise_slope;\n> @@ -69,8 +69,8 @@ void Noise::Prepare(Metadata *image_metadata)\n> }\n> \n> // Register algorithm with the system.\n> -static Algorithm *Create(Controller *controller)\n> +static Algorithm *create(Controller *controller)\n> {\n> \treturn new Noise(controller);\n> }\n> -static RegisterAlgorithm reg(NAME, &Create);\n> +static RegisterAlgorithm reg(NAME, &create);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> index 1c9de5c87d08..ed6ffe910e27 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp\n> @@ -17,16 +17,16 @@ class Noise : public Algorithm\n> {\n> public:\n> \tNoise(Controller *controller);\n> -\tchar const *Name() const override;\n> -\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> -\tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> -\tvoid Prepare(Metadata *image_metadata) override;\n> +\tchar const *name() const override;\n> +\tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata) override;\n> +\tvoid read(boost::property_tree::ptree const ¶ms) override;\n> +\tvoid prepare(Metadata *imageMetadata) override;\n> \n> private:\n> \t// the noise profile for analogue gain of 1.0\n> -\tdouble reference_constant_;\n> -\tdouble reference_slope_;\n> -\tdouble mode_factor_;\n> +\tdouble referenceConstant_;\n> +\tdouble referenceSlope_;\n> +\tdouble modeFactor_;\n> };\n> \n> } // namespace RPiController\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> index 9384550983e7..8707b6d9cd9e 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> @@ -27,49 +27,51 @@ Sdn::Sdn(Controller *controller)\n> {\n> }\n> \n> -char const *Sdn::Name() const\n> +char const *Sdn::name() const\n> {\n> \treturn NAME;\n> }\n> \n> -void Sdn::Read(boost::property_tree::ptree const ¶ms)\n> +void Sdn::read(boost::property_tree::ptree const ¶ms)\n> {\n> \tdeviation_ = params.get(\"deviation\", 3.2);\n> \tstrength_ = params.get(\"strength\", 0.75);\n> }\n> \n> -void Sdn::Initialise() {}\n> +void Sdn::initialise()\n> +{\n> +}\n> \n> -void Sdn::Prepare(Metadata *image_metadata)\n> +void Sdn::prepare(Metadata *imageMetadata)\n> {\n> -\tstruct NoiseStatus noise_status = {};\n> -\tnoise_status.noise_slope = 3.0; // in case no metadata\n> -\tif (image_metadata->Get(\"noise.status\", noise_status) != 0)\n> +\tstruct NoiseStatus noiseStatus = {};\n> +\tnoiseStatus.noise_slope = 3.0; // in case no metadata\n> +\tif (imageMetadata->get(\"noise.status\", noiseStatus) != 0)\n> \t\tLOG(RPiSdn, Warning) << \"no noise profile found\";\n> \tLOG(RPiSdn, Debug)\n> -\t\t<< \"Noise profile: constant \" << noise_status.noise_constant\n> -\t\t<< \" slope \" << noise_status.noise_slope;\n> +\t\t<< \"Noise profile: constant \" << noiseStatus.noise_constant\n> +\t\t<< \" slope \" << noiseStatus.noise_slope;\n> \tstruct DenoiseStatus status;\n> -\tstatus.noise_constant = noise_status.noise_constant * deviation_;\n> -\tstatus.noise_slope = noise_status.noise_slope * deviation_;\n> +\tstatus.noise_constant = noiseStatus.noise_constant * deviation_;\n> +\tstatus.noise_slope = noiseStatus.noise_slope * deviation_;\n> \tstatus.strength = strength_;\n> \tstatus.mode = static_cast>(mode_);\n> -\timage_metadata->Set(\"denoise.status\", status);\n> +\timageMetadata->set(\"denoise.status\", status);\n> \tLOG(RPiSdn, Debug)\n> \t\t<< \"programmed constant \" << status.noise_constant\n> \t\t<< \" slope \" << status.noise_slope\n> \t\t<< \" strength \" << status.strength;\n> }\n> \n> -void Sdn::SetMode(DenoiseMode mode)\n> +void Sdn::setMode(DenoiseMode mode)\n> {\n> \t// We only distinguish between off and all other modes.\n> \tmode_ = mode;\n> }\n> \n> // Register algorithm with the system.\n> -static Algorithm *Create(Controller *controller)\n> +static Algorithm *create(Controller *controller)\n> {\n> \treturn (Algorithm *)new Sdn(controller);\n> }\n> -static RegisterAlgorithm reg(NAME, &Create);\n> +static RegisterAlgorithm reg(NAME, &create);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.hpp b/src/ipa/raspberrypi/controller/rpi/sdn.hpp\n> index 2371ce04163f..d9b18f296635 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sdn.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.hpp\n> @@ -17,11 +17,11 @@ class Sdn : public DenoiseAlgorithm\n> {\n> public:\n> \tSdn(Controller *controller = NULL);\n> -\tchar const *Name() const override;\n> -\tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> -\tvoid Initialise() override;\n> -\tvoid Prepare(Metadata *image_metadata) override;\n> -\tvoid SetMode(DenoiseMode mode) override;\n> +\tchar const *name() const override;\n> +\tvoid read(boost::property_tree::ptree const ¶ms) override;\n> +\tvoid initialise() override;\n> +\tvoid prepare(Metadata *imageMetadata) override;\n> +\tvoid setMode(DenoiseMode mode) override;\n> \n> private:\n> \tdouble deviation_;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> index 18825a43867b..775ed0fd2c46 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> @@ -21,23 +21,23 @@ LOG_DEFINE_CATEGORY(RPiSharpen)\n> #define NAME \"rpi.sharpen\"\n> \n> Sharpen::Sharpen(Controller *controller)\n> -\t: SharpenAlgorithm(controller), user_strength_(1.0)\n> +\t: SharpenAlgorithm(controller), userStrength_(1.0)\n> {\n> }\n> \n> -char const *Sharpen::Name() const\n> +char const *Sharpen::name() const\n> {\n> \treturn NAME;\n> }\n> \n> -void Sharpen::SwitchMode(CameraMode const &camera_mode,\n> +void Sharpen::switchMode(CameraMode const &cameraMode,\n> \t\t\t [[maybe_unused]] Metadata *metadata)\n> {\n> \t// can't be less than one, right?\n> -\tmode_factor_ = std::max(1.0, camera_mode.noise_factor);\n> +\tmodeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n> }\n> \n> -void Sharpen::Read(boost::property_tree::ptree const ¶ms)\n> +void Sharpen::read(boost::property_tree::ptree const ¶ms)\n> {\n> \tthreshold_ = params.get(\"threshold\", 1.0);\n> \tstrength_ = params.get(\"strength\", 1.0);\n> @@ -48,38 +48,38 @@ void Sharpen::Read(boost::property_tree::ptree const ¶ms)\n> \t\t<< \" limit \" << limit_;\n> }\n> \n> -void Sharpen::SetStrength(double strength)\n> +void Sharpen::setStrength(double strength)\n> {\n> \t// Note that this function is how an application sets the overall\n> \t// sharpening \"strength\". We call this the \"user strength\" field\n> \t// as there already is a strength_ field - being an internal gain\n> \t// parameter that gets passed to the ISP control code. Negative\n> \t// values are not allowed - coerce them to zero (no sharpening).\n> -\tuser_strength_ = std::max(0.0, strength);\n> +\tuserStrength_ = std::max(0.0, strength);\n> }\n> \n> -void Sharpen::Prepare(Metadata *image_metadata)\n> +void Sharpen::prepare(Metadata *imageMetadata)\n> {\n> \t// The user_strength_ affects the algorithm's internal gain directly, but\n\ns/user_strength_/userStrength_/\n\n> \t// we adjust the limit and threshold less aggressively. Using a sqrt\n> \t// function is an arbitrary but gentle way of accomplishing this.\n> -\tdouble user_strength_sqrt = sqrt(user_strength_);\n> +\tdouble userStrengthSqrt = sqrt(userStrength_);\n> \tstruct SharpenStatus status;\n> \t// Binned modes seem to need the sharpening toned down with this\n> \t// pipeline, thus we use the mode_factor here. Also avoid\n\ns/mode_factor/modeFactor/\n\nReviewed-by: Laurent Pinchart \n\n> -\t// divide-by-zero with the user_strength_sqrt.\n> -\tstatus.threshold = threshold_ * mode_factor_ /\n> -\t\t\t std::max(0.01, user_strength_sqrt);\n> -\tstatus.strength = strength_ / mode_factor_ * user_strength_;\n> -\tstatus.limit = limit_ / mode_factor_ * user_strength_sqrt;\n> -\t// Finally, report any application-supplied parameters that were used.\n> -\tstatus.user_strength = user_strength_;\n> -\timage_metadata->Set(\"sharpen.status\", status);\n> +\t// divide-by-zero with the userStrengthSqrt.\n> +\tstatus.threshold = threshold_ * modeFactor_ /\n> +\t\t\t std::max(0.01, userStrengthSqrt);\n> +\tstatus.strength = strength_ / modeFactor_ * userStrength_;\n> +\tstatus.limit = limit_ / modeFactor_ * userStrengthSqrt;\n> +\t/* Finally, report any application-supplied parameters that were used. */\n> +\tstatus.userStrength = userStrength_;\n> +\timageMetadata->set(\"sharpen.status\", status);\n> }\n> \n> // Register algorithm with the system.\n> -static Algorithm *Create(Controller *controller)\n> +static Algorithm *create(Controller *controller)\n> {\n> \treturn new Sharpen(controller);\n> }\n> -static RegisterAlgorithm reg(NAME, &Create);\n> +static RegisterAlgorithm reg(NAME, &create);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> index 13a076a86895..ced917f3c42b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp\n> @@ -17,18 +17,18 @@ class Sharpen : public SharpenAlgorithm\n> {\n> public:\n> \tSharpen(Controller *controller);\n> -\tchar const *Name() const override;\n> -\tvoid SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;\n> -\tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> -\tvoid SetStrength(double strength) override;\n> -\tvoid Prepare(Metadata *image_metadata) override;\n> +\tchar const *name() const override;\n> +\tvoid switchMode(CameraMode const &cameraMode, Metadata *metadata) override;\n> +\tvoid read(boost::property_tree::ptree const ¶ms) override;\n> +\tvoid setStrength(double strength) override;\n> +\tvoid prepare(Metadata *imageMetadata) override;\n> \n> private:\n> \tdouble threshold_;\n> \tdouble strength_;\n> \tdouble limit_;\n> -\tdouble mode_factor_;\n> -\tdouble user_strength_;\n> +\tdouble modeFactor_;\n> +\tdouble userStrength_;\n> };\n> \n> } // namespace RPiController","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 28B2EC3275\n\tfor ;\n\tMon, 25 Jul 2022 20:58:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79FB863312;\n\tMon, 25 Jul 2022 22:58:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4BF076330A\n\tfor ;\n\tMon, 25 Jul 2022 22:58:30 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B92CF6D1;\n\tMon, 25 Jul 2022 22:58:29 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658782711;\n\tbh=FP1nKBctFcAok/+G+wU9UxJg024M5RoNfgEJWb+89Zw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=zdYPiPtbveBMrmhBQ+70qo3oJipkQtAizodxQlyy2dX+oqkKFfBtVOsZGP0QOhlWe\n\tfxhLJpAkPaWAhAaFlttVAyz+04ZFgQk+VP78NUjH4rXDLMoURk73T2rpCpLCQWxApn\n\tPu/GLFn0eGHQ32EffA2qNw4YzoKdczFFO79ZHA1E5k6wvb911h2WOO+vcqPs6QpHE7\n\t59eXsjIALCqcUXSdZA8yeXoiXGRsUHcqn73qxIqQLmgTyLe1lzLZbZSkXVgy6LhJyA\n\tp2WB4I3aXGPXo1yMNPKrgghTFbE+hBVQEYhKyanv3RNYdO2/MwOipCN8Qm8bIcsHSU\n\tUoo/Rtq34LVqg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658782709;\n\tbh=FP1nKBctFcAok/+G+wU9UxJg024M5RoNfgEJWb+89Zw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ByxRPB6FZBiuLkLUXLsmdqrYRG/cu+jxMFjXWorWkcmCq3E+BSp5TsFKONbde/9Vw\n\tgCNtqTWQcGGUjiNy1PvqZ0fKC9tOFtRlVq5qRCKuaYVfP0JiMQ1EJwu1BxkobLD++x\n\tm2Dbu7J15/qS1UvHtpL1me6krPcEvEHLwaw+8R5I="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ByxRPB6F\"; dkim-atps=neutral","Date":"Mon, 25 Jul 2022 23:58:25 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220725134639.4572-1-naush@raspberrypi.com>\n\t<20220725134639.4572-11-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220725134639.4572-11-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 10/15] DNI: ipa: raspberrypi: Code\n\trefactoring to match style guidelines","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]