[libcamera-devel] [RFC PATCH 00/12] Replace boost JSON parser with libyaml in Raspberry Pi IPA
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed May 25 14:28:30 CEST 2022
Hi Naush,
On Wed, May 25, 2022 at 01:15:00PM +0100, Naushir Patuck wrote:
> 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?
Thanks for testing. This is useful feedback. I'll try to preserve the
order in YamlObject.
> On Tue, 24 May 2022 at 23:58, Laurent Pinchart 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
More information about the libcamera-devel
mailing list