Conversation
| greyWorld = false; | ||
| } else { | ||
| greyWorld = true; | ||
| sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */ |
There was a problem hiding this comment.
What's the story with neural network AWB and sensitivities, are we stll respecting them?
src/ipa/rpi/controller/rpi/awb.cpp
Outdated
| * elsewhere (ALSC and AGC). | ||
| */ | ||
|
|
||
| int AwbMode::read(const libcamera::YamlObject ¶ms) |
There was a problem hiding this comment.
Maybe we should put this method back here in the cpp file?
src/ipa/rpi/controller/rpi/awb.cpp
Outdated
| for (auto &z : zones_) { | ||
| double deltaR = gainR * z.R - 1 - config_.whitepointR; | ||
| double deltaB = gainB * z.B - 1 - config_.whitepointB; | ||
| double deltaR = gainR * z.R - whitepointR; |
There was a problem hiding this comment.
Just wondering what happened to the -1 here... is that included in the whitepointR/B values now?
src/ipa/rpi/controller/rpi/awb.h
Outdated
| struct AwbMode { | ||
| int read(const libcamera::YamlObject ¶ms); | ||
| int read(const libcamera::YamlObject ¶ms) | ||
| { |
There was a problem hiding this comment.
Maybe move back to cpp?
| * Given 3 points on a curve, find the extremum of the function in that | ||
| * interval by fitting a quadratic. | ||
| */ | ||
| * Given 3 points on a curve, find the extremum of the function in that |
There was a problem hiding this comment.
Not 100% convinced by the indentation here...
src/ipa/rpi/controller/rpi/awb.cpp
Outdated
| generateStats(zones_, statistics_, config_.minPixels, | ||
| config_.minG, getGlobalMetadata(), | ||
| config_.biasProportion, biasCtR, biasCtB); | ||
| generateStats(zones_, statistics_, 0.0, 0.0, getGlobalMetadata(), 0.0, 0.0, 0.0); |
There was a problem hiding this comment.
AwbBayes seems to have its own prepareStats, is that right? So is this the AwbNN version? Might be better to leave it pure virtual in the base class and define it in awb_nn.cpp.
| continue; | ||
| zone.R = region.val.rSum / region.counted; | ||
| zone.B = region.val.bSum / region.counted; | ||
| /* |
There was a problem hiding this comment.
Has the indentation gone wrong here?
| float minTemp; | ||
| float maxTemp; | ||
|
|
||
| bool enable_nn; |
There was a problem hiding this comment.
libcamera will want enableNn instead of enable_nn.
| modelPath = root + modelPath; | ||
| } else { | ||
| modelPath = LIBCAMERA_DATA_DIR + modelPath; | ||
| } |
There was a problem hiding this comment.
Might be worth printing out the model path in an Info statement. People waste so much time thinking they're loading something that they aren't (myself included)!
There was a problem hiding this comment.
Ah I see we do it once it's loaded. Well, myabe a Debug statement then, so that folks can double-check if it's not being found.
| } | ||
| } | ||
|
|
||
| void AwbNN::transverseSearch(double t, double &r, double &b) |
There was a problem hiding this comment.
I sometimes wonder if we should do a very short search along the curve too, but not sure. TBD, I guess.
|
|
||
| static double getElementPadded(const std::vector<double> &array, int i, int j) | ||
| { | ||
| int i_padded = std::clamp(i, 0, 31); |
There was a problem hiding this comment.
libcamera will want snakecase iPadded.
| * Decreases the difference between the colour channels when it is higher | ||
| * than nearby pixels. | ||
| */ | ||
| const int total_pixels = 32 * 32; |
There was a problem hiding this comment.
Same throughout this function, totalPixels etc.
|
|
||
| void AwbNN::awbNN() | ||
| { | ||
| float *input_data = interpreter_->typed_input_tensor<float>(0); |
There was a problem hiding this comment.
inputData etc.
| enable_nn = false; | ||
| } | ||
|
|
||
| bool valid_ccm = true; |
| return; | ||
| } | ||
|
|
||
| const int expected_input_size = 32 * 32 * 3; |
There was a problem hiding this comment.
libcamera will want expectedInputSize. Same for all the others...
| zone_log.B += 1.0; | ||
|
|
||
| zone_log.R /= 2.0; | ||
| zone_log.B /= 2.0; |
There was a problem hiding this comment.
Is there any particular rationale for these adjustment? Might be worth a comment.
| for (int i = 0; i < num_outputs; i++) { | ||
| float temp = min_temp + i * (max_temp - min_temp) / (num_outputs - 1); | ||
| float weight = output_data[i]; | ||
| weight *= weight; |
There was a problem hiding this comment.
Interesting. Makes all the weights +ve, among other things! I wonder a bit about putting a softmax on the end of the network instead, is that something we've tried?
e283aef to
0b3ea0b
Compare
Move parts of the AWB algorithm specific to the Bayesian algorithm into a new class. This will make it easier to add new Awb algorithms in the future. Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
edabec9 to
202185e
Compare
Add an Awb algorithm which uses neural networks. Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
Update the tuning files to include the new Awb algorithm. It is enabled by renaming disable.rpi.nn.awb to rpi.nn.awb and rpi.awb to disable.rpi.awb. Add a model for the Awb algorithm to use by default. Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
Prevent an algorithm starting with "disable" from being loaded. Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
202185e to
a68be7e
Compare
| ctHi = *value; | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Super-minor nitpick. If we put this above above readCtCurve would that minimise the diffs a bit?
| greyWorld = false; | ||
| } else { | ||
| greyWorld = true; | ||
| sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */ |
There was a problem hiding this comment.
The CT curve belongs to the base AWB class, just want to check if we think the sensitivities should belong with the CT curve or not?
| } | ||
| } | ||
| whitepointR = params["whitepoint_r"].get<double>(0.0); | ||
| whitepointB = params["whitepoint_b"].get<double>(0.0); |
There was a problem hiding this comment.
I guess I wonder a little bit if the whitepoint might not be a base class thing. What do we think?
|
|
||
| bool validCcm = true; | ||
| for (int i = 0; i < 9; i++) | ||
| if (ccm[i] == 0.0) |
There was a problem hiding this comment.
I think you can have zeroes in a CCM. But every row does have to sum to 1 (otherwise it doesn't preserve greyness).
| -0.2613746357615894, -0.668015927152318, 1.9293905629139072 | ||
| ], | ||
| "enable_nn": 1 | ||
| } |
There was a problem hiding this comment.
Might there be any other useful options? I wonder about a "run on one/all core(s)" option, or is it so quick it's not worth bothering?
Initial implementation of AWB using neural networks. This is an alternative to the old Bayesian method.