<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 11 Jul 2022 at 21:18, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
Would you be able to review this ? And could I volunteer you to handle<br>
the items listed below to make this a non-RFC series ? :-) That would be<br>
<br>
- Adding support for a new tuning data format that doesn't rely on YAML<br>
mapping ordering (it's really just about turning the mapping into a<br>
list of single-entry mappings, and while at it, adding a top-level<br>
version number element).<br>
<br>
- Update ctt to generate the new format.<br>
<br>
- Add a tool (Python seems to be the best option) to convert tuning<br>
files from the existing format to the new format.<br></blockquote><div><br></div><div>Sure, I'll get to that as soon as I can.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
On Thu, Jun 16, 2022 at 06:05:31PM +0300, Laurent Pinchart via libcamera-devel wrote:<br>
> Hello,<br>
> <br>
> As mentioned in the subject, this patch series replaces the boost JSON<br>
> parser with the YamlParser implementation based on libyaml for the<br>
> Raspberry Pi IPA module.<br>
> <br>
> Compared to v2, the base work that introduces the iterator API has been<br>
> sent separately in "[PATCH 0/7] libcamera: yaml_parser: Add iterator<br>
> API". This series then addresses the specific needs of the Raspberry Pi<br>
> IPA module.<br>
> <br>
> The Raspberry Pi IPA tuning data contains a list of algorithms stored in<br>
> a mapping. Both the JSON and the YAML specifications explicitly state<br>
> that mappings are not ordered, but the IPA relies on the JSON data order<br>
> being preserved by the parser. This is an implementation-specific<br>
> behaviour that happens to be match the boost JSON parser implementation,<br>
> but not the YamlParser class.<br>
> <br>
> While in a perfect world we would update the tuning data format to avoid<br>
> this problem, we don't want to break every tuning file out there, and<br>
> thus need to ensure backward compatibility. This is why patches 1/4 and<br>
> 2/4 update the parser and iterator implementation to preserve the data<br>
> order, and patches 3/4 and 4/4 then move the Raspberry Pi IPA from boost<br>
> to YamlParser.<br>
> <br>
> If this approach is deemed reasonable, then the next non-RFC version of<br>
> the series should also include a new format for the Raspberry Pi IPA<br>
> tuning data that would be compliant with the JSON specification, an<br>
> update to the camera tuning tool to generate tuning files in that<br>
> format, and possibly a Python script to convert existing files to the<br>
> new format.<br>
> <br>
> Laurent Pinchart (4):<br>
> test: yaml-parser: Test dictionary items ordering<br>
> libcamera: yaml_parser: Preserve order of items in dictionary<br>
> ipa: raspberrypi: Replace tabs with spaces in tuning data files<br>
> ipa: raspberrypi: Use YamlParser to replace dependency on boost<br>
> <br>
> README.rst | 6 --<br>
> include/libcamera/internal/yaml_parser.h | 37 +++++---<br>
> src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-<br>
> src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-<br>
> src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--<br>
> src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-<br>
> src/ipa/raspberrypi/controller/pwl.hpp | 5 +-<br>
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------<br>
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-<br>
> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------<br>
> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------<br>
> src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-<br>
> .../controller/rpi/black_level.cpp | 12 +--<br>
> .../controller/rpi/black_level.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---<br>
> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-<br>
> .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--<br>
> .../raspberrypi/controller/rpi/contrast.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-<br>
> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-<br>
> src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--<br>
> src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-<br>
> src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-<br>
> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-<br>
> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-<br>
> .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-<br>
> .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-<br>
> src/ipa/raspberrypi/data/imx219.json | 8 +-<br>
> src/ipa/raspberrypi/data/imx219_noir.json | 10 +-<br>
> src/ipa/raspberrypi/data/imx290.json | 18 ++--<br>
> src/ipa/raspberrypi/data/imx477.json | 8 +-<br>
> src/ipa/raspberrypi/data/imx477_noir.json | 10 +-<br>
> src/ipa/raspberrypi/data/ov5647.json | 10 +-<br>
> src/ipa/raspberrypi/data/ov5647_noir.json | 12 +--<br>
> src/ipa/raspberrypi/data/se327m12.json | 6 +-<br>
> src/ipa/raspberrypi/meson.build | 1 -<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 1 +<br>
> src/libcamera/yaml_parser.cpp | 35 ++++---<br>
> test/yaml-parser.cpp | 17 ++--<br>
> 43 files changed, 334 insertions(+), 318 deletions(-)<br>
> <br>
> <br>
> base-commit: c5ab0f3b64280733a10b2da39e522fe87d0d51f0<br>
> -- <br>
> Regards,<br>
> <br>
> Laurent Pinchart<br>
> <br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>