[libcamera-devel] [PATCH 07/17] DNI: ipa: raspberrypi: Code refactoring to match style guidelines
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 27 03:51:06 CEST 2022
Hi Naush,
Thank you for the patch.
On Tue, Jul 26, 2022 at 01:45:39PM +0100, Naushir Patuck via libcamera-devel wrote:
> Refactor the source files under src/ipa/raspberrypi/controller/rpi/a* 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/agc_status.h | 2 +-
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 732 ++++++++++----------
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 130 ++--
> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 641 ++++++++---------
> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 86 +--
> src/ipa/raspberrypi/controller/rpi/awb.cpp | 564 ++++++++-------
> src/ipa/raspberrypi/controller/rpi/awb.hpp | 110 +--
> 7 files changed, 1097 insertions(+), 1168 deletions(-)
[snip]
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index f6a9cb0a2cd8..c19769f4e27b 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
[snip]
> @@ -441,97 +432,97 @@ void Agc::housekeepConfig()
[snip]
> -static double compute_initial_Y(bcm2835_isp_stats *stats, AwbStatus const &awb,
> - double weights[], double gain)
> +static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const &awb,
> + double weights[], double gain)
> {
> bcm2835_isp_stats_region *regions = stats->agc_stats;
> // Note how the calculation below means that equal weights give you
> // "average" metering (i.e. all pixels equally important).
> - double R_sum = 0, G_sum = 0, B_sum = 0, pixel_sum = 0;
> + double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;
> for (int i = 0; i < AGC_STATS_SIZE; i++) {
> double counted = regions[i].counted;
> - double r_sum = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> - double g_sum = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> - double b_sum = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> - R_sum += r_sum * weights[i];
> - G_sum += g_sum * weights[i];
> - B_sum += b_sum * weights[i];
> - pixel_sum += counted * weights[i];
> + double rAcc = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> + double gAcc = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> + double bAcc = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);
> + rSum += rAcc * weights[i];
> + gSum += gAcc * weights[i];
> + bSum += bAcc * weights[i];
> + pixelSum += counted * weights[i];
> }
> - if (pixel_sum == 0.0) {
> - LOG(RPiAgc, Warning) << "compute_initial_Y: pixel_sum is zero";
> + if (pixelSum == 0.0) {
> + LOG(RPiAgc, Warning) << "computeInitialY: pixel_sum is zero";
s/pixel_sum/pixelSum/
> return 0;
> }
> - double Y_sum = R_sum * awb.gain_r * .299 +
> - G_sum * awb.gain_g * .587 +
> - B_sum * awb.gain_b * .114;
> - return Y_sum / pixel_sum / (1 << PIPELINE_BITS);
> + double ySum = rSum * awb.gainR * .299 +
> + gSum * awb.gainG * .587 +
> + bSum * awb.gainB * .114;
> + return ySum / pixelSum / (1 << PIPELINE_BITS);
> }
>
> // We handle extra gain through EV by adjusting our Y targets. However, you
[snip]
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> index ac3dca6f42fc..91251d6be2da 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp
[snip]
> @@ -110,29 +110,29 @@ private:
> bool isAutoEnabled() const;
> // configuration is read-only, and available to both threads
> AwbConfig config_;
> - std::thread async_thread_;
> + std::thread asyncThread_;
> void asyncFunc(); // asynchronous thread function
> std::mutex mutex_;
> // condvar for async thread to wait on
> - std::condition_variable async_signal_;
> + std::condition_variable asyncSignal_;
> // condvar for synchronous thread to wait on
> - std::condition_variable sync_signal_;
> + std::condition_variable syncSignal_;
> // for sync thread to check if async thread finished (requires mutex)
> - bool async_finished_;
> + bool asyncFinished_;
> // for async thread to check if it's been told to run (requires mutex)
> - bool async_start_;
> + bool asyncStart_;
> // for async thread to check if it's been told to quit (requires mutex)
> - bool async_abort_;
> + bool asyncAbort_;
>
> // The following are only for the synchronous thread to use:
> // for sync thread to note its has asked async thread to run
> - bool async_started_;
> - // counts up to frame_period before restarting the async thread
> - int frame_phase_;
> - int frame_count_; // counts up to startup_frames
> - AwbStatus sync_results_;
> - AwbStatus prev_sync_results_;
> - std::string mode_name_;
> + bool asyncStarted_;
> + // counts up to framePeriod before restarting the async thread
> + int framePhase_;
> + int frameCount_; // counts up to startup_frames
s/startup_frames/startupFrames/
> + AwbStatus syncResults_;
> + AwbStatus prevSyncResults_;
> + std::string modeName_;
> // The following are for the asynchronous thread to use, though the main
> // thread can set/reset them if the async thread is known to be idle:
> void restartAsync(StatisticsPtr &stats, double lux);
> @@ -141,22 +141,22 @@ private:
> StatisticsPtr statistics_;
> AwbMode *mode_;
> double lux_;
> - AwbStatus async_results_;
> + AwbStatus asyncResults_;
> void doAwb();
> void awbBayes();
> void awbGrey();
> void prepareStats();
> - double computeDelta2Sum(double gain_r, double gain_b);
> + double computeDelta2Sum(double gain_r, double gainB);
s/gain_r/gainR/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Pwl interpolatePrior();
> double coarseSearch(Pwl const &prior);
> void fineSearch(double &t, double &r, double &b, Pwl const &prior);
> std::vector<RGB> zones_;
> std::vector<Pwl::Point> points_;
> // manual r setting
> - double manual_r_;
> + double manualR_;
> // manual b setting
> - double manual_b_;
> - bool first_switch_mode_; // is this the first call to SwitchMode?
> + double manualB_;
> + bool firstSwitchMode_; // is this the first call to SwitchMode?
> };
>
> static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list