[libcamera-devel,v7,05/14] ipa: raspberrypi: Return an error code from Algorithm::read()

16817 diff mbox series

When encountering errors, the Algorithm::read() function either uses
LOG(Fatal) or throws exceptions from the boost property_tree functions.
To prepare for replacing boost JSON parse with the YamlParser class,
give the Algorithm::read() function the ability to return an error code,
and propagate it all the way to the IPA module init() function.

All algorithm classes return a hardcoded 0 value for now, subsequent
commits will change that.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/raspberrypi/controller/algorithm.cpp  |  3 +-
 src/ipa/raspberrypi/controller/algorithm.h    |  2 +-
 src/ipa/raspberrypi/controller/controller.cpp |  8 +++-
 src/ipa/raspberrypi/controller/controller.h   |  2 +-
 src/ipa/raspberrypi/controller/pwl.cpp        |  3 +-
 src/ipa/raspberrypi/controller/pwl.h          |  2 +-
 src/ipa/raspberrypi/controller/rpi/agc.cpp    | 28 ++++++++++----
 src/ipa/raspberrypi/controller/rpi/agc.h      | 10 ++---
 src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 38 +++++++++++++------
 src/ipa/raspberrypi/controller/rpi/alsc.h     |  2 +-
 src/ipa/raspberrypi/controller/rpi/awb.cpp    | 35 +++++++++++------
 src/ipa/raspberrypi/controller/rpi/awb.h      |  8 ++--
 .../controller/rpi/black_level.cpp            |  3 +-
 .../raspberrypi/controller/rpi/black_level.h  |  2 +-
 src/ipa/raspberrypi/controller/rpi/ccm.cpp    | 22 ++++++++---
 src/ipa/raspberrypi/controller/rpi/ccm.h      |  4 +-
 .../raspberrypi/controller/rpi/contrast.cpp   |  4 +-
 src/ipa/raspberrypi/controller/rpi/contrast.h |  2 +-
 src/ipa/raspberrypi/controller/rpi/dpc.cpp    |  3 +-
 src/ipa/raspberrypi/controller/rpi/dpc.h      |  2 +-
 src/ipa/raspberrypi/controller/rpi/geq.cpp    | 12 ++++--
 src/ipa/raspberrypi/controller/rpi/geq.h      |  2 +-
 src/ipa/raspberrypi/controller/rpi/lux.cpp    |  3 +-
 src/ipa/raspberrypi/controller/rpi/lux.h      |  2 +-
 src/ipa/raspberrypi/controller/rpi/noise.cpp  |  3 +-
 src/ipa/raspberrypi/controller/rpi/noise.h    |  2 +-
 src/ipa/raspberrypi/controller/rpi/sdn.cpp    |  3 +-
 src/ipa/raspberrypi/controller/rpi/sdn.h      |  2 +-
 .../raspberrypi/controller/rpi/sharpen.cpp    |  3 +-
 src/ipa/raspberrypi/controller/rpi/sharpen.h  |  2 +-
 src/ipa/raspberrypi/raspberrypi.cpp           |  9 ++++-
 31 files changed, 151 insertions(+), 75 deletions(-)

Hi Laurent,

Thank you for your patch.

On Wed, 27 Jul 2022 at 03:38, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> When encountering errors, the Algorithm::read() function either uses
> LOG(Fatal) or throws exceptions from the boost property_tree functions.
> To prepare for replacing boost JSON parse with the YamlParser class,
> give the Algorithm::read() function the ability to return an error code,
> and propagate it all the way to the IPA module init() function.
>
> All algorithm classes return a hardcoded 0 value for now, subsequent
> commits will change that.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/raspberrypi/controller/algorithm.cpp  |  3 +-
>  src/ipa/raspberrypi/controller/algorithm.h    |  2 +-
>  src/ipa/raspberrypi/controller/controller.cpp |  8 +++-
>  src/ipa/raspberrypi/controller/controller.h   |  2 +-
>  src/ipa/raspberrypi/controller/pwl.cpp        |  3 +-
>  src/ipa/raspberrypi/controller/pwl.h          |  2 +-
>  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 28 ++++++++++----
>  src/ipa/raspberrypi/controller/rpi/agc.h      | 10 ++---
>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 38 +++++++++++++------
>  src/ipa/raspberrypi/controller/rpi/alsc.h     |  2 +-
>  src/ipa/raspberrypi/controller/rpi/awb.cpp    | 35 +++++++++++------
>  src/ipa/raspberrypi/controller/rpi/awb.h      |  8 ++--
>  .../controller/rpi/black_level.cpp            |  3 +-
>  .../raspberrypi/controller/rpi/black_level.h  |  2 +-
>  src/ipa/raspberrypi/controller/rpi/ccm.cpp    | 22 ++++++++---
>  src/ipa/raspberrypi/controller/rpi/ccm.h      |  4 +-
>  .../raspberrypi/controller/rpi/contrast.cpp   |  4 +-
>  src/ipa/raspberrypi/controller/rpi/contrast.h |  2 +-
>  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |  3 +-
>  src/ipa/raspberrypi/controller/rpi/dpc.h      |  2 +-
>  src/ipa/raspberrypi/controller/rpi/geq.cpp    | 12 ++++--
>  src/ipa/raspberrypi/controller/rpi/geq.h      |  2 +-
>  src/ipa/raspberrypi/controller/rpi/lux.cpp    |  3 +-
>  src/ipa/raspberrypi/controller/rpi/lux.h      |  2 +-
>  src/ipa/raspberrypi/controller/rpi/noise.cpp  |  3 +-
>  src/ipa/raspberrypi/controller/rpi/noise.h    |  2 +-
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |  3 +-
>  src/ipa/raspberrypi/controller/rpi/sdn.h      |  2 +-
>  .../raspberrypi/controller/rpi/sharpen.cpp    |  3 +-
>  src/ipa/raspberrypi/controller/rpi/sharpen.h  |  2 +-
>  src/ipa/raspberrypi/raspberrypi.cpp           |  9 ++++-
>  31 files changed, 151 insertions(+), 75 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp
> b/src/ipa/raspberrypi/controller/algorithm.cpp
> index 1a7d20a4731b..d73cb36fe2d6 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.cpp
> +++ b/src/ipa/raspberrypi/controller/algorithm.cpp
> @@ -9,8 +9,9 @@
>
>  using namespace RPiController;
>
> -void Algorithm::read([[maybe_unused]] boost::property_tree::ptree const
> &params)
> +int Algorithm::read([[maybe_unused]] boost::property_tree::ptree const
> &params)
>  {
> +       return 0;
>  }
>
>  void Algorithm::initialise()
> diff --git a/src/ipa/raspberrypi/controller/algorithm.h
> b/src/ipa/raspberrypi/controller/algorithm.h
> index 92fd895d2b06..0c5566fda874 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.h
> +++ b/src/ipa/raspberrypi/controller/algorithm.h
> @@ -35,7 +35,7 @@ public:
>         virtual bool isPaused() const { return paused_; }
>         virtual void pause() { paused_ = true; }
>         virtual void resume() { paused_ = false; }
> -       virtual void read(boost::property_tree::ptree const &params);
> +       virtual int read(boost::property_tree::ptree const &params);
>         virtual void initialise();
>         virtual void switchMode(CameraMode const &cameraMode, Metadata
> *metadata);
>         virtual void prepare(Metadata *imageMetadata);
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp
> b/src/ipa/raspberrypi/controller/controller.cpp
> index 872a32302410..d91ac90704cb 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -32,19 +32,23 @@ Controller::Controller(char const *jsonFilename)
>
>  Controller::~Controller() {}
>
> -void Controller::read(char const *filename)
> +int Controller::read(char const *filename)
>

Since read now returns an error code, we probably ought to remove the
Controller(char const *jsonFilename) constructor.  This never gets used
anyway.  But that can be done on top of this work.

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>



>  {
>         boost::property_tree::ptree root;
>         boost::property_tree::read_json(filename, root);
>         for (auto const &keyAndValue : root) {
>                 Algorithm *algo =
> createAlgorithm(keyAndValue.first.c_str());
>                 if (algo) {
> -                       algo->read(keyAndValue.second);
> +                       int ret = algo->read(keyAndValue.second);
> +                       if (ret)
> +                               return ret;
>                         algorithms_.push_back(AlgorithmPtr(algo));
>                 } else
>                         LOG(RPiController, Warning)
>                                 << "No algorithm found for \"" <<
> keyAndValue.first << "\"";
>         }
> +
> +       return 0;
>  }
>
>  Algorithm *Controller::createAlgorithm(char const *name)
> diff --git a/src/ipa/raspberrypi/controller/controller.h
> b/src/ipa/raspberrypi/controller/controller.h
> index e28e30d7d574..841783bb7a5c 100644
> --- a/src/ipa/raspberrypi/controller/controller.h
> +++ b/src/ipa/raspberrypi/controller/controller.h
> @@ -41,7 +41,7 @@ public:
>         Controller(char const *jsonFilename);
>         ~Controller();
>         Algorithm *createAlgorithm(char const *name);
> -       void read(char const *filename);
> +       int read(char const *filename);
>         void initialise();
>         void switchMode(CameraMode const &cameraMode, Metadata *metadata);
>         void prepare(Metadata *imageMetadata);
> diff --git a/src/ipa/raspberrypi/controller/pwl.cpp
> b/src/ipa/raspberrypi/controller/pwl.cpp
> index 8b8db722966b..fde0b298c6ce 100644
> --- a/src/ipa/raspberrypi/controller/pwl.cpp
> +++ b/src/ipa/raspberrypi/controller/pwl.cpp
> @@ -12,7 +12,7 @@
>
>  using namespace RPiController;
>
> -void Pwl::read(boost::property_tree::ptree const &params)
> +int Pwl::read(boost::property_tree::ptree const &params)
>  {
>         for (auto it = params.begin(); it != params.end(); it++) {
>                 double x = it->second.get_value<double>();
> @@ -22,6 +22,7 @@ void Pwl::read(boost::property_tree::ptree const &params)
>                 points_.push_back(Point(x, y));
>         }
>         assert(points_.size() >= 2);
> +       return 0;
>  }
>
>  void Pwl::append(double x, double y, const double eps)
> diff --git a/src/ipa/raspberrypi/controller/pwl.h
> b/src/ipa/raspberrypi/controller/pwl.h
> index 973efdf59945..ef1cc2ed113a 100644
> --- a/src/ipa/raspberrypi/controller/pwl.h
> +++ b/src/ipa/raspberrypi/controller/pwl.h
> @@ -57,7 +57,7 @@ public:
>         };
>         Pwl() {}
>         Pwl(std::vector<Point> const &points) : points_(points) {}
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>         void append(double x, double y, const double eps = 1e-6);
>         void prepend(double x, double y, const double eps = 1e-6);
>         Interval domain() const;
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index adec8592626d..130c606d4136 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -30,7 +30,7 @@ LOG_DEFINE_CATEGORY(RPiAgc)
>
>  static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit
> pipeline */
>
> -void AgcMeteringMode::read(boost::property_tree::ptree const &params)
> +int AgcMeteringMode::read(boost::property_tree::ptree const &params)
>  {
>         int num = 0;
>         for (auto &p : params.get_child("weights")) {
> @@ -40,6 +40,7 @@ void AgcMeteringMode::read(boost::property_tree::ptree
> const &params)
>         }
>         if (num != AgcStatsSize)
>                 LOG(RPiAgc, Fatal) << "AgcMeteringMode: insufficient
> weights";
> +       return 0;
>  }
>
>  static std::string
> @@ -73,7 +74,7 @@ static int readList(std::vector<Duration> &list,
>         return list.size();
>  }
>
> -void AgcExposureMode::read(boost::property_tree::ptree const &params)
> +int AgcExposureMode::read(boost::property_tree::ptree const &params)
>  {
>         int numShutters = readList(shutter, params.get_child("shutter"));
>         int numAgs = readList(gain, params.get_child("gain"));
> @@ -83,6 +84,7 @@ void AgcExposureMode::read(boost::property_tree::ptree
> const &params)
>         if (numShutters != numAgs)
>                 LOG(RPiAgc, Fatal)
>                         << "AgcExposureMode: expect same number of
> exposure and gain entries in exposure profile";
> +       return 0;
>  }
>
>  static std::string
> @@ -100,7 +102,7 @@ readExposureModes(std::map<std::string,
> AgcExposureMode> &exposureModes,
>         return first;
>  }
>
> -void AgcConstraint::read(boost::property_tree::ptree const &params)
> +int AgcConstraint::read(boost::property_tree::ptree const &params)
>  {
>         std::string boundString = params.get<std::string>("bound", "");
>         transform(boundString.begin(), boundString.end(),
> @@ -110,7 +112,7 @@ void AgcConstraint::read(boost::property_tree::ptree
> const &params)
>         bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER;
>         qLo = params.get<double>("q_lo");
>         qHi = params.get<double>("q_hi");
> -       yTarget.read(params.get_child("y_target"));
> +       return yTarget.read(params.get_child("y_target"));
>  }
>
>  static AgcConstraintMode
> @@ -137,13 +139,17 @@ static std::string
> readConstraintModes(std::map<std::string, AgcConstraintMode>
>         return first;
>  }
>
> -void AgcConfig::read(boost::property_tree::ptree const &params)
> +int AgcConfig::read(boost::property_tree::ptree const &params)
>  {
>         LOG(RPiAgc, Debug) << "AgcConfig";
>         defaultMeteringMode = readMeteringModes(meteringModes,
> params.get_child("metering_modes"));
>         defaultExposureMode = readExposureModes(exposureModes,
> params.get_child("exposure_modes"));
>         defaultConstraintMode = readConstraintModes(constraintModes,
> params.get_child("constraint_modes"));
> -       yTarget.read(params.get_child("y_target"));
> +
> +       int ret = yTarget.read(params.get_child("y_target"));
> +       if (ret)
> +               return ret;
> +
>         speed = params.get<double>("speed", 0.2);
>         startupFrames = params.get<uint16_t>("startup_frames", 10);
>         convergenceFrames = params.get<unsigned int>("convergence_frames",
> 6);
> @@ -152,6 +158,7 @@ void AgcConfig::read(boost::property_tree::ptree const
> &params)
>         /* Start with quite a low value as ramping up is easier than
> ramping down. */
>         defaultExposureTime = params.get<double>("default_exposure_time",
> 1000) * 1us;
>         defaultAnalogueGain = params.get<double>("default_analogueGain",
> 1.0);
> +       return 0;
>  }
>
>  Agc::ExposureValues::ExposureValues()
> @@ -182,10 +189,14 @@ char const *Agc::name() const
>         return NAME;
>  }
>
> -void Agc::read(boost::property_tree::ptree const &params)
> +int Agc::read(boost::property_tree::ptree const &params)
>  {
>         LOG(RPiAgc, Debug) << "Agc";
> -       config_.read(params);
> +
> +       int ret = config_.read(params);
> +       if (ret)
> +               return ret;
> +
>         /*
>          * Set the config's defaults (which are the first ones it read) as
> our
>          * current modes, until someone changes them.  (they're all known
> to
> @@ -200,6 +211,7 @@ void Agc::read(boost::property_tree::ptree const
> &params)
>         /* Set up the "last shutter/gain" values, in case AGC starts
> "disabled". */
>         status_.shutterTime = config_.defaultExposureTime;
>         status_.analogueGain = config_.defaultAnalogueGain;
> +       return 0;
>  }
>
>  bool Agc::isPaused() const
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h
> b/src/ipa/raspberrypi/controller/rpi/agc.h
> index f57afa6dc80c..1351b0ee079c 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.h
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.h
> @@ -28,13 +28,13 @@ namespace RPiController {
>
>  struct AgcMeteringMode {
>         double weights[AgcStatsSize];
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>  };
>
>  struct AgcExposureMode {
>         std::vector<libcamera::utils::Duration> shutter;
>         std::vector<double> gain;
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>  };
>
>  struct AgcConstraint {
> @@ -43,13 +43,13 @@ struct AgcConstraint {
>         double qLo;
>         double qHi;
>         Pwl yTarget;
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>  };
>
>  typedef std::vector<AgcConstraint> AgcConstraintMode;
>
>  struct AgcConfig {
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>         std::map<std::string, AgcMeteringMode> meteringModes;
>         std::map<std::string, AgcExposureMode> exposureModes;
>         std::map<std::string, AgcConstraintMode> constraintModes;
> @@ -74,7 +74,7 @@ class Agc : public AgcAlgorithm
>  public:
>         Agc(Controller *controller);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         /* AGC handles "pausing" for itself. */
>         bool isPaused() const override;
>         void pause() override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> index 03ae33501dc0..b36277696a52 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> @@ -50,7 +50,7 @@ char const *Alsc::name() const
>         return NAME;
>  }
>
> -static void generateLut(double *lut, boost::property_tree::ptree const
> &params)
> +static int generateLut(double *lut, boost::property_tree::ptree const
> &params)
>  {
>         double cstrength = params.get<double>("corner_strength", 2.0);
>         if (cstrength <= 1.0)
> @@ -71,9 +71,10 @@ static void generateLut(double *lut,
> boost::property_tree::ptree const &params)
>                                 (f2 * f2); /* this reproduces the cos^4
> rule */
>                 }
>         }
> +       return 0;
>  }
>
> -static void readLut(double *lut, boost::property_tree::ptree const
> &params)
> +static int readLut(double *lut, boost::property_tree::ptree const &params)
>  {
>         int num = 0;
>         const int maxNum = XY;
> @@ -84,11 +85,12 @@ static void readLut(double *lut,
> boost::property_tree::ptree const &params)
>         }
>         if (num < maxNum)
>                 LOG(RPiAlsc, Fatal) << "Alsc: too few entries in LSC
> table";
> +       return 0;
>  }
>
> -static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> -                            boost::property_tree::ptree const &params,
> -                            std::string const &name)
> +static int readCalibrations(std::vector<AlscCalibration> &calibrations,
> +                           boost::property_tree::ptree const &params,
> +                           std::string const &name)
>  {
>         if (params.get_child_optional(name)) {
>                 double lastCt = 0;
> @@ -117,9 +119,10 @@ static void
> readCalibrations(std::vector<AlscCalibration> &calibrations,
>                                 << "Read " << name << " calibration for ct
> " << ct;
>                 }
>         }
> +       return 0;
>  }
>
> -void Alsc::read(boost::property_tree::ptree const &params)
> +int Alsc::read(boost::property_tree::ptree const &params)
>  {
>         config_.framePeriod = params.get<uint16_t>("frame_period", 12);
>         config_.startupFrames = params.get<uint16_t>("startup_frames", 10);
> @@ -135,19 +138,32 @@ void Alsc::read(boost::property_tree::ptree const
> &params)
>                 params.get<double>("luminance_strength", 1.0);
>         for (int i = 0; i < XY; i++)
>                 config_.luminanceLut[i] = 1.0;
> +
> +       int ret = 0;
> +
>         if (params.get_child_optional("corner_strength"))
> -               generateLut(config_.luminanceLut, params);
> +               ret = generateLut(config_.luminanceLut, params);
>         else if (params.get_child_optional("luminance_lut"))
> -               readLut(config_.luminanceLut,
> -                       params.get_child("luminance_lut"));
> +               ret = readLut(config_.luminanceLut,
> +                             params.get_child("luminance_lut"));
>         else
>                 LOG(RPiAlsc, Warning)
>                         << "no luminance table - assume unity everywhere";
> -       readCalibrations(config_.calibrationsCr, params,
> "calibrations_Cr");
> -       readCalibrations(config_.calibrationsCb, params,
> "calibrations_Cb");
> +       if (ret)
> +               return ret;
> +
> +       ret = readCalibrations(config_.calibrationsCr, params,
> "calibrations_Cr");
> +       if (ret)
> +               return ret;
> +       ret = readCalibrations(config_.calibrationsCb, params,
> "calibrations_Cb");
> +       if (ret)
> +               return ret;
> +
>         config_.defaultCt = params.get<double>("default_ct", 4500.0);
>         config_.threshold = params.get<double>("threshold", 1e-3);
>         config_.lambdaBound = params.get<double>("lambda_bound", 0.05);
> +
> +       return 0;
>  }
>
>  static double getCt(Metadata *metadata, double defaultCt);
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h
> b/src/ipa/raspberrypi/controller/rpi/alsc.h
> index 4e9a715ae0ab..773fd338ea8e 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.h
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h
> @@ -52,7 +52,7 @@ public:
>         char const *name() const override;
>         void initialise() override;
>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>         void process(StatisticsPtr &stats, Metadata *imageMetadata)
> override;
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index ad75d55f0976..a9e776a344a1 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -26,20 +26,21 @@ static constexpr unsigned int AwbStatsSizeY =
> DEFAULT_AWB_REGIONS_Y;
>   * elsewhere (ALSC and AGC).
>   */
>
> -void AwbMode::read(boost::property_tree::ptree const &params)
> +int AwbMode::read(boost::property_tree::ptree const &params)
>  {
>         ctLo = params.get<double>("lo");
>         ctHi = params.get<double>("hi");
> +       return 0;
>  }
>
> -void AwbPrior::read(boost::property_tree::ptree const &params)
> +int AwbPrior::read(boost::property_tree::ptree const &params)
>  {
>         lux = params.get<double>("lux");
> -       prior.read(params.get_child("prior"));
> +       return prior.read(params.get_child("prior"));
>  }
>
> -static void readCtCurve(Pwl &ctR, Pwl &ctB,
> -                       boost::property_tree::ptree const &params)
> +static int readCtCurve(Pwl &ctR, Pwl &ctB,
> +                      boost::property_tree::ptree const &params)
>  {
>         int num = 0;
>         for (auto it = params.begin(); it != params.end(); it++) {
> @@ -55,21 +56,28 @@ static void readCtCurve(Pwl &ctR, Pwl &ctB,
>         }
>         if (num < 2)
>                 LOG(RPiAwb, Fatal) << "AwbConfig: insufficient points in
> CT curve";
> +       return 0;
>  }
>
> -void AwbConfig::read(boost::property_tree::ptree const &params)
> +int AwbConfig::read(boost::property_tree::ptree const &params)
>  {
> +       int ret;
>         bayes = params.get<int>("bayes", 1);
>         framePeriod = params.get<uint16_t>("framePeriod", 10);
>         startupFrames = params.get<uint16_t>("startupFrames", 10);
>         convergenceFrames = params.get<unsigned int>("convergence_frames",
> 3);
>         speed = params.get<double>("speed", 0.05);
> -       if (params.get_child_optional("ct_curve"))
> -               readCtCurve(ctR, ctB, params.get_child("ct_curve"));
> +       if (params.get_child_optional("ct_curve")) {
> +               ret = readCtCurve(ctR, ctB, params.get_child("ct_curve"));
> +               if (ret)
> +                       return ret;
> +       }
>         if (params.get_child_optional("priors")) {
>                 for (auto &p : params.get_child("priors")) {
>                         AwbPrior prior;
> -                       prior.read(p.second);
> +                       ret = prior.read(p.second);
> +                       if (ret)
> +                               return ret;
>                         if (!priors.empty() && prior.lux <=
> priors.back().lux)
>                                 LOG(RPiAwb, Fatal) << "AwbConfig: Prior
> must be ordered in increasing lux value";
>                         priors.push_back(prior);
> @@ -79,7 +87,9 @@ void AwbConfig::read(boost::property_tree::ptree const
> &params)
>         }
>         if (params.get_child_optional("modes")) {
>                 for (auto &p : params.get_child("modes")) {
> -                       modes[p.first].read(p.second);
> +                       ret = modes[p.first].read(p.second);
> +                       if (ret)
> +                               return ret;
>                         if (defaultMode == nullptr)
>                                 defaultMode = &modes[p.first];
>                 }
> @@ -110,6 +120,7 @@ void AwbConfig::read(boost::property_tree::ptree const
> &params)
>         whitepointB = params.get<double>("whitepoint_b", 0.0);
>         if (bayes == false)
>                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities
> make any sense */
> +       return 0;
>  }
>
>  Awb::Awb(Controller *controller)
> @@ -137,9 +148,9 @@ char const *Awb::name() const
>         return NAME;
>  }
>
> -void Awb::read(boost::property_tree::ptree const &params)
> +int Awb::read(boost::property_tree::ptree const &params)
>  {
> -       config_.read(params);
> +       return config_.read(params);
>  }
>
>  void Awb::initialise()
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> b/src/ipa/raspberrypi/controller/rpi/awb.h
> index 9e075624c429..fa0715735fcf 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> @@ -19,20 +19,20 @@ namespace RPiController {
>  /* Control algorithm to perform AWB calculations. */
>
>  struct AwbMode {
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>         double ctLo; /* low CT value for search */
>         double ctHi; /* high CT value for search */
>  };
>
>  struct AwbPrior {
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>         double lux; /* lux level */
>         Pwl prior; /* maps CT to prior log likelihood for this lux level */
>  };
>
>  struct AwbConfig {
>         AwbConfig() : defaultMode(nullptr) {}
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>         /* Only repeat the AWB calculation every "this many" frames */
>         uint16_t framePeriod;
>         /* number of initial frames for which speed taken as 1.0 (maximum)
> */
> @@ -92,7 +92,7 @@ public:
>         ~Awb();
>         char const *name() const override;
>         void initialise() override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         /* AWB handles "pausing" for itself. */
>         bool isPaused() const override;
>         void pause() override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> index 0799d7b9195a..aa6f602acd7d 100644
> --- a/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> @@ -31,7 +31,7 @@ char const *BlackLevel::name() const
>         return NAME;
>  }
>
> -void BlackLevel::read(boost::property_tree::ptree const &params)
> +int BlackLevel::read(boost::property_tree::ptree const &params)
>  {
>         uint16_t blackLevel = params.get<uint16_t>(
>                 "black_level", 4096); /* 64 in 10 bits scaled to 16 bits */
> @@ -42,6 +42,7 @@ void BlackLevel::read(boost::property_tree::ptree const
> &params)
>                 << " Read black levels red " << blackLevelR_
>                 << " green " << blackLevelG_
>                 << " blue " << blackLevelB_;
> +       return 0;
>  }
>
>  void BlackLevel::prepare(Metadata *imageMetadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.h
> b/src/ipa/raspberrypi/controller/rpi/black_level.h
> index 7789f261cf03..6ec8c4f9ba8f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/black_level.h
> +++ b/src/ipa/raspberrypi/controller/rpi/black_level.h
> @@ -18,7 +18,7 @@ class BlackLevel : public Algorithm
>  public:
>         BlackLevel(Controller *controller);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> index cf0c85d26cf9..f0110d38f548 100644
> --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> @@ -39,7 +39,7 @@ Matrix::Matrix(double m0, double m1, double m2, double
> m3, double m4, double m5,
>         m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] =
> m4,
>         m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;
>  }
> -void Matrix::read(boost::property_tree::ptree const &params)
> +int Matrix::read(boost::property_tree::ptree const &params)
>  {
>         double *ptr = (double *)m;
>         int n = 0;
> @@ -50,6 +50,7 @@ void Matrix::read(boost::property_tree::ptree const
> &params)
>         }
>         if (n < 9)
>                 LOG(RPiCcm, Fatal) << "Ccm: too few values in CCM";
> +       return 0;
>  }
>
>  Ccm::Ccm(Controller *controller)
> @@ -60,21 +61,32 @@ char const *Ccm::name() const
>         return NAME;
>  }
>
> -void Ccm::read(boost::property_tree::ptree const &params)
> +int Ccm::read(boost::property_tree::ptree const &params)
>  {
> -       if (params.get_child_optional("saturation"))
> -               config_.saturation.read(params.get_child("saturation"));
> +       int ret;
> +
> +       if (params.get_child_optional("saturation")) {
> +               ret =
> config_.saturation.read(params.get_child("saturation"));
> +               if (ret)
> +                       return ret;
> +       }
> +
>         for (auto &p : params.get_child("ccms")) {
>                 CtCcm ctCcm;
>                 ctCcm.ct = p.second.get<double>("ct");
> -               ctCcm.ccm.read(p.second.get_child("ccm"));
> +               ret = ctCcm.ccm.read(p.second.get_child("ccm"));
> +               if (ret)
> +                       return ret;
>                 if (!config_.ccms.empty() &&
>                     ctCcm.ct <= config_.ccms.back().ct)
>                         LOG(RPiCcm, Fatal) << "Ccm: CCM not in increasing
> colour temperature order";
>                 config_.ccms.push_back(std::move(ctCcm));
>         }
> +
>         if (config_.ccms.empty())
>                 LOG(RPiCcm, Fatal) << "Ccm: no CCMs specified";
> +
> +       return 0;
>  }
>
>  void Ccm::setSaturation(double saturation)
> diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.h
> b/src/ipa/raspberrypi/controller/rpi/ccm.h
> index b44f1576528f..6b65c7aea8a6 100644
> --- a/src/ipa/raspberrypi/controller/rpi/ccm.h
> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.h
> @@ -20,7 +20,7 @@ struct Matrix {
>                double m6, double m7, double m8);
>         Matrix();
>         double m[3][3];
> -       void read(boost::property_tree::ptree const &params);
> +       int read(boost::property_tree::ptree const &params);
>  };
>  static inline Matrix operator*(double d, Matrix const &m)
>  {
> @@ -62,7 +62,7 @@ class Ccm : public CcmAlgorithm
>  public:
>         Ccm(Controller *controller = NULL);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void setSaturation(double saturation) override;
>         void initialise() override;
>         void prepare(Metadata *imageMetadata) override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> index 9e60dc5d47e7..d76dc43b837f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> @@ -38,7 +38,7 @@ char const *Contrast::name() const
>         return NAME;
>  }
>
> -void Contrast::read(boost::property_tree::ptree const &params)
> +int Contrast::read(boost::property_tree::ptree const &params)
>  {
>         /* enable adaptive enhancement by default */
>         config_.ceEnable = params.get<int>("ce_enable", 1);
> @@ -52,7 +52,7 @@ void Contrast::read(boost::property_tree::ptree const
> &params)
>         config_.hiHistogram = params.get<double>("hi_histogram", 0.95);
>         config_.hiLevel = params.get<double>("hi_level", 0.95);
>         config_.hiMax = params.get<double>("hi_max", 2000);
> -       config_.gammaCurve.read(params.get_child("gamma_curve"));
> +       return config_.gammaCurve.read(params.get_child("gamma_curve"));
>  }
>
>  void Contrast::setBrightness(double brightness)
> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.h
> b/src/ipa/raspberrypi/controller/rpi/contrast.h
> index b1375d819dbc..5c3d2f56b310 100644
> --- a/src/ipa/raspberrypi/controller/rpi/contrast.h
> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.h
> @@ -34,7 +34,7 @@ class Contrast : public ContrastAlgorithm
>  public:
>         Contrast(Controller *controller = NULL);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void setBrightness(double brightness) override;
>         void setContrast(double contrast) override;
>         void initialise() override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> index d5d784e7ca64..be014a05fd41 100644
> --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> @@ -31,11 +31,12 @@ char const *Dpc::name() const
>         return NAME;
>  }
>
> -void Dpc::read(boost::property_tree::ptree const &params)
> +int Dpc::read(boost::property_tree::ptree const &params)
>  {
>         config_.strength = params.get<int>("strength", 1);
>         if (config_.strength < 0 || config_.strength > 2)
>                 LOG(RPiDpc, Fatal) << "Dpc: bad strength value";
> +       return 0;
>  }
>
>  void Dpc::prepare(Metadata *imageMetadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.h
> b/src/ipa/raspberrypi/controller/rpi/dpc.h
> index 4c1bdec6dd87..2bb6cef565a7 100644
> --- a/src/ipa/raspberrypi/controller/rpi/dpc.h
> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.h
> @@ -22,7 +22,7 @@ class Dpc : public Algorithm
>  public:
>         Dpc(Controller *controller);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> index 696da4aeea59..a74447877bf8 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> @@ -35,14 +35,20 @@ char const *Geq::name() const
>         return NAME;
>  }
>
> -void Geq::read(boost::property_tree::ptree const &params)
> +int Geq::read(boost::property_tree::ptree const &params)
>  {
>         config_.offset = params.get<uint16_t>("offset", 0);
>         config_.slope = params.get<double>("slope", 0.0);
>         if (config_.slope < 0.0 || config_.slope >= 1.0)
>                 LOG(RPiGeq, Fatal) << "Geq: bad slope value";
> -       if (params.get_child_optional("strength"))
> -               config_.strength.read(params.get_child("strength"));
> +
> +       if (params.get_child_optional("strength")) {
> +               int ret =
> config_.strength.read(params.get_child("strength"));
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
>  }
>
>  void Geq::prepare(Metadata *imageMetadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.h
> b/src/ipa/raspberrypi/controller/rpi/geq.h
> index 5d06b9cbd529..677a0510c6ac 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.h
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.h
> @@ -24,7 +24,7 @@ class Geq : public Algorithm
>  public:
>         Geq(Controller *controller);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> index 95c0a93b2810..ca1e827191fd 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> @@ -38,7 +38,7 @@ char const *Lux::name() const
>         return NAME;
>  }
>
> -void Lux::read(boost::property_tree::ptree const &params)
> +int Lux::read(boost::property_tree::ptree const &params)
>  {
>         referenceShutterSpeed_ =
>                 params.get<double>("reference_shutter_speed") * 1.0us;
> @@ -47,6 +47,7 @@ void Lux::read(boost::property_tree::ptree const &params)
>         referenceY_ = params.get<double>("reference_Y");
>         referenceLux_ = params.get<double>("reference_lux");
>         currentAperture_ = referenceAperture_;
> +       return 0;
>  }
>
>  void Lux::setCurrentAperture(double aperture)
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.h
> b/src/ipa/raspberrypi/controller/rpi/lux.h
> index 26af8185911c..6416dfb52553 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.h
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.h
> @@ -22,7 +22,7 @@ class Lux : public Algorithm
>  public:
>         Lux(Controller *controller);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>         void process(StatisticsPtr &stats, Metadata *imageMetadata)
> override;
>         void setCurrentAperture(double aperture);
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> index 5d2c85ad3e74..74fa99bafe48 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> @@ -41,10 +41,11 @@ void Noise::switchMode(CameraMode const &cameraMode,
>         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
>  }
>
> -void Noise::read(boost::property_tree::ptree const &params)
> +int Noise::read(boost::property_tree::ptree const &params)
>  {
>         referenceConstant_ = params.get<double>("reference_constant");
>         referenceSlope_ = params.get<double>("reference_slope");
> +       return 0;
>  }
>
>  void Noise::prepare(Metadata *imageMetadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.h
> b/src/ipa/raspberrypi/controller/rpi/noise.h
> index 144e36529f4c..b33a5fc75ec7 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.h
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.h
> @@ -19,7 +19,7 @@ public:
>         Noise(Controller *controller);
>         char const *name() const override;
>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void prepare(Metadata *imageMetadata) override;
>
>  private:
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> index af31bd0881f0..03d9f119a338 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -34,10 +34,11 @@ char const *Sdn::name() const
>         return NAME;
>  }
>
> -void Sdn::read(boost::property_tree::ptree const &params)
> +int Sdn::read(boost::property_tree::ptree const &params)
>  {
>         deviation_ = params.get<double>("deviation", 3.2);
>         strength_ = params.get<double>("strength", 0.75);
> +       return 0;
>  }
>
>  void Sdn::initialise()
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.h
> b/src/ipa/raspberrypi/controller/rpi/sdn.h
> index 90ea37ff5550..4287ef08a79f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.h
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.h
> @@ -18,7 +18,7 @@ class Sdn : public DenoiseAlgorithm
>  public:
>         Sdn(Controller *controller = NULL);
>         char const *name() const override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void initialise() override;
>         void prepare(Metadata *imageMetadata) override;
>         void setMode(DenoiseMode mode) override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> index 36b75f69afce..9c4cffa4128c 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> @@ -37,7 +37,7 @@ void Sharpen::switchMode(CameraMode const &cameraMode,
>         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
>  }
>
> -void Sharpen::read(boost::property_tree::ptree const &params)
> +int Sharpen::read(boost::property_tree::ptree const &params)
>  {
>         threshold_ = params.get<double>("threshold", 1.0);
>         strength_ = params.get<double>("strength", 1.0);
> @@ -46,6 +46,7 @@ void Sharpen::read(boost::property_tree::ptree const
> &params)
>                 << "Read threshold " << threshold_
>                 << " strength " << strength_
>                 << " limit " << limit_;
> +       return 0;
>  }
>
>  void Sharpen::setStrength(double strength)
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.h
> b/src/ipa/raspberrypi/controller/rpi/sharpen.h
> index 8846f2ae09a2..0cd8b92f2224 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.h
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.h
> @@ -19,7 +19,7 @@ public:
>         Sharpen(Controller *controller);
>         char const *name() const override;
>         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> override;
> -       void read(boost::property_tree::ptree const &params) override;
> +       int read(boost::property_tree::ptree const &params) override;
>         void setStrength(double strength) override;
>         void prepare(Metadata *imageMetadata) override;
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 9d550354d78e..b9e9b814e886 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -229,7 +229,14 @@ int IPARPi::init(const IPASettings &settings,
> IPAInitResult *result)
>         result->sensorConfig.sensorMetadata = sensorMetadata;
>
>         /* Load the tuning file for this sensor. */
> -       controller_.read(settings.configurationFile.c_str());
> +       int ret = controller_.read(settings.configurationFile.c_str());
> +       if (ret) {
> +               LOG(IPARPI, Error)
> +                       << "Failed to load tuning data file "
> +                       << settings.configurationFile;
> +               return ret;
> +       }
> +
>         controller_.initialise();
>
>         /* Return the controls handled by the IPA */
> --
> Regards,
>
> Laurent Pinchart
>
>

Hi Naush,

On Wed, Jul 27, 2022 at 11:08:32AM +0100, Naushir Patuck wrote:
> On Wed, 27 Jul 2022 at 03:38, Laurent Pinchart wrote:
> 
> > When encountering errors, the Algorithm::read() function either uses
> > LOG(Fatal) or throws exceptions from the boost property_tree functions.
> > To prepare for replacing boost JSON parse with the YamlParser class,
> > give the Algorithm::read() function the ability to return an error code,
> > and propagate it all the way to the IPA module init() function.
> >
> > All algorithm classes return a hardcoded 0 value for now, subsequent
> > commits will change that.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/ipa/raspberrypi/controller/algorithm.cpp  |  3 +-
> >  src/ipa/raspberrypi/controller/algorithm.h    |  2 +-
> >  src/ipa/raspberrypi/controller/controller.cpp |  8 +++-
> >  src/ipa/raspberrypi/controller/controller.h   |  2 +-
> >  src/ipa/raspberrypi/controller/pwl.cpp        |  3 +-
> >  src/ipa/raspberrypi/controller/pwl.h          |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/agc.cpp    | 28 ++++++++++----
> >  src/ipa/raspberrypi/controller/rpi/agc.h      | 10 ++---
> >  src/ipa/raspberrypi/controller/rpi/alsc.cpp   | 38 +++++++++++++------
> >  src/ipa/raspberrypi/controller/rpi/alsc.h     |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/awb.cpp    | 35 +++++++++++------
> >  src/ipa/raspberrypi/controller/rpi/awb.h      |  8 ++--
> >  .../controller/rpi/black_level.cpp            |  3 +-
> >  .../raspberrypi/controller/rpi/black_level.h  |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/ccm.cpp    | 22 ++++++++---
> >  src/ipa/raspberrypi/controller/rpi/ccm.h      |  4 +-
> >  .../raspberrypi/controller/rpi/contrast.cpp   |  4 +-
> >  src/ipa/raspberrypi/controller/rpi/contrast.h |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |  3 +-
> >  src/ipa/raspberrypi/controller/rpi/dpc.h      |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/geq.cpp    | 12 ++++--
> >  src/ipa/raspberrypi/controller/rpi/geq.h      |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/lux.cpp    |  3 +-
> >  src/ipa/raspberrypi/controller/rpi/lux.h      |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/noise.cpp  |  3 +-
> >  src/ipa/raspberrypi/controller/rpi/noise.h    |  2 +-
> >  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |  3 +-
> >  src/ipa/raspberrypi/controller/rpi/sdn.h      |  2 +-
> >  .../raspberrypi/controller/rpi/sharpen.cpp    |  3 +-
> >  src/ipa/raspberrypi/controller/rpi/sharpen.h  |  2 +-
> >  src/ipa/raspberrypi/raspberrypi.cpp           |  9 ++++-
> >  31 files changed, 151 insertions(+), 75 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp
> > index 1a7d20a4731b..d73cb36fe2d6 100644
> > --- a/src/ipa/raspberrypi/controller/algorithm.cpp
> > +++ b/src/ipa/raspberrypi/controller/algorithm.cpp
> > @@ -9,8 +9,9 @@
> >
> >  using namespace RPiController;
> >
> > -void Algorithm::read([[maybe_unused]] boost::property_tree::ptree const &params)
> > +int Algorithm::read([[maybe_unused]] boost::property_tree::ptree const &params)
> >  {
> > +       return 0;
> >  }
> >
> >  void Algorithm::initialise()
> > diff --git a/src/ipa/raspberrypi/controller/algorithm.h b/src/ipa/raspberrypi/controller/algorithm.h
> > index 92fd895d2b06..0c5566fda874 100644
> > --- a/src/ipa/raspberrypi/controller/algorithm.h
> > +++ b/src/ipa/raspberrypi/controller/algorithm.h
> > @@ -35,7 +35,7 @@ public:
> >         virtual bool isPaused() const { return paused_; }
> >         virtual void pause() { paused_ = true; }
> >         virtual void resume() { paused_ = false; }
> > -       virtual void read(boost::property_tree::ptree const &params);
> > +       virtual int read(boost::property_tree::ptree const &params);
> >         virtual void initialise();
> >         virtual void switchMode(CameraMode const &cameraMode, Metadata *metadata);
> >         virtual void prepare(Metadata *imageMetadata);
> > diff --git a/src/ipa/raspberrypi/controller/controller.cpp
> > b/src/ipa/raspberrypi/controller/controller.cpp
> > index 872a32302410..d91ac90704cb 100644
> > --- a/src/ipa/raspberrypi/controller/controller.cpp
> > +++ b/src/ipa/raspberrypi/controller/controller.cpp
> > @@ -32,19 +32,23 @@ Controller::Controller(char const *jsonFilename)
> >
> >  Controller::~Controller() {}
> >
> > -void Controller::read(char const *filename)
> > +int Controller::read(char const *filename)
> 
> Since read now returns an error code, we probably ought to remove the
> Controller(char const *jsonFilename) constructor.  This never gets used
> anyway.  But that can be done on top of this work.

Feel free to perform further cleanups :-) Eventually it could be nice
for the Raspberry Pi IPA to use the Algorithm and Module classes
provided by libipa to replace the Controller class, but I think we need
some more work there first.

> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
> 
> >  {
> >         boost::property_tree::ptree root;
> >         boost::property_tree::read_json(filename, root);
> >         for (auto const &keyAndValue : root) {
> >                 Algorithm *algo = createAlgorithm(keyAndValue.first.c_str());
> >                 if (algo) {
> > -                       algo->read(keyAndValue.second);
> > +                       int ret = algo->read(keyAndValue.second);
> > +                       if (ret)
> > +                               return ret;
> >                         algorithms_.push_back(AlgorithmPtr(algo));
> >                 } else
> >                         LOG(RPiController, Warning)
> >                                 << "No algorithm found for \"" << keyAndValue.first << "\"";
> >         }
> > +
> > +       return 0;
> >  }
> >
> >  Algorithm *Controller::createAlgorithm(char const *name)
> > diff --git a/src/ipa/raspberrypi/controller/controller.h b/src/ipa/raspberrypi/controller/controller.h
> > index e28e30d7d574..841783bb7a5c 100644
> > --- a/src/ipa/raspberrypi/controller/controller.h
> > +++ b/src/ipa/raspberrypi/controller/controller.h
> > @@ -41,7 +41,7 @@ public:
> >         Controller(char const *jsonFilename);
> >         ~Controller();
> >         Algorithm *createAlgorithm(char const *name);
> > -       void read(char const *filename);
> > +       int read(char const *filename);
> >         void initialise();
> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata);
> >         void prepare(Metadata *imageMetadata);
> > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp b/src/ipa/raspberrypi/controller/pwl.cpp
> > index 8b8db722966b..fde0b298c6ce 100644
> > --- a/src/ipa/raspberrypi/controller/pwl.cpp
> > +++ b/src/ipa/raspberrypi/controller/pwl.cpp
> > @@ -12,7 +12,7 @@
> >
> >  using namespace RPiController;
> >
> > -void Pwl::read(boost::property_tree::ptree const &params)
> > +int Pwl::read(boost::property_tree::ptree const &params)
> >  {
> >         for (auto it = params.begin(); it != params.end(); it++) {
> >                 double x = it->second.get_value<double>();
> > @@ -22,6 +22,7 @@ void Pwl::read(boost::property_tree::ptree const &params)
> >                 points_.push_back(Point(x, y));
> >         }
> >         assert(points_.size() >= 2);
> > +       return 0;
> >  }
> >
> >  void Pwl::append(double x, double y, const double eps)
> > diff --git a/src/ipa/raspberrypi/controller/pwl.h b/src/ipa/raspberrypi/controller/pwl.h
> > index 973efdf59945..ef1cc2ed113a 100644
> > --- a/src/ipa/raspberrypi/controller/pwl.h
> > +++ b/src/ipa/raspberrypi/controller/pwl.h
> > @@ -57,7 +57,7 @@ public:
> >         };
> >         Pwl() {}
> >         Pwl(std::vector<Point> const &points) : points_(points) {}
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >         void append(double x, double y, const double eps = 1e-6);
> >         void prepend(double x, double y, const double eps = 1e-6);
> >         Interval domain() const;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > index adec8592626d..130c606d4136 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > @@ -30,7 +30,7 @@ LOG_DEFINE_CATEGORY(RPiAgc)
> >
> >  static constexpr unsigned int PipelineBits = 13; /* seems to be a 13-bit pipeline */
> >
> > -void AgcMeteringMode::read(boost::property_tree::ptree const &params)
> > +int AgcMeteringMode::read(boost::property_tree::ptree const &params)
> >  {
> >         int num = 0;
> >         for (auto &p : params.get_child("weights")) {
> > @@ -40,6 +40,7 @@ void AgcMeteringMode::read(boost::property_tree::ptree const &params)
> >         }
> >         if (num != AgcStatsSize)
> >                 LOG(RPiAgc, Fatal) << "AgcMeteringMode: insufficient weights";
> > +       return 0;
> >  }
> >
> >  static std::string
> > @@ -73,7 +74,7 @@ static int readList(std::vector<Duration> &list,
> >         return list.size();
> >  }
> >
> > -void AgcExposureMode::read(boost::property_tree::ptree const &params)
> > +int AgcExposureMode::read(boost::property_tree::ptree const &params)
> >  {
> >         int numShutters = readList(shutter, params.get_child("shutter"));
> >         int numAgs = readList(gain, params.get_child("gain"));
> > @@ -83,6 +84,7 @@ void AgcExposureMode::read(boost::property_tree::ptree const &params)
> >         if (numShutters != numAgs)
> >                 LOG(RPiAgc, Fatal)
> >                         << "AgcExposureMode: expect same number of exposure and gain entries in exposure profile";
> > +       return 0;
> >  }
> >
> >  static std::string
> > @@ -100,7 +102,7 @@ readExposureModes(std::map<std::string, AgcExposureMode> &exposureModes,
> >         return first;
> >  }
> >
> > -void AgcConstraint::read(boost::property_tree::ptree const &params)
> > +int AgcConstraint::read(boost::property_tree::ptree const &params)
> >  {
> >         std::string boundString = params.get<std::string>("bound", "");
> >         transform(boundString.begin(), boundString.end(),
> > @@ -110,7 +112,7 @@ void AgcConstraint::read(boost::property_tree::ptree const &params)
> >         bound = boundString == "UPPER" ? Bound::UPPER : Bound::LOWER;
> >         qLo = params.get<double>("q_lo");
> >         qHi = params.get<double>("q_hi");
> > -       yTarget.read(params.get_child("y_target"));
> > +       return yTarget.read(params.get_child("y_target"));
> >  }
> >
> >  static AgcConstraintMode
> > @@ -137,13 +139,17 @@ static std::string readConstraintModes(std::map<std::string, AgcConstraintMode>
> >         return first;
> >  }
> >
> > -void AgcConfig::read(boost::property_tree::ptree const &params)
> > +int AgcConfig::read(boost::property_tree::ptree const &params)
> >  {
> >         LOG(RPiAgc, Debug) << "AgcConfig";
> >         defaultMeteringMode = readMeteringModes(meteringModes, params.get_child("metering_modes"));
> >         defaultExposureMode = readExposureModes(exposureModes, params.get_child("exposure_modes"));
> >         defaultConstraintMode = readConstraintModes(constraintModes, params.get_child("constraint_modes"));
> > -       yTarget.read(params.get_child("y_target"));
> > +
> > +       int ret = yTarget.read(params.get_child("y_target"));
> > +       if (ret)
> > +               return ret;
> > +
> >         speed = params.get<double>("speed", 0.2);
> >         startupFrames = params.get<uint16_t>("startup_frames", 10);
> >         convergenceFrames = params.get<unsigned int>("convergence_frames", 6);
> > @@ -152,6 +158,7 @@ void AgcConfig::read(boost::property_tree::ptree const &params)
> >         /* Start with quite a low value as ramping up is easier than ramping down. */
> >         defaultExposureTime = params.get<double>("default_exposure_time", 1000) * 1us;
> >         defaultAnalogueGain = params.get<double>("default_analogueGain", 1.0);
> > +       return 0;
> >  }
> >
> >  Agc::ExposureValues::ExposureValues()
> > @@ -182,10 +189,14 @@ char const *Agc::name() const
> >         return NAME;
> >  }
> >
> > -void Agc::read(boost::property_tree::ptree const &params)
> > +int Agc::read(boost::property_tree::ptree const &params)
> >  {
> >         LOG(RPiAgc, Debug) << "Agc";
> > -       config_.read(params);
> > +
> > +       int ret = config_.read(params);
> > +       if (ret)
> > +               return ret;
> > +
> >         /*
> >          * Set the config's defaults (which are the first ones it read) as our
> >          * current modes, until someone changes them.  (they're all known to
> > @@ -200,6 +211,7 @@ void Agc::read(boost::property_tree::ptree const &params)
> >         /* Set up the "last shutter/gain" values, in case AGC starts "disabled". */
> >         status_.shutterTime = config_.defaultExposureTime;
> >         status_.analogueGain = config_.defaultAnalogueGain;
> > +       return 0;
> >  }
> >
> >  bool Agc::isPaused() const
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h b/src/ipa/raspberrypi/controller/rpi/agc.h
> > index f57afa6dc80c..1351b0ee079c 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h
> > @@ -28,13 +28,13 @@ namespace RPiController {
> >
> >  struct AgcMeteringMode {
> >         double weights[AgcStatsSize];
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >  };
> >
> >  struct AgcExposureMode {
> >         std::vector<libcamera::utils::Duration> shutter;
> >         std::vector<double> gain;
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >  };
> >
> >  struct AgcConstraint {
> > @@ -43,13 +43,13 @@ struct AgcConstraint {
> >         double qLo;
> >         double qHi;
> >         Pwl yTarget;
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >  };
> >
> >  typedef std::vector<AgcConstraint> AgcConstraintMode;
> >
> >  struct AgcConfig {
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >         std::map<std::string, AgcMeteringMode> meteringModes;
> >         std::map<std::string, AgcExposureMode> exposureModes;
> >         std::map<std::string, AgcConstraintMode> constraintModes;
> > @@ -74,7 +74,7 @@ class Agc : public AgcAlgorithm
> >  public:
> >         Agc(Controller *controller);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         /* AGC handles "pausing" for itself. */
> >         bool isPaused() const override;
> >         void pause() override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > index 03ae33501dc0..b36277696a52 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> > @@ -50,7 +50,7 @@ char const *Alsc::name() const
> >         return NAME;
> >  }
> >
> > -static void generateLut(double *lut, boost::property_tree::ptree const &params)
> > +static int generateLut(double *lut, boost::property_tree::ptree const &params)
> >  {
> >         double cstrength = params.get<double>("corner_strength", 2.0);
> >         if (cstrength <= 1.0)
> > @@ -71,9 +71,10 @@ static void generateLut(double *lut, boost::property_tree::ptree const &params)
> >                                 (f2 * f2); /* this reproduces the cos^4 rule */
> >                 }
> >         }
> > +       return 0;
> >  }
> >
> > -static void readLut(double *lut, boost::property_tree::ptree const &params)
> > +static int readLut(double *lut, boost::property_tree::ptree const &params)
> >  {
> >         int num = 0;
> >         const int maxNum = XY;
> > @@ -84,11 +85,12 @@ static void readLut(double *lut, boost::property_tree::ptree const &params)
> >         }
> >         if (num < maxNum)
> >                 LOG(RPiAlsc, Fatal) << "Alsc: too few entries in LSC table";
> > +       return 0;
> >  }
> >
> > -static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> > -                            boost::property_tree::ptree const &params,
> > -                            std::string const &name)
> > +static int readCalibrations(std::vector<AlscCalibration> &calibrations,
> > +                           boost::property_tree::ptree const &params,
> > +                           std::string const &name)
> >  {
> >         if (params.get_child_optional(name)) {
> >                 double lastCt = 0;
> > @@ -117,9 +119,10 @@ static void readCalibrations(std::vector<AlscCalibration> &calibrations,
> >                                 << "Read " << name << " calibration for ct " << ct;
> >                 }
> >         }
> > +       return 0;
> >  }
> >
> > -void Alsc::read(boost::property_tree::ptree const &params)
> > +int Alsc::read(boost::property_tree::ptree const &params)
> >  {
> >         config_.framePeriod = params.get<uint16_t>("frame_period", 12);
> >         config_.startupFrames = params.get<uint16_t>("startup_frames", 10);
> > @@ -135,19 +138,32 @@ void Alsc::read(boost::property_tree::ptree const &params)
> >                 params.get<double>("luminance_strength", 1.0);
> >         for (int i = 0; i < XY; i++)
> >                 config_.luminanceLut[i] = 1.0;
> > +
> > +       int ret = 0;
> > +
> >         if (params.get_child_optional("corner_strength"))
> > -               generateLut(config_.luminanceLut, params);
> > +               ret = generateLut(config_.luminanceLut, params);
> >         else if (params.get_child_optional("luminance_lut"))
> > -               readLut(config_.luminanceLut,
> > -                       params.get_child("luminance_lut"));
> > +               ret = readLut(config_.luminanceLut,
> > +                             params.get_child("luminance_lut"));
> >         else
> >                 LOG(RPiAlsc, Warning)
> >                         << "no luminance table - assume unity everywhere";
> > -       readCalibrations(config_.calibrationsCr, params, "calibrations_Cr");
> > -       readCalibrations(config_.calibrationsCb, params, "calibrations_Cb");
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = readCalibrations(config_.calibrationsCr, params, "calibrations_Cr");
> > +       if (ret)
> > +               return ret;
> > +       ret = readCalibrations(config_.calibrationsCb, params, "calibrations_Cb");
> > +       if (ret)
> > +               return ret;
> > +
> >         config_.defaultCt = params.get<double>("default_ct", 4500.0);
> >         config_.threshold = params.get<double>("threshold", 1e-3);
> >         config_.lambdaBound = params.get<double>("lambda_bound", 0.05);
> > +
> > +       return 0;
> >  }
> >
> >  static double getCt(Metadata *metadata, double defaultCt);
> > diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.h b/src/ipa/raspberrypi/controller/rpi/alsc.h
> > index 4e9a715ae0ab..773fd338ea8e 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/alsc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/alsc.h
> > @@ -52,7 +52,7 @@ public:
> >         char const *name() const override;
> >         void initialise() override;
> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> > override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >         void process(StatisticsPtr &stats, Metadata *imageMetadata)
> > override;
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > index ad75d55f0976..a9e776a344a1 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > @@ -26,20 +26,21 @@ static constexpr unsigned int AwbStatsSizeY =
> > DEFAULT_AWB_REGIONS_Y;
> >   * elsewhere (ALSC and AGC).
> >   */
> >
> > -void AwbMode::read(boost::property_tree::ptree const &params)
> > +int AwbMode::read(boost::property_tree::ptree const &params)
> >  {
> >         ctLo = params.get<double>("lo");
> >         ctHi = params.get<double>("hi");
> > +       return 0;
> >  }
> >
> > -void AwbPrior::read(boost::property_tree::ptree const &params)
> > +int AwbPrior::read(boost::property_tree::ptree const &params)
> >  {
> >         lux = params.get<double>("lux");
> > -       prior.read(params.get_child("prior"));
> > +       return prior.read(params.get_child("prior"));
> >  }
> >
> > -static void readCtCurve(Pwl &ctR, Pwl &ctB,
> > -                       boost::property_tree::ptree const &params)
> > +static int readCtCurve(Pwl &ctR, Pwl &ctB,
> > +                      boost::property_tree::ptree const &params)
> >  {
> >         int num = 0;
> >         for (auto it = params.begin(); it != params.end(); it++) {
> > @@ -55,21 +56,28 @@ static void readCtCurve(Pwl &ctR, Pwl &ctB,
> >         }
> >         if (num < 2)
> >                 LOG(RPiAwb, Fatal) << "AwbConfig: insufficient points in
> > CT curve";
> > +       return 0;
> >  }
> >
> > -void AwbConfig::read(boost::property_tree::ptree const &params)
> > +int AwbConfig::read(boost::property_tree::ptree const &params)
> >  {
> > +       int ret;
> >         bayes = params.get<int>("bayes", 1);
> >         framePeriod = params.get<uint16_t>("framePeriod", 10);
> >         startupFrames = params.get<uint16_t>("startupFrames", 10);
> >         convergenceFrames = params.get<unsigned int>("convergence_frames",
> > 3);
> >         speed = params.get<double>("speed", 0.05);
> > -       if (params.get_child_optional("ct_curve"))
> > -               readCtCurve(ctR, ctB, params.get_child("ct_curve"));
> > +       if (params.get_child_optional("ct_curve")) {
> > +               ret = readCtCurve(ctR, ctB, params.get_child("ct_curve"));
> > +               if (ret)
> > +                       return ret;
> > +       }
> >         if (params.get_child_optional("priors")) {
> >                 for (auto &p : params.get_child("priors")) {
> >                         AwbPrior prior;
> > -                       prior.read(p.second);
> > +                       ret = prior.read(p.second);
> > +                       if (ret)
> > +                               return ret;
> >                         if (!priors.empty() && prior.lux <=
> > priors.back().lux)
> >                                 LOG(RPiAwb, Fatal) << "AwbConfig: Prior
> > must be ordered in increasing lux value";
> >                         priors.push_back(prior);
> > @@ -79,7 +87,9 @@ void AwbConfig::read(boost::property_tree::ptree const
> > &params)
> >         }
> >         if (params.get_child_optional("modes")) {
> >                 for (auto &p : params.get_child("modes")) {
> > -                       modes[p.first].read(p.second);
> > +                       ret = modes[p.first].read(p.second);
> > +                       if (ret)
> > +                               return ret;
> >                         if (defaultMode == nullptr)
> >                                 defaultMode = &modes[p.first];
> >                 }
> > @@ -110,6 +120,7 @@ void AwbConfig::read(boost::property_tree::ptree const
> > &params)
> >         whitepointB = params.get<double>("whitepoint_b", 0.0);
> >         if (bayes == false)
> >                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities
> > make any sense */
> > +       return 0;
> >  }
> >
> >  Awb::Awb(Controller *controller)
> > @@ -137,9 +148,9 @@ char const *Awb::name() const
> >         return NAME;
> >  }
> >
> > -void Awb::read(boost::property_tree::ptree const &params)
> > +int Awb::read(boost::property_tree::ptree const &params)
> >  {
> > -       config_.read(params);
> > +       return config_.read(params);
> >  }
> >
> >  void Awb::initialise()
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> > b/src/ipa/raspberrypi/controller/rpi/awb.h
> > index 9e075624c429..fa0715735fcf 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> > @@ -19,20 +19,20 @@ namespace RPiController {
> >  /* Control algorithm to perform AWB calculations. */
> >
> >  struct AwbMode {
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >         double ctLo; /* low CT value for search */
> >         double ctHi; /* high CT value for search */
> >  };
> >
> >  struct AwbPrior {
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >         double lux; /* lux level */
> >         Pwl prior; /* maps CT to prior log likelihood for this lux level */
> >  };
> >
> >  struct AwbConfig {
> >         AwbConfig() : defaultMode(nullptr) {}
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >         /* Only repeat the AWB calculation every "this many" frames */
> >         uint16_t framePeriod;
> >         /* number of initial frames for which speed taken as 1.0 (maximum)
> > */
> > @@ -92,7 +92,7 @@ public:
> >         ~Awb();
> >         char const *name() const override;
> >         void initialise() override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         /* AWB handles "pausing" for itself. */
> >         bool isPaused() const override;
> >         void pause() override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> > b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> > index 0799d7b9195a..aa6f602acd7d 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> > @@ -31,7 +31,7 @@ char const *BlackLevel::name() const
> >         return NAME;
> >  }
> >
> > -void BlackLevel::read(boost::property_tree::ptree const &params)
> > +int BlackLevel::read(boost::property_tree::ptree const &params)
> >  {
> >         uint16_t blackLevel = params.get<uint16_t>(
> >                 "black_level", 4096); /* 64 in 10 bits scaled to 16 bits */
> > @@ -42,6 +42,7 @@ void BlackLevel::read(boost::property_tree::ptree const
> > &params)
> >                 << " Read black levels red " << blackLevelR_
> >                 << " green " << blackLevelG_
> >                 << " blue " << blackLevelB_;
> > +       return 0;
> >  }
> >
> >  void BlackLevel::prepare(Metadata *imageMetadata)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.h
> > b/src/ipa/raspberrypi/controller/rpi/black_level.h
> > index 7789f261cf03..6ec8c4f9ba8f 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/black_level.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/black_level.h
> > @@ -18,7 +18,7 @@ class BlackLevel : public Algorithm
> >  public:
> >         BlackLevel(Controller *controller);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >
> >  private:
> > diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> > b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> > index cf0c85d26cf9..f0110d38f548 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> > @@ -39,7 +39,7 @@ Matrix::Matrix(double m0, double m1, double m2, double
> > m3, double m4, double m5,
> >         m[0][0] = m0, m[0][1] = m1, m[0][2] = m2, m[1][0] = m3, m[1][1] =
> > m4,
> >         m[1][2] = m5, m[2][0] = m6, m[2][1] = m7, m[2][2] = m8;
> >  }
> > -void Matrix::read(boost::property_tree::ptree const &params)
> > +int Matrix::read(boost::property_tree::ptree const &params)
> >  {
> >         double *ptr = (double *)m;
> >         int n = 0;
> > @@ -50,6 +50,7 @@ void Matrix::read(boost::property_tree::ptree const
> > &params)
> >         }
> >         if (n < 9)
> >                 LOG(RPiCcm, Fatal) << "Ccm: too few values in CCM";
> > +       return 0;
> >  }
> >
> >  Ccm::Ccm(Controller *controller)
> > @@ -60,21 +61,32 @@ char const *Ccm::name() const
> >         return NAME;
> >  }
> >
> > -void Ccm::read(boost::property_tree::ptree const &params)
> > +int Ccm::read(boost::property_tree::ptree const &params)
> >  {
> > -       if (params.get_child_optional("saturation"))
> > -               config_.saturation.read(params.get_child("saturation"));
> > +       int ret;
> > +
> > +       if (params.get_child_optional("saturation")) {
> > +               ret =
> > config_.saturation.read(params.get_child("saturation"));
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         for (auto &p : params.get_child("ccms")) {
> >                 CtCcm ctCcm;
> >                 ctCcm.ct = p.second.get<double>("ct");
> > -               ctCcm.ccm.read(p.second.get_child("ccm"));
> > +               ret = ctCcm.ccm.read(p.second.get_child("ccm"));
> > +               if (ret)
> > +                       return ret;
> >                 if (!config_.ccms.empty() &&
> >                     ctCcm.ct <= config_.ccms.back().ct)
> >                         LOG(RPiCcm, Fatal) << "Ccm: CCM not in increasing
> > colour temperature order";
> >                 config_.ccms.push_back(std::move(ctCcm));
> >         }
> > +
> >         if (config_.ccms.empty())
> >                 LOG(RPiCcm, Fatal) << "Ccm: no CCMs specified";
> > +
> > +       return 0;
> >  }
> >
> >  void Ccm::setSaturation(double saturation)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.h
> > b/src/ipa/raspberrypi/controller/rpi/ccm.h
> > index b44f1576528f..6b65c7aea8a6 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/ccm.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/ccm.h
> > @@ -20,7 +20,7 @@ struct Matrix {
> >                double m6, double m7, double m8);
> >         Matrix();
> >         double m[3][3];
> > -       void read(boost::property_tree::ptree const &params);
> > +       int read(boost::property_tree::ptree const &params);
> >  };
> >  static inline Matrix operator*(double d, Matrix const &m)
> >  {
> > @@ -62,7 +62,7 @@ class Ccm : public CcmAlgorithm
> >  public:
> >         Ccm(Controller *controller = NULL);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void setSaturation(double saturation) override;
> >         void initialise() override;
> >         void prepare(Metadata *imageMetadata) override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > index 9e60dc5d47e7..d76dc43b837f 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> > @@ -38,7 +38,7 @@ char const *Contrast::name() const
> >         return NAME;
> >  }
> >
> > -void Contrast::read(boost::property_tree::ptree const &params)
> > +int Contrast::read(boost::property_tree::ptree const &params)
> >  {
> >         /* enable adaptive enhancement by default */
> >         config_.ceEnable = params.get<int>("ce_enable", 1);
> > @@ -52,7 +52,7 @@ void Contrast::read(boost::property_tree::ptree const
> > &params)
> >         config_.hiHistogram = params.get<double>("hi_histogram", 0.95);
> >         config_.hiLevel = params.get<double>("hi_level", 0.95);
> >         config_.hiMax = params.get<double>("hi_max", 2000);
> > -       config_.gammaCurve.read(params.get_child("gamma_curve"));
> > +       return config_.gammaCurve.read(params.get_child("gamma_curve"));
> >  }
> >
> >  void Contrast::setBrightness(double brightness)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.h
> > b/src/ipa/raspberrypi/controller/rpi/contrast.h
> > index b1375d819dbc..5c3d2f56b310 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/contrast.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/contrast.h
> > @@ -34,7 +34,7 @@ class Contrast : public ContrastAlgorithm
> >  public:
> >         Contrast(Controller *controller = NULL);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void setBrightness(double brightness) override;
> >         void setContrast(double contrast) override;
> >         void initialise() override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> > b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> > index d5d784e7ca64..be014a05fd41 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> > @@ -31,11 +31,12 @@ char const *Dpc::name() const
> >         return NAME;
> >  }
> >
> > -void Dpc::read(boost::property_tree::ptree const &params)
> > +int Dpc::read(boost::property_tree::ptree const &params)
> >  {
> >         config_.strength = params.get<int>("strength", 1);
> >         if (config_.strength < 0 || config_.strength > 2)
> >                 LOG(RPiDpc, Fatal) << "Dpc: bad strength value";
> > +       return 0;
> >  }
> >
> >  void Dpc::prepare(Metadata *imageMetadata)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.h
> > b/src/ipa/raspberrypi/controller/rpi/dpc.h
> > index 4c1bdec6dd87..2bb6cef565a7 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/dpc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/dpc.h
> > @@ -22,7 +22,7 @@ class Dpc : public Algorithm
> >  public:
> >         Dpc(Controller *controller);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >
> >  private:
> > diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> > b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> > index 696da4aeea59..a74447877bf8 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> > @@ -35,14 +35,20 @@ char const *Geq::name() const
> >         return NAME;
> >  }
> >
> > -void Geq::read(boost::property_tree::ptree const &params)
> > +int Geq::read(boost::property_tree::ptree const &params)
> >  {
> >         config_.offset = params.get<uint16_t>("offset", 0);
> >         config_.slope = params.get<double>("slope", 0.0);
> >         if (config_.slope < 0.0 || config_.slope >= 1.0)
> >                 LOG(RPiGeq, Fatal) << "Geq: bad slope value";
> > -       if (params.get_child_optional("strength"))
> > -               config_.strength.read(params.get_child("strength"));
> > +
> > +       if (params.get_child_optional("strength")) {
> > +               int ret =
> > config_.strength.read(params.get_child("strength"));
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       return 0;
> >  }
> >
> >  void Geq::prepare(Metadata *imageMetadata)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/geq.h
> > b/src/ipa/raspberrypi/controller/rpi/geq.h
> > index 5d06b9cbd529..677a0510c6ac 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/geq.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/geq.h
> > @@ -24,7 +24,7 @@ class Geq : public Algorithm
> >  public:
> >         Geq(Controller *controller);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >
> >  private:
> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > index 95c0a93b2810..ca1e827191fd 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> > @@ -38,7 +38,7 @@ char const *Lux::name() const
> >         return NAME;
> >  }
> >
> > -void Lux::read(boost::property_tree::ptree const &params)
> > +int Lux::read(boost::property_tree::ptree const &params)
> >  {
> >         referenceShutterSpeed_ =
> >                 params.get<double>("reference_shutter_speed") * 1.0us;
> > @@ -47,6 +47,7 @@ void Lux::read(boost::property_tree::ptree const &params)
> >         referenceY_ = params.get<double>("reference_Y");
> >         referenceLux_ = params.get<double>("reference_lux");
> >         currentAperture_ = referenceAperture_;
> > +       return 0;
> >  }
> >
> >  void Lux::setCurrentAperture(double aperture)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/lux.h
> > b/src/ipa/raspberrypi/controller/rpi/lux.h
> > index 26af8185911c..6416dfb52553 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/lux.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/lux.h
> > @@ -22,7 +22,7 @@ class Lux : public Algorithm
> >  public:
> >         Lux(Controller *controller);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >         void process(StatisticsPtr &stats, Metadata *imageMetadata)
> > override;
> >         void setCurrentAperture(double aperture);
> > diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> > b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> > index 5d2c85ad3e74..74fa99bafe48 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> > @@ -41,10 +41,11 @@ void Noise::switchMode(CameraMode const &cameraMode,
> >         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
> >  }
> >
> > -void Noise::read(boost::property_tree::ptree const &params)
> > +int Noise::read(boost::property_tree::ptree const &params)
> >  {
> >         referenceConstant_ = params.get<double>("reference_constant");
> >         referenceSlope_ = params.get<double>("reference_slope");
> > +       return 0;
> >  }
> >
> >  void Noise::prepare(Metadata *imageMetadata)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/noise.h
> > b/src/ipa/raspberrypi/controller/rpi/noise.h
> > index 144e36529f4c..b33a5fc75ec7 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/noise.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/noise.h
> > @@ -19,7 +19,7 @@ public:
> >         Noise(Controller *controller);
> >         char const *name() const override;
> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> > override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void prepare(Metadata *imageMetadata) override;
> >
> >  private:
> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > index af31bd0881f0..03d9f119a338 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> > @@ -34,10 +34,11 @@ char const *Sdn::name() const
> >         return NAME;
> >  }
> >
> > -void Sdn::read(boost::property_tree::ptree const &params)
> > +int Sdn::read(boost::property_tree::ptree const &params)
> >  {
> >         deviation_ = params.get<double>("deviation", 3.2);
> >         strength_ = params.get<double>("strength", 0.75);
> > +       return 0;
> >  }
> >
> >  void Sdn::initialise()
> > diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.h
> > b/src/ipa/raspberrypi/controller/rpi/sdn.h
> > index 90ea37ff5550..4287ef08a79f 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/sdn.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/sdn.h
> > @@ -18,7 +18,7 @@ class Sdn : public DenoiseAlgorithm
> >  public:
> >         Sdn(Controller *controller = NULL);
> >         char const *name() const override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void initialise() override;
> >         void prepare(Metadata *imageMetadata) override;
> >         void setMode(DenoiseMode mode) override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> > b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> > index 36b75f69afce..9c4cffa4128c 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> > @@ -37,7 +37,7 @@ void Sharpen::switchMode(CameraMode const &cameraMode,
> >         modeFactor_ = std::max(1.0, cameraMode.noiseFactor);
> >  }
> >
> > -void Sharpen::read(boost::property_tree::ptree const &params)
> > +int Sharpen::read(boost::property_tree::ptree const &params)
> >  {
> >         threshold_ = params.get<double>("threshold", 1.0);
> >         strength_ = params.get<double>("strength", 1.0);
> > @@ -46,6 +46,7 @@ void Sharpen::read(boost::property_tree::ptree const
> > &params)
> >                 << "Read threshold " << threshold_
> >                 << " strength " << strength_
> >                 << " limit " << limit_;
> > +       return 0;
> >  }
> >
> >  void Sharpen::setStrength(double strength)
> > diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.h
> > b/src/ipa/raspberrypi/controller/rpi/sharpen.h
> > index 8846f2ae09a2..0cd8b92f2224 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/sharpen.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.h
> > @@ -19,7 +19,7 @@ public:
> >         Sharpen(Controller *controller);
> >         char const *name() const override;
> >         void switchMode(CameraMode const &cameraMode, Metadata *metadata)
> > override;
> > -       void read(boost::property_tree::ptree const &params) override;
> > +       int read(boost::property_tree::ptree const &params) override;
> >         void setStrength(double strength) override;
> >         void prepare(Metadata *imageMetadata) override;
> >
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> > b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 9d550354d78e..b9e9b814e886 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -229,7 +229,14 @@ int IPARPi::init(const IPASettings &settings,
> > IPAInitResult *result)
> >         result->sensorConfig.sensorMetadata = sensorMetadata;
> >
> >         /* Load the tuning file for this sensor. */
> > -       controller_.read(settings.configurationFile.c_str());
> > +       int ret = controller_.read(settings.configurationFile.c_str());
> > +       if (ret) {
> > +               LOG(IPARPI, Error)
> > +                       << "Failed to load tuning data file "
> > +                       << settings.configurationFile;
> > +               return ret;
> > +       }
> > +
> >         controller_.initialise();
> >
> >         /* Return the controls handled by the IPA */