[{"id":23347,"web_url":"https://patchwork.libcamera.org/comment/23347/","msgid":"","date":"2022-06-07T16:15:22","subject":"Re: [libcamera-devel] [RFC PATCH v2 00/14] Replace boost JSON\n\tparser with libyaml in Raspberry Pi IPA","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sat, Jun 04, 2022 at 09:59:25PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hello,\n> \n> As mentioned in the subject, this patch series replaces the boost JSON\n> parser with the YamlParser implementation based on libyaml for the\n> Raspberry Pi IPA module.\n> \n> The series starts with a few cleanups and extensions in YamlParser, with\n> patch 01/14 turning the enum Type into an enum class, patch 02/14\n> extending the size() function to support dictionaries in addition to\n> lists, and patch 03/14 moving the parser from using FILE to File for a\n> better RAII-style management of files. Patch 04/14 continues with a\n> small cleanup in the YamlParser unit tests.\n> \n> Patches 05/14 and 06/14 introduces the first important functional change\n> in the series: they preserve the order of items in YAML dictionaries.\n> This is needed by the Raspberry Pi IPA, as it instantiates algorithms in\n> the same order as they are declared in the tuning file. Patch 05/14\n> extends the YamlParser unit test to ensure order is preserved (which\n> fails), and patch 06/14 preserves the order (which fixes the test).\n> \n> Patch 07/14 contains the second big change: a new iterator API for\n> YamlObject. This allows iterating over the elements of a list of\n> dictionaly object. Unlike the boost property_tree implementation that\n> iterates over a pair of std::string key and ptree value, the YamlObject\n> iterators differ for lists and dictionaries. This provides a more\n> familiar API for lists, at the expense of requiring adapter objects (see\n> the asDict() and asList() functions). I'm interested in feedback on this\n> approach, hence the RFC. Patch 08/14 then extends the unit tests to\n> cover the iterator API, and patch 09/14 uses the API in the Android\n> camera HAL. Patch 10/14 completes that part of the series by removing\n> the then unused memberNames() function.\n> \n> The series continue with two extensions to the YamlParser. In patch\n> 11/14 the get() function receives a fix to properly check the value\n> range for 32-bit integers, and in patch 12/14 new specializations of\n> the function are added for 16-bit integers, needed by the Raspberry Pi\n> IPA.\n> \n> The last part of the series converts the Raspberry Pi IPA from boost to\n> YamlParser. Small changes are needed in patch 13/14 to convert tabs to\n> spaces in the tuning JSON files, as they confuse the YAML parser. Patch\n> 14/14 then converts the IPA module, and drops the dependency on boost.\n> \n> When compiled in release mode with clang-13, the Raspberry Pi IPA module\n> .text section shrank from 1317086 to 963614 bytes.\n\nI forgot to update the numbers. It looks like I had probably compiled\nlibcamera in debug mode, not release mode, as it has now shrunk from\n244788 to 195876 bytes.\n\n> I haven't been able to test the series on a Raspberry Pi yet (second\n> reason for the RFC) as I don't have access to my board at the moment.\n> \n> Laurent Pinchart (14):\n> libcamera: yaml_object: Turn Type into an enum class\n> libcamera: yaml_parser: Extend YamlObject::size() to dictionaries\n> libcamera: yaml_parser: Switch from FILE to File\n> test: yaml-parser: Use write() instead of fwrite()\n> test: yaml-parser: Test dictionary items ordering\n> libcamera: yaml_parser: Preserve order of items in dictionary\n> libcamera: yaml_parser: Add iterator API\n> test: yaml_parser: Extend tests to cover the iterator API\n> android: Use the YamlObject iterator API\n> libcamera: yaml_parser: Remove memberNames() function\n> libcamera: yaml_parser: Fix range checks for 32-bit integers\n> libcamera: yaml_parser: Add get() specializations for 16-bit integers\n> ipa: raspberrypi: Replace tabs with spaces in tuning data files\n> ipa: raspberrypi: Use YamlParser to replace dependency on boost\n> \n> README.rst | 6 -\n> include/libcamera/internal/yaml_parser.h | 142 +++++++++-\n> src/android/camera_hal_config.cpp | 22 +-\n> src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> src/ipa/raspberrypi/controller/controller.cpp | 27 +-\n> src/ipa/raspberrypi/controller/pwl.cpp | 12 +-\n> src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++----\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++----\n> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++---\n> src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> .../controller/rpi/black_level.cpp | 12 +-\n> .../controller/rpi/black_level.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +-\n> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> .../raspberrypi/controller/rpi/contrast.cpp | 18 +-\n> .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +-\n> src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> src/ipa/raspberrypi/data/imx219.json | 8 +-\n> src/ipa/raspberrypi/data/imx219_noir.json | 10 +-\n> src/ipa/raspberrypi/data/imx290.json | 18 +-\n> src/ipa/raspberrypi/data/imx477.json | 8 +-\n> src/ipa/raspberrypi/data/imx477_noir.json | 10 +-\n> src/ipa/raspberrypi/data/ov5647.json | 10 +-\n> src/ipa/raspberrypi/data/ov5647_noir.json | 12 +-\n> src/ipa/raspberrypi/data/se327m12.json | 6 +-\n> src/ipa/raspberrypi/meson.build | 1 -\n> src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> src/libcamera/yaml_parser.cpp | 263 +++++++++++++-----\n> test/yaml-parser.cpp | 112 +++++---\n> 44 files changed, 691 insertions(+), 411 deletions(-)\n> \n> \n> base-commit: 7ef83e02284715db95e3f4cd61744290baff2840","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A61FCBD160\n\tfor ;\n\tTue, 7 Jun 2022 16:15:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D5B1C65631;\n\tTue, 7 Jun 2022 18:15:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 916AB633A5\n\tfor ;\n\tTue, 7 Jun 2022 18:15:29 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-66-225-nat.elisa-mobile.fi\n\t[85.76.66.225])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7FA3580A\n\tfor ;\n\tTue, 7 Jun 2022 18:15:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654618531;\n\tbh=xwZ6ewLsdWIx5oKFAlLcueBvPdUqmgJJwtFuzdmtoZ4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=QLFRoOCgQY3wrAe4oT1eA/Rg9L9TBULbaLs5SD55SVg3j4U3o6IIo0JELp4bAh8oU\n\tABUrZ+AKtu9EsoyRytNCTg7zHwcFQDJd5UCRneSNJR5tSolmKUyRmTpWcmyvmR7wxu\n\thkYZTnQgN9prdLajl+jqzcg7bQWm4lZx6HV799dpYLRzyk8TPPp1PL5EBl03NmMRP5\n\tirnkq71NQ0zXa8nzux+/bNOdHNzLhZEMoT79CzjZYeIyVw6VhJvog+AT/45T3poY6+\n\tKujfaIsKP7bt/1tMJizmOE5DhudwCNd9iSKuQU2nydGsdWDP6Db+GpVWgkJIjVVQ35\n\tV19Uqd5Yl/pww==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654618529;\n\tbh=xwZ6ewLsdWIx5oKFAlLcueBvPdUqmgJJwtFuzdmtoZ4=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=Z27DjjaCR2gfM4zVoJQKK8B7z7tH1AKAGlRzgE1oc9dxnVx/rI0YjSoB9ZZ7zotuE\n\t5DwdBp2r7BqrMeguEIsf5vF/8DXuF5N0qSXpCSKPFJ1A6LxfYWo4rERAFgTjO4n00u\n\t3cL3i3ZniU/+9xARqTDxFUMDcLvNhmkeE0lzmIZ4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Z27DjjaC\"; dkim-atps=neutral","Date":"Tue, 7 Jun 2022 19:15:22 +0300","To":"libcamera-devel@lists.libcamera.org","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 00/14] Replace boost JSON\n\tparser with libyaml in Raspberry Pi IPA","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]