[libcamera-devel] [RFC PATCH 00/12] Replace boost JSON parser with libyaml in Raspberry Pi IPA

Naushir Patuck naush at raspberrypi.com
Wed May 25 14:15:00 CEST 2022 

Hi Laurent,

Thank you for your work.  It would indeed be very nice to get rid of the
boost
json parser dependency if we can.  I've had a quick go at trying this
series out
and unfortunately hit a fundamental problem.

In the existing boost parser, Controller::Read() would populate the
algorithms
vector in the order they appeared in the json file.  With the new yaml
parser,
they are populated in alphabetical order.  This breaks certain behavior in
our
controllers where, for example, we want the Lux algorithm to always run
first
and populate the metadata with the estimated lux value for other algorithms
to
use.  I've not fully checked, but I think Lux, Noise and AWB needs to run
(in
that order) before any other algorithm. The other algorithms can run in any
given order.

Perhaps a way forward would be to have an "order" key in our json files that
specify algorithm ordering, so we decouple it from the parser used?  I worry
this could lead to unexpected problems where a user might add a block but
it may
not run as it's not in the order list.  Maybe this order list does not need
to
list all algorithms, just the one it wants ordered?

Regards,
Naush


On Tue, 24 May 2022 at 23:58, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hello,
>
> As mentioned in the subject, this patch series replaces the boost JSON
> parser with the YamlParser implementation based on libyaml for the
> Raspberry Pi IPA module.
>
> The series starts with a few cleanups and extensions in YamlParser, with
> patch 01/12 turning the enum Type into an enum class, patch 02/12
> extending the size() function to support dictionaries in addition to
> lists, and patch 03/12 moving the parser from using FILE to File for a
> better RAII-style management of files. Patch 04/12 continues with a
> small cleanup in the YamlParser unit tests.
>
> Patch 05/12 introduces the first big change in the series: a new
> iterator API for YamlObject. This allows iterating over the elements of
> a list of dictionaly object. Unlike the boost property_tree
> implementation that iterates over a pair of std::string key and ptree
> value, the YamlObject iterators differ for lists and dictionaries. This
> provides a more familiar API for lists, at the expense of requiring
> adapter objects (see the asDict() and asList() functions). I'm
> interested in feedback on this approach, hence the RFC. Patch 06/12
> then extends the unit tests to cover the iterator API, and patch 07/12
> uses the API in the Android camera HAL. Patch 08/12 completes that part
> of the series by removing the then unused memberNames() function.
>
> The series continue with two extensions to the YamlParser. In patch
> 09/12 the get() function receives a fix to properly check the value
> range for 32-bit integers, and in patch 10/12 new specializations of
> the function are added for 16-bit integers, needed by the Raspberry Pi
> IPA.
>
> The last part of the series converts the Raspberry Pi IPA from boost to
> YamlParser. Small changes are needed in patch 11/12 to convert tabs to
> spaces in the tuning JSON files, as they confuse the YAML parser. Patch
> 12/12 then converts the IPA module, and drops the dependency on boost.
>
> When compiled in release mode with clang-13, the Raspberry Pi IPA module
> .text section shrank from 1317086 to 963614 bytes.
>
> I haven't been able to test the series on a Raspberry Pi yet (second
> reason for the RFC) as I don't have access to my board at the moment.
>
> Laurent Pinchart (12):
>   libcamera: yaml_object: Turn Type into an enum class
>   libcamera: yaml_parser: Extend YamlObject::size() to dictionaries
>   libcamera: yaml_parser: Switch from FILE to File
>   test: yaml-parser: Use write() instead of fwrite()
>   libcamera: yaml_parser: Add iterator API
>   test: yaml_parser: Extend tests to cover the iterator API
>   android: Use the YamlObject iterator API
>   libcamera: yaml_parser: Remove memberNames() function
>   libcamera: yaml_parser: Fix range checks for 32-bit integers
>   libcamera: yaml_parser: Add get() specializations for 16-bit integers
>   ipa: raspberrypi: Replace tabs with spaces in tuning data files
>   ipa: raspberrypi: Use YamlParser to replace dependency on boost
>
>  README.rst                                    |   6 -
>  include/libcamera/internal/yaml_parser.h      | 133 +++++++++-
>  src/android/camera_hal_config.cpp             |  22 +-
>  src/ipa/raspberrypi/controller/algorithm.cpp  |   2 +-
>  src/ipa/raspberrypi/controller/algorithm.hpp  |   6 +-
>  src/ipa/raspberrypi/controller/controller.cpp |  27 +-
>  src/ipa/raspberrypi/controller/pwl.cpp        |  12 +-
>  src/ipa/raspberrypi/controller/pwl.hpp        |   5 +-
>  src/ipa/raspberrypi/controller/rpi/agc.cpp    |  94 +++----
>  src/ipa/raspberrypi/controller/rpi/agc.hpp    |  10 +-
>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  94 ++++---
>  src/ipa/raspberrypi/controller/rpi/alsc.hpp   |   2 +-
>  src/ipa/raspberrypi/controller/rpi/awb.cpp    |  89 +++----
>  src/ipa/raspberrypi/controller/rpi/awb.hpp    |   8 +-
>  .../controller/rpi/black_level.cpp            |  12 +-
>  .../controller/rpi/black_level.hpp            |   2 +-
>  src/ipa/raspberrypi/controller/rpi/ccm.cpp    |  28 +--
>  src/ipa/raspberrypi/controller/rpi/ccm.hpp    |   4 +-
>  .../raspberrypi/controller/rpi/contrast.cpp   |  18 +-
>  .../raspberrypi/controller/rpi/contrast.hpp   |   2 +-
>  src/ipa/raspberrypi/controller/rpi/dpc.cpp    |   4 +-
>  src/ipa/raspberrypi/controller/rpi/dpc.hpp    |   2 +-
>  src/ipa/raspberrypi/controller/rpi/geq.cpp    |  10 +-
>  src/ipa/raspberrypi/controller/rpi/geq.hpp    |   2 +-
>  src/ipa/raspberrypi/controller/rpi/lux.cpp    |  12 +-
>  src/ipa/raspberrypi/controller/rpi/lux.hpp    |   2 +-
>  src/ipa/raspberrypi/controller/rpi/noise.cpp  |   6 +-
>  src/ipa/raspberrypi/controller/rpi/noise.hpp  |   2 +-
>  src/ipa/raspberrypi/controller/rpi/sdn.cpp    |   6 +-
>  src/ipa/raspberrypi/controller/rpi/sdn.hpp    |   2 +-
>  .../raspberrypi/controller/rpi/sharpen.cpp    |   8 +-
>  .../raspberrypi/controller/rpi/sharpen.hpp    |   2 +-
>  src/ipa/raspberrypi/data/imx219.json          |   8 +-
>  src/ipa/raspberrypi/data/imx219_noir.json     |  10 +-
>  src/ipa/raspberrypi/data/imx290.json          |  18 +-
>  src/ipa/raspberrypi/data/imx477.json          |   8 +-
>  src/ipa/raspberrypi/data/imx477_noir.json     |  10 +-
>  src/ipa/raspberrypi/data/ov5647.json          |  10 +-
>  src/ipa/raspberrypi/data/ov5647_noir.json     |  12 +-
>  src/ipa/raspberrypi/data/se327m12.json        |   6 +-
>  src/ipa/raspberrypi/meson.build               |   1 -
>  src/ipa/raspberrypi/raspberrypi.cpp           |   1 +
>  src/libcamera/yaml_parser.cpp                 | 232 +++++++++++++-----
>  test/yaml-parser.cpp                          | 109 +++++---
>  44 files changed, 659 insertions(+), 400 deletions(-)
>
>
> base-commit: 153b468930a9df22debb28889312f8a5c511ee04
> --
> Regards,
>
> Laurent Pinchart
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220525/00e81d99/attachment-0001.htm>

More information about the libcamera-devel mailing list