<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Jul 2022 at 14:14, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Wed, Jul 27, 2022 at 09:55:17AM +0100, Naushir Patuck wrote:<br>
> Refactor all the source files in src/ipa/raspberrypi/ to match the recommended<br>
> formatting guidelines for the libcamera project. The vast majority of changes<br>
> in this commit comprise of switching from snake_case to CamelCase, and starting<br>
> class member functions with a lower case character.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> src/ipa/raspberrypi/cam_helper.cpp | 88 +--<br>
> src/ipa/raspberrypi/cam_helper.hpp | 40 +-<br>
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 34 +-<br>
> src/ipa/raspberrypi/cam_helper_imx290.cpp | 32 +-<br>
> src/ipa/raspberrypi/cam_helper_imx296.cpp | 24 +-<br>
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 72 +-<br>
> src/ipa/raspberrypi/cam_helper_imx519.cpp | 70 +-<br>
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 44 +-<br>
> src/ipa/raspberrypi/cam_helper_ov9281.cpp | 28 +-<br>
> .../raspberrypi/controller/agc_algorithm.hpp | 19 +-<br>
> src/ipa/raspberrypi/controller/agc_status.h | 24 +-<br>
> src/ipa/raspberrypi/controller/algorithm.cpp | 20 +-<br>
> src/ipa/raspberrypi/controller/algorithm.hpp | 26 +-<br>
> .../raspberrypi/controller/awb_algorithm.hpp | 6 +-<br>
> src/ipa/raspberrypi/controller/awb_status.h | 8 +-<br>
> .../controller/black_level_status.h | 6 +-<br>
> src/ipa/raspberrypi/controller/camera_mode.h | 16 +-<br>
> .../raspberrypi/controller/ccm_algorithm.hpp | 2 +-<br>
> .../controller/contrast_algorithm.hpp | 4 +-<br>
> src/ipa/raspberrypi/controller/controller.cpp | 74 +-<br>
> src/ipa/raspberrypi/controller/controller.hpp | 22 +-<br>
> .../controller/denoise_algorithm.hpp | 2 +-<br>
> .../raspberrypi/controller/denoise_status.h | 4 +-<br>
> .../raspberrypi/controller/device_status.cpp | 18 +-<br>
> .../raspberrypi/controller/device_status.h | 16 +-<br>
> src/ipa/raspberrypi/controller/focus_status.h | 2 +-<br>
> src/ipa/raspberrypi/controller/histogram.cpp | 34 +-<br>
> src/ipa/raspberrypi/controller/histogram.hpp | 10 +-<br>
> src/ipa/raspberrypi/controller/metadata.hpp | 16 +-<br>
> src/ipa/raspberrypi/controller/noise_status.h | 4 +-<br>
> src/ipa/raspberrypi/controller/pwl.cpp | 130 ++--<br>
> src/ipa/raspberrypi/controller/pwl.hpp | 48 +-<br>
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 732 +++++++++---------<br>
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 130 ++--<br>
> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 641 ++++++++-------<br>
> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 86 +-<br>
> src/ipa/raspberrypi/controller/rpi/awb.cpp | 564 +++++++-------<br>
> src/ipa/raspberrypi/controller/rpi/awb.hpp | 110 +--<br>
> .../controller/rpi/black_level.cpp | 34 +-<br>
> .../controller/rpi/black_level.hpp | 12 +-<br>
> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 84 +-<br>
> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 12 +-<br>
> .../raspberrypi/controller/rpi/contrast.cpp | 118 ++-<br>
> .../raspberrypi/controller/rpi/contrast.hpp | 30 +-<br>
> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 18 +-<br>
> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 6 +-<br>
> src/ipa/raspberrypi/controller/rpi/focus.cpp | 14 +-<br>
> src/ipa/raspberrypi/controller/rpi/focus.hpp | 4 +-<br>
> src/ipa/raspberrypi/controller/rpi/geq.cpp | 48 +-<br>
> src/ipa/raspberrypi/controller/rpi/geq.hpp | 6 +-<br>
> src/ipa/raspberrypi/controller/rpi/lux.cpp | 70 +-<br>
> src/ipa/raspberrypi/controller/rpi/lux.hpp | 22 +-<br>
> src/ipa/raspberrypi/controller/rpi/noise.cpp | 38 +-<br>
> src/ipa/raspberrypi/controller/rpi/noise.hpp | 14 +-<br>
> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 36 +-<br>
> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 10 +-<br>
> .../raspberrypi/controller/rpi/sharpen.cpp | 42 +-<br>
> .../raspberrypi/controller/rpi/sharpen.hpp | 14 +-<br>
> .../controller/sharpen_algorithm.hpp | 2 +-<br>
> .../raspberrypi/controller/sharpen_status.h | 2 +-<br>
> src/ipa/raspberrypi/md_parser.hpp | 44 +-<br>
> src/ipa/raspberrypi/md_parser_smia.cpp | 108 +--<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 272 +++----<br>
> 63 files changed, 2099 insertions(+), 2167 deletions(-)<br>
<br>
[snip]<br>
<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> index f6a9cb0a2cd8..408ff9cf296d 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
<br>
[snip]<br>
<br>
> -static double compute_initial_Y(bcm2835_isp_stats *stats, AwbStatus const &awb,<br>
> - double weights[], double gain)<br>
> +static double computeInitialY(bcm2835_isp_stats *stats, AwbStatus const &awb,<br>
> + double weights[], double gain)<br>
> {<br>
> bcm2835_isp_stats_region *regions = stats->agc_stats;<br>
> // Note how the calculation below means that equal weights give you<br>
> // "average" metering (i.e. all pixels equally important).<br>
> - double R_sum = 0, G_sum = 0, B_sum = 0, pixel_sum = 0;<br>
> + double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;<br>
> for (int i = 0; i < AGC_STATS_SIZE; i++) {<br>
> double counted = regions[i].counted;<br>
> - double r_sum = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> - double g_sum = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> - double b_sum = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> - R_sum += r_sum * weights[i];<br>
> - G_sum += g_sum * weights[i];<br>
> - B_sum += b_sum * weights[i];<br>
> - pixel_sum += counted * weights[i];<br>
> + double rAcc = std::min(regions[i].r_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> + double gAcc = std::min(regions[i].g_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> + double bAcc = std::min(regions[i].b_sum * gain, ((1 << PIPELINE_BITS) - 1) * counted);<br>
> + rSum += rAcc * weights[i];<br>
> + gSum += gAcc * weights[i];<br>
> + bSum += bAcc * weights[i];<br>
> + pixelSum += counted * weights[i];<br>
> }<br>
> - if (pixel_sum == 0.0) {<br>
> - LOG(RPiAgc, Warning) << "compute_initial_Y: pixel_sum is zero";<br>
> + if (pixelSum == 0.0) {<br>
> + LOG(RPiAgc, Warning) << "computeInitialY: pixel_sum is zero";<br>
<br>
s/pixel_sum/pixelSum/<br>
<br>
> return 0;<br>
> }<br>
> - double Y_sum = R_sum * awb.gain_r * .299 +<br>
> - G_sum * awb.gain_g * .587 +<br>
> - B_sum * awb.gain_b * .114;<br>
> - return Y_sum / pixel_sum / (1 << PIPELINE_BITS);<br>
> + double ySum = rSum * awb.gainR * .299 +<br>
> + gSum * awb.gainG * .587 +<br>
> + bSum * awb.gainB * .114;<br>
> + return ySum / pixelSum / (1 << PIPELINE_BITS);<br>
> }<br>
<br>
[snip]<br>
<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
> index d4c934473832..a305237f31fb 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
<br>
[snip]<br>
<br>
> -double Awb::computeDelta2Sum(double gain_r, double gain_b)<br>
> +double Awb::computeDelta2Sum(double gainR, double gainB)<br>
> {<br>
> // Compute the sum of the squared colour error (non-greyness) as it<br>
> // appears in the log likelihood equation.<br>
> - double delta2_sum = 0;<br>
> + double delta2Sum = 0;<br>
> for (auto &z : zones_) {<br>
> - double delta_r = gain_r * z.R - 1 - config_.whitepoint_r;<br>
> - double delta_b = gain_b * z.B - 1 - config_.whitepoint_b;<br>
> - double delta2 = delta_r * delta_r + delta_b * delta_b;<br>
> + double deltaR = gainR * z.R - 1 - config_.whitepointR;<br>
> + double deltaB = gainB * z.B - 1 - config_.whitepointB;<br>
> + double delta2 = deltaR * deltaR + deltaB * deltaB;<br>
> //LOG(RPiAwb, Debug) << "delta_r " << delta_r << " delta_b " << delta_b << " delta2 " << delta2;<br>
<br>
The variables should be updated here too.<br>
<br>
> - delta2 = std::min(delta2, config_.delta_limit);<br>
> - delta2_sum += delta2;<br>
> + delta2 = std::min(delta2, config_.deltaLimit);<br>
> + delta2Sum += delta2;<br>
> }<br>
> - return delta2_sum;<br>
> + return delta2Sum;<br>
> }<br>
<br>
[snip]<br>
<br>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp<br>
> index ac3dca6f42fc..91251d6be2da 100644<br>
> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp<br>
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp<br>
<br>
[snip]<br>
<br>
> @@ -141,22 +141,22 @@ private:<br>
> StatisticsPtr statistics_;<br>
> AwbMode *mode_;<br>
> double lux_;<br>
> - AwbStatus async_results_;<br>
> + AwbStatus asyncResults_;<br>
> void doAwb();<br>
> void awbBayes();<br>
> void awbGrey();<br>
> void prepareStats();<br>
> - double computeDelta2Sum(double gain_r, double gain_b);<br>
> + double computeDelta2Sum(double gain_r, double gainB);<br>
<br>
s/gain_r/gainR/<br>
<br>
> Pwl interpolatePrior();<br>
> double coarseSearch(Pwl const &prior);<br>
> void fineSearch(double &t, double &r, double &b, Pwl const &prior);<br>
> std::vector<RGB> zones_;<br>
> std::vector<Pwl::Point> points_;<br>
> // manual r setting<br>
> - double manual_r_;<br>
> + double manualR_;<br>
> // manual b setting<br>
> - double manual_b_;<br>
> - bool first_switch_mode_; // is this the first call to SwitchMode?<br>
> + double manualB_;<br>
> + bool firstSwitchMode_; // is this the first call to SwitchMode?<br>
> };<br>
<br>
[snip]<br>
<br>
> diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp<br>
> index ea5eac414b36..9fab6594baac 100644<br>
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp<br>
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp<br>
<br>
[snip]<br>
<br>
> @@ -76,74 +76,74 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t><br>
> {<br>
> ASSERT(offsets_.size());<br>
> <br>
> - if (buffer[0] != LINE_START)<br>
> - return NO_LINE_START;<br>
> + if (buffer[0] != LineStart)<br>
> + return NoLineStart;<br>
> <br>
> - unsigned int current_offset = 1; /* after the LINE_START */<br>
> - unsigned int current_line_start = 0, current_line = 0;<br>
> - unsigned int reg_num = 0, regs_done = 0;<br>
> + unsigned int currentOffset = 1; /* after the LineStart */<br>
> + unsigned int currentLineStart = 0, currentLine = 0;<br>
> + unsigned int regNum = 0, regsDone = 0;<br>
> <br>
> while (1) {<br>
> - int tag = buffer[current_offset++];<br>
> -<br>
> - if ((bits_per_pixel_ == 10 &&<br>
> - (current_offset + 1 - current_line_start) % 5 == 0) ||<br>
> - (bits_per_pixel_ == 12 &&<br>
> - (current_offset + 1 - current_line_start) % 3 == 0)) {<br>
> - if (buffer[current_offset++] != REG_SKIP)<br>
> - return BAD_DUMMY;<br>
> + int tag = buffer[currentOffset++];<br>
> +<br>
> + if ((bitsPerPixel_ == 10 &&<br>
> + (currentOffset + 1 - currentLineStart) % 5 == 0) ||<br>
> + (bitsPerPixel_ == 12 &&<br>
> + (currentOffset + 1 - currentLineStart) % 3 == 0)) {<br>
> + if (buffer[currentOffset++] != RegSkip)<br>
> + return BadDummy;<br>
> }<br>
> <br>
> - int data_byte = buffer[current_offset++];<br>
> + int dataByte = buffer[currentOffset++];<br>
> <br>
> - if (tag == LINE_END_TAG) {<br>
> - if (data_byte != LINE_END_TAG)<br>
> - return BAD_LINE_END;<br>
> + if (tag == LineEndTag) {<br>
> + if (dataByte != LineEndTag)<br>
> + return BadLineEnd;<br>
> <br>
> - if (num_lines_ && ++current_line == num_lines_)<br>
> - return MISSING_REGS;<br>
> + if (numLines_ && ++currentLine == numLines_)<br>
> + return MissingRegs;<br>
> <br>
> - if (line_length_bytes_) {<br>
> - current_offset = current_line_start + line_length_bytes_;<br>
> + if (lineLengthBytes_) {<br>
> + currentOffset = currentLineStart + lineLengthBytes_;<br>
> <br>
> /* Require whole line to be in the buffer (if buffer size set). */<br>
> if (buffer.size() &&<br>
> - current_offset + line_length_bytes_ > buffer.size())<br>
> - return MISSING_REGS;<br>
> + currentOffset + lineLengthBytes_ > buffer.size())<br>
> + return MissingRegs;<br>
> <br>
> - if (buffer[current_offset] != LINE_START)<br>
> - return NO_LINE_START;<br>
> + if (buffer[currentOffset] != LineStart)<br>
> + return NoLineStart;<br>
> } else {<br>
> /* allow a zero line length to mean "hunt for the next line" */<br>
> - while (current_offset < buffer.size() &&<br>
> - buffer[current_offset] != LINE_START)<br>
> - current_offset++;<br>
> + while (currentOffset < buffer.size() &&<br>
> + buffer[currentOffset] != LineStart)<br>
> + currentOffset++;<br>
> <br>
> - if (current_offset == buffer.size())<br>
> - return NO_LINE_START;<br>
> + if (currentOffset == buffer.size())<br>
> + return NoLineStart;<br>
> }<br>
> <br>
> - /* inc current_offset to after LINE_START */<br>
> - current_line_start = current_offset++;<br>
> + /* inc current_offset to after LineStart */<br>
<br>
s/current_offset/currentOffset/<br>
<br>
> + currentLineStart = currentOffset++;<br>
> } else {<br>
> - if (tag == REG_HI_BITS)<br>
> - reg_num = (reg_num & 0xff) | (data_byte << 8);<br>
> - else if (tag == REG_LOW_BITS)<br>
> - reg_num = (reg_num & 0xff00) | data_byte;<br>
> - else if (tag == REG_SKIP)<br>
> - reg_num++;<br>
> - else if (tag == REG_VALUE) {<br>
> - auto reg = offsets_.find(reg_num);<br>
> + if (tag == RegHiBits)<br>
> + regNum = (regNum & 0xff) | (dataByte << 8);<br>
> + else if (tag == RegLowBits)<br>
> + regNum = (regNum & 0xff00) | dataByte;<br>
> + else if (tag == RegSkip)<br>
> + regNum++;<br>
> + else if (tag == RegValue) {<br>
> + auto reg = offsets_.find(regNum);<br>
> <br>
> if (reg != offsets_.end()) {<br>
> - offsets_[reg_num] = current_offset - 1;<br>
> + offsets_[regNum] = currentOffset - 1;<br>
> <br>
> - if (++regs_done == offsets_.size())<br>
> - return PARSE_OK;<br>
> + if (++regsDone == offsets_.size())<br>
> + return ParseOk;<br>
> }<br>
> - reg_num++;<br>
> + regNum++;<br>
> } else<br>
> - return ILLEGAL_TAG;<br>
> + return IllegalTag;<br>
> }<br>
> }<br>
> }<br>
<br>
[snip]<br>
<br>
I'll handle all these smalll issues when applying.<br></blockquote><div><br></div><div>Groan! I'm sure there are still more lurking around... :-(</div><div>Thanks for the fixups!<br></div><div><br></div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>