[libcamera-devel] [PATCH 15/15] ipa: raspberryip: Remove all exception throw statements
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 25 23:14:46 CEST 2022
Hi Naush,
Thank you for the patch.
On Mon, Jul 25, 2022 at 02:46:39PM +0100, Naushir Patuck via libcamera-devel wrote:
> Replace all exception throw statements with LOG(RPi*, Fatal) error messages.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 26 +++++++++------------
> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 25 ++++++++------------
> src/ipa/raspberrypi/controller/rpi/awb.cpp | 18 ++++++--------
> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 9 ++++---
> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 2 +-
> src/ipa/raspberrypi/controller/rpi/geq.cpp | 2 +-
> 6 files changed, 34 insertions(+), 48 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index 2c7928bbac4d..0f44ceb3feda 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -35,11 +35,11 @@ void AgcMeteringMode::read(boost::property_tree::ptree const ¶ms)
> int num = 0;
> for (auto &p : params.get_child("weights")) {
> if (num == AGC_STATS_SIZE)
> - throw std::runtime_error("AgcConfig: too many weights");
> + LOG(RPiAgc, Fatal) << "AgcConfig: too many weights";
A patch on top to s/AgcConfig/AgcMeteringMode/ would be nice.
> weights[num++] = p.second.get_value<double>();
> }
> if (num != AGC_STATS_SIZE)
> - throw std::runtime_error("AgcConfig: insufficient weights");
> + LOG(RPiAgc, Fatal) << "AgcConfig: insufficient weights";
> }
>
> static std::string
> @@ -78,11 +78,11 @@ void 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)
> - throw std::runtime_error(
> - "AgcConfig: must have at least two entries in exposure profile");
> + LOG(RPiAgc, Fatal)
> + << "AgcConfig: must have at least two entries in exposure profile";
> if (numShutters != numAgs)
> - throw std::runtime_error(
> - "AgcConfig: expect same number of exposure and gain entries in exposure profile");
> + LOG(RPiAgc, Fatal)
> + << "AgcConfig: expect same number of exposure and gain entries in exposure profile";
> }
>
> static std::string
> @@ -106,8 +106,7 @@ void AgcConstraint::read(boost::property_tree::ptree const ¶ms)
> transform(boundString.begin(), boundString.end(),
> boundString.begin(), ::toupper);
> if (boundString != "UPPER" && boundString != "LOWER")
> - throw std::runtime_error(
> - "AGC constraint type should be UPPER or LOWER");
> + LOG(RPiAgc, Fatal) << "AGC constraint type should be UPPER or LOWER";
> bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER;
> qLo = params.get<double>("q_lo");
> qHi = params.get<double>("q_hi");
> @@ -465,8 +464,7 @@ void Agc::housekeepConfig()
> if (strcmp(meteringModeName_.c_str(), status_.meteringMode)) {
> auto it = config_.meteringModes.find(meteringModeName_);
> if (it == config_.meteringModes.end())
> - throw std::runtime_error("Agc: no metering mode " +
> - meteringModeName_);
> + LOG(RPiAgc, Fatal) << "Agc: no metering mode " << meteringModeName_;
You can drop the "Agc:" prefix as the log category name will make it
clear. Same comment for messages below.
Most of the Fatal log statements occur at config read time. While it's
not nice to abort, there's no urgency to fix that. The calls that occur
at runtime bother me a bit more, it would be good to fix that (on top).
At the moment, if a configuration file doesn't define, for instance, a
matrix metering mode, I think an application will crash libcamera if it
tries to select it.
The problem predates this patch, so it's not a blocker to get this
merged. With the prefix issue addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> meteringMode_ = &it->second;
> copyString(meteringModeName_, status_.meteringMode,
> sizeof(status_.meteringMode));
> @@ -474,8 +472,7 @@ void Agc::housekeepConfig()
> if (strcmp(exposureModeName_.c_str(), status_.exposureMode)) {
> auto it = config_.exposureModes.find(exposureModeName_);
> if (it == config_.exposureModes.end())
> - throw std::runtime_error("Agc: no exposure profile " +
> - exposureModeName_);
> + LOG(RPiAgc, Fatal) << "Agc: no exposure profile " << exposureModeName_;
> exposureMode_ = &it->second;
> copyString(exposureModeName_, status_.exposureMode,
> sizeof(status_.exposureMode));
> @@ -484,8 +481,7 @@ void Agc::housekeepConfig()
> auto it =
> config_.constraintModes.find(constraintModeName_);
> if (it == config_.constraintModes.end())
> - throw std::runtime_error("Agc: no constraint list " +
> - constraintModeName_);
> + LOG(RPiAgc, Fatal) << "Agc: no constraint list " << constraintModeName_;
> constraintMode_ = &it->second;
> copyString(constraintModeName_, status_.constraint_mode,
> sizeof(status_.constraint_mode));
> @@ -502,7 +498,7 @@ void Agc::fetchCurrentExposure(Metadata *imageMetadata)
> DeviceStatus *deviceStatus =
> imageMetadata->getLocked<DeviceStatus>("device.status");
> if (!deviceStatus)
> - throw std::runtime_error("Agc: no device metadata");
> + LOG(RPiAgc, Fatal) << "Agc: no device metadata";
> current_.shutter = deviceStatus->shutterSpeed;
> current_.analogueGain = deviceStatus->analogueGain;
> AgcStatus *agcStatus =
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> index 7dd35aef10c4..f2111a7f63ec 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> @@ -54,10 +54,10 @@ static void generateLut(double *lut, boost::property_tree::ptree const ¶ms)
> {
> double cstrength = params.get<double>("corner_strength", 2.0);
> if (cstrength <= 1.0)
> - throw std::runtime_error("Alsc: corner_strength must be > 1.0");
> + LOG(RPiAlsc, Fatal) << "Alsc: corner_strength must be > 1.0";
> double asymmetry = params.get<double>("asymmetry", 1.0);
> if (asymmetry < 0)
> - throw std::runtime_error("Alsc: asymmetry must be >= 0");
> + LOG(RPiAlsc, Fatal) << "Alsc: asymmetry must be >= 0";
> double f1 = cstrength - 1, f2 = 1 + sqrt(cstrength);
> double R2 = X * Y / 4 * (1 + asymmetry * asymmetry);
> int num = 0;
> @@ -79,12 +79,11 @@ static void readLut(double *lut, boost::property_tree::ptree const ¶ms)
> const int maxNum = XY;
> for (auto &p : params) {
> if (num == maxNum)
> - throw std::runtime_error(
> - "Alsc: too many entries in LSC table");
> + LOG(RPiAlsc, Fatal) << "Alsc: too many entries in LSC table";
> lut[num++] = p.second.get_value<double>();
> }
> if (num < maxNum)
> - throw std::runtime_error("Alsc: too few entries in LSC table");
> + LOG(RPiAlsc, Fatal) << "Alsc: too few entries in LSC table";
> }
>
> static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> @@ -96,9 +95,8 @@ static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> for (auto &p : params.get_child(name)) {
> double ct = p.second.get<double>("ct");
> if (ct <= lastCt)
> - throw std::runtime_error(
> - "Alsc: entries in " + name +
> - " must be in increasing ct order");
> + LOG(RPiAlsc, Fatal)
> + << "Alsc: entries in " << name << " must be in increasing ct order";
> AlscCalibration calibration;
> calibration.ct = lastCt = ct;
> boost::property_tree::ptree const &table =
> @@ -106,17 +104,14 @@ static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> int num = 0;
> for (auto it = table.begin(); it != table.end(); it++) {
> if (num == XY)
> - throw std::runtime_error(
> - "Alsc: too many values for ct " +
> - std::to_string(ct) + " in " +
> - name);
> + LOG(RPiAlsc, Fatal)
> + << "Alsc: too many values for ct " << ct << " in " << name;
> calibration.table[num++] =
> it->second.get_value<double>();
> }
> if (num != XY)
> - throw std::runtime_error(
> - "Alsc: too few values for ct " +
> - std::to_string(ct) + " in " + name);
> + LOG(RPiAlsc, Fatal)
> + << "Alsc: too few values for ct " << ct << " in " << name;
> 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 5f601290ce72..aabfec6bd672 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -46,18 +46,15 @@ static void readCtCurve(Pwl &ctR, Pwl &ctB,
> double ct = it->second.get_value<double>();
> assert(it == params.begin() || ct != ctR.domain().end);
> if (++it == params.end())
> - throw std::runtime_error(
> - "AwbConfig: incomplete CT curve entry");
> + LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry";
> ctR.append(ct, it->second.get_value<double>());
> if (++it == params.end())
> - throw std::runtime_error(
> - "AwbConfig: incomplete CT curve entry");
> + LOG(RPiAwb, Fatal) << "AwbConfig: incomplete CT curve entry";
> ctB.append(ct, it->second.get_value<double>());
> num++;
> }
> if (num < 2)
> - throw std::runtime_error(
> - "AwbConfig: insufficient points in CT curve");
> + LOG(RPiAwb, Fatal) << "AwbConfig: insufficient points in CT curve";
> }
>
> void AwbConfig::read(boost::property_tree::ptree const ¶ms)
> @@ -74,12 +71,11 @@ void AwbConfig::read(boost::property_tree::ptree const ¶ms)
> AwbPrior prior;
> prior.read(p.second);
> if (!priors.empty() && prior.lux <= priors.back().lux)
> - throw std::runtime_error("AwbConfig: Prior must be ordered in increasing lux value");
> + LOG(RPiAwb, Fatal) << "AwbConfig: Prior must be ordered in increasing lux value";
> priors.push_back(prior);
> }
> if (priors.empty())
> - throw std::runtime_error(
> - "AwbConfig: no AWB priors configured");
> + LOG(RPiAwb, Fatal) << "AwbConfig: no AWB priors configured";
> }
> if (params.get_child_optional("modes")) {
> for (auto &p : params.get_child("modes")) {
> @@ -88,7 +84,7 @@ void AwbConfig::read(boost::property_tree::ptree const ¶ms)
> defaultMode = &modes[p.first];
> }
> if (defaultMode == nullptr)
> - throw std::runtime_error("AwbConfig: no AWB modes configured");
> + LOG(RPiAwb, Fatal) << "AwbConfig: no AWB modes configured";
> }
> minPixels = params.get<double>("min_pixels", 16.0);
> minG = params.get<uint16_t>("min_G", 32);
> @@ -98,7 +94,7 @@ void AwbConfig::read(boost::property_tree::ptree const ¶ms)
> transversePos = params.get<double>("transverse_pos", 0.01);
> transverseNeg = params.get<double>("transverse_neg", 0.01);
> if (transversePos <= 0 || transverseNeg <= 0)
> - throw std::runtime_error("AwbConfig: transverse_pos/neg must be > 0");
> + LOG(RPiAwb, Fatal) << "AwbConfig: transverse_pos/neg must be > 0";
> sensitivityR = params.get<double>("sensitivity_r", 1.0);
> sensitivityB = params.get<double>("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 8095c42dc0c8..cf0c85d26cf9 100644
> --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> @@ -45,11 +45,11 @@ void Matrix::read(boost::property_tree::ptree const ¶ms)
> int n = 0;
> for (auto it = params.begin(); it != params.end(); it++) {
> if (n++ == 9)
> - throw std::runtime_error("Ccm: too many values in CCM");
> + LOG(RPiCcm, Fatal) << "Ccm: too many values in CCM";
> *ptr++ = it->second.get_value<double>();
> }
> if (n < 9)
> - throw std::runtime_error("Ccm: too few values in CCM");
> + LOG(RPiCcm, Fatal) << "Ccm: too few values in CCM";
> }
>
> Ccm::Ccm(Controller *controller)
> @@ -70,12 +70,11 @@ void Ccm::read(boost::property_tree::ptree const ¶ms)
> ctCcm.ccm.read(p.second.get_child("ccm"));
> if (!config_.ccms.empty() &&
> ctCcm.ct <= config_.ccms.back().ct)
> - throw std::runtime_error(
> - "Ccm: CCM not in increasing colour temperature order");
> + LOG(RPiCcm, Fatal) << "Ccm: CCM not in increasing colour temperature order";
> config_.ccms.push_back(std::move(ctCcm));
> }
> if (config_.ccms.empty())
> - throw std::runtime_error("Ccm: no CCMs specified");
> + LOG(RPiCcm, Fatal) << "Ccm: no CCMs specified";
> }
>
> void Ccm::setSaturation(double saturation)
> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> index 9cd78abe28d6..d5d784e7ca64 100644
> --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> @@ -35,7 +35,7 @@ void Dpc::read(boost::property_tree::ptree const ¶ms)
> {
> config_.strength = params.get<int>("strength", 1);
> if (config_.strength < 0 || config_.strength > 2)
> - throw std::runtime_error("Dpc: bad strength value");
> + LOG(RPiDpc, Fatal) << "Dpc: bad strength value";
> }
>
> void Dpc::prepare(Metadata *imageMetadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> index 4e8ed696851b..696da4aeea59 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> @@ -40,7 +40,7 @@ void Geq::read(boost::property_tree::ptree const ¶ms)
> config_.offset = params.get<uint16_t>("offset", 0);
> config_.slope = params.get<double>("slope", 0.0);
> if (config_.slope < 0.0 || config_.slope >= 1.0)
> - throw std::runtime_error("Geq: bad slope value");
> + LOG(RPiGeq, Fatal) << "Geq: bad slope value";
> if (params.get_child_optional("strength"))
> config_.strength.read(params.get_child("strength"));
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list