[{"id":24168,"web_url":"https://patchwork.libcamera.org/comment/24168/","msgid":"","date":"2022-07-27T01:51:06","subject":"Re: [libcamera-devel] [PATCH 07/17] 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 Tue, Jul 26, 2022 at 01:45:39PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Refactor the source files under src/ipa/raspberrypi/controller/rpi/a* 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/agc_status.h | 2 +-\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 732 ++++++++++----------\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 130 ++--\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 641 ++++++++---------\n> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 86 +--\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 564 ++++++++-------\n> src/ipa/raspberrypi/controller/rpi/awb.hpp | 110 +--\n> 7 files changed, 1097 insertions(+), 1168 deletions(-)\n\n[snip]\n\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index f6a9cb0a2cd8..c19769f4e27b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n\n[snip]\n\n> @@ -441,97 +432,97 @@ void Agc::housekeepConfig()\n\n[snip]\n\n> -static double compute_initial_Y(bcm2835_isp_stats *stats, AwbStatus const &awb,\n> -\t\t\t\tdouble weights[], double gain)\n> +static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const &awb,\n> +\t\t\t double weights[], double gain)\n> {\n> \tbcm2835_isp_stats_region *regions = stats->agc_stats;\n> \t// Note how the calculation below means that equal weights give you\n> \t// \"average\" metering (i.e. all pixels equally important).\n> -\tdouble R_sum = 0, G_sum = 0, B_sum = 0, pixel_sum = 0;\n> +\tdouble rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> \tfor (int i = 0; i < AGC_STATS_SIZE; i++) {\n> \t\tdouble counted = regions[i].counted;\n> -\t\tdouble r_sum = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> -\t\tdouble g_sum = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> -\t\tdouble b_sum = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> -\t\tR_sum += r_sum * weights[i];\n> -\t\tG_sum += g_sum * weights[i];\n> -\t\tB_sum += b_sum * weights[i];\n> -\t\tpixel_sum += counted * weights[i];\n> +\t\tdouble rAcc = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> +\t\tdouble gAcc = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> +\t\tdouble bAcc = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);\n> +\t\trSum += rAcc * weights[i];\n> +\t\tgSum += gAcc * weights[i];\n> +\t\tbSum += bAcc * weights[i];\n> +\t\tpixelSum += counted * weights[i];\n> \t}\n> -\tif (pixel_sum == 0.0) {\n> -\t\tLOG(RPiAgc, Warning) << \"compute_initial_Y: pixel_sum is zero\";\n> +\tif (pixelSum == 0.0) {\n> +\t\tLOG(RPiAgc, Warning) << \"computeInitialY: pixel_sum is zero\";\n\ns/pixel_sum/pixelSum/\n\n> \t\treturn 0;\n> \t}\n> -\tdouble Y_sum = R_sum * awb.gain_r * .299 +\n> -\t\t G_sum * awb.gain_g * .587 +\n> -\t\t B_sum * awb.gain_b * .114;\n> -\treturn Y_sum / pixel_sum / (1 << PIPELINE_BITS);\n> +\tdouble ySum = rSum * awb.gainR * .299 +\n> +\t\t gSum * awb.gainG * .587 +\n> +\t\t bSum * awb.gainB * .114;\n> +\treturn ySum / pixelSum / (1 << PIPELINE_BITS);\n> }\n> \n> // We handle extra gain through EV by adjusting our Y targets. However, you\n\n[snip]\n\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> index ac3dca6f42fc..91251d6be2da 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n\n[snip]\n\n> @@ -110,29 +110,29 @@ private:\n> \tbool isAutoEnabled() const;\n> \t// configuration is read-only, and available to both threads\n> \tAwbConfig config_;\n> -\tstd::thread async_thread_;\n> +\tstd::thread asyncThread_;\n> \tvoid asyncFunc(); // asynchronous thread function\n> \tstd::mutex mutex_;\n> \t// condvar for async thread to wait on\n> -\tstd::condition_variable async_signal_;\n> +\tstd::condition_variable asyncSignal_;\n> \t// condvar for synchronous thread to wait on\n> -\tstd::condition_variable sync_signal_;\n> +\tstd::condition_variable syncSignal_;\n> \t// for sync thread to check if async thread finished (requires mutex)\n> -\tbool async_finished_;\n> +\tbool asyncFinished_;\n> \t// for async thread to check if it's been told to run (requires mutex)\n> -\tbool async_start_;\n> +\tbool asyncStart_;\n> \t// for async thread to check if it's been told to quit (requires mutex)\n> -\tbool async_abort_;\n> +\tbool asyncAbort_;\n> \n> \t// The following are only for the synchronous thread to use:\n> \t// for sync thread to note its has asked async thread to run\n> -\tbool async_started_;\n> -\t// counts up to frame_period before restarting the async thread\n> -\tint frame_phase_;\n> -\tint frame_count_; // counts up to startup_frames\n> -\tAwbStatus sync_results_;\n> -\tAwbStatus prev_sync_results_;\n> -\tstd::string mode_name_;\n> +\tbool asyncStarted_;\n> +\t// counts up to framePeriod before restarting the async thread\n> +\tint framePhase_;\n> +\tint frameCount_; // counts up to startup_frames\n\ns/startup_frames/startupFrames/\n\n> +\tAwbStatus syncResults_;\n> +\tAwbStatus prevSyncResults_;\n> +\tstd::string modeName_;\n> \t// The following are for the asynchronous thread to use, though the main\n> \t// thread can set/reset them if the async thread is known to be idle:\n> \tvoid restartAsync(StatisticsPtr &stats, double lux);\n> @@ -141,22 +141,22 @@ private:\n> \tStatisticsPtr statistics_;\n> \tAwbMode *mode_;\n> \tdouble lux_;\n> -\tAwbStatus async_results_;\n> +\tAwbStatus asyncResults_;\n> \tvoid doAwb();\n> \tvoid awbBayes();\n> \tvoid awbGrey();\n> \tvoid prepareStats();\n> -\tdouble computeDelta2Sum(double gain_r, double gain_b);\n> +\tdouble computeDelta2Sum(double gain_r, double gainB);\n\ns/gain_r/gainR/\n\nReviewed-by: Laurent Pinchart \n\n> \tPwl interpolatePrior();\n> \tdouble coarseSearch(Pwl const &prior);\n> \tvoid fineSearch(double &t, double &r, double &b, Pwl const &prior);\n> \tstd::vector zones_;\n> \tstd::vector points_;\n> \t// manual r setting\n> -\tdouble manual_r_;\n> +\tdouble manualR_;\n> \t// manual b setting\n> -\tdouble manual_b_;\n> -\tbool first_switch_mode_; // is this the first call to SwitchMode?\n> +\tdouble manualB_;\n> +\tbool firstSwitchMode_; // is this the first call to SwitchMode?\n> };\n> \n> static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)","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 117B4BE173\n\tfor ;\n\tWed, 27 Jul 2022 01:51:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 737AB63312;\n\tWed, 27 Jul 2022 03:51:10 +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 08A3F603E8\n\tfor ;\n\tWed, 27 Jul 2022 03:51:09 +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 7AFD5415;\n\tWed, 27 Jul 2022 03:51:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658886670;\n\tbh=ovH7gBf/Zk5fQMI9tmOKDffe8aNfBMiKam5XmGKgaGM=;\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=HVRafj/5S3qn5+MLtEihj4GiwcpZICgR9r5jFzMaxvwtyHHOu9Ey/2X9YWrsKU4BJ\n\tMn5ESKCbLWRdDRohKwW33qMcGUsNfbx7j6dZG3ZCRz0i0qtyboYOeUh75fmRLgUwCZ\n\tn5rwLfFQXzENdA+kS4itvON/rzbzdL5eNOmbLa4rzP7z6AFGnZ69A/Ag46u16BtSGw\n\tuurFJziPyL9hKi8Gg+w+QOfjbPsw/L1ht6nunZXFbbLeha+PSpTxBIC14sHx8MF322\n\tUm5nQdlz5i+eeyVRIwCRJZaNFR+a9UQ+LqBHahp2UA3ijHPfFU3OaHVTM4O3CmX+xD\n\tI7phyMpV179zA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658886668;\n\tbh=ovH7gBf/Zk5fQMI9tmOKDffe8aNfBMiKam5XmGKgaGM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Gyvz+4qAzLhbTy8FWt9cnHyu+/Tm0VE8Az5S6hvxfLQuKAmgLj2zztXnz7vrqV5wv\n\t02xgNPsqUEAET//ar0QcM8Ry1GiM0DoBOTmzpYGaG9EOXQ67Iu9gVGGazcdbMW5wwb\n\tcoxbfp9GL1PTjHBy38SKQ2pBHjnV2VtzP86dW2ik="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Gyvz+4qA\"; dkim-atps=neutral","Date":"Wed, 27 Jul 2022 04:51:06 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220726124549.1646-1-naush@raspberrypi.com>\n\t<20220726124549.1646-8-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220726124549.1646-8-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 07/17] 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\" "}}]