[{"id":24182,"web_url":"https://patchwork.libcamera.org/comment/24182/","msgid":"<CAEmqJPoiJO73YXY_BV7Ue4D8f+d3Ci6mEQgY4DnynWvtKi6XDA@mail.gmail.com>","date":"2022-07-27T10:08:32","subject":"Re: [libcamera-devel] [PATCH v7 05/14] ipa: raspberrypi: Return an\n\terror code from Algorithm::read()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your patch.\n\nOn Wed, 27 Jul 2022 at 03:38, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> When encountering errors, the Algorithm::read() function either uses\n> LOG(Fatal) or throws exceptions from the boost property_tree functions.\n> To prepare for replacing boost JSON parse with the YamlParser class,\n> give the Algorithm::read() function the ability to return an error code,\n> and propagate it all the way to the IPA module init() function.\n>\n> All algorithm classes return a hardcoded 0 value for now, subsequent\n> commits will change that.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/ipa/raspberrypi/controller/algorithm.cpp  |  3 +-\n>  src/ipa/raspberrypi/controller/algorithm.h    |  2 +-\n>  src/ipa/raspberrypi/controller/controller.cpp |  8 +++-\n>  src/ipa/raspberrypi/controller/controller.h   |  2 +-\n>  src/ipa/raspberrypi/controller/pwl.cpp        |  3 +-\n>  src/ipa/raspberrypi/controller/pwl.h          |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 28 ++++++++++----\n>  src/ipa/raspberrypi/controller/rpi/agc.h      | 10 ++---\n>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 38 +++++++++++++------\n>  src/ipa/raspberrypi/controller/rpi/alsc.h     |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/awb.cpp    | 35 +++++++++++------\n>  src/ipa/raspberrypi/controller/rpi/awb.h      |  8 ++--\n>  .../controller/rpi/black_level.cpp            |  3 +-\n>  .../raspberrypi/controller/rpi/black_level.h  |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/ccm.cpp    | 22 ++++++++---\n>  src/ipa/raspberrypi/controller/rpi/ccm.h      |  4 +-\n>  .../raspberrypi/controller/rpi/contrast.cpp   |  4 +-\n>  src/ipa/raspberrypi/controller/rpi/contrast.h |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/dpc.h      |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/geq.cpp    | 12 ++++--\n>  src/ipa/raspberrypi/controller/rpi/geq.h      |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/lux.cpp    |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/lux.h      |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/noise.cpp  |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/noise.h    |  2 +-\n>  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/sdn.h      |  2 +-\n>  .../raspberrypi/controller/rpi/sharpen.cpp    |  3 +-\n>  src/ipa/raspberrypi/controller/rpi/sharpen.h  |  2 +-\n>  src/ipa/raspberrypi/raspberrypi.cpp           |  9 ++++-\n>  31 files changed, 151 insertions(+), 75 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp\n> b/src/ipa/raspberrypi/controller/algorithm.cpp\n> index 1a7d20a4731b..d73cb36fe2d6 100644\n> --- a/src/ipa/raspberrypi/controller/algorithm.cpp\n> +++ b/src/ipa/raspberrypi/controller/algorithm.cpp\n> @@ -9,8 +9,9 @@\n>\n>  using namespace RPiController;\n>\n> -void Algorithm::read([[maybe_unused]] boost::property_tree::ptree const\n> &params)\n> +int Algorithm::read([[maybe_unused]] boost::property_tree::ptree const\n> &params)\n>  {\n> +       return 0;\n>  }\n>\n>  void Algorithm::initialise()\n> diff --git a/src/ipa/raspberrypi/controller/algorithm.h\n> b/src/ipa/raspberrypi/controller/algorithm.h\n> index 92fd895d2b06..0c5566fda874 100644\n> --- a/src/ipa/raspberrypi/controller/algorithm.h\n> +++ b/src/ipa/raspberrypi/controller/algorithm.h\n> @@ -35,7 +35,7 @@ public:\n>         virtual bool isPaused() const { return paused_; }\n>         virtual void pause() { paused_ = true; }\n>         virtual void resume() { paused_ = false; }\n> -       virtual void read(boost::property_tree::ptree const &params);\n> +       virtual int read(boost::property_tree::ptree const &params);\n>         virtual void initialise();\n>         virtual void switchMode(CameraMode const &cameraMode, Metadata\n> *metadata);\n>         virtual void prepare(Metadata *imageMetadata);\n> diff --git a/src/ipa/raspberrypi/controller/controller.cpp\n> b/src/ipa/raspberrypi/controller/controller.cpp\n> index 872a32302410..d91ac90704cb 100644\n> --- a/src/ipa/raspberrypi/controller/controller.cpp\n> +++ b/src/ipa/raspberrypi/controller/controller.cpp\n> @@ -32,19 +32,23 @@ Controller::Controller(char const *jsonFilename)\n>\n>  Controller::~Controller() {}\n>\n> -void Controller::read(char const *filename)\n> +int Controller::read(char const *filename)\n>\n\nSince read now returns an error code, we probably ought to remove the\nController(char const *jsonFilename) constructor.  This never gets used\nanyway.  But that can be done on top of this work.\n\nReviewed-by: Naushir Patuck <naush@raspberrypi.com>\n\n\n\n>  {\n>         boost::property_tree::ptree root;\n>         boost::property_tree::read_json(filename, root);\n>         for (auto const &keyAndValue : root) {\n>                 Algorithm *algo =\n> createAlgorithm(keyAndValue.first.c_str());\n>                 if (algo) {\n> -                       algo->read(keyAndValue.second);\n> +                       int ret = algo->read(keyAndValue.second);\n> +                       if (ret)\n> +                               return ret;\n>                         algorithms_.push_back(AlgorithmPtr(algo));\n>                 } else\n>                         LOG(RPiController, Warning)\n>                                 << \"No algorithm found for \\\"\" <<\n> keyAndValue.first << \"\\\"\";\n>         }\n> +\n> +       return 0;\n>  }\n>\n>  Algorithm *Controller::createAlgorithm(char const *name)\n> diff --git a/src/ipa/raspberrypi/controller/controller.h\n> b/src/ipa/raspberrypi/controller/controller.h\n> index e28e30d7d574..841783bb7a5c 100644\n> --- a/src/ipa/raspberrypi/controller/controller.h\n> +++ b/src/ipa/raspberrypi/controller/controller.h\n> @@ -41,7 +41,7 @@ public:\n>         Controller(char const *jsonFilename);\n>         ~Controller();\n>         Algorithm *createAlgorithm(char const *name);\n> -       void read(char const *filename);\n> +       int read(char const *filename);\n>         void initialise();\n>         void switchMode(CameraMode const &cameraMode, Metadata *metadata);\n>         void prepare(Metadata *imageMetadata);\n> diff --git a/src/ipa/raspberrypi/controller/pwl.cpp\n> b/src/ipa/raspberrypi/controller/pwl.cpp\n> index 8b8db722966b..fde0b298c6ce 100644\n> --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> @@ -12,7 +12,7 @@\n>\n>  using namespace RPiController;\n>\n> -void Pwl::read(boost::property_tree::ptree const &params)\n> +int Pwl::read(boost::property_tree::ptree const &params)\n>  {\n>         for (auto it = params.begin(); it != params.end(); it++) {\n>                 double x = it->second.get_value<double>();\n> @@ -22,6 +22,7 @@ void Pwl::read(boost::property_tree::ptree const &params)\n>                 points_.push_back(Point(x, y));\n>         }\n>         assert(points_.size() >= 2);\n> +       return 0;\n>  }\n>\n>  void Pwl::append(double x, double y, const double eps)\n> diff --git a/src/ipa/raspberrypi/controller/pwl.h\n> b/src/ipa/raspberrypi/controller/pwl.h\n> index 973efdf59945..ef1cc2ed113a 100644\n> --- a/src/ipa/raspberrypi/controller/pwl.h\n> +++ b/src/ipa/raspberrypi/controller/pwl.h\n> @@ -57,7 +57,7 @@ public:\n>         };\n>         Pwl() {}\n>         Pwl(std::vector<Point> const &points) : points_(points) {}\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>         void append(double x, double y, const double eps = 1e-6);\n>         void prepend(double x, double y, const double eps = 1e-6);\n>         Interval domain() const;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index adec8592626d..130c606d4136 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -30,7 +30,7 @@ LOG_DEFINE_CATEGORY(RPiAgc)\n>\n>  static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit\n> pipeline */\n>\n> -void AgcMeteringMode::read(boost::property_tree::ptree const &params)\n> +int AgcMeteringMode::read(boost::property_tree::ptree const &params)\n>  {\n>         int num = 0;\n>         for (auto &p : params.get_child(\"weights\")) {\n> @@ -40,6 +40,7 @@ void AgcMeteringMode::read(boost::property_tree::ptree\n> const &params)\n>         }\n>         if (num != AgcStatsSize)\n>                 LOG(RPiAgc, Fatal) << \"AgcMeteringMode: insufficient\n> weights\";\n> +       return 0;\n>  }\n>\n>  static std::string\n> @@ -73,7 +74,7 @@ static int readList(std::vector<Duration> &list,\n>         return list.size();\n>  }\n>\n> -void AgcExposureMode::read(boost::property_tree::ptree const &params)\n> +int AgcExposureMode::read(boost::property_tree::ptree const &params)\n>  {\n>         int numShutters = readList(shutter, params.get_child(\"shutter\"));\n>         int numAgs = readList(gain, params.get_child(\"gain\"));\n> @@ -83,6 +84,7 @@ void AgcExposureMode::read(boost::property_tree::ptree\n> const &params)\n>         if (numShutters != numAgs)\n>                 LOG(RPiAgc, Fatal)\n>                         << \"AgcExposureMode: expect same number of\n> exposure and gain entries in exposure profile\";\n> +       return 0;\n>  }\n>\n>  static std::string\n> @@ -100,7 +102,7 @@ readExposureModes(std::map<std::string,\n> AgcExposureMode> &exposureModes,\n>         return first;\n>  }\n>\n> -void AgcConstraint::read(boost::property_tree::ptree const &params)\n> +int AgcConstraint::read(boost::property_tree::ptree const &params)\n>  {\n>         std::string boundString = params.get<std::string>(\"bound\", \"\");\n>         transform(boundString.begin(), boundString.end(),\n> @@ -110,7 +112,7 @@ void AgcConstraint::read(boost::property_tree::ptree\n> const &params)\n>         bound = boundString == \"UPPER\" ? Bound::UPPER : Bound::LOWER;\n>         qLo = params.get<double>(\"q_lo\");\n>         qHi = params.get<double>(\"q_hi\");\n> -       yTarget.read(params.get_child(\"y_target\"));\n> +       return yTarget.read(params.get_child(\"y_target\"));\n>  }\n>\n>  static AgcConstraintMode\n> @@ -137,13 +139,17 @@ static std::string\n> readConstraintModes(std::map<std::string, AgcConstraintMode>\n>         return first;\n>  }\n>\n> -void AgcConfig::read(boost::property_tree::ptree const &params)\n> +int AgcConfig::read(boost::property_tree::ptree const &params)\n>  {\n>         LOG(RPiAgc, Debug) << \"AgcConfig\";\n>         defaultMeteringMode = readMeteringModes(meteringModes,\n> params.get_child(\"metering_modes\"));\n>         defaultExposureMode = readExposureModes(exposureModes,\n> params.get_child(\"exposure_modes\"));\n>         defaultConstraintMode = readConstraintModes(constraintModes,\n> params.get_child(\"constraint_modes\"));\n> -       yTarget.read(params.get_child(\"y_target\"));\n> +\n> +       int ret = yTarget.read(params.get_child(\"y_target\"));\n> +       if (ret)\n> +               return ret;\n> +\n>         speed = params.get<double>(\"speed\", 0.2);\n>         startupFrames = params.get<uint16_t>(\"startup_frames\", 10);\n>         convergenceFrames = params.get<unsigned int>(\"convergence_frames\",\n> 6);\n> @@ -152,6 +158,7 @@ void AgcConfig::read(boost::property_tree::ptree const\n> &params)\n>         /* Start with quite a low value as ramping up is easier than\n> ramping down. */\n>         defaultExposureTime = params.get<double>(\"default_exposure_time\",\n> 1000) * 1us;\n>         defaultAnalogueGain = params.get<double>(\"default_analogueGain\",\n> 1.0);\n> +       return 0;\n>  }\n>\n>  Agc::ExposureValues::ExposureValues()\n> @@ -182,10 +189,14 @@ char const *Agc::name() const\n>         return NAME;\n>  }\n>\n> -void Agc::read(boost::property_tree::ptree const &params)\n> +int Agc::read(boost::property_tree::ptree const &params)\n>  {\n>         LOG(RPiAgc, Debug) << \"Agc\";\n> -       config_.read(params);\n> +\n> +       int ret = config_.read(params);\n> +       if (ret)\n> +               return ret;\n> +\n>         /*\n>          * Set the config's defaults (which are the first ones it read) as\n> our\n>          * current modes, until someone changes them.  (they're all known\n> to\n> @@ -200,6 +211,7 @@ void Agc::read(boost::property_tree::ptree const\n> &params)\n>         /* Set up the \"last shutter/gain\" values, in case AGC starts\n> \"disabled\". */\n>         status_.shutterTime = config_.defaultExposureTime;\n>         status_.analogueGain = config_.defaultAnalogueGain;\n> +       return 0;\n>  }\n>\n>  bool Agc::isPaused() const\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h\n> b/src/ipa/raspberrypi/controller/rpi/agc.h\n> index f57afa6dc80c..1351b0ee079c 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> @@ -28,13 +28,13 @@ namespace RPiController {\n>\n>  struct AgcMeteringMode {\n>         double weights[AgcStatsSize];\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>  };\n>\n>  struct AgcExposureMode {\n>         std::vector<libcamera::utils::Duration> shutter;\n>         std::vector<double> gain;\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>  };\n>\n>  struct AgcConstraint {\n> @@ -43,13 +43,13 @@ struct AgcConstraint {\n>         double qLo;\n>         double qHi;\n>         Pwl yTarget;\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>  };\n>\n>  typedef std::vector<AgcConstraint> AgcConstraintMode;\n>\n>  struct AgcConfig {\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>         std::map<std::string, AgcMeteringMode> meteringModes;\n>         std::map<std::string, AgcExposureMode> exposureModes;\n>         std::map<std::string, AgcConstraintMode> constraintModes;\n> @@ -74,7 +74,7 @@ class Agc : public AgcAlgorithm\n>  public:\n>         Agc(Controller *controller);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         /* AGC handles \"pausing\" for itself. */\n>         bool isPaused() const override;\n>         void pause() override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> index 03ae33501dc0..b36277696a52 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> @@ -50,7 +50,7 @@ char const *Alsc::name() const\n>         return NAME;\n>  }\n>\n> -static void generateLut(double *lut, boost::property_tree::ptree const\n> &params)\n> +static int generateLut(double *lut, boost::property_tree::ptree const\n> &params)\n>  {\n>         double cstrength = params.get<double>(\"corner_strength\", 2.0);\n>         if (cstrength <= 1.0)\n> @@ -71,9 +71,10 @@ static void generateLut(double *lut,\n> boost::property_tree::ptree const &params)\n>                                 (f2 * f2); /* this reproduces the cos^4\n> rule */\n>                 }\n>         }\n> +       return 0;\n>  }\n>\n> -static void readLut(double *lut, boost::property_tree::ptree const\n> &params)\n> +static int readLut(double *lut, boost::property_tree::ptree const &params)\n>  {\n>         int num = 0;\n>         const int maxNum = XY;\n> @@ -84,11 +85,12 @@ static void readLut(double *lut,\n> boost::property_tree::ptree const &params)\n>         }\n>         if (num < maxNum)\n>                 LOG(RPiAlsc, Fatal) << \"Alsc: too few entries in LSC\n> table\";\n> +       return 0;\n>  }\n>\n> -static void readCalibrations(std::vector<AlscCalibration> &calibrations,\n> -                            boost::property_tree::ptree const &params,\n> -                            std::string const &name)\n> +static int readCalibrations(std::vector<AlscCalibration> &calibrations,\n> +                           boost::property_tree::ptree const &params,\n> +                           std::string const &name)\n>  {\n>         if (params.get_child_optional(name)) {\n>                 double lastCt = 0;\n> @@ -117,9 +119,10 @@ static void\n> readCalibrations(std::vector<AlscCalibration> &calibrations,\n>                                 << \"Read \" << name << \" calibration for ct\n> \" << ct;\n>                 }\n>         }\n> +       return 0;\n>  }\n>\n> -void Alsc::read(boost::property_tree::ptree const &params)\n> +int Alsc::read(boost::property_tree::ptree const &params)\n>  {\n>         config_.framePeriod = params.get<uint16_t>(\"frame_period\", 12);\n>         config_.startupFrames = params.get<uint16_t>(\"startup_frames\", 10);\n> @@ -135,19 +138,32 @@ void Alsc::read(boost::property_tree::ptree const\n> &params)\n>                 params.get<double>(\"luminance_strength\", 1.0);\n>         for (int i = 0; i < XY; i++)\n>                 config_.luminanceLut[i] = 1.0;\n> +\n> +       int ret = 0;\n> +\n>         if (params.get_child_optional(\"corner_strength\"))\n> -               generateLut(config_.luminanceLut, params);\n> +               ret = generateLut(config_.luminanceLut, params);\n>         else if (params.get_child_optional(\"luminance_lut\"))\n> -               readLut(config_.luminanceLut,\n> -                       params.get_child(\"luminance_lut\"));\n> +               ret = readLut(config_.luminanceLut,\n> +                             params.get_child(\"luminance_lut\"));\n>         else\n>                 LOG(RPiAlsc, Warning)\n>                         << \"no luminance table - assume unity everywhere\";\n> -       readCalibrations(config_.calibrationsCr, params,\n> \"calibrations_Cr\");\n> -       readCalibrations(config_.calibrationsCb, params,\n> \"calibrations_Cb\");\n> +       if (ret)\n> +               return ret;\n> +\n> +       ret = readCalibrations(config_.calibrationsCr, params,\n> \"calibrations_Cr\");\n> +       if (ret)\n> +               return ret;\n> +       ret = readCalibrations(config_.calibrationsCb, params,\n> \"calibrations_Cb\");\n> +       if (ret)\n> +               return ret;\n> +\n>         config_.defaultCt = params.get<double>(\"default_ct\", 4500.0);\n>         config_.threshold = params.get<double>(\"threshold\", 1e-3);\n>         config_.lambdaBound = params.get<double>(\"lambda_bound\", 0.05);\n> +\n> +       return 0;\n>  }\n>\n>  static double getCt(Metadata *metadata, double defaultCt);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h\n> b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> index 4e9a715ae0ab..773fd338ea8e 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> @@ -52,7 +52,7 @@ public:\n>         char const *name() const override;\n>         void initialise() override;\n>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>         void process(StatisticsPtr &stats, Metadata *imageMetadata)\n> override;\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> index ad75d55f0976..a9e776a344a1 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> @@ -26,20 +26,21 @@ static constexpr unsigned int AwbStatsSizeY =\n> DEFAULT_AWB_REGIONS_Y;\n>   * elsewhere (ALSC and AGC).\n>   */\n>\n> -void AwbMode::read(boost::property_tree::ptree const &params)\n> +int AwbMode::read(boost::property_tree::ptree const &params)\n>  {\n>         ctLo = params.get<double>(\"lo\");\n>         ctHi = params.get<double>(\"hi\");\n> +       return 0;\n>  }\n>\n> -void AwbPrior::read(boost::property_tree::ptree const &params)\n> +int AwbPrior::read(boost::property_tree::ptree const &params)\n>  {\n>         lux = params.get<double>(\"lux\");\n> -       prior.read(params.get_child(\"prior\"));\n> +       return prior.read(params.get_child(\"prior\"));\n>  }\n>\n> -static void readCtCurve(Pwl &ctR, Pwl &ctB,\n> -                       boost::property_tree::ptree const &params)\n> +static int readCtCurve(Pwl &ctR, Pwl &ctB,\n> +                      boost::property_tree::ptree const &params)\n>  {\n>         int num = 0;\n>         for (auto it = params.begin(); it != params.end(); it++) {\n> @@ -55,21 +56,28 @@ static void readCtCurve(Pwl &ctR, Pwl &ctB,\n>         }\n>         if (num < 2)\n>                 LOG(RPiAwb, Fatal) << \"AwbConfig: insufficient points in\n> CT curve\";\n> +       return 0;\n>  }\n>\n> -void AwbConfig::read(boost::property_tree::ptree const &params)\n> +int AwbConfig::read(boost::property_tree::ptree const &params)\n>  {\n> +       int ret;\n>         bayes = params.get<int>(\"bayes\", 1);\n>         framePeriod = params.get<uint16_t>(\"framePeriod\", 10);\n>         startupFrames = params.get<uint16_t>(\"startupFrames\", 10);\n>         convergenceFrames = params.get<unsigned int>(\"convergence_frames\",\n> 3);\n>         speed = params.get<double>(\"speed\", 0.05);\n> -       if (params.get_child_optional(\"ct_curve\"))\n> -               readCtCurve(ctR, ctB, params.get_child(\"ct_curve\"));\n> +       if (params.get_child_optional(\"ct_curve\")) {\n> +               ret = readCtCurve(ctR, ctB, params.get_child(\"ct_curve\"));\n> +               if (ret)\n> +                       return ret;\n> +       }\n>         if (params.get_child_optional(\"priors\")) {\n>                 for (auto &p : params.get_child(\"priors\")) {\n>                         AwbPrior prior;\n> -                       prior.read(p.second);\n> +                       ret = prior.read(p.second);\n> +                       if (ret)\n> +                               return ret;\n>                         if (!priors.empty() && prior.lux <=\n> priors.back().lux)\n>                                 LOG(RPiAwb, Fatal) << \"AwbConfig: Prior\n> must be ordered in increasing lux value\";\n>                         priors.push_back(prior);\n> @@ -79,7 +87,9 @@ void AwbConfig::read(boost::property_tree::ptree const\n> &params)\n>         }\n>         if (params.get_child_optional(\"modes\")) {\n>                 for (auto &p : params.get_child(\"modes\")) {\n> -                       modes[p.first].read(p.second);\n> +                       ret = modes[p.first].read(p.second);\n> +                       if (ret)\n> +                               return ret;\n>                         if (defaultMode == nullptr)\n>                                 defaultMode = &modes[p.first];\n>                 }\n> @@ -110,6 +120,7 @@ void AwbConfig::read(boost::property_tree::ptree const\n> &params)\n>         whitepointB = params.get<double>(\"whitepoint_b\", 0.0);\n>         if (bayes == false)\n>                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities\n> make any sense */\n> +       return 0;\n>  }\n>\n>  Awb::Awb(Controller *controller)\n> @@ -137,9 +148,9 @@ char const *Awb::name() const\n>         return NAME;\n>  }\n>\n> -void Awb::read(boost::property_tree::ptree const &params)\n> +int Awb::read(boost::property_tree::ptree const &params)\n>  {\n> -       config_.read(params);\n> +       return config_.read(params);\n>  }\n>\n>  void Awb::initialise()\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h\n> b/src/ipa/raspberrypi/controller/rpi/awb.h\n> index 9e075624c429..fa0715735fcf 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.h\n> @@ -19,20 +19,20 @@ namespace RPiController {\n>  /* Control algorithm to perform AWB calculations. */\n>\n>  struct AwbMode {\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>         double ctLo; /* low CT value for search */\n>         double ctHi; /* high CT value for search */\n>  };\n>\n>  struct AwbPrior {\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>         double lux; /* lux level */\n>         Pwl prior; /* maps CT to prior log likelihood for this lux level */\n>  };\n>\n>  struct AwbConfig {\n>         AwbConfig() : defaultMode(nullptr) {}\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>         /* Only repeat the AWB calculation every \"this many\" frames */\n>         uint16_t framePeriod;\n>         /* number of initial frames for which speed taken as 1.0 (maximum)\n> */\n> @@ -92,7 +92,7 @@ public:\n>         ~Awb();\n>         char const *name() const override;\n>         void initialise() override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         /* AWB handles \"pausing\" for itself. */\n>         bool isPaused() const override;\n>         void pause() override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> b/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> index 0799d7b9195a..aa6f602acd7d 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> @@ -31,7 +31,7 @@ char const *BlackLevel::name() const\n>         return NAME;\n>  }\n>\n> -void BlackLevel::read(boost::property_tree::ptree const &params)\n> +int BlackLevel::read(boost::property_tree::ptree const &params)\n>  {\n>         uint16_t blackLevel = params.get<uint16_t>(\n>                 \"black_level\", 4096); /* 64 in 10 bits scaled to 16 bits */\n> @@ -42,6 +42,7 @@ void BlackLevel::read(boost::property_tree::ptree const\n> &params)\n>                 << \" Read black levels red \" << blackLevelR_\n>                 << \" green \" << blackLevelG_\n>                 << \" blue \" << blackLevelB_;\n> +       return 0;\n>  }\n>\n>  void BlackLevel::prepare(Metadata *imageMetadata)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.h\n> b/src/ipa/raspberrypi/controller/rpi/black_level.h\n> index 7789f261cf03..6ec8c4f9ba8f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/black_level.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/black_level.h\n> @@ -18,7 +18,7 @@ class BlackLevel : public Algorithm\n>  public:\n>         BlackLevel(Controller *controller);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>\n>  private:\n> diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> b/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> index cf0c85d26cf9..f0110d38f548 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> @@ -39,7 +39,7 @@ Matrix::Matrix(double m0, double m1, double m2, double\n> m3, double m4, double m5,\n>         m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] =\n> m4,\n>         m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;\n>  }\n> -void Matrix::read(boost::property_tree::ptree const &params)\n> +int Matrix::read(boost::property_tree::ptree const &params)\n>  {\n>         double *ptr = (double *)m;\n>         int n = 0;\n> @@ -50,6 +50,7 @@ void Matrix::read(boost::property_tree::ptree const\n> &params)\n>         }\n>         if (n < 9)\n>                 LOG(RPiCcm, Fatal) << \"Ccm: too few values in CCM\";\n> +       return 0;\n>  }\n>\n>  Ccm::Ccm(Controller *controller)\n> @@ -60,21 +61,32 @@ char const *Ccm::name() const\n>         return NAME;\n>  }\n>\n> -void Ccm::read(boost::property_tree::ptree const &params)\n> +int Ccm::read(boost::property_tree::ptree const &params)\n>  {\n> -       if (params.get_child_optional(\"saturation\"))\n> -               config_.saturation.read(params.get_child(\"saturation\"));\n> +       int ret;\n> +\n> +       if (params.get_child_optional(\"saturation\")) {\n> +               ret =\n> config_.saturation.read(params.get_child(\"saturation\"));\n> +               if (ret)\n> +                       return ret;\n> +       }\n> +\n>         for (auto &p : params.get_child(\"ccms\")) {\n>                 CtCcm ctCcm;\n>                 ctCcm.ct = p.second.get<double>(\"ct\");\n> -               ctCcm.ccm.read(p.second.get_child(\"ccm\"));\n> +               ret = ctCcm.ccm.read(p.second.get_child(\"ccm\"));\n> +               if (ret)\n> +                       return ret;\n>                 if (!config_.ccms.empty() &&\n>                     ctCcm.ct <= config_.ccms.back().ct)\n>                         LOG(RPiCcm, Fatal) << \"Ccm: CCM not in increasing\n> colour temperature order\";\n>                 config_.ccms.push_back(std::move(ctCcm));\n>         }\n> +\n>         if (config_.ccms.empty())\n>                 LOG(RPiCcm, Fatal) << \"Ccm: no CCMs specified\";\n> +\n> +       return 0;\n>  }\n>\n>  void Ccm::setSaturation(double saturation)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.h\n> b/src/ipa/raspberrypi/controller/rpi/ccm.h\n> index b44f1576528f..6b65c7aea8a6 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/ccm.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.h\n> @@ -20,7 +20,7 @@ struct Matrix {\n>                double m6, double m7, double m8);\n>         Matrix();\n>         double m[3][3];\n> -       void read(boost::property_tree::ptree const &params);\n> +       int read(boost::property_tree::ptree const &params);\n>  };\n>  static inline Matrix operator*(double d, Matrix const &m)\n>  {\n> @@ -62,7 +62,7 @@ class Ccm : public CcmAlgorithm\n>  public:\n>         Ccm(Controller *controller = NULL);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void setSaturation(double saturation) override;\n>         void initialise() override;\n>         void prepare(Metadata *imageMetadata) override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> index 9e60dc5d47e7..d76dc43b837f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> @@ -38,7 +38,7 @@ char const *Contrast::name() const\n>         return NAME;\n>  }\n>\n> -void Contrast::read(boost::property_tree::ptree const &params)\n> +int Contrast::read(boost::property_tree::ptree const &params)\n>  {\n>         /* enable adaptive enhancement by default */\n>         config_.ceEnable = params.get<int>(\"ce_enable\", 1);\n> @@ -52,7 +52,7 @@ void Contrast::read(boost::property_tree::ptree const\n> &params)\n>         config_.hiHistogram = params.get<double>(\"hi_histogram\", 0.95);\n>         config_.hiLevel = params.get<double>(\"hi_level\", 0.95);\n>         config_.hiMax = params.get<double>(\"hi_max\", 2000);\n> -       config_.gammaCurve.read(params.get_child(\"gamma_curve\"));\n> +       return config_.gammaCurve.read(params.get_child(\"gamma_curve\"));\n>  }\n>\n>  void Contrast::setBrightness(double brightness)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.h\n> b/src/ipa/raspberrypi/controller/rpi/contrast.h\n> index b1375d819dbc..5c3d2f56b310 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/contrast.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.h\n> @@ -34,7 +34,7 @@ class Contrast : public ContrastAlgorithm\n>  public:\n>         Contrast(Controller *controller = NULL);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void setBrightness(double brightness) override;\n>         void setContrast(double contrast) override;\n>         void initialise() override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> b/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> index d5d784e7ca64..be014a05fd41 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> @@ -31,11 +31,12 @@ char const *Dpc::name() const\n>         return NAME;\n>  }\n>\n> -void Dpc::read(boost::property_tree::ptree const &params)\n> +int Dpc::read(boost::property_tree::ptree const &params)\n>  {\n>         config_.strength = params.get<int>(\"strength\", 1);\n>         if (config_.strength < 0 || config_.strength > 2)\n>                 LOG(RPiDpc, Fatal) << \"Dpc: bad strength value\";\n> +       return 0;\n>  }\n>\n>  void Dpc::prepare(Metadata *imageMetadata)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.h\n> b/src/ipa/raspberrypi/controller/rpi/dpc.h\n> index 4c1bdec6dd87..2bb6cef565a7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/dpc.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.h\n> @@ -22,7 +22,7 @@ class Dpc : public Algorithm\n>  public:\n>         Dpc(Controller *controller);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>\n>  private:\n> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> b/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> index 696da4aeea59..a74447877bf8 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> @@ -35,14 +35,20 @@ char const *Geq::name() const\n>         return NAME;\n>  }\n>\n> -void Geq::read(boost::property_tree::ptree const &params)\n> +int Geq::read(boost::property_tree::ptree const &params)\n>  {\n>         config_.offset = params.get<uint16_t>(\"offset\", 0);\n>         config_.slope = params.get<double>(\"slope\", 0.0);\n>         if (config_.slope < 0.0 || config_.slope >= 1.0)\n>                 LOG(RPiGeq, Fatal) << \"Geq: bad slope value\";\n> -       if (params.get_child_optional(\"strength\"))\n> -               config_.strength.read(params.get_child(\"strength\"));\n> +\n> +       if (params.get_child_optional(\"strength\")) {\n> +               int ret =\n> config_.strength.read(params.get_child(\"strength\"));\n> +               if (ret)\n> +                       return ret;\n> +       }\n> +\n> +       return 0;\n>  }\n>\n>  void Geq::prepare(Metadata *imageMetadata)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.h\n> b/src/ipa/raspberrypi/controller/rpi/geq.h\n> index 5d06b9cbd529..677a0510c6ac 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/geq.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/geq.h\n> @@ -24,7 +24,7 @@ class Geq : public Algorithm\n>  public:\n>         Geq(Controller *controller);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>\n>  private:\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> index 95c0a93b2810..ca1e827191fd 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> @@ -38,7 +38,7 @@ char const *Lux::name() const\n>         return NAME;\n>  }\n>\n> -void Lux::read(boost::property_tree::ptree const &params)\n> +int Lux::read(boost::property_tree::ptree const &params)\n>  {\n>         referenceShutterSpeed_ =\n>                 params.get<double>(\"reference_shutter_speed\") * 1.0us;\n> @@ -47,6 +47,7 @@ void Lux::read(boost::property_tree::ptree const &params)\n>         referenceY_ = params.get<double>(\"reference_Y\");\n>         referenceLux_ = params.get<double>(\"reference_lux\");\n>         currentAperture_ = referenceAperture_;\n> +       return 0;\n>  }\n>\n>  void Lux::setCurrentAperture(double aperture)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.h\n> b/src/ipa/raspberrypi/controller/rpi/lux.h\n> index 26af8185911c..6416dfb52553 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/lux.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/lux.h\n> @@ -22,7 +22,7 @@ class Lux : public Algorithm\n>  public:\n>         Lux(Controller *controller);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>         void process(StatisticsPtr &stats, Metadata *imageMetadata)\n> override;\n>         void setCurrentAperture(double aperture);\n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> index 5d2c85ad3e74..74fa99bafe48 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> @@ -41,10 +41,11 @@ void Noise::switchMode(CameraMode const &cameraMode,\n>         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n>  }\n>\n> -void Noise::read(boost::property_tree::ptree const &params)\n> +int Noise::read(boost::property_tree::ptree const &params)\n>  {\n>         referenceConstant_ = params.get<double>(\"reference_constant\");\n>         referenceSlope_ = params.get<double>(\"reference_slope\");\n> +       return 0;\n>  }\n>\n>  void Noise::prepare(Metadata *imageMetadata)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.h\n> b/src/ipa/raspberrypi/controller/rpi/noise.h\n> index 144e36529f4c..b33a5fc75ec7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/noise.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/noise.h\n> @@ -19,7 +19,7 @@ public:\n>         Noise(Controller *controller);\n>         char const *name() const override;\n>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void prepare(Metadata *imageMetadata) override;\n>\n>  private:\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> index af31bd0881f0..03d9f119a338 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> @@ -34,10 +34,11 @@ char const *Sdn::name() const\n>         return NAME;\n>  }\n>\n> -void Sdn::read(boost::property_tree::ptree const &params)\n> +int Sdn::read(boost::property_tree::ptree const &params)\n>  {\n>         deviation_ = params.get<double>(\"deviation\", 3.2);\n>         strength_ = params.get<double>(\"strength\", 0.75);\n> +       return 0;\n>  }\n>\n>  void Sdn::initialise()\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.h\n> b/src/ipa/raspberrypi/controller/rpi/sdn.h\n> index 90ea37ff5550..4287ef08a79f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sdn.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.h\n> @@ -18,7 +18,7 @@ class Sdn : public DenoiseAlgorithm\n>  public:\n>         Sdn(Controller *controller = NULL);\n>         char const *name() const override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void initialise() override;\n>         void prepare(Metadata *imageMetadata) override;\n>         void setMode(DenoiseMode mode) override;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> index 36b75f69afce..9c4cffa4128c 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> @@ -37,7 +37,7 @@ void Sharpen::switchMode(CameraMode const &cameraMode,\n>         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n>  }\n>\n> -void Sharpen::read(boost::property_tree::ptree const &params)\n> +int Sharpen::read(boost::property_tree::ptree const &params)\n>  {\n>         threshold_ = params.get<double>(\"threshold\", 1.0);\n>         strength_ = params.get<double>(\"strength\", 1.0);\n> @@ -46,6 +46,7 @@ void Sharpen::read(boost::property_tree::ptree const\n> &params)\n>                 << \"Read threshold \" << threshold_\n>                 << \" strength \" << strength_\n>                 << \" limit \" << limit_;\n> +       return 0;\n>  }\n>\n>  void Sharpen::setStrength(double strength)\n> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> b/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> index 8846f2ae09a2..0cd8b92f2224 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> @@ -19,7 +19,7 @@ public:\n>         Sharpen(Controller *controller);\n>         char const *name() const override;\n>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> override;\n> -       void read(boost::property_tree::ptree const &params) override;\n> +       int read(boost::property_tree::ptree const &params) override;\n>         void setStrength(double strength) override;\n>         void prepare(Metadata *imageMetadata) override;\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 9d550354d78e..b9e9b814e886 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -229,7 +229,14 @@ int IPARPi::init(const IPASettings &settings,\n> IPAInitResult *result)\n>         result->sensorConfig.sensorMetadata = sensorMetadata;\n>\n>         /* Load the tuning file for this sensor. */\n> -       controller_.read(settings.configurationFile.c_str());\n> +       int ret = controller_.read(settings.configurationFile.c_str());\n> +       if (ret) {\n> +               LOG(IPARPI, Error)\n> +                       << \"Failed to load tuning data file \"\n> +                       << settings.configurationFile;\n> +               return ret;\n> +       }\n> +\n>         controller_.initialise();\n>\n>         /* Return the controls handled by the IPA */\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1755CBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Jul 2022 10:08:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3525763313;\n\tWed, 27 Jul 2022 12:08:51 +0200 (CEST)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1C4A63309\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jul 2022 12:08:48 +0200 (CEST)","by mail-lf1-x12b.google.com with SMTP id a23so24043893lfm.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jul 2022 03:08:48 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658916531;\n\tbh=WmU9ZEIza4rIMqCjx5DD8V5Bmd47Hxqy72KbxWxR6oA=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=4hEqp8CcAhgIQYIzwa5wYxaAYeDwsEi3e4vu1F/rder6ktgJJTHDyohIp4LbOu7ds\n\twSDzPhQauLwMSwXFDrzK2XTtt1hR7rQFUAMBAYshQxgy8g6NcVTYSkh/b+lM46fiUG\n\tmylG3CFG1yUo5PjGIBPVlxarM3B7iAGsOca2caoF0UdFZyO/G5YIz/IzocYJq7+G1E\n\tbh1AwptF3px7mSY6zntButCF42y3g/HGzh+ne6Npj/hcdqQwc2FZfydQ1pesyiLwzO\n\tSo40qcjDKkZnj977c5TbYDueg8qi4vZGToxysXaMCDzGj20GL4N4QQMcA28ltDdVgn\n\twyeuXhOCPozRA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=MiVD/MlyH0pD4P+UkszIeXo5siKcAUBOKUcaMsWnVdk=;\n\tb=LqjD29VRjJMegZYtSz3VPwCbYVxXDtE04/kwBUokECroFaymtyGaw7zk+/GtNHqIKm\n\tx9C2DiwYpadB1XDb+r9gVXl2JKiC65r1s58HQXXCk9RepToakgujRtk25g8OwNdt3n4b\n\tYMx1EQc5Y4VEWZNlbyxci1yS4uMv2aT0KFsFHhD/+uFkHAx7om+tlkJAe4LdRlAi8LDt\n\tGmwEevt2jQ92vBQhbT5X8uLrLEbKP3/npEAh2nrQoESxeGjD178DxYarpv+K2tNRS8uo\n\twnMPRXDyna+QXS6YL/2yzcCm/PNh44EThGavPFg6rGl7Xs2B7bCzUy9n98xRyS5l92+/\n\tam8Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"LqjD29VR\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=MiVD/MlyH0pD4P+UkszIeXo5siKcAUBOKUcaMsWnVdk=;\n\tb=X1ZnQf/3N1VwYjeJD7GDBwqOFaT2ZzOh9cB8IX4P2jLmTBxNxOWCjFCI2tzN+947gl\n\tbI1SMsVwPTgGRKon5F5X/XBaRVf2/d9oe0yb641FyDCB6dAvT4TStGAsMx9wd8zM7oLV\n\tQd5Odp0j8mwJ1Cjo1X7xjPOwXBPgm0iIXQdaF01e3tUS28cLkDS56bJLhbVA8xMZV8L4\n\tb1bpzRtShnzSUp5L2d2bggAHHnWbW8NSg8u8eKViG1SCInJ5a+k0zpIMKoxkTDW1oZm7\n\tHLuCAVe+8Y6vW/zemn/rDlBLg8Jb9AWrxKiRi/GEtq8tsz8gx6ztC1/lDAnVSLYDNR73\n\ttENA==","X-Gm-Message-State":"AJIora8qoPGfn3PnHXRyrC9Ih9aZssLA239Z9W+LJ3mYul917wpuwWmA\n\tObUuGgs88tSpt5HAGJMU5c0eB/ADU9zGxcx7w9QQ+Q==","X-Google-Smtp-Source":"AGRyM1vVkz2+eBStuACRaZZIrOL34YxihJeOh8Axwn5+PGeIY/HAYtvLWYHuahFQKCBHOZbRIJQcEUEdn0p9Id0n9AM=","X-Received":"by 2002:ac2:4c4d:0:b0:48a:7a96:470d with SMTP id\n\to13-20020ac24c4d000000b0048a7a96470dmr7561082lfk.682.1658916527987;\n\tWed, 27 Jul 2022 03:08:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20220727023816.30008-1-laurent.pinchart@ideasonboard.com>\n\t<20220727023816.30008-6-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220727023816.30008-6-laurent.pinchart@ideasonboard.com>","Date":"Wed, 27 Jul 2022 11:08:32 +0100","Message-ID":"<CAEmqJPoiJO73YXY_BV7Ue4D8f+d3Ci6mEQgY4DnynWvtKi6XDA@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"0000000000005cf0bb05e4c69a17\"","Subject":"Re: [libcamera-devel] [PATCH v7 05/14] ipa: raspberrypi: Return an\n\terror code from Algorithm::read()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24194,"web_url":"https://patchwork.libcamera.org/comment/24194/","msgid":"<YuFvIA9/kiitQRff@pendragon.ideasonboard.com>","date":"2022-07-27T17:00:16","subject":"Re: [libcamera-devel] [PATCH v7 05/14] ipa: raspberrypi: Return an\n\terror code from Algorithm::read()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Wed, Jul 27, 2022 at 11:08:32AM +0100, Naushir Patuck wrote:\n> On Wed, 27 Jul 2022 at 03:38, Laurent Pinchart wrote:\n> \n> > When encountering errors, the Algorithm::read() function either uses\n> > LOG(Fatal) or throws exceptions from the boost property_tree functions.\n> > To prepare for replacing boost JSON parse with the YamlParser class,\n> > give the Algorithm::read() function the ability to return an error code,\n> > and propagate it all the way to the IPA module init() function.\n> >\n> > All algorithm classes return a hardcoded 0 value for now, subsequent\n> > commits will change that.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/ipa/raspberrypi/controller/algorithm.cpp  |  3 +-\n> >  src/ipa/raspberrypi/controller/algorithm.h    |  2 +-\n> >  src/ipa/raspberrypi/controller/controller.cpp |  8 +++-\n> >  src/ipa/raspberrypi/controller/controller.h   |  2 +-\n> >  src/ipa/raspberrypi/controller/pwl.cpp        |  3 +-\n> >  src/ipa/raspberrypi/controller/pwl.h          |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 28 ++++++++++----\n> >  src/ipa/raspberrypi/controller/rpi/agc.h      | 10 ++---\n> >  src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 38 +++++++++++++------\n> >  src/ipa/raspberrypi/controller/rpi/alsc.h     |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/awb.cpp    | 35 +++++++++++------\n> >  src/ipa/raspberrypi/controller/rpi/awb.h      |  8 ++--\n> >  .../controller/rpi/black_level.cpp            |  3 +-\n> >  .../raspberrypi/controller/rpi/black_level.h  |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/ccm.cpp    | 22 ++++++++---\n> >  src/ipa/raspberrypi/controller/rpi/ccm.h      |  4 +-\n> >  .../raspberrypi/controller/rpi/contrast.cpp   |  4 +-\n> >  src/ipa/raspberrypi/controller/rpi/contrast.h |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |  3 +-\n> >  src/ipa/raspberrypi/controller/rpi/dpc.h      |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/geq.cpp    | 12 ++++--\n> >  src/ipa/raspberrypi/controller/rpi/geq.h      |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/lux.cpp    |  3 +-\n> >  src/ipa/raspberrypi/controller/rpi/lux.h      |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/noise.cpp  |  3 +-\n> >  src/ipa/raspberrypi/controller/rpi/noise.h    |  2 +-\n> >  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |  3 +-\n> >  src/ipa/raspberrypi/controller/rpi/sdn.h      |  2 +-\n> >  .../raspberrypi/controller/rpi/sharpen.cpp    |  3 +-\n> >  src/ipa/raspberrypi/controller/rpi/sharpen.h  |  2 +-\n> >  src/ipa/raspberrypi/raspberrypi.cpp           |  9 ++++-\n> >  31 files changed, 151 insertions(+), 75 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp\n> > index 1a7d20a4731b..d73cb36fe2d6 100644\n> > --- a/src/ipa/raspberrypi/controller/algorithm.cpp\n> > +++ b/src/ipa/raspberrypi/controller/algorithm.cpp\n> > @@ -9,8 +9,9 @@\n> >\n> >  using namespace RPiController;\n> >\n> > -void Algorithm::read([[maybe_unused]] boost::property_tree::ptree const &params)\n> > +int Algorithm::read([[maybe_unused]] boost::property_tree::ptree const &params)\n> >  {\n> > +       return 0;\n> >  }\n> >\n> >  void Algorithm::initialise()\n> > diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h\n> > index 92fd895d2b06..0c5566fda874 100644\n> > --- a/src/ipa/raspberrypi/controller/algorithm.h\n> > +++ b/src/ipa/raspberrypi/controller/algorithm.h\n> > @@ -35,7 +35,7 @@ public:\n> >         virtual bool isPaused() const { return paused_; }\n> >         virtual void pause() { paused_ = true; }\n> >         virtual void resume() { paused_ = false; }\n> > -       virtual void read(boost::property_tree::ptree const &params);\n> > +       virtual int read(boost::property_tree::ptree const &params);\n> >         virtual void initialise();\n> >         virtual void switchMode(CameraMode const &cameraMode, Metadata *metadata);\n> >         virtual void prepare(Metadata *imageMetadata);\n> > diff --git a/src/ipa/raspberrypi/controller/controller.cpp\n> > b/src/ipa/raspberrypi/controller/controller.cpp\n> > index 872a32302410..d91ac90704cb 100644\n> > --- a/src/ipa/raspberrypi/controller/controller.cpp\n> > +++ b/src/ipa/raspberrypi/controller/controller.cpp\n> > @@ -32,19 +32,23 @@ Controller::Controller(char const *jsonFilename)\n> >\n> >  Controller::~Controller() {}\n> >\n> > -void Controller::read(char const *filename)\n> > +int Controller::read(char const *filename)\n> \n> Since read now returns an error code, we probably ought to remove the\n> Controller(char const *jsonFilename) constructor.  This never gets used\n> anyway.  But that can be done on top of this work.\n\nFeel free to perform further cleanups :-) Eventually it could be nice\nfor the Raspberry Pi IPA to use the Algorithm and Module classes\nprovided by libipa to replace the Controller class, but I think we need\nsome more work there first.\n\n> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> \n> >  {\n> >         boost::property_tree::ptree root;\n> >         boost::property_tree::read_json(filename, root);\n> >         for (auto const &keyAndValue : root) {\n> >                 Algorithm *algo = createAlgorithm(keyAndValue.first.c_str());\n> >                 if (algo) {\n> > -                       algo->read(keyAndValue.second);\n> > +                       int ret = algo->read(keyAndValue.second);\n> > +                       if (ret)\n> > +                               return ret;\n> >                         algorithms_.push_back(AlgorithmPtr(algo));\n> >                 } else\n> >                         LOG(RPiController, Warning)\n> >                                 << \"No algorithm found for \\\"\" << keyAndValue.first << \"\\\"\";\n> >         }\n> > +\n> > +       return 0;\n> >  }\n> >\n> >  Algorithm *Controller::createAlgorithm(char const *name)\n> > diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h\n> > index e28e30d7d574..841783bb7a5c 100644\n> > --- a/src/ipa/raspberrypi/controller/controller.h\n> > +++ b/src/ipa/raspberrypi/controller/controller.h\n> > @@ -41,7 +41,7 @@ public:\n> >         Controller(char const *jsonFilename);\n> >         ~Controller();\n> >         Algorithm *createAlgorithm(char const *name);\n> > -       void read(char const *filename);\n> > +       int read(char const *filename);\n> >         void initialise();\n> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata);\n> >         void prepare(Metadata *imageMetadata);\n> > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp b/src/ipa/raspberrypi/controller/pwl.cpp\n> > index 8b8db722966b..fde0b298c6ce 100644\n> > --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> > +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> > @@ -12,7 +12,7 @@\n> >\n> >  using namespace RPiController;\n> >\n> > -void Pwl::read(boost::property_tree::ptree const &params)\n> > +int Pwl::read(boost::property_tree::ptree const &params)\n> >  {\n> >         for (auto it = params.begin(); it != params.end(); it++) {\n> >                 double x = it->second.get_value<double>();\n> > @@ -22,6 +22,7 @@ void Pwl::read(boost::property_tree::ptree const &params)\n> >                 points_.push_back(Point(x, y));\n> >         }\n> >         assert(points_.size() >= 2);\n> > +       return 0;\n> >  }\n> >\n> >  void Pwl::append(double x, double y, const double eps)\n> > diff --git a/src/ipa/raspberrypi/controller/pwl.h b/src/ipa/raspberrypi/controller/pwl.h\n> > index 973efdf59945..ef1cc2ed113a 100644\n> > --- a/src/ipa/raspberrypi/controller/pwl.h\n> > +++ b/src/ipa/raspberrypi/controller/pwl.h\n> > @@ -57,7 +57,7 @@ public:\n> >         };\n> >         Pwl() {}\n> >         Pwl(std::vector<Point> const &points) : points_(points) {}\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >         void append(double x, double y, const double eps = 1e-6);\n> >         void prepend(double x, double y, const double eps = 1e-6);\n> >         Interval domain() const;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > index adec8592626d..130c606d4136 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> > @@ -30,7 +30,7 @@ LOG_DEFINE_CATEGORY(RPiAgc)\n> >\n> >  static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit pipeline */\n> >\n> > -void AgcMeteringMode::read(boost::property_tree::ptree const &params)\n> > +int AgcMeteringMode::read(boost::property_tree::ptree const &params)\n> >  {\n> >         int num = 0;\n> >         for (auto &p : params.get_child(\"weights\")) {\n> > @@ -40,6 +40,7 @@ void AgcMeteringMode::read(boost::property_tree::ptree const &params)\n> >         }\n> >         if (num != AgcStatsSize)\n> >                 LOG(RPiAgc, Fatal) << \"AgcMeteringMode: insufficient weights\";\n> > +       return 0;\n> >  }\n> >\n> >  static std::string\n> > @@ -73,7 +74,7 @@ static int readList(std::vector<Duration> &list,\n> >         return list.size();\n> >  }\n> >\n> > -void AgcExposureMode::read(boost::property_tree::ptree const &params)\n> > +int AgcExposureMode::read(boost::property_tree::ptree const &params)\n> >  {\n> >         int numShutters = readList(shutter, params.get_child(\"shutter\"));\n> >         int numAgs = readList(gain, params.get_child(\"gain\"));\n> > @@ -83,6 +84,7 @@ void AgcExposureMode::read(boost::property_tree::ptree const &params)\n> >         if (numShutters != numAgs)\n> >                 LOG(RPiAgc, Fatal)\n> >                         << \"AgcExposureMode: expect same number of exposure and gain entries in exposure profile\";\n> > +       return 0;\n> >  }\n> >\n> >  static std::string\n> > @@ -100,7 +102,7 @@ readExposureModes(std::map<std::string, AgcExposureMode> &exposureModes,\n> >         return first;\n> >  }\n> >\n> > -void AgcConstraint::read(boost::property_tree::ptree const &params)\n> > +int AgcConstraint::read(boost::property_tree::ptree const &params)\n> >  {\n> >         std::string boundString = params.get<std::string>(\"bound\", \"\");\n> >         transform(boundString.begin(), boundString.end(),\n> > @@ -110,7 +112,7 @@ void AgcConstraint::read(boost::property_tree::ptree const &params)\n> >         bound = boundString == \"UPPER\" ? Bound::UPPER : Bound::LOWER;\n> >         qLo = params.get<double>(\"q_lo\");\n> >         qHi = params.get<double>(\"q_hi\");\n> > -       yTarget.read(params.get_child(\"y_target\"));\n> > +       return yTarget.read(params.get_child(\"y_target\"));\n> >  }\n> >\n> >  static AgcConstraintMode\n> > @@ -137,13 +139,17 @@ static std::string readConstraintModes(std::map<std::string, AgcConstraintMode>\n> >         return first;\n> >  }\n> >\n> > -void AgcConfig::read(boost::property_tree::ptree const &params)\n> > +int AgcConfig::read(boost::property_tree::ptree const &params)\n> >  {\n> >         LOG(RPiAgc, Debug) << \"AgcConfig\";\n> >         defaultMeteringMode = readMeteringModes(meteringModes, params.get_child(\"metering_modes\"));\n> >         defaultExposureMode = readExposureModes(exposureModes, params.get_child(\"exposure_modes\"));\n> >         defaultConstraintMode = readConstraintModes(constraintModes, params.get_child(\"constraint_modes\"));\n> > -       yTarget.read(params.get_child(\"y_target\"));\n> > +\n> > +       int ret = yTarget.read(params.get_child(\"y_target\"));\n> > +       if (ret)\n> > +               return ret;\n> > +\n> >         speed = params.get<double>(\"speed\", 0.2);\n> >         startupFrames = params.get<uint16_t>(\"startup_frames\", 10);\n> >         convergenceFrames = params.get<unsigned int>(\"convergence_frames\", 6);\n> > @@ -152,6 +158,7 @@ void AgcConfig::read(boost::property_tree::ptree const &params)\n> >         /* Start with quite a low value as ramping up is easier than ramping down. */\n> >         defaultExposureTime = params.get<double>(\"default_exposure_time\", 1000) * 1us;\n> >         defaultAnalogueGain = params.get<double>(\"default_analogueGain\", 1.0);\n> > +       return 0;\n> >  }\n> >\n> >  Agc::ExposureValues::ExposureValues()\n> > @@ -182,10 +189,14 @@ char const *Agc::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Agc::read(boost::property_tree::ptree const &params)\n> > +int Agc::read(boost::property_tree::ptree const &params)\n> >  {\n> >         LOG(RPiAgc, Debug) << \"Agc\";\n> > -       config_.read(params);\n> > +\n> > +       int ret = config_.read(params);\n> > +       if (ret)\n> > +               return ret;\n> > +\n> >         /*\n> >          * Set the config's defaults (which are the first ones it read) as our\n> >          * current modes, until someone changes them.  (they're all known to\n> > @@ -200,6 +211,7 @@ void Agc::read(boost::property_tree::ptree const &params)\n> >         /* Set up the \"last shutter/gain\" values, in case AGC starts \"disabled\". */\n> >         status_.shutterTime = config_.defaultExposureTime;\n> >         status_.analogueGain = config_.defaultAnalogueGain;\n> > +       return 0;\n> >  }\n> >\n> >  bool Agc::isPaused() const\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > index f57afa6dc80c..1351b0ee079c 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/agc.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h\n> > @@ -28,13 +28,13 @@ namespace RPiController {\n> >\n> >  struct AgcMeteringMode {\n> >         double weights[AgcStatsSize];\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >  };\n> >\n> >  struct AgcExposureMode {\n> >         std::vector<libcamera::utils::Duration> shutter;\n> >         std::vector<double> gain;\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >  };\n> >\n> >  struct AgcConstraint {\n> > @@ -43,13 +43,13 @@ struct AgcConstraint {\n> >         double qLo;\n> >         double qHi;\n> >         Pwl yTarget;\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >  };\n> >\n> >  typedef std::vector<AgcConstraint> AgcConstraintMode;\n> >\n> >  struct AgcConfig {\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >         std::map<std::string, AgcMeteringMode> meteringModes;\n> >         std::map<std::string, AgcExposureMode> exposureModes;\n> >         std::map<std::string, AgcConstraintMode> constraintModes;\n> > @@ -74,7 +74,7 @@ class Agc : public AgcAlgorithm\n> >  public:\n> >         Agc(Controller *controller);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         /* AGC handles \"pausing\" for itself. */\n> >         bool isPaused() const override;\n> >         void pause() override;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> > index 03ae33501dc0..b36277696a52 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> > @@ -50,7 +50,7 @@ char const *Alsc::name() const\n> >         return NAME;\n> >  }\n> >\n> > -static void generateLut(double *lut, boost::property_tree::ptree const &params)\n> > +static int generateLut(double *lut, boost::property_tree::ptree const &params)\n> >  {\n> >         double cstrength = params.get<double>(\"corner_strength\", 2.0);\n> >         if (cstrength <= 1.0)\n> > @@ -71,9 +71,10 @@ static void generateLut(double *lut, boost::property_tree::ptree const &params)\n> >                                 (f2 * f2); /* this reproduces the cos^4 rule */\n> >                 }\n> >         }\n> > +       return 0;\n> >  }\n> >\n> > -static void readLut(double *lut, boost::property_tree::ptree const &params)\n> > +static int readLut(double *lut, boost::property_tree::ptree const &params)\n> >  {\n> >         int num = 0;\n> >         const int maxNum = XY;\n> > @@ -84,11 +85,12 @@ static void readLut(double *lut, boost::property_tree::ptree const &params)\n> >         }\n> >         if (num < maxNum)\n> >                 LOG(RPiAlsc, Fatal) << \"Alsc: too few entries in LSC table\";\n> > +       return 0;\n> >  }\n> >\n> > -static void readCalibrations(std::vector<AlscCalibration> &calibrations,\n> > -                            boost::property_tree::ptree const &params,\n> > -                            std::string const &name)\n> > +static int readCalibrations(std::vector<AlscCalibration> &calibrations,\n> > +                           boost::property_tree::ptree const &params,\n> > +                           std::string const &name)\n> >  {\n> >         if (params.get_child_optional(name)) {\n> >                 double lastCt = 0;\n> > @@ -117,9 +119,10 @@ static void readCalibrations(std::vector<AlscCalibration> &calibrations,\n> >                                 << \"Read \" << name << \" calibration for ct \" << ct;\n> >                 }\n> >         }\n> > +       return 0;\n> >  }\n> >\n> > -void Alsc::read(boost::property_tree::ptree const &params)\n> > +int Alsc::read(boost::property_tree::ptree const &params)\n> >  {\n> >         config_.framePeriod = params.get<uint16_t>(\"frame_period\", 12);\n> >         config_.startupFrames = params.get<uint16_t>(\"startup_frames\", 10);\n> > @@ -135,19 +138,32 @@ void Alsc::read(boost::property_tree::ptree const &params)\n> >                 params.get<double>(\"luminance_strength\", 1.0);\n> >         for (int i = 0; i < XY; i++)\n> >                 config_.luminanceLut[i] = 1.0;\n> > +\n> > +       int ret = 0;\n> > +\n> >         if (params.get_child_optional(\"corner_strength\"))\n> > -               generateLut(config_.luminanceLut, params);\n> > +               ret = generateLut(config_.luminanceLut, params);\n> >         else if (params.get_child_optional(\"luminance_lut\"))\n> > -               readLut(config_.luminanceLut,\n> > -                       params.get_child(\"luminance_lut\"));\n> > +               ret = readLut(config_.luminanceLut,\n> > +                             params.get_child(\"luminance_lut\"));\n> >         else\n> >                 LOG(RPiAlsc, Warning)\n> >                         << \"no luminance table - assume unity everywhere\";\n> > -       readCalibrations(config_.calibrationsCr, params, \"calibrations_Cr\");\n> > -       readCalibrations(config_.calibrationsCb, params, \"calibrations_Cb\");\n> > +       if (ret)\n> > +               return ret;\n> > +\n> > +       ret = readCalibrations(config_.calibrationsCr, params, \"calibrations_Cr\");\n> > +       if (ret)\n> > +               return ret;\n> > +       ret = readCalibrations(config_.calibrationsCb, params, \"calibrations_Cb\");\n> > +       if (ret)\n> > +               return ret;\n> > +\n> >         config_.defaultCt = params.get<double>(\"default_ct\", 4500.0);\n> >         config_.threshold = params.get<double>(\"threshold\", 1e-3);\n> >         config_.lambdaBound = params.get<double>(\"lambda_bound\", 0.05);\n> > +\n> > +       return 0;\n> >  }\n> >\n> >  static double getCt(Metadata *metadata, double defaultCt);\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> > index 4e9a715ae0ab..773fd338ea8e 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h\n> > @@ -52,7 +52,7 @@ public:\n> >         char const *name() const override;\n> >         void initialise() override;\n> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> > override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >         void process(StatisticsPtr &stats, Metadata *imageMetadata)\n> > override;\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > index ad75d55f0976..a9e776a344a1 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > @@ -26,20 +26,21 @@ static constexpr unsigned int AwbStatsSizeY =\n> > DEFAULT_AWB_REGIONS_Y;\n> >   * elsewhere (ALSC and AGC).\n> >   */\n> >\n> > -void AwbMode::read(boost::property_tree::ptree const &params)\n> > +int AwbMode::read(boost::property_tree::ptree const &params)\n> >  {\n> >         ctLo = params.get<double>(\"lo\");\n> >         ctHi = params.get<double>(\"hi\");\n> > +       return 0;\n> >  }\n> >\n> > -void AwbPrior::read(boost::property_tree::ptree const &params)\n> > +int AwbPrior::read(boost::property_tree::ptree const &params)\n> >  {\n> >         lux = params.get<double>(\"lux\");\n> > -       prior.read(params.get_child(\"prior\"));\n> > +       return prior.read(params.get_child(\"prior\"));\n> >  }\n> >\n> > -static void readCtCurve(Pwl &ctR, Pwl &ctB,\n> > -                       boost::property_tree::ptree const &params)\n> > +static int readCtCurve(Pwl &ctR, Pwl &ctB,\n> > +                      boost::property_tree::ptree const &params)\n> >  {\n> >         int num = 0;\n> >         for (auto it = params.begin(); it != params.end(); it++) {\n> > @@ -55,21 +56,28 @@ static void readCtCurve(Pwl &ctR, Pwl &ctB,\n> >         }\n> >         if (num < 2)\n> >                 LOG(RPiAwb, Fatal) << \"AwbConfig: insufficient points in\n> > CT curve\";\n> > +       return 0;\n> >  }\n> >\n> > -void AwbConfig::read(boost::property_tree::ptree const &params)\n> > +int AwbConfig::read(boost::property_tree::ptree const &params)\n> >  {\n> > +       int ret;\n> >         bayes = params.get<int>(\"bayes\", 1);\n> >         framePeriod = params.get<uint16_t>(\"framePeriod\", 10);\n> >         startupFrames = params.get<uint16_t>(\"startupFrames\", 10);\n> >         convergenceFrames = params.get<unsigned int>(\"convergence_frames\",\n> > 3);\n> >         speed = params.get<double>(\"speed\", 0.05);\n> > -       if (params.get_child_optional(\"ct_curve\"))\n> > -               readCtCurve(ctR, ctB, params.get_child(\"ct_curve\"));\n> > +       if (params.get_child_optional(\"ct_curve\")) {\n> > +               ret = readCtCurve(ctR, ctB, params.get_child(\"ct_curve\"));\n> > +               if (ret)\n> > +                       return ret;\n> > +       }\n> >         if (params.get_child_optional(\"priors\")) {\n> >                 for (auto &p : params.get_child(\"priors\")) {\n> >                         AwbPrior prior;\n> > -                       prior.read(p.second);\n> > +                       ret = prior.read(p.second);\n> > +                       if (ret)\n> > +                               return ret;\n> >                         if (!priors.empty() && prior.lux <=\n> > priors.back().lux)\n> >                                 LOG(RPiAwb, Fatal) << \"AwbConfig: Prior\n> > must be ordered in increasing lux value\";\n> >                         priors.push_back(prior);\n> > @@ -79,7 +87,9 @@ void AwbConfig::read(boost::property_tree::ptree const\n> > &params)\n> >         }\n> >         if (params.get_child_optional(\"modes\")) {\n> >                 for (auto &p : params.get_child(\"modes\")) {\n> > -                       modes[p.first].read(p.second);\n> > +                       ret = modes[p.first].read(p.second);\n> > +                       if (ret)\n> > +                               return ret;\n> >                         if (defaultMode == nullptr)\n> >                                 defaultMode = &modes[p.first];\n> >                 }\n> > @@ -110,6 +120,7 @@ void AwbConfig::read(boost::property_tree::ptree const\n> > &params)\n> >         whitepointB = params.get<double>(\"whitepoint_b\", 0.0);\n> >         if (bayes == false)\n> >                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities\n> > make any sense */\n> > +       return 0;\n> >  }\n> >\n> >  Awb::Awb(Controller *controller)\n> > @@ -137,9 +148,9 @@ char const *Awb::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Awb::read(boost::property_tree::ptree const &params)\n> > +int Awb::read(boost::property_tree::ptree const &params)\n> >  {\n> > -       config_.read(params);\n> > +       return config_.read(params);\n> >  }\n> >\n> >  void Awb::initialise()\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h\n> > b/src/ipa/raspberrypi/controller/rpi/awb.h\n> > index 9e075624c429..fa0715735fcf 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h\n> > @@ -19,20 +19,20 @@ namespace RPiController {\n> >  /* Control algorithm to perform AWB calculations. */\n> >\n> >  struct AwbMode {\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >         double ctLo; /* low CT value for search */\n> >         double ctHi; /* high CT value for search */\n> >  };\n> >\n> >  struct AwbPrior {\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >         double lux; /* lux level */\n> >         Pwl prior; /* maps CT to prior log likelihood for this lux level */\n> >  };\n> >\n> >  struct AwbConfig {\n> >         AwbConfig() : defaultMode(nullptr) {}\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >         /* Only repeat the AWB calculation every \"this many\" frames */\n> >         uint16_t framePeriod;\n> >         /* number of initial frames for which speed taken as 1.0 (maximum)\n> > */\n> > @@ -92,7 +92,7 @@ public:\n> >         ~Awb();\n> >         char const *name() const override;\n> >         void initialise() override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         /* AWB handles \"pausing\" for itself. */\n> >         bool isPaused() const override;\n> >         void pause() override;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> > index 0799d7b9195a..aa6f602acd7d 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/black_level.cpp\n> > @@ -31,7 +31,7 @@ char const *BlackLevel::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void BlackLevel::read(boost::property_tree::ptree const &params)\n> > +int BlackLevel::read(boost::property_tree::ptree const &params)\n> >  {\n> >         uint16_t blackLevel = params.get<uint16_t>(\n> >                 \"black_level\", 4096); /* 64 in 10 bits scaled to 16 bits */\n> > @@ -42,6 +42,7 @@ void BlackLevel::read(boost::property_tree::ptree const\n> > &params)\n> >                 << \" Read black levels red \" << blackLevelR_\n> >                 << \" green \" << blackLevelG_\n> >                 << \" blue \" << blackLevelB_;\n> > +       return 0;\n> >  }\n> >\n> >  void BlackLevel::prepare(Metadata *imageMetadata)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.h\n> > b/src/ipa/raspberrypi/controller/rpi/black_level.h\n> > index 7789f261cf03..6ec8c4f9ba8f 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/black_level.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/black_level.h\n> > @@ -18,7 +18,7 @@ class BlackLevel : public Algorithm\n> >  public:\n> >         BlackLevel(Controller *controller);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >\n> >  private:\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> > index cf0c85d26cf9..f0110d38f548 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp\n> > @@ -39,7 +39,7 @@ Matrix::Matrix(double m0, double m1, double m2, double\n> > m3, double m4, double m5,\n> >         m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] =\n> > m4,\n> >         m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;\n> >  }\n> > -void Matrix::read(boost::property_tree::ptree const &params)\n> > +int Matrix::read(boost::property_tree::ptree const &params)\n> >  {\n> >         double *ptr = (double *)m;\n> >         int n = 0;\n> > @@ -50,6 +50,7 @@ void Matrix::read(boost::property_tree::ptree const\n> > &params)\n> >         }\n> >         if (n < 9)\n> >                 LOG(RPiCcm, Fatal) << \"Ccm: too few values in CCM\";\n> > +       return 0;\n> >  }\n> >\n> >  Ccm::Ccm(Controller *controller)\n> > @@ -60,21 +61,32 @@ char const *Ccm::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Ccm::read(boost::property_tree::ptree const &params)\n> > +int Ccm::read(boost::property_tree::ptree const &params)\n> >  {\n> > -       if (params.get_child_optional(\"saturation\"))\n> > -               config_.saturation.read(params.get_child(\"saturation\"));\n> > +       int ret;\n> > +\n> > +       if (params.get_child_optional(\"saturation\")) {\n> > +               ret =\n> > config_.saturation.read(params.get_child(\"saturation\"));\n> > +               if (ret)\n> > +                       return ret;\n> > +       }\n> > +\n> >         for (auto &p : params.get_child(\"ccms\")) {\n> >                 CtCcm ctCcm;\n> >                 ctCcm.ct = p.second.get<double>(\"ct\");\n> > -               ctCcm.ccm.read(p.second.get_child(\"ccm\"));\n> > +               ret = ctCcm.ccm.read(p.second.get_child(\"ccm\"));\n> > +               if (ret)\n> > +                       return ret;\n> >                 if (!config_.ccms.empty() &&\n> >                     ctCcm.ct <= config_.ccms.back().ct)\n> >                         LOG(RPiCcm, Fatal) << \"Ccm: CCM not in increasing\n> > colour temperature order\";\n> >                 config_.ccms.push_back(std::move(ctCcm));\n> >         }\n> > +\n> >         if (config_.ccms.empty())\n> >                 LOG(RPiCcm, Fatal) << \"Ccm: no CCMs specified\";\n> > +\n> > +       return 0;\n> >  }\n> >\n> >  void Ccm::setSaturation(double saturation)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.h\n> > b/src/ipa/raspberrypi/controller/rpi/ccm.h\n> > index b44f1576528f..6b65c7aea8a6 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/ccm.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/ccm.h\n> > @@ -20,7 +20,7 @@ struct Matrix {\n> >                double m6, double m7, double m8);\n> >         Matrix();\n> >         double m[3][3];\n> > -       void read(boost::property_tree::ptree const &params);\n> > +       int read(boost::property_tree::ptree const &params);\n> >  };\n> >  static inline Matrix operator*(double d, Matrix const &m)\n> >  {\n> > @@ -62,7 +62,7 @@ class Ccm : public CcmAlgorithm\n> >  public:\n> >         Ccm(Controller *controller = NULL);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void setSaturation(double saturation) override;\n> >         void initialise() override;\n> >         void prepare(Metadata *imageMetadata) override;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> > index 9e60dc5d47e7..d76dc43b837f 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp\n> > @@ -38,7 +38,7 @@ char const *Contrast::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Contrast::read(boost::property_tree::ptree const &params)\n> > +int Contrast::read(boost::property_tree::ptree const &params)\n> >  {\n> >         /* enable adaptive enhancement by default */\n> >         config_.ceEnable = params.get<int>(\"ce_enable\", 1);\n> > @@ -52,7 +52,7 @@ void Contrast::read(boost::property_tree::ptree const\n> > &params)\n> >         config_.hiHistogram = params.get<double>(\"hi_histogram\", 0.95);\n> >         config_.hiLevel = params.get<double>(\"hi_level\", 0.95);\n> >         config_.hiMax = params.get<double>(\"hi_max\", 2000);\n> > -       config_.gammaCurve.read(params.get_child(\"gamma_curve\"));\n> > +       return config_.gammaCurve.read(params.get_child(\"gamma_curve\"));\n> >  }\n> >\n> >  void Contrast::setBrightness(double brightness)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.h\n> > b/src/ipa/raspberrypi/controller/rpi/contrast.h\n> > index b1375d819dbc..5c3d2f56b310 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/contrast.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/contrast.h\n> > @@ -34,7 +34,7 @@ class Contrast : public ContrastAlgorithm\n> >  public:\n> >         Contrast(Controller *controller = NULL);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void setBrightness(double brightness) override;\n> >         void setContrast(double contrast) override;\n> >         void initialise() override;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> > index d5d784e7ca64..be014a05fd41 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp\n> > @@ -31,11 +31,12 @@ char const *Dpc::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Dpc::read(boost::property_tree::ptree const &params)\n> > +int Dpc::read(boost::property_tree::ptree const &params)\n> >  {\n> >         config_.strength = params.get<int>(\"strength\", 1);\n> >         if (config_.strength < 0 || config_.strength > 2)\n> >                 LOG(RPiDpc, Fatal) << \"Dpc: bad strength value\";\n> > +       return 0;\n> >  }\n> >\n> >  void Dpc::prepare(Metadata *imageMetadata)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.h\n> > b/src/ipa/raspberrypi/controller/rpi/dpc.h\n> > index 4c1bdec6dd87..2bb6cef565a7 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/dpc.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/dpc.h\n> > @@ -22,7 +22,7 @@ class Dpc : public Algorithm\n> >  public:\n> >         Dpc(Controller *controller);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >\n> >  private:\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> > index 696da4aeea59..a74447877bf8 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp\n> > @@ -35,14 +35,20 @@ char const *Geq::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Geq::read(boost::property_tree::ptree const &params)\n> > +int Geq::read(boost::property_tree::ptree const &params)\n> >  {\n> >         config_.offset = params.get<uint16_t>(\"offset\", 0);\n> >         config_.slope = params.get<double>(\"slope\", 0.0);\n> >         if (config_.slope < 0.0 || config_.slope >= 1.0)\n> >                 LOG(RPiGeq, Fatal) << \"Geq: bad slope value\";\n> > -       if (params.get_child_optional(\"strength\"))\n> > -               config_.strength.read(params.get_child(\"strength\"));\n> > +\n> > +       if (params.get_child_optional(\"strength\")) {\n> > +               int ret =\n> > config_.strength.read(params.get_child(\"strength\"));\n> > +               if (ret)\n> > +                       return ret;\n> > +       }\n> > +\n> > +       return 0;\n> >  }\n> >\n> >  void Geq::prepare(Metadata *imageMetadata)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/geq.h\n> > b/src/ipa/raspberrypi/controller/rpi/geq.h\n> > index 5d06b9cbd529..677a0510c6ac 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/geq.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/geq.h\n> > @@ -24,7 +24,7 @@ class Geq : public Algorithm\n> >  public:\n> >         Geq(Controller *controller);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >\n> >  private:\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > index 95c0a93b2810..ca1e827191fd 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp\n> > @@ -38,7 +38,7 @@ char const *Lux::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Lux::read(boost::property_tree::ptree const &params)\n> > +int Lux::read(boost::property_tree::ptree const &params)\n> >  {\n> >         referenceShutterSpeed_ =\n> >                 params.get<double>(\"reference_shutter_speed\") * 1.0us;\n> > @@ -47,6 +47,7 @@ void Lux::read(boost::property_tree::ptree const &params)\n> >         referenceY_ = params.get<double>(\"reference_Y\");\n> >         referenceLux_ = params.get<double>(\"reference_lux\");\n> >         currentAperture_ = referenceAperture_;\n> > +       return 0;\n> >  }\n> >\n> >  void Lux::setCurrentAperture(double aperture)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.h\n> > b/src/ipa/raspberrypi/controller/rpi/lux.h\n> > index 26af8185911c..6416dfb52553 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/lux.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.h\n> > @@ -22,7 +22,7 @@ class Lux : public Algorithm\n> >  public:\n> >         Lux(Controller *controller);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >         void process(StatisticsPtr &stats, Metadata *imageMetadata)\n> > override;\n> >         void setCurrentAperture(double aperture);\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> > index 5d2c85ad3e74..74fa99bafe48 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp\n> > @@ -41,10 +41,11 @@ void Noise::switchMode(CameraMode const &cameraMode,\n> >         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n> >  }\n> >\n> > -void Noise::read(boost::property_tree::ptree const &params)\n> > +int Noise::read(boost::property_tree::ptree const &params)\n> >  {\n> >         referenceConstant_ = params.get<double>(\"reference_constant\");\n> >         referenceSlope_ = params.get<double>(\"reference_slope\");\n> > +       return 0;\n> >  }\n> >\n> >  void Noise::prepare(Metadata *imageMetadata)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/noise.h\n> > b/src/ipa/raspberrypi/controller/rpi/noise.h\n> > index 144e36529f4c..b33a5fc75ec7 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/noise.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/noise.h\n> > @@ -19,7 +19,7 @@ public:\n> >         Noise(Controller *controller);\n> >         char const *name() const override;\n> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> > override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >\n> >  private:\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > index af31bd0881f0..03d9f119a338 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp\n> > @@ -34,10 +34,11 @@ char const *Sdn::name() const\n> >         return NAME;\n> >  }\n> >\n> > -void Sdn::read(boost::property_tree::ptree const &params)\n> > +int Sdn::read(boost::property_tree::ptree const &params)\n> >  {\n> >         deviation_ = params.get<double>(\"deviation\", 3.2);\n> >         strength_ = params.get<double>(\"strength\", 0.75);\n> > +       return 0;\n> >  }\n> >\n> >  void Sdn::initialise()\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.h\n> > b/src/ipa/raspberrypi/controller/rpi/sdn.h\n> > index 90ea37ff5550..4287ef08a79f 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.h\n> > @@ -18,7 +18,7 @@ class Sdn : public DenoiseAlgorithm\n> >  public:\n> >         Sdn(Controller *controller = NULL);\n> >         char const *name() const override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void initialise() override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >         void setMode(DenoiseMode mode) override;\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> > b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> > index 36b75f69afce..9c4cffa4128c 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp\n> > @@ -37,7 +37,7 @@ void Sharpen::switchMode(CameraMode const &cameraMode,\n> >         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);\n> >  }\n> >\n> > -void Sharpen::read(boost::property_tree::ptree const &params)\n> > +int Sharpen::read(boost::property_tree::ptree const &params)\n> >  {\n> >         threshold_ = params.get<double>(\"threshold\", 1.0);\n> >         strength_ = params.get<double>(\"strength\", 1.0);\n> > @@ -46,6 +46,7 @@ void Sharpen::read(boost::property_tree::ptree const\n> > &params)\n> >                 << \"Read threshold \" << threshold_\n> >                 << \" strength \" << strength_\n> >                 << \" limit \" << limit_;\n> > +       return 0;\n> >  }\n> >\n> >  void Sharpen::setStrength(double strength)\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> > b/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> > index 8846f2ae09a2..0cd8b92f2224 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.h\n> > @@ -19,7 +19,7 @@ public:\n> >         Sharpen(Controller *controller);\n> >         char const *name() const override;\n> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)\n> > override;\n> > -       void read(boost::property_tree::ptree const &params) override;\n> > +       int read(boost::property_tree::ptree const &params) override;\n> >         void setStrength(double strength) override;\n> >         void prepare(Metadata *imageMetadata) override;\n> >\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> > b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 9d550354d78e..b9e9b814e886 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -229,7 +229,14 @@ int IPARPi::init(const IPASettings &settings,\n> > IPAInitResult *result)\n> >         result->sensorConfig.sensorMetadata = sensorMetadata;\n> >\n> >         /* Load the tuning file for this sensor. */\n> > -       controller_.read(settings.configurationFile.c_str());\n> > +       int ret = controller_.read(settings.configurationFile.c_str());\n> > +       if (ret) {\n> > +               LOG(IPARPI, Error)\n> > +                       << \"Failed to load tuning data file \"\n> > +                       << settings.configurationFile;\n> > +               return ret;\n> > +       }\n> > +\n> >         controller_.initialise();\n> >\n> >         /* Return the controls handled by the IPA */","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 66218C3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Jul 2022 17:00:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A6ACF63311;\n\tWed, 27 Jul 2022 19:00:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 943F263309\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jul 2022 19:00:18 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5A2A835;\n\tWed, 27 Jul 2022 19:00:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658941219;\n\tbh=IyVZN1nO/ypIpHnHijRWgpwNeRgpyWcmskTD/64r4ps=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H8rzEf2YrqWbLujusZ2FpO8L+Ofn6UdD+FgsN3+I9Za05bAoFS27lAZ7JbiY948hN\n\tWSnx/AahPZBvoE0NdZZKYM7Vc0IkimAxvcvS6EN6n4dPT0TP4lwmL1km+xU1DwQ1hr\n\tpwyAzNC0OTAbXg5NgatLLG/B/Sg2TCSXdzK2V/VhjiemdVn6i4DKR+VSs7w9Gt3El8\n\tjf2QwNxa02hY6Lj31wZCHggSr6d2U6e+UTUrCen6KE/CWU5gZc+Xr3IwTOPSPpKwhj\n\tuI/gE6IDwdDPaVP83PQVw/fnrodkfDWT2t40Jj5qW4PxpogVKBmQoiL3xizQwxWqec\n\t4XH7KKFoOsKDA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658941218;\n\tbh=IyVZN1nO/ypIpHnHijRWgpwNeRgpyWcmskTD/64r4ps=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FkQGYsjNbYPeFiyZMajIzyrwabMZ9xOMjEOAU0ECZPIgDMDLJ8Di/lo/59Oc3VzPT\n\tbedYu+a/LrJh2KVWim4mCRANh3XgCbRok0Wf4iaAvB4biUxafk5Upks6hiPrxjAN/n\n\t4MHKiwyG3a2xtK7sdSS9UNGPXCQUbfzPXmOdjPjE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FkQGYsjN\"; dkim-atps=neutral","Date":"Wed, 27 Jul 2022 20:00:16 +0300","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YuFvIA9/kiitQRff@pendragon.ideasonboard.com>","References":"<20220727023816.30008-1-laurent.pinchart@ideasonboard.com>\n\t<20220727023816.30008-6-laurent.pinchart@ideasonboard.com>\n\t<CAEmqJPoiJO73YXY_BV7Ue4D8f+d3Ci6mEQgY4DnynWvtKi6XDA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPoiJO73YXY_BV7Ue4D8f+d3Ci6mEQgY4DnynWvtKi6XDA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v7 05/14] ipa: raspberrypi: Return an\n\terror code from Algorithm::read()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]