[libcamera-devel] [PATCH v3 1/8] ipa: raspberrypi: Code refactoring to match style guidelines
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 27 15:14:21 CEST 2022
Hi Naush,
Thank you for the patch.
On Wed, Jul 27, 2022 at 09:55:17AM +0100, Naushir Patuck wrote:
> Refactor all the source files in src/ipa/raspberrypi/ 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/cam_helper.cpp | 88 +--
> src/ipa/raspberrypi/cam_helper.hpp | 40 +-
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 34 +-
> src/ipa/raspberrypi/cam_helper_imx290.cpp | 32 +-
> src/ipa/raspberrypi/cam_helper_imx296.cpp | 24 +-
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 72 +-
> src/ipa/raspberrypi/cam_helper_imx519.cpp | 70 +-
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 44 +-
> src/ipa/raspberrypi/cam_helper_ov9281.cpp | 28 +-
> .../raspberrypi/controller/agc_algorithm.hpp | 19 +-
> src/ipa/raspberrypi/controller/agc_status.h | 24 +-
> src/ipa/raspberrypi/controller/algorithm.cpp | 20 +-
> src/ipa/raspberrypi/controller/algorithm.hpp | 26 +-
> .../raspberrypi/controller/awb_algorithm.hpp | 6 +-
> src/ipa/raspberrypi/controller/awb_status.h | 8 +-
> .../controller/black_level_status.h | 6 +-
> src/ipa/raspberrypi/controller/camera_mode.h | 16 +-
> .../raspberrypi/controller/ccm_algorithm.hpp | 2 +-
> .../controller/contrast_algorithm.hpp | 4 +-
> src/ipa/raspberrypi/controller/controller.cpp | 74 +-
> src/ipa/raspberrypi/controller/controller.hpp | 22 +-
> .../controller/denoise_algorithm.hpp | 2 +-
> .../raspberrypi/controller/denoise_status.h | 4 +-
> .../raspberrypi/controller/device_status.cpp | 18 +-
> .../raspberrypi/controller/device_status.h | 16 +-
> src/ipa/raspberrypi/controller/focus_status.h | 2 +-
> src/ipa/raspberrypi/controller/histogram.cpp | 34 +-
> src/ipa/raspberrypi/controller/histogram.hpp | 10 +-
> src/ipa/raspberrypi/controller/metadata.hpp | 16 +-
> src/ipa/raspberrypi/controller/noise_status.h | 4 +-
> src/ipa/raspberrypi/controller/pwl.cpp | 130 ++--
> src/ipa/raspberrypi/controller/pwl.hpp | 48 +-
> 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 +--
> .../controller/rpi/black_level.cpp | 34 +-
> .../controller/rpi/black_level.hpp | 12 +-
> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 84 +-
> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 12 +-
> .../raspberrypi/controller/rpi/contrast.cpp | 118 ++-
> .../raspberrypi/controller/rpi/contrast.hpp | 30 +-
> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 18 +-
> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 6 +-
> 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 +-
> src/ipa/raspberrypi/controller/rpi/noise.cpp | 38 +-
> src/ipa/raspberrypi/controller/rpi/noise.hpp | 14 +-
> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 36 +-
> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 10 +-
> .../raspberrypi/controller/rpi/sharpen.cpp | 42 +-
> .../raspberrypi/controller/rpi/sharpen.hpp | 14 +-
> .../controller/sharpen_algorithm.hpp | 2 +-
> .../raspberrypi/controller/sharpen_status.h | 2 +-
> src/ipa/raspberrypi/md_parser.hpp | 44 +-
> src/ipa/raspberrypi/md_parser_smia.cpp | 108 +--
> src/ipa/raspberrypi/raspberrypi.cpp | 272 +++----
> 63 files changed, 2099 insertions(+), 2167 deletions(-)
[snip]
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index f6a9cb0a2cd8..408ff9cf296d 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
[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);
> }
[snip]
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index d4c934473832..a305237f31fb 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
[snip]
> -double Awb::computeDelta2Sum(double gain_r, double gain_b)
> +double Awb::computeDelta2Sum(double gainR, double gainB)
> {
> // Compute the sum of the squared colour error (non-greyness) as it
> // appears in the log likelihood equation.
> - double delta2_sum = 0;
> + double delta2Sum = 0;
> for (auto &z : zones_) {
> - double delta_r = gain_r * z.R - 1 - config_.whitepoint_r;
> - double delta_b = gain_b * z.B - 1 - config_.whitepoint_b;
> - double delta2 = delta_r * delta_r + delta_b * delta_b;
> + double deltaR = gainR * z.R - 1 - config_.whitepointR;
> + double deltaB = gainB * z.B - 1 - config_.whitepointB;
> + double delta2 = deltaR * deltaR + deltaB * deltaB;
> //LOG(RPiAwb, Debug) << "delta_r " << delta_r << " delta_b " << delta_b << " delta2 " << delta2;
The variables should be updated here too.
> - delta2 = std::min(delta2, config_.delta_limit);
> - delta2_sum += delta2;
> + delta2 = std::min(delta2, config_.deltaLimit);
> + delta2Sum += delta2;
> }
> - return delta2_sum;
> + return delta2Sum;
> }
[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]
> @@ -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/
> 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?
> };
[snip]
> diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp
> index ea5eac414b36..9fab6594baac 100644
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp
[snip]
> @@ -76,74 +76,74 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>
> {
> ASSERT(offsets_.size());
>
> - if (buffer[0] != LINE_START)
> - return NO_LINE_START;
> + if (buffer[0] != LineStart)
> + return NoLineStart;
>
> - unsigned int current_offset = 1; /* after the LINE_START */
> - unsigned int current_line_start = 0, current_line = 0;
> - unsigned int reg_num = 0, regs_done = 0;
> + unsigned int currentOffset = 1; /* after the LineStart */
> + unsigned int currentLineStart = 0, currentLine = 0;
> + unsigned int regNum = 0, regsDone = 0;
>
> while (1) {
> - int tag = buffer[current_offset++];
> -
> - if ((bits_per_pixel_ == 10 &&
> - (current_offset + 1 - current_line_start) % 5 == 0) ||
> - (bits_per_pixel_ == 12 &&
> - (current_offset + 1 - current_line_start) % 3 == 0)) {
> - if (buffer[current_offset++] != REG_SKIP)
> - return BAD_DUMMY;
> + int tag = buffer[currentOffset++];
> +
> + if ((bitsPerPixel_ == 10 &&
> + (currentOffset + 1 - currentLineStart) % 5 == 0) ||
> + (bitsPerPixel_ == 12 &&
> + (currentOffset + 1 - currentLineStart) % 3 == 0)) {
> + if (buffer[currentOffset++] != RegSkip)
> + return BadDummy;
> }
>
> - int data_byte = buffer[current_offset++];
> + int dataByte = buffer[currentOffset++];
>
> - if (tag == LINE_END_TAG) {
> - if (data_byte != LINE_END_TAG)
> - return BAD_LINE_END;
> + if (tag == LineEndTag) {
> + if (dataByte != LineEndTag)
> + return BadLineEnd;
>
> - if (num_lines_ && ++current_line == num_lines_)
> - return MISSING_REGS;
> + if (numLines_ && ++currentLine == numLines_)
> + return MissingRegs;
>
> - if (line_length_bytes_) {
> - current_offset = current_line_start + line_length_bytes_;
> + if (lineLengthBytes_) {
> + currentOffset = currentLineStart + lineLengthBytes_;
>
> /* Require whole line to be in the buffer (if buffer size set). */
> if (buffer.size() &&
> - current_offset + line_length_bytes_ > buffer.size())
> - return MISSING_REGS;
> + currentOffset + lineLengthBytes_ > buffer.size())
> + return MissingRegs;
>
> - if (buffer[current_offset] != LINE_START)
> - return NO_LINE_START;
> + if (buffer[currentOffset] != LineStart)
> + return NoLineStart;
> } else {
> /* allow a zero line length to mean "hunt for the next line" */
> - while (current_offset < buffer.size() &&
> - buffer[current_offset] != LINE_START)
> - current_offset++;
> + while (currentOffset < buffer.size() &&
> + buffer[currentOffset] != LineStart)
> + currentOffset++;
>
> - if (current_offset == buffer.size())
> - return NO_LINE_START;
> + if (currentOffset == buffer.size())
> + return NoLineStart;
> }
>
> - /* inc current_offset to after LINE_START */
> - current_line_start = current_offset++;
> + /* inc current_offset to after LineStart */
s/current_offset/currentOffset/
> + currentLineStart = currentOffset++;
> } else {
> - if (tag == REG_HI_BITS)
> - reg_num = (reg_num & 0xff) | (data_byte << 8);
> - else if (tag == REG_LOW_BITS)
> - reg_num = (reg_num & 0xff00) | data_byte;
> - else if (tag == REG_SKIP)
> - reg_num++;
> - else if (tag == REG_VALUE) {
> - auto reg = offsets_.find(reg_num);
> + if (tag == RegHiBits)
> + regNum = (regNum & 0xff) | (dataByte << 8);
> + else if (tag == RegLowBits)
> + regNum = (regNum & 0xff00) | dataByte;
> + else if (tag == RegSkip)
> + regNum++;
> + else if (tag == RegValue) {
> + auto reg = offsets_.find(regNum);
>
> if (reg != offsets_.end()) {
> - offsets_[reg_num] = current_offset - 1;
> + offsets_[regNum] = currentOffset - 1;
>
> - if (++regs_done == offsets_.size())
> - return PARSE_OK;
> + if (++regsDone == offsets_.size())
> + return ParseOk;
> }
> - reg_num++;
> + regNum++;
> } else
> - return ILLEGAL_TAG;
> + return IllegalTag;
> }
> }
> }
[snip]
I'll handle all these smalll issues when applying.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list