From patchwork Wed Jul 27 02:38:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 16819 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C4DBEBE173 for ; Wed, 27 Jul 2022 02:38:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 523656332F; Wed, 27 Jul 2022 04:38:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1658889511; bh=B2W9uXKQJikhxUM8xy+8cNGQA6jiCxgAksDwLjCzH6g=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=1Q1PJ8Cc/Ef5GHFZ1dbst4tdMhXCDEHP2pO9jpz0osIZxNg4mP2LdVPXXT4Ov9vzS hiz7OJEGeSoCi/rUVitdg5VNMGzlweWxn+evnUBmsce/6CilNpR9phQACs7h++Hqgl lfutax0iVRUttlZ44x8Q6snA9io84kPPOIs4w2aJfucZOJytkPxQpOz4Ig61F8/yHB NgISyxXjBm+GSZlRTvd1TeZuNk1t0eGXq/miLuQF836qh0Wt/ma73Jx0FF9yAVXdpb sWkhcK9oY/u5INWl1b82mIlcWQDXBZsWbOKPBSfjqgaIJLIzusPdlqyeJ7t/Te0hBQ QzWW6ubIZDZHw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AD0563325 for ; Wed, 27 Jul 2022 04:38:26 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="N9hdvLBb"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D8A0856D; Wed, 27 Jul 2022 04:38:25 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1658889506; bh=B2W9uXKQJikhxUM8xy+8cNGQA6jiCxgAksDwLjCzH6g=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=N9hdvLBb9Dti+PiPMjaQ2MpFysMahlrUKZkSaxlez379oLd5H+UsJHJ/OdK44/+12 YXIvvJmUkrEwJYtGGpTjA/26RndZMZ/trST/hqzmnRJb4DT89kEWhJybiafoaKF7ux RGmPNMVBius+oO83taKlhYGr5EVneX1Fl+CmmxW8= To: libcamera-devel@lists.libcamera.org Date: Wed, 27 Jul 2022 05:38:09 +0300 Message-Id: <20220727023816.30008-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220727023816.30008-1-laurent.pinchart@ideasonboard.com> References: <20220727023816.30008-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 07/14] ipa: raspberrypi: Replace Fatal log by error propagation X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Replace the Fatal log messages that cause an abort during tuning data read with Error messages and proper error propagation to the caller. Signed-off-by: Laurent Pinchart Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/controller/rpi/agc.cpp | 36 ++++++++++---- src/ipa/raspberrypi/controller/rpi/alsc.cpp | 52 ++++++++++++++------- src/ipa/raspberrypi/controller/rpi/awb.cpp | 42 +++++++++++------ src/ipa/raspberrypi/controller/rpi/ccm.cpp | 24 ++++++---- src/ipa/raspberrypi/controller/rpi/dpc.cpp | 6 ++- src/ipa/raspberrypi/controller/rpi/geq.cpp | 6 ++- 6 files changed, 113 insertions(+), 53 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index 93f966a1d5ce..153493f8b8e2 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -34,13 +34,20 @@ static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit pipelin int AgcMeteringMode::read(boost::property_tree::ptree const ¶ms) { int num = 0; + for (auto &p : params.get_child("weights")) { - if (num == AgcStatsSize) - LOG(RPiAgc, Fatal) << "AgcMeteringMode: too many weights"; + if (num == AgcStatsSize) { + LOG(RPiAgc, Error) << "AgcMeteringMode: too many weights"; + return -EINVAL; + } weights[num++] = p.second.get_value(); } - if (num != AgcStatsSize) - LOG(RPiAgc, Fatal) << "AgcMeteringMode: insufficient weights"; + + if (num != AgcStatsSize) { + LOG(RPiAgc, Error) << "AgcMeteringMode: insufficient weights"; + return -EINVAL; + } + return 0; } @@ -85,12 +92,19 @@ int AgcExposureMode::read(boost::property_tree::ptree const ¶ms) { int numShutters = readList(shutter, params.get_child("shutter")); int numAgs = readList(gain, params.get_child("gain")); - if (numShutters < 2 || numAgs < 2) - LOG(RPiAgc, Fatal) + + if (numShutters < 2 || numAgs < 2) { + LOG(RPiAgc, Error) << "AgcExposureMode: must have at least two entries in exposure profile"; - if (numShutters != numAgs) - LOG(RPiAgc, Fatal) + return -EINVAL; + } + + if (numShutters != numAgs) { + LOG(RPiAgc, Error) << "AgcExposureMode: expect same number of exposure and gain entries in exposure profile"; + return -EINVAL; + } + return 0; } @@ -120,8 +134,10 @@ int AgcConstraint::read(boost::property_tree::ptree const ¶ms) std::string boundString = params.get("bound", ""); transform(boundString.begin(), boundString.end(), boundString.begin(), ::toupper); - if (boundString != "UPPER" && boundString != "LOWER") - LOG(RPiAgc, Fatal) << "AGC constraint type should be UPPER or LOWER"; + if (boundString != "UPPER" && boundString != "LOWER") { + LOG(RPiAgc, Error) << "AGC constraint type should be UPPER or LOWER"; + return -EINVAL; + } bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER; qLo = params.get("q_lo"); qHi = params.get("q_hi"); diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp index b36277696a52..49aaf6b74603 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp @@ -53,11 +53,17 @@ char const *Alsc::name() const static int generateLut(double *lut, boost::property_tree::ptree const ¶ms) { double cstrength = params.get("corner_strength", 2.0); - if (cstrength <= 1.0) - LOG(RPiAlsc, Fatal) << "Alsc: corner_strength must be > 1.0"; + if (cstrength <= 1.0) { + LOG(RPiAlsc, Error) << "corner_strength must be > 1.0"; + return -EINVAL; + } + double asymmetry = params.get("asymmetry", 1.0); - if (asymmetry < 0) - LOG(RPiAlsc, Fatal) << "Alsc: asymmetry must be >= 0"; + if (asymmetry < 0) { + LOG(RPiAlsc, Error) << "asymmetry must be >= 0"; + return -EINVAL; + } + double f1 = cstrength - 1, f2 = 1 + sqrt(cstrength); double R2 = X * Y / 4 * (1 + asymmetry * asymmetry); int num = 0; @@ -78,13 +84,19 @@ static int readLut(double *lut, boost::property_tree::ptree const ¶ms) { int num = 0; const int maxNum = XY; + for (auto &p : params) { - if (num == maxNum) - LOG(RPiAlsc, Fatal) << "Alsc: too many entries in LSC table"; + if (num == maxNum) { + LOG(RPiAlsc, Error) << "Too many entries in LSC table"; + return -EINVAL; + } lut[num++] = p.second.get_value(); } - if (num < maxNum) - LOG(RPiAlsc, Fatal) << "Alsc: too few entries in LSC table"; + + if (num < maxNum) { + LOG(RPiAlsc, Error) << "Too few entries in LSC table"; + return -EINVAL; + } return 0; } @@ -96,24 +108,30 @@ static int readCalibrations(std::vector &calibrations, double lastCt = 0; for (auto &p : params.get_child(name)) { double ct = p.second.get("ct"); - if (ct <= lastCt) - LOG(RPiAlsc, Fatal) - << "Alsc: entries in " << name << " must be in increasing ct order"; + if (ct <= lastCt) { + LOG(RPiAlsc, Error) + << "Entries in " << name << " must be in increasing ct order"; + return -EINVAL; + } AlscCalibration calibration; calibration.ct = lastCt = ct; boost::property_tree::ptree const &table = p.second.get_child("table"); int num = 0; for (auto it = table.begin(); it != table.end(); it++) { - if (num == XY) - LOG(RPiAlsc, Fatal) - << "Alsc: too many values for ct " << ct << " in " << name; + if (num == XY) { + LOG(RPiAlsc, Error) + << "Too many values for ct " << ct << " in " << name; + return -EINVAL; + } calibration.table[num++] = it->second.get_value(); } - if (num != XY) - LOG(RPiAlsc, Fatal) - << "Alsc: too few values for ct " << ct << " in " << name; + if (num != XY) { + LOG(RPiAlsc, Error) + << "Too few values for ct " << ct << " in " << name; + return -EINVAL; + } calibrations.push_back(calibration); LOG(RPiAlsc, Debug) << "Read " << name << " calibration for ct " << ct; diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp index a9e776a344a1..10c49c126341 100644 --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp @@ -46,16 +46,22 @@ static int readCtCurve(Pwl &ctR, Pwl &ctB, for (auto it = params.begin(); it != params.end(); it++) { double ct = it->second.get_value(); assert(it == params.begin() || ct != ctR.domain().end); - if (++it == params.end()) - LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry"; + if (++it == params.end()) { + LOG(RPiAwb, Error) << "AwbConfig: incomplete CT curve entry"; + return -EINVAL; + } ctR.append(ct, it->second.get_value()); - if (++it == params.end()) - LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry"; + if (++it == params.end()) { + LOG(RPiAwb, Error) << "AwbConfig: incomplete CT curve entry"; + return -EINVAL; + } ctB.append(ct, it->second.get_value()); num++; } - if (num < 2) - LOG(RPiAwb, Fatal) << "AwbConfig: insufficient points in CT curve"; + if (num < 2) { + LOG(RPiAwb, Error) << "AwbConfig: insufficient points in CT curve"; + return -EINVAL; + } return 0; } @@ -78,12 +84,16 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms) ret = prior.read(p.second); if (ret) return ret; - if (!priors.empty() && prior.lux <= priors.back().lux) - LOG(RPiAwb, Fatal) << "AwbConfig: Prior must be ordered in increasing lux value"; + if (!priors.empty() && prior.lux <= priors.back().lux) { + LOG(RPiAwb, Error) << "AwbConfig: Prior must be ordered in increasing lux value"; + return -EINVAL; + } priors.push_back(prior); } - if (priors.empty()) - LOG(RPiAwb, Fatal) << "AwbConfig: no AWB priors configured"; + if (priors.empty()) { + LOG(RPiAwb, Error) << "AwbConfig: no AWB priors configured"; + return ret; + } } if (params.get_child_optional("modes")) { for (auto &p : params.get_child("modes")) { @@ -93,8 +103,10 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms) if (defaultMode == nullptr) defaultMode = &modes[p.first]; } - if (defaultMode == nullptr) - LOG(RPiAwb, Fatal) << "AwbConfig: no AWB modes configured"; + if (defaultMode == nullptr) { + LOG(RPiAwb, Error) << "AwbConfig: no AWB modes configured"; + return -EINVAL; + } } minPixels = params.get("min_pixels", 16.0); minG = params.get("min_G", 32); @@ -103,8 +115,10 @@ int AwbConfig::read(boost::property_tree::ptree const ¶ms) coarseStep = params.get("coarse_step", 0.2); transversePos = params.get("transverse_pos", 0.01); transverseNeg = params.get("transverse_neg", 0.01); - if (transversePos <= 0 || transverseNeg <= 0) - LOG(RPiAwb, Fatal) << "AwbConfig: transverse_pos/neg must be > 0"; + if (transversePos <= 0 || transverseNeg <= 0) { + LOG(RPiAwb, Error) << "AwbConfig: transverse_pos/neg must be > 0"; + return -EINVAL; + } sensitivityR = params.get("sensitivity_r", 1.0); sensitivityB = params.get("sensitivity_b", 1.0); if (bayes) { diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp b/src/ipa/raspberrypi/controller/rpi/ccm.cpp index f0110d38f548..9588e94adeb1 100644 --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp @@ -44,12 +44,16 @@ int Matrix::read(boost::property_tree::ptree const ¶ms) double *ptr = (double *)m; int n = 0; for (auto it = params.begin(); it != params.end(); it++) { - if (n++ == 9) - LOG(RPiCcm, Fatal) << "Ccm: too many values in CCM"; + if (n++ == 9) { + LOG(RPiCcm, Error) << "Too many values in CCM"; + return -EINVAL; + } *ptr++ = it->second.get_value(); } - if (n < 9) - LOG(RPiCcm, Fatal) << "Ccm: too few values in CCM"; + if (n < 9) { + LOG(RPiCcm, Error) << "Too few values in CCM"; + return -EINVAL; + } return 0; } @@ -78,13 +82,17 @@ int Ccm::read(boost::property_tree::ptree const ¶ms) if (ret) return ret; if (!config_.ccms.empty() && - ctCcm.ct <= config_.ccms.back().ct) - LOG(RPiCcm, Fatal) << "Ccm: CCM not in increasing colour temperature order"; + ctCcm.ct <= config_.ccms.back().ct) { + LOG(RPiCcm, Error) << "CCM not in increasing colour temperature order"; + return -EINVAL; + } config_.ccms.push_back(std::move(ctCcm)); } - if (config_.ccms.empty()) - LOG(RPiCcm, Fatal) << "Ccm: no CCMs specified"; + if (config_.ccms.empty()) { + LOG(RPiCcm, Error) << "No CCMs specified"; + return -EINVAL; + } return 0; } diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp b/src/ipa/raspberrypi/controller/rpi/dpc.cpp index be014a05fd41..def39bb7416a 100644 --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp @@ -34,8 +34,10 @@ char const *Dpc::name() const int Dpc::read(boost::property_tree::ptree const ¶ms) { config_.strength = params.get("strength", 1); - if (config_.strength < 0 || config_.strength > 2) - LOG(RPiDpc, Fatal) << "Dpc: bad strength value"; + if (config_.strength < 0 || config_.strength > 2) { + LOG(RPiDpc, Error) << "bad strength value"; + return -EINVAL; + } return 0; } diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp b/src/ipa/raspberrypi/controller/rpi/geq.cpp index a74447877bf8..106f0a40dfc1 100644 --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp @@ -39,8 +39,10 @@ int Geq::read(boost::property_tree::ptree const ¶ms) { config_.offset = params.get("offset", 0); config_.slope = params.get("slope", 0.0); - if (config_.slope < 0.0 || config_.slope >= 1.0) - LOG(RPiGeq, Fatal) << "Geq: bad slope value"; + if (config_.slope < 0.0 || config_.slope >= 1.0) { + LOG(RPiGeq, Error) << "Bad slope value"; + return -EINVAL; + } if (params.get_child_optional("strength")) { int ret = config_.strength.read(params.get_child("strength"));