Message ID | 20220604185939.29163-1-laurent.pinchart@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
On Sat, Jun 04, 2022 at 09:59:25PM +0300, Laurent Pinchart via libcamera-devel 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/14 turning the enum Type into an enum class, patch 02/14 > extending the size() function to support dictionaries in addition to > lists, and patch 03/14 moving the parser from using FILE to File for a > better RAII-style management of files. Patch 04/14 continues with a > small cleanup in the YamlParser unit tests. > > Patches 05/14 and 06/14 introduces the first important functional change > in the series: they preserve the order of items in YAML dictionaries. > This is needed by the Raspberry Pi IPA, as it instantiates algorithms in > the same order as they are declared in the tuning file. Patch 05/14 > extends the YamlParser unit test to ensure order is preserved (which > fails), and patch 06/14 preserves the order (which fixes the test). > > Patch 07/14 contains the second big change: 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 08/14 then extends the unit tests to > cover the iterator API, and patch 09/14 uses the API in the Android > camera HAL. Patch 10/14 completes that part of the series by removing > the then unused memberNames() function. > > The series continue with two extensions to the YamlParser. In patch > 11/14 the get() function receives a fix to properly check the value > range for 32-bit integers, and in patch 12/14 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 13/14 to convert tabs to > spaces in the tuning JSON files, as they confuse the YAML parser. Patch > 14/14 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 forgot to update the numbers. It looks like I had probably compiled libcamera in debug mode, not release mode, as it has now shrunk from 244788 to 195876 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 (14): > 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() > test: yaml-parser: Test dictionary items ordering > libcamera: yaml_parser: Preserve order of items in dictionary > 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 | 142 +++++++++- > 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 | 263 +++++++++++++----- > test/yaml-parser.cpp | 112 +++++--- > 44 files changed, 691 insertions(+), 411 deletions(-) > > > base-commit: 7ef83e02284715db95e3f4cd61744290baff2840