<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 27 Jul 2022 at 03:38, 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">Replace the Fatal log messages that cause an abort during tuning data<br>
read with Error messages and proper error propagation to the caller.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br></blockquote><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
src/ipa/raspberrypi/controller/rpi/agc.cpp | 36 ++++++++++----<br>
src/ipa/raspberrypi/controller/rpi/alsc.cpp | 52 ++++++++++++++-------<br>
src/ipa/raspberrypi/controller/rpi/awb.cpp | 42 +++++++++++------<br>
src/ipa/raspberrypi/controller/rpi/ccm.cpp | 24 ++++++----<br>
src/ipa/raspberrypi/controller/rpi/dpc.cpp | 6 ++-<br>
src/ipa/raspberrypi/controller/rpi/geq.cpp | 6 ++-<br>
6 files changed, 113 insertions(+), 53 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
index 93f966a1d5ce..153493f8b8e2 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp<br>
@@ -34,13 +34,20 @@ static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit pipelin<br>
int AgcMeteringMode::read(boost::property_tree::ptree const ¶ms)<br>
{<br>
int num = 0;<br>
+<br>
for (auto &p : params.get_child("weights")) {<br>
- if (num == AgcStatsSize)<br>
- LOG(RPiAgc, Fatal) << "AgcMeteringMode: too many weights";<br>
+ if (num == AgcStatsSize) {<br>
+ LOG(RPiAgc, Error) << "AgcMeteringMode: too many weights";<br>
+ return -EINVAL;<br>
+ }<br>
weights[num++] = p.second.get_value<double>();<br>
}<br>
- if (num != AgcStatsSize)<br>
- LOG(RPiAgc, Fatal) << "AgcMeteringMode: insufficient weights";<br>
+<br>
+ if (num != AgcStatsSize) {<br>
+ LOG(RPiAgc, Error) << "AgcMeteringMode: insufficient weights";<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
return 0;<br>
}<br>
<br>
@@ -85,12 +92,19 @@ int AgcExposureMode::read(boost::property_tree::ptree const ¶ms)<br>
{<br>
int numShutters = readList(shutter, params.get_child("shutter"));<br>
int numAgs = readList(gain, params.get_child("gain"));<br>
- if (numShutters < 2 || numAgs < 2)<br>
- LOG(RPiAgc, Fatal)<br>
+<br>
+ if (numShutters < 2 || numAgs < 2) {<br>
+ LOG(RPiAgc, Error)<br>
<< "AgcExposureMode: must have at least two entries in exposure profile";<br>
- if (numShutters != numAgs)<br>
- LOG(RPiAgc, Fatal)<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
+ if (numShutters != numAgs) {<br>
+ LOG(RPiAgc, Error)<br>
<< "AgcExposureMode: expect same number of exposure and gain entries in exposure profile";<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
return 0;<br>
}<br>
<br>
@@ -120,8 +134,10 @@ int AgcConstraint::read(boost::property_tree::ptree const ¶ms)<br>
std::string boundString = params.get<std::string>("bound", "");<br>
transform(boundString.begin(), boundString.end(),<br>
boundString.begin(), ::toupper);<br>
- if (boundString != "UPPER" && boundString != "LOWER")<br>
- LOG(RPiAgc, Fatal) << "AGC constraint type should be UPPER or LOWER";<br>
+ if (boundString != "UPPER" && boundString != "LOWER") {<br>
+ LOG(RPiAgc, Error) << "AGC constraint type should be UPPER or LOWER";<br>
+ return -EINVAL;<br>
+ }<br>
bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER;<br>
qLo = params.get<double>("q_lo");<br>
qHi = params.get<double>("q_hi");<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
index b36277696a52..49aaf6b74603 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
@@ -53,11 +53,17 @@ char const *Alsc::name() const<br>
static int generateLut(double *lut, boost::property_tree::ptree const ¶ms)<br>
{<br>
double cstrength = params.get<double>("corner_strength", 2.0);<br>
- if (cstrength <= 1.0)<br>
- LOG(RPiAlsc, Fatal) << "Alsc: corner_strength must be > 1.0";<br>
+ if (cstrength <= 1.0) {<br>
+ LOG(RPiAlsc, Error) << "corner_strength must be > 1.0";<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
double asymmetry = params.get<double>("asymmetry", 1.0);<br>
- if (asymmetry < 0)<br>
- LOG(RPiAlsc, Fatal) << "Alsc: asymmetry must be >= 0";<br>
+ if (asymmetry < 0) {<br>
+ LOG(RPiAlsc, Error) << "asymmetry must be >= 0";<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
double f1 = cstrength - 1, f2 = 1 + sqrt(cstrength);<br>
double R2 = X * Y / 4 * (1 + asymmetry * asymmetry);<br>
int num = 0;<br>
@@ -78,13 +84,19 @@ static int readLut(double *lut, boost::property_tree::ptree const ¶ms)<br>
{<br>
int num = 0;<br>
const int maxNum = XY;<br>
+<br>
for (auto &p : params) {<br>
- if (num == maxNum)<br>
- LOG(RPiAlsc, Fatal) << "Alsc: too many entries in LSC table";<br>
+ if (num == maxNum) {<br>
+ LOG(RPiAlsc, Error) << "Too many entries in LSC table";<br>
+ return -EINVAL;<br>
+ }<br>
lut[num++] = p.second.get_value<double>();<br>
}<br>
- if (num < maxNum)<br>
- LOG(RPiAlsc, Fatal) << "Alsc: too few entries in LSC table";<br>
+<br>
+ if (num < maxNum) {<br>
+ LOG(RPiAlsc, Error) << "Too few entries in LSC table";<br>
+ return -EINVAL;<br>
+ }<br>
return 0;<br>
}<br>
<br>
@@ -96,24 +108,30 @@ static int readCalibrations(std::vector<AlscCalibration> &calibrations,<br>
double lastCt = 0;<br>
for (auto &p : params.get_child(name)) {<br>
double ct = p.second.get<double>("ct");<br>
- if (ct <= lastCt)<br>
- LOG(RPiAlsc, Fatal)<br>
- << "Alsc: entries in " << name << " must be in increasing ct order";<br>
+ if (ct <= lastCt) {<br>
+ LOG(RPiAlsc, Error)<br>
+ << "Entries in " << name << " must be in increasing ct order";<br>
+ return -EINVAL;<br>
+ }<br>
AlscCalibration calibration;<br>
calibration.ct = lastCt = ct;<br>
boost::property_tree::ptree const &table =<br>
p.second.get_child("table");<br>
int num = 0;<br>
for (auto it = table.begin(); it != table.end(); it++) {<br>
- if (num == XY)<br>
- LOG(RPiAlsc, Fatal)<br>
- << "Alsc: too many values for ct " << ct << " in " << name;<br>
+ if (num == XY) {<br>
+ LOG(RPiAlsc, Error)<br>
+ << "Too many values for ct " << ct << " in " << name;<br>
+ return -EINVAL;<br>
+ }<br>
calibration.table[num++] =<br>
it->second.get_value<double>();<br>
}<br>
- if (num != XY)<br>
- LOG(RPiAlsc, Fatal)<br>
- << "Alsc: too few values for ct " << ct << " in " << name;<br>
+ if (num != XY) {<br>
+ LOG(RPiAlsc, Error)<br>
+ << "Too few values for ct " << ct << " in " << name;<br>
+ return -EINVAL;<br>
+ }<br>
calibrations.push_back(calibration);<br>
LOG(RPiAlsc, Debug)<br>
<< "Read " << name << " calibration for ct " << ct;<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
index a9e776a344a1..10c49c126341 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp<br>
@@ -46,16 +46,22 @@ static int readCtCurve(Pwl &ctR, Pwl &ctB,<br>
for (auto it = params.begin(); it != params.end(); it++) {<br>
double ct = it->second.get_value<double>();<br>
assert(it == params.begin() || ct != ctR.domain().end);<br>
- if (++it == params.end())<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry";<br>
+ if (++it == params.end()) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: incomplete CT curve entry";<br>
+ return -EINVAL;<br>
+ }<br>
ctR.append(ct, it->second.get_value<double>());<br>
- if (++it == params.end())<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry";<br>
+ if (++it == params.end()) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: incomplete CT curve entry";<br>
+ return -EINVAL;<br>
+ }<br>
ctB.append(ct, it->second.get_value<double>());<br>
num++;<br>
}<br>
- if (num < 2)<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: insufficient points in CT curve";<br>
+ if (num < 2) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: insufficient points in CT curve";<br>
+ return -EINVAL;<br>
+ }<br>
return 0;<br>
}<br>
<br>
@@ -78,12 +84,16 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms)<br>
ret = prior.read(p.second);<br>
if (ret)<br>
return ret;<br>
- if (!priors.empty() && prior.lux <= priors.back().lux)<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: Prior must be ordered in increasing lux value";<br>
+ if (!priors.empty() && prior.lux <= priors.back().lux) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: Prior must be ordered in increasing lux value";<br>
+ return -EINVAL;<br>
+ }<br>
priors.push_back(prior);<br>
}<br>
- if (priors.empty())<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: no AWB priors configured";<br>
+ if (priors.empty()) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: no AWB priors configured";<br>
+ return ret;<br>
+ }<br>
}<br>
if (params.get_child_optional("modes")) {<br>
for (auto &p : params.get_child("modes")) {<br>
@@ -93,8 +103,10 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms)<br>
if (defaultMode == nullptr)<br>
defaultMode = &modes[p.first];<br>
}<br>
- if (defaultMode == nullptr)<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: no AWB modes configured";<br>
+ if (defaultMode == nullptr) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: no AWB modes configured";<br>
+ return -EINVAL;<br>
+ }<br>
}<br>
minPixels = params.get<double>("min_pixels", 16.0);<br>
minG = params.get<uint16_t>("min_G", 32);<br>
@@ -103,8 +115,10 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms)<br>
coarseStep = params.get<double>("coarse_step", 0.2);<br>
transversePos = params.get<double>("transverse_pos", 0.01);<br>
transverseNeg = params.get<double>("transverse_neg", 0.01);<br>
- if (transversePos <= 0 || transverseNeg <= 0)<br>
- LOG(RPiAwb, Fatal) << "AwbConfig: transverse_pos/neg must be > 0";<br>
+ if (transversePos <= 0 || transverseNeg <= 0) {<br>
+ LOG(RPiAwb, Error) << "AwbConfig: transverse_pos/neg must be > 0";<br>
+ return -EINVAL;<br>
+ }<br>
sensitivityR = params.get<double>("sensitivity_r", 1.0);<br>
sensitivityB = params.get<double>("sensitivity_b", 1.0);<br>
if (bayes) {<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp b/src/ipa/raspberrypi/controller/rpi/ccm.cpp<br>
index f0110d38f548..9588e94adeb1 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp<br>
@@ -44,12 +44,16 @@ int Matrix::read(boost::property_tree::ptree const ¶ms)<br>
double *ptr = (double *)m;<br>
int n = 0;<br>
for (auto it = params.begin(); it != params.end(); it++) {<br>
- if (n++ == 9)<br>
- LOG(RPiCcm, Fatal) << "Ccm: too many values in CCM";<br>
+ if (n++ == 9) {<br>
+ LOG(RPiCcm, Error) << "Too many values in CCM";<br>
+ return -EINVAL;<br>
+ }<br>
*ptr++ = it->second.get_value<double>();<br>
}<br>
- if (n < 9)<br>
- LOG(RPiCcm, Fatal) << "Ccm: too few values in CCM";<br>
+ if (n < 9) {<br>
+ LOG(RPiCcm, Error) << "Too few values in CCM";<br>
+ return -EINVAL;<br>
+ }<br>
return 0;<br>
}<br>
<br>
@@ -78,13 +82,17 @@ int Ccm::read(boost::property_tree::ptree const ¶ms)<br>
if (ret)<br>
return ret;<br>
if (!config_.ccms.empty() &&<br>
- ctCcm.ct <= config_.ccms.back().ct)<br>
- LOG(RPiCcm, Fatal) << "Ccm: CCM not in increasing colour temperature order";<br>
+ ctCcm.ct <= config_.ccms.back().ct) {<br>
+ LOG(RPiCcm, Error) << "CCM not in increasing colour temperature order";<br>
+ return -EINVAL;<br>
+ }<br>
config_.ccms.push_back(std::move(ctCcm));<br>
}<br>
<br>
- if (config_.ccms.empty())<br>
- LOG(RPiCcm, Fatal) << "Ccm: no CCMs specified";<br>
+ if (config_.ccms.empty()) {<br>
+ LOG(RPiCcm, Error) << "No CCMs specified";<br>
+ return -EINVAL;<br>
+ }<br>
<br>
return 0;<br>
}<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp b/src/ipa/raspberrypi/controller/rpi/dpc.cpp<br>
index be014a05fd41..def39bb7416a 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp<br>
@@ -34,8 +34,10 @@ char const *Dpc::name() const<br>
int Dpc::read(boost::property_tree::ptree const ¶ms)<br>
{<br>
config_.strength = params.get<int>("strength", 1);<br>
- if (config_.strength < 0 || config_.strength > 2)<br>
- LOG(RPiDpc, Fatal) << "Dpc: bad strength value";<br>
+ if (config_.strength < 0 || config_.strength > 2) {<br>
+ LOG(RPiDpc, Error) << "bad strength value";<br>
+ return -EINVAL;<br>
+ }<br>
return 0;<br>
}<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp b/src/ipa/raspberrypi/controller/rpi/geq.cpp<br>
index a74447877bf8..106f0a40dfc1 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/geq.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp<br>
@@ -39,8 +39,10 @@ int Geq::read(boost::property_tree::ptree const ¶ms)<br>
{<br>
config_.offset = params.get<uint16_t>("offset", 0);<br>
config_.slope = params.get<double>("slope", 0.0);<br>
- if (config_.slope < 0.0 || config_.slope >= 1.0)<br>
- LOG(RPiGeq, Fatal) << "Geq: bad slope value";<br>
+ if (config_.slope < 0.0 || config_.slope >= 1.0) {<br>
+ LOG(RPiGeq, Error) << "Bad slope value";<br>
+ return -EINVAL;<br>
+ }<br>
<br>
if (params.get_child_optional("strength")) {<br>
int ret = config_.strength.read(params.get_child("strength"));<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>