[{"id":23392,"web_url":"https://patchwork.libcamera.org/comment/23392/","msgid":"<20220613042219.GG2369877@pyrite.rasen.tech>","date":"2022-06-13T04:22:19","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Laurent,\n\nOn Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> The order of items in a YAML dictionary may matter. Update the test to\n> ensure that it is preserved. The test currently fails at the YamlParser\n\nMy understanding is that YAML mappings are unordered [1] [2], and if\norder in the mapping is significant, then either a sequence of mappings\n[3] or flow mapping adjacent values [4] should be used.\n\n[1] https://yaml.org/spec/1.2.2/#mapping\n[2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n[3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n[4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n\n\nPaul\n\n> doesn't correctly preserve the order, this will be fixed by the next\n> commit.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> test/yaml-parser.cpp | 7 +++----\n> 1 file changed, 3 insertions(+), 4 deletions(-)\n> \n> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> index 5ff4c3236dbf..582c9caed836 100644\n> --- a/test/yaml-parser.cpp\n> +++ b/test/yaml-parser.cpp\n> @@ -29,8 +29,8 @@ static const string testYaml =\n> \t\" - Mary\\n\"\n> \t\"dictionary:\\n\"\n> \t\" a: 1\\n\"\n> -\t\" b: 2\\n\"\n> \t\" c: 3\\n\"\n> +\t\" b: 2\\n\"\n> \t\"level1:\\n\"\n> \t\" level2:\\n\"\n> \t\" - [1, 2]\\n\"\n> @@ -428,7 +428,6 @@ protected:\n> \t\t}\n> \n> \t\tauto memeberNames = dictObj.memberNames();\n> -\t\tsort(memeberNames.begin(), memeberNames.end());\n> \n> \t\tif (memeberNames.size() != 3) {\n> \t\t\tcerr << \"Dictionary object fail to extra member names\" << std::endl;\n> @@ -436,8 +435,8 @@ protected:\n> \t\t}\n> \n> \t\tif (memeberNames[0] != \"a\" ||\n> -\t\t memeberNames[1] != \"b\" ||\n> -\t\t memeberNames[2] != \"c\") {\n> +\t\t memeberNames[1] != \"c\" ||\n> +\t\t memeberNames[2] != \"b\") {\n> \t\t\tcerr << \"Dictionary object fail to parse member names\" << std::endl;\n> \t\t\treturn TestFail;\n> \t\t}","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 6162CBD808\n\tfor ;\n\tMon, 13 Jun 2022 04:22:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEF0865635;\n\tMon, 13 Jun 2022 06:22:29 +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 B674D600F0\n\tfor ;\n\tMon, 13 Jun 2022 06:22:27 +0200 (CEST)","from pyrite.rasen.tech (softbank036240126034.bbtec.net\n\t[36.240.126.34])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FE10440;\n\tMon, 13 Jun 2022 06:22:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655094149;\n\tbh=nLVTYSCBC04k6zMOTJ0TJ0EWWsJfUnwRUXbPwC9r97I=;\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:Cc:\n\tFrom;\n\tb=Wn5rZABDCLHNYYcJewddOq2HQidbRnfrgbsMWQcvOU+HjE2/of0s1hQmHOdCWQQ9M\n\tOSW6XmOcix0myTjy5aD4pkIYXUoCWHfJ73GHu2ckz5Mzo/2OpX7URkLrxn/lGj+2QR\n\tmZnlO8cV3ox/Rddgyxb+ZhepmRCE5ROYiDA+CwutfZMFlnVEdX7xXuBAvOUhzLed8B\n\t7b5NyCp1RgdbIZKLo8vdUgSmw7Eoong8EnXYPo14FwU7imb9CVJ4ZqS2eYPhnRzRX8\n\txcIqwVxQwiIlywmjXrq0Yiz0IFKfbsHAGWYd+vFLQ4LW9kv24DMju747qFAyz5a2SV\n\t9MHpJt1ao0Wpw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655094147;\n\tbh=nLVTYSCBC04k6zMOTJ0TJ0EWWsJfUnwRUXbPwC9r97I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KX95m/hhJahXrFL0hfTnLzY0Mj7eVh6Aw+de/isJtZ0jWxqv4Y5IzgedelaLnDs/g\n\tBDiU0tjK8Ql0A73SLtROzoZAwWR9cICZh3GGSYOnFN/OGGDFI6ve/Ou1CxhrpkspvC\n\trnnC8hQ9qWSn5yGt5f2vGpQHNiyghaQ5JuF0yvDg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KX95m/hh\"; dkim-atps=neutral","Date":"Mon, 13 Jun 2022 13:22:19 +0900","To":"Laurent Pinchart ","Message-ID":"<20220613042219.GG2369877@pyrite.rasen.tech>","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23393,"web_url":"https://patchwork.libcamera.org/comment/23393/","msgid":"","date":"2022-06-13T06:17:53","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Laurent,\n> \n> On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > The order of items in a YAML dictionary may matter. Update the test to\n> > ensure that it is preserved. The test currently fails at the YamlParser\n> \n> My understanding is that YAML mappings are unordered [1] [2], and if\n> order in the mapping is significant, then either a sequence of mappings\n> [3] or flow mapping adjacent values [4] should be used.\n\nThat's a very good point. [5] even mentions\n\n\"while imposing a order on mapping keys is necessary for flattening YAML\nrepresentations to a sequential access medium, this serialization detail\nmust not be used to convey application level information.\"\n\nThe same applies to JSON ([6]).\n\nFixing this would require changing the syntax of the tuning files. It's\ninconvenient, but not doing so opens the door to more issues in the\nfuture :-S\n\n> [1] https://yaml.org/spec/1.2.2/#mapping\n> [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n\n[5] https://yaml.org/spec/1.2.2/#32-information-models\n[6] https://www.json.org/json-en.html\n\n> > doesn't correctly preserve the order, this will be fixed by the next\n> > commit.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > test/yaml-parser.cpp | 7 +++----\n> > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > index 5ff4c3236dbf..582c9caed836 100644\n> > --- a/test/yaml-parser.cpp\n> > +++ b/test/yaml-parser.cpp\n> > @@ -29,8 +29,8 @@ static const string testYaml =\n> > \t\" - Mary\\n\"\n> > \t\"dictionary:\\n\"\n> > \t\" a: 1\\n\"\n> > -\t\" b: 2\\n\"\n> > \t\" c: 3\\n\"\n> > +\t\" b: 2\\n\"\n> > \t\"level1:\\n\"\n> > \t\" level2:\\n\"\n> > \t\" - [1, 2]\\n\"\n> > @@ -428,7 +428,6 @@ protected:\n> > \t\t}\n> > \n> > \t\tauto memeberNames = dictObj.memberNames();\n> > -\t\tsort(memeberNames.begin(), memeberNames.end());\n> > \n> > \t\tif (memeberNames.size() != 3) {\n> > \t\t\tcerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > @@ -436,8 +435,8 @@ protected:\n> > \t\t}\n> > \n> > \t\tif (memeberNames[0] != \"a\" ||\n> > -\t\t memeberNames[1] != \"b\" ||\n> > -\t\t memeberNames[2] != \"c\") {\n> > +\t\t memeberNames[1] != \"c\" ||\n> > +\t\t memeberNames[2] != \"b\") {\n> > \t\t\tcerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > \t\t\treturn TestFail;\n> > \t\t}","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 DD320BD161\n\tfor ;\n\tMon, 13 Jun 2022 06:18:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3017765635;\n\tMon, 13 Jun 2022 08:18:03 +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 7F959600F0\n\tfor ;\n\tMon, 13 Jun 2022 08:18:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 12EE9305;\n\tMon, 13 Jun 2022 08:18:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655101083;\n\tbh=WV7baQ5vBrWKZFs4at6nHbKzLKNVEEg8+xuruRAWcOA=;\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:Cc:\n\tFrom;\n\tb=EpkYOtGHvNtfj3+WeN7cEemLAVq/mwueK+hOflYgY73F0Yzn1lCNtyyI6DkPfJGgN\n\tZQmn2q38dQLleC7RBR1ZUPcM/n4j6fQjQr9ff9EyNmq4N+L91qMqzxiAtfldBKMqRJ\n\t8bLw9aOs/k2G94aYGOPKCGx6OqKzA6FnD0kj8PyWkHkR2m8aw570DzF9/G2j7iY4r7\n\t2si6Z10gcRbkTz+NoTI8VaWbg/MVbVAFYXixalcBRRATFBi59FjKATd6XMVpYtW4GL\n\t7Fh1s3rETHoXMr+g1a6kgeaZbm2W5dYK9sRv7LsMe1oCElTuec+azcfR3VitUHtH+5\n\tSqHSdQ3nIzqjg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655101081;\n\tbh=WV7baQ5vBrWKZFs4at6nHbKzLKNVEEg8+xuruRAWcOA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KfFsDvc98q40V75H0w4zkOi0IUbgHFHS15hA7Sn9f2aii1R71nCSpxV2F+JdjZ2fZ\n\tYQYIsnTItaf+j/KIT5LEdAphiGhxZx5LHwvp3WzgnIpGIAwYe8PFjlExeZq0+5EyvK\n\tOxlfHHVUanl1idZQFxYcnwi+qNamrtxldhD3MKuw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KfFsDvc9\"; dkim-atps=neutral","Date":"Mon, 13 Jun 2022 09:17:53 +0300","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220613042219.GG2369877@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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 ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23394,"web_url":"https://patchwork.libcamera.org/comment/23394/","msgid":"","date":"2022-06-13T08:05:21","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi,\n\nOn Mon, 13 Jun 2022 at 07:18, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Paul,\n>\n> On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com\n> wrote:\n> > Hi Laurent,\n> >\n> > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart via\n> libcamera-devel wrote:\n> > > The order of items in a YAML dictionary may matter. Update the test to\n> > > ensure that it is preserved. The test currently fails at the YamlParser\n> >\n> > My understanding is that YAML mappings are unordered [1] [2], and if\n> > order in the mapping is significant, then either a sequence of mappings\n> > [3] or flow mapping adjacent values [4] should be used.\n>\n> That's a very good point. [5] even mentions\n>\n> \"while imposing a order on mapping keys is necessary for flattening YAML\n> representations to a sequential access medium, this serialization detail\n> must not be used to convey application level information.\"\n>\n> The same applies to JSON ([6]).\n>\n> Fixing this would require changing the syntax of the tuning files. It's\n> inconvenient, but not doing so opens the door to more issues in the\n> future :-S\n>\n\nGiven that the IPA does require ordering and we have been lucky with\nBoost preserving order in the JSON parser, I think we probably ought\nto specify ordering in the config file with a specific key. When I get a\nchange, I'll look to add a patch to allow this on the existing codebase,\nand this series ought to \"just work\" after that.\n\nThanks,\nNaush\n\n> [1] https://yaml.org/spec/1.2.2/#mapping\n> > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n>\n> [5] https://yaml.org/spec/1.2.2/#32-information-models\n> [6] https://www.json.org/json-en.html\n>\n> > > doesn't correctly preserve the order, this will be fixed by the next\n> > > commit.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > test/yaml-parser.cpp | 7 +++----\n> > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > index 5ff4c3236dbf..582c9caed836 100644\n> > > --- a/test/yaml-parser.cpp\n> > > +++ b/test/yaml-parser.cpp\n> > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > \" - Mary\\n\"\n> > > \"dictionary:\\n\"\n> > > \" a: 1\\n\"\n> > > - \" b: 2\\n\"\n> > > \" c: 3\\n\"\n> > > + \" b: 2\\n\"\n> > > \"level1:\\n\"\n> > > \" level2:\\n\"\n> > > \" - [1, 2]\\n\"\n> > > @@ -428,7 +428,6 @@ protected:\n> > > }\n> > >\n> > > auto memeberNames = dictObj.memberNames();\n> > > - sort(memeberNames.begin(), memeberNames.end());\n> > >\n> > > if (memeberNames.size() != 3) {\n> > > cerr << \"Dictionary object fail to extra member\n> names\" << std::endl;\n> > > @@ -436,8 +435,8 @@ protected:\n> > > }\n> > >\n> > > if (memeberNames[0] != \"a\" ||\n> > > - memeberNames[1] != \"b\" ||\n> > > - memeberNames[2] != \"c\") {\n> > > + memeberNames[1] != \"c\" ||\n> > > + memeberNames[2] != \"b\") {\n> > > cerr << \"Dictionary object fail to parse member\n> names\" << std::endl;\n> > > return TestFail;\n> > > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","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 69F18BD808\n\tfor ;\n\tMon, 13 Jun 2022 08:05:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8799565637;\n\tMon, 13 Jun 2022 10:05:39 +0200 (CEST)","from mail-lj1-x22c.google.com (mail-lj1-x22c.google.com\n\t[IPv6:2a00:1450:4864:20::22c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 10B33601F0\n\tfor ;\n\tMon, 13 Jun 2022 10:05:38 +0200 (CEST)","by mail-lj1-x22c.google.com with SMTP id y29so5290860ljd.7\n\tfor ;\n\tMon, 13 Jun 2022 01:05:38 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655107539;\n\tbh=XHIHqLmkoqkaiDJvXfa9sLthaLMmy+6omwqtkQgM+r8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=xjp1wWzXszq5IhySTn7+kGmA2AGvQwy8+/WTOKpAjUQQc820Wacl/OcJzkHeUh4bA\n\tZHCT/ko5pkN4FDt5RnK3IDujLRYwagpGTzKRm/c38KTq6/dHKYS/O6iq2Z2MzrZG5x\n\tAgvOkAQ0hbYeGinVAQ6HKqiR/BGQBlbRSvQxt3eBiS6I/j9sDvvEP+7rmgix5n3bzt\n\tFFminjYBeF9Fr5EmER/JflVATGszGcaisQE8o9RCielm2/5bGXEG8VNWKdNKlpFdBP\n\t+paievD2N/MlD5OyTZQGi+uPX74O++uF3sIMsSTruS3l8aNqPK/pcz9xsTAopLRaVA\n\t0SU3k1bdOodWA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=dwCQBxf8CULq+vTjB40lC7pExQ42EgESAnv0TE+nIS0=;\n\tb=Di6AyEYlj4UVgAG6Ry0lHcopTram9vxAcDk0Py3K3kKN5zmpUEeh6Hf+sqBkB3Trdn\n\tiAUzw+zDjQq09y5tDFTlOSCkDQ2CYeEhmTyi5uAq3qqsu0+jOs5qPLqh9/vnC9tVaoFG\n\t9zJWMXtovb9QZu/pfegC3ZE+Po67PXbyGUwRnLBIFyG1xb4yVAZE3RMQ4uHRlSwHc/2M\n\t17Zene+wnazTct4x110b0kmLgVR3Hc43mMtH1b4SF6luNA0Vjc6I+Mgb4TC/ERrh+iyu\n\t4GGzL1Xtk7suWObNqr8wQhHi3DlnYBopWQKnX4goHSN0lEbokpEYqU9AiPdMnoEHRgkJ\n\t0FAA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"Di6AyEYl\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=dwCQBxf8CULq+vTjB40lC7pExQ42EgESAnv0TE+nIS0=;\n\tb=PnSaDREFv+lNcOvr6qPBlMxm0OYvP05j4HTsVm/07QCJwSQzPbYuJcWnbotnSe5Imb\n\tTq1F9bTRsdgE6gKa2dE8F/nAvhW2Abq8K9QpBvC8+HbisvNudmSOV9b3UoCO68EXJ7Z7\n\t0gLHET8/k67khRYH9+WYyKHHAP6wYTsTUTaEBHgea+vS3pGx1SFoFxpVyxGGj33xiusq\n\tUoj6HBB8U32WfsbaskM8ayvTJNi4Vli04gDDRVtJ0ZSuIVL5TZTXOt3Tqn9nc5ptIqMs\n\t39zHaQ5BaBE8n653RjZkGq58VYWQ8RSW5s2R5lgpKOR34LEOnTk/O+CgVRcNHFrJa2wf\n\tOXiA==","X-Gm-Message-State":"AOAM530ipAPx64i7rSsUTdq6N9Pm3niAhYJtO9wGJ60uYY0vN634daT4\n\tBkQxGpwWVd0HqFmWVXHHbjQXsQtKi/5IbrpEFSOWOQ==","X-Google-Smtp-Source":"ABdhPJxhthuWwi5vEtMkC4WALcC4Wk/BvOUgUD4qrUpPG00r+a7wzWC0KijeAyG1j5nuXiUzx6V5e7YNSVb/fd/q0To=","X-Received":"by 2002:a2e:8805:0:b0:255:6e73:9a67 with SMTP id\n\tx5-20020a2e8805000000b002556e739a67mr31159483ljh.426.1655107537270;\n\tMon, 13 Jun 2022 01:05:37 -0700 (PDT)","MIME-Version":"1.0","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t","In-Reply-To":"","Date":"Mon, 13 Jun 2022 09:05:21 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000d3149e05e14fc056\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23395,"web_url":"https://patchwork.libcamera.org/comment/23395/","msgid":"","date":"2022-06-13T08:46:10","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > >\n> > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > order in the mapping is significant, then either a sequence of mappings\n> > > [3] or flow mapping adjacent values [4] should be used.\n> >\n> > That's a very good point. [5] even mentions\n> >\n> > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > representations to a sequential access medium, this serialization detail\n> > must not be used to convey application level information.\"\n> >\n> > The same applies to JSON ([6]).\n> >\n> > Fixing this would require changing the syntax of the tuning files. It's\n> > inconvenient, but not doing so opens the door to more issues in the\n> > future :-S\n> \n> Given that the IPA does require ordering and we have been lucky with\n> Boost preserving order in the JSON parser, I think we probably ought\n> to specify ordering in the config file with a specific key.\n\nSounds good to me.\n\nIt could be done through a priority key in each algorithm, or by\nconverting the mapping to a list. In YAML format, that would be moving\nfrom\n\n{\n \"rpi.black_level\":\n {\n \"black_level\": 4096\n },\n \"rpi.noise\":\n {\n \"reference_constant\": 0,\n \"reference_slope\": 3.67\n },\n}\n\nto\n\n[\n {\n \"rpi.black_level\":\n {\n \"black_level\": 4096\n },\n },\n {\n \"rpi.noise\":\n {\n \"reference_constant\": 0,\n \"reference_slope\": 3.67\n },\n }\n]\n\nIn YAML format, it would translate as a move from\n\nrpi.black_level:\n black_level: 4096\nrpi.noise:\n reference_constant: 0\n reference_slope: 3.67\n\nto\n\n- rpi.black_level:\n black_level: 4096\n- rpi.noise:\n reference_constant: 0\n reference_slope: 3.67\n\nAs we have to change the tuning files, I'd like to take this as an\nopportunity to add a format version. Something along those lines maybe ?\n\nversion: 1.0\nalgorithms:\n - rpi.black_level:\n black_level: 4096\n - rpi.noise:\n reference_constant: 0\n reference_slope: 3.67\n\nAnd while we're discussing this, does someone know about best practice\nrules to design JSON/YAML grammars ? I've been wondering for a long time\nif the following grammar would have any advantage:\n\nversion: 1.0\nalgorithms:\n - name: rpi.black_level\n data:\n black_level: 4096\n - name: rpi.noise:\n data:\n reference_constant: 0\n reference_slope: 3.67\n\n> When I get a\n> change, I'll look to add a patch to allow this on the existing codebase,\n> and this series ought to \"just work\" after that.\n> \n> > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> >\n> > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > [6] https://www.json.org/json-en.html\n> >\n> > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > commit.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > test/yaml-parser.cpp | 7 +++----\n> > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > >\n> > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > --- a/test/yaml-parser.cpp\n> > > > +++ b/test/yaml-parser.cpp\n> > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > \" - Mary\\n\"\n> > > > \"dictionary:\\n\"\n> > > > \" a: 1\\n\"\n> > > > - \" b: 2\\n\"\n> > > > \" c: 3\\n\"\n> > > > + \" b: 2\\n\"\n> > > > \"level1:\\n\"\n> > > > \" level2:\\n\"\n> > > > \" - [1, 2]\\n\"\n> > > > @@ -428,7 +428,6 @@ protected:\n> > > > }\n> > > >\n> > > > auto memeberNames = dictObj.memberNames();\n> > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > >\n> > > > if (memeberNames.size() != 3) {\n> > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > @@ -436,8 +435,8 @@ protected:\n> > > > }\n> > > >\n> > > > if (memeberNames[0] != \"a\" ||\n> > > > - memeberNames[1] != \"b\" ||\n> > > > - memeberNames[2] != \"c\") {\n> > > > + memeberNames[1] != \"c\" ||\n> > > > + memeberNames[2] != \"b\") {\n> > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > return TestFail;\n> > > > }","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 E1D4CBD808\n\tfor ;\n\tMon, 13 Jun 2022 08:46:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEE8765635;\n\tMon, 13 Jun 2022 10:46:19 +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 42FD3601F0\n\tfor ;\n\tMon, 13 Jun 2022 10:46:19 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B6ACE305;\n\tMon, 13 Jun 2022 10:46:18 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655109980;\n\tbh=OjNNxgYVeZ50yFdLwy9sgW7I28T8AISdMpJ1lL5Lcjo=;\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:Cc:\n\tFrom;\n\tb=NBcYShBPbJ6IM7eFrgXrlsnrdMLf6PqxLNOVvK0YMz21akaiKbxYGbg0ZFRvc3GI+\n\tWX4QG0wc/g7FB2IwFwFZCdZyZ1pP3+nZJmNJJRqEFspIMs+nxqvQ2HtIJ89VpzSEjw\n\tbwRCwZx0+0gi8l5R2MOWGaem0Q3j98SHHBEY5tANsOhAbkYF+MNuX7WsVEOOD2BYun\n\tEDVlNRYk3JWSn4KZCn/2V1zcoWplqL3y/hUVwy/RHloZXiueJYnPwRCwB8h2Nnf+wj\n\teatJtMAG3R3Fl4b/nuVTIV7nLcap/84QEIbfluCPorT0zJAvzuYkzJtaYjXZ1Sw9e0\n\tyxRLzrMbga9gQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655109978;\n\tbh=OjNNxgYVeZ50yFdLwy9sgW7I28T8AISdMpJ1lL5Lcjo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dDkeA+li1mshnceqEl/8ikIj8f2gB+v+5ZhNsQt6YEhDasR+8hhXH84dr9Wa9u/tw\n\tDfmU++hMamL6E2UxFAPOhw5ukAOqpGaf3IHpkdfcuK7Ta2CD8Na9cVOXGaVwLBGmQM\n\trzL9b8aBIYy+w6Ua04//wvC6wBvN2C2+zg8SBq3U="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dDkeA+li\"; dkim-atps=neutral","Date":"Mon, 13 Jun 2022 11:46:10 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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 ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23398,"web_url":"https://patchwork.libcamera.org/comment/23398/","msgid":"","date":"2022-06-13T09:25:15","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent, everyone\n\nThanks for the suggestions. I'm fine to change the syntax to make\nthings clearer, but I wonder if we could avoid breaking existing JSON\nfiles? There are probably low numbers of them out there beyond the\nones that we've supplied, but you never quite know and backwards\ncompatibility is still a nice thing. Do you think that's something we\ncan arrange?\n\nDavid\n\nOn Mon, 13 Jun 2022 at 09:46, Laurent Pinchart\n wrote:\n>\n> Hi Naush,\n>\n> On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > >\n> > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > order in the mapping is significant, then either a sequence of mappings\n> > > > [3] or flow mapping adjacent values [4] should be used.\n> > >\n> > > That's a very good point. [5] even mentions\n> > >\n> > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > representations to a sequential access medium, this serialization detail\n> > > must not be used to convey application level information.\"\n> > >\n> > > The same applies to JSON ([6]).\n> > >\n> > > Fixing this would require changing the syntax of the tuning files. It's\n> > > inconvenient, but not doing so opens the door to more issues in the\n> > > future :-S\n> >\n> > Given that the IPA does require ordering and we have been lucky with\n> > Boost preserving order in the JSON parser, I think we probably ought\n> > to specify ordering in the config file with a specific key.\n>\n> Sounds good to me.\n>\n> It could be done through a priority key in each algorithm, or by\n> converting the mapping to a list. In YAML format, that would be moving\n> from\n>\n> {\n> \"rpi.black_level\":\n> {\n> \"black_level\": 4096\n> },\n> \"rpi.noise\":\n> {\n> \"reference_constant\": 0,\n> \"reference_slope\": 3.67\n> },\n> }\n>\n> to\n>\n> [\n> {\n> \"rpi.black_level\":\n> {\n> \"black_level\": 4096\n> },\n> },\n> {\n> \"rpi.noise\":\n> {\n> \"reference_constant\": 0,\n> \"reference_slope\": 3.67\n> },\n> }\n> ]\n>\n> In YAML format, it would translate as a move from\n>\n> rpi.black_level:\n> black_level: 4096\n> rpi.noise:\n> reference_constant: 0\n> reference_slope: 3.67\n>\n> to\n>\n> - rpi.black_level:\n> black_level: 4096\n> - rpi.noise:\n> reference_constant: 0\n> reference_slope: 3.67\n>\n> As we have to change the tuning files, I'd like to take this as an\n> opportunity to add a format version. Something along those lines maybe ?\n>\n> version: 1.0\n> algorithms:\n> - rpi.black_level:\n> black_level: 4096\n> - rpi.noise:\n> reference_constant: 0\n> reference_slope: 3.67\n>\n> And while we're discussing this, does someone know about best practice\n> rules to design JSON/YAML grammars ? I've been wondering for a long time\n> if the following grammar would have any advantage:\n>\n> version: 1.0\n> algorithms:\n> - name: rpi.black_level\n> data:\n> black_level: 4096\n> - name: rpi.noise:\n> data:\n> reference_constant: 0\n> reference_slope: 3.67\n>\n> > When I get a\n> > change, I'll look to add a patch to allow this on the existing codebase,\n> > and this series ought to \"just work\" after that.\n> >\n> > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > >\n> > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > [6] https://www.json.org/json-en.html\n> > >\n> > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > commit.\n> > > > >\n> > > > > Signed-off-by: Laurent Pinchart \n> > > > > ---\n> > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > >\n> > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > --- a/test/yaml-parser.cpp\n> > > > > +++ b/test/yaml-parser.cpp\n> > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > \" - Mary\\n\"\n> > > > > \"dictionary:\\n\"\n> > > > > \" a: 1\\n\"\n> > > > > - \" b: 2\\n\"\n> > > > > \" c: 3\\n\"\n> > > > > + \" b: 2\\n\"\n> > > > > \"level1:\\n\"\n> > > > > \" level2:\\n\"\n> > > > > \" - [1, 2]\\n\"\n> > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > }\n> > > > >\n> > > > > auto memeberNames = dictObj.memberNames();\n> > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > >\n> > > > > if (memeberNames.size() != 3) {\n> > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > }\n> > > > >\n> > > > > if (memeberNames[0] != \"a\" ||\n> > > > > - memeberNames[1] != \"b\" ||\n> > > > > - memeberNames[2] != \"c\") {\n> > > > > + memeberNames[1] != \"c\" ||\n> > > > > + memeberNames[2] != \"b\") {\n> > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > return TestFail;\n> > > > > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 87F3BBD161\n\tfor ;\n\tMon, 13 Jun 2022 09:25:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBF1865637;\n\tMon, 13 Jun 2022 11:25:28 +0200 (CEST)","from mail-ej1-x636.google.com (mail-ej1-x636.google.com\n\t[IPv6:2a00:1450:4864:20::636])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F0E26601F0\n\tfor ;\n\tMon, 13 Jun 2022 11:25:26 +0200 (CEST)","by mail-ej1-x636.google.com with SMTP id o7so9994513eja.1\n\tfor ;\n\tMon, 13 Jun 2022 02:25:26 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655112328;\n\tbh=2Ydi46YQH0X9RSlFwbxjSK+AejccBYQpYdtCVGWGY6E=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=aIwtE5EKvbC2hoi0DZ0lGhkUxf4b28+rjpyt/FR1ONpBdWBdq1BlN5JZYvGk2XOex\n\thOWLSmW1/HI1BEXQLrcCBXqEYz0+GyVZc7CH/nXU1mq+DRX0bHLw51pLZ5IWRksiIb\n\tyvCaICSiBvmuzfK6vDIAKu/dgamsDEMFyLjcdzu5glycI4YvjnNR88ys+13QL4cMuE\n\tBmW47T2scpIgChZ4hWiNxZ/6KRgAcYeFAuIjkJxwgLyA0dgGEiOHbawnE+36HNaaAK\n\ta/0B600nxhmreTYGlGVFxytUSW9e1I6od6he8xIc6r21qE+Jr2h1gbkk2msqjhzVVc\n\tPjJ79/+DIZFRg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=DGM7Qi4EzSrh3p8ySS//MzgBqlY6m/VuWuP7gByRf1Q=;\n\tb=B5JD8oXjj7OBujrn7Sr+2juUg5Gqac4WT2WfejYFkgeA15qjH70DeZ4R08d5AoFk01\n\t+RcSLAQ1VoB9o2S7DvTo7sgakKcyuuGxtOhKREimAFTdA/dAC08AMoB5x2pY2Fmr24ug\n\thR3TeogJbOiIUow1A5pGyuIyMVzaofA7gdH+m9zfqgi1gXYT0nxUh2GBm3G6i8vtIDz+\n\tpSjWivhUBsSLEWLD4DQNSSx/ukj22Y/89iH8NRrFiGeIqrCptQNwFvZDakPQASxa140M\n\tOKZIK/UzyGnQJXYgHREpzlgZrzX/jLEv/A7PeQl5PRY6QEi/UwEJ0V8/PDwxDGlDYF8F\n\tEN7A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"B5JD8oXj\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=DGM7Qi4EzSrh3p8ySS//MzgBqlY6m/VuWuP7gByRf1Q=;\n\tb=vCPTK3j3Zo34ZS35R7gQsihcpcDfldv0NbdQzKxpC7XVWWXFtXCDUcoJdkMdoYKeXO\n\tV3vCkurpVosGJv42UgqufSrYRnOYHdoo+BwBEpFRlJbVWI0/pRQFBY0ZtiLAE8wJK+Wx\n\tbqLvGXno1HmZdNvkgdog9T7L271VLTXJslEKHysw6eTenQuxOAwpqvHHs+SQ+Tsk9wAy\n\tYPYOKyVkedEQ2iOizKB/Po+cL1DwdIdsiiAvDJaKNzYd/k3tlOo9jN7F2lvrF4ByvLvY\n\txPDpulfidpVrK0gScZNTnjyLfXFVCVy6g0B0dtySOF2ckWo9Ltwl/Bzu57DrA6P7m32x\n\tBlXw==","X-Gm-Message-State":"AOAM533/I3bL1K+g6S9fs+SunQsgFJMaOF9fSVku62XMmlhqtJnOfY44\n\tuAOKTFtI2/B33CiJE/SCdKkGDD+OmHY7LR/84G7mlWhz5eIWbg==","X-Google-Smtp-Source":"ABdhPJyItIPwpNUJMpKr5TRZFZ7f0kvfoIORkubETxV7DVvU1flORUefFVfN75yJHMviioZF1IAAZLvJm6JbhsmlxpM=","X-Received":"by 2002:a17:907:1c01:b0:6f4:2692:e23 with SMTP id\n\tnc1-20020a1709071c0100b006f426920e23mr51443046ejc.243.1655112326547;\n\tMon, 13 Jun 2022 02:25:26 -0700 (PDT)","MIME-Version":"1.0","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t","In-Reply-To":"","Date":"Mon, 13 Jun 2022 10:25:15 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23399,"web_url":"https://patchwork.libcamera.org/comment/23399/","msgid":"","date":"2022-06-13T09:48:55","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> Hi Laurent, everyone\n> \n> Thanks for the suggestions. I'm fine to change the syntax to make\n> things clearer, but I wonder if we could avoid breaking existing JSON\n> files? There are probably low numbers of them out there beyond the\n> ones that we've supplied, but you never quite know and backwards\n> compatibility is still a nice thing. Do you think that's something we\n> can arrange?\n\nThis patch series guarantees ordering of entries in mapping (at the\nexpense of duplicated key storage in memory, but that could probably be\nfixed), so there should be no breakage. The main drawback is that\nYamlParser and YamlObject will then expose an order that is\nimplementation-specific and shouldn't be relied on by applications\naccording to the JSON and YAML specifications. There's thus a risk of\nintroducing new code that relies on mappings being ordered, which\nwouldn't be the case if the implementation didn't guarantee ordering\n(although one may argue that in the case the users of YamlObject may\nstill rely on a different implementation-specific order without\nrealizing it, we would need to randomize the order to avoid that, which\nI don't think is a good idea).\n\nWith versioned tuning files the IPA could fairly easily support both the\ncurrent format and the new format, so that shouldn't be a problem. I\nthink we should then provide a Python script to convert the old format\nto the new one, and print a warning to the log. Would you expect the\nRaspberry Pi IPA module to support the current format forever, or only\nfor a fixed duration to help users transition ?\n\n> On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > >\n> > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > >\n> > > > That's a very good point. [5] even mentions\n> > > >\n> > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > representations to a sequential access medium, this serialization detail\n> > > > must not be used to convey application level information.\"\n> > > >\n> > > > The same applies to JSON ([6]).\n> > > >\n> > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > future :-S\n> > >\n> > > Given that the IPA does require ordering and we have been lucky with\n> > > Boost preserving order in the JSON parser, I think we probably ought\n> > > to specify ordering in the config file with a specific key.\n> >\n> > Sounds good to me.\n> >\n> > It could be done through a priority key in each algorithm, or by\n> > converting the mapping to a list. In YAML format, that would be moving\n> > from\n> >\n> > {\n> > \"rpi.black_level\":\n> > {\n> > \"black_level\": 4096\n> > },\n> > \"rpi.noise\":\n> > {\n> > \"reference_constant\": 0,\n> > \"reference_slope\": 3.67\n> > },\n> > }\n> >\n> > to\n> >\n> > [\n> > {\n> > \"rpi.black_level\":\n> > {\n> > \"black_level\": 4096\n> > },\n> > },\n> > {\n> > \"rpi.noise\":\n> > {\n> > \"reference_constant\": 0,\n> > \"reference_slope\": 3.67\n> > },\n> > }\n> > ]\n> >\n> > In YAML format, it would translate as a move from\n> >\n> > rpi.black_level:\n> > black_level: 4096\n> > rpi.noise:\n> > reference_constant: 0\n> > reference_slope: 3.67\n> >\n> > to\n> >\n> > - rpi.black_level:\n> > black_level: 4096\n> > - rpi.noise:\n> > reference_constant: 0\n> > reference_slope: 3.67\n> >\n> > As we have to change the tuning files, I'd like to take this as an\n> > opportunity to add a format version. Something along those lines maybe ?\n> >\n> > version: 1.0\n> > algorithms:\n> > - rpi.black_level:\n> > black_level: 4096\n> > - rpi.noise:\n> > reference_constant: 0\n> > reference_slope: 3.67\n> >\n> > And while we're discussing this, does someone know about best practice\n> > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > if the following grammar would have any advantage:\n> >\n> > version: 1.0\n> > algorithms:\n> > - name: rpi.black_level\n> > data:\n> > black_level: 4096\n> > - name: rpi.noise:\n> > data:\n> > reference_constant: 0\n> > reference_slope: 3.67\n> >\n> > > When I get a\n> > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > and this series ought to \"just work\" after that.\n> > >\n> > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > >\n> > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > [6] https://www.json.org/json-en.html\n> > > >\n> > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > commit.\n> > > > > >\n> > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > ---\n> > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > >\n> > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > --- a/test/yaml-parser.cpp\n> > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > \" - Mary\\n\"\n> > > > > > \"dictionary:\\n\"\n> > > > > > \" a: 1\\n\"\n> > > > > > - \" b: 2\\n\"\n> > > > > > \" c: 3\\n\"\n> > > > > > + \" b: 2\\n\"\n> > > > > > \"level1:\\n\"\n> > > > > > \" level2:\\n\"\n> > > > > > \" - [1, 2]\\n\"\n> > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > }\n> > > > > >\n> > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > >\n> > > > > > if (memeberNames.size() != 3) {\n> > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > }\n> > > > > >\n> > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > - memeberNames[2] != \"c\") {\n> > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > + memeberNames[2] != \"b\") {\n> > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > return TestFail;\n> > > > > > }","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 2667EBD808\n\tfor ;\n\tMon, 13 Jun 2022 09:49:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AA4465635;\n\tMon, 13 Jun 2022 11:49:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D67B601F0\n\tfor ;\n\tMon, 13 Jun 2022 11:49:04 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D1E2E305;\n\tMon, 13 Jun 2022 11:49:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655113746;\n\tbh=sXhtdtjGEMuzmaRBk57WQwB7vcsjoHmoxHPqB2fUpP0=;\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:Cc:\n\tFrom;\n\tb=LU4F50NXeW0JsKdLCgTo4r301iWzpi02u48t0eJr5RzQp+e3uTNEv3wyl6CTouTAG\n\tlPqXzGIZpvqveQZs676AbzfncbIxPtfcZPgRKpjY/1bx+H3gU4WfaBlmlI0iJ4fbC5\n\tkahwP2KaUuVjVxxafFTxpb+P5g2sfdqhAfJ0/OYi54lJRAThBK1zJSp20+E57/HUFT\n\tNpsbo1n8NQ1KHbqUWNaoFtCvu6Xn37vrTcA1Phzdh+cVr2cdUt6QYgmGsEhlA7i4Gn\n\tcUkTw/ThHGN3UlDvGG2gC7RKWKz2iEbklzhbaLfUMYfm3UferaFj6d6kQd9Al1d5oc\n\tzSw1D4rXoIQgQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655113744;\n\tbh=sXhtdtjGEMuzmaRBk57WQwB7vcsjoHmoxHPqB2fUpP0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZuomWRQ+cgFE2ZSRi8pvgcEXV2RAiN8HLC4qoRNDLPAMyHSZ2FvxYV90M1WQfs2Xv\n\t8WYP/+x+lDHzpfB4CD9KTUPe6kVFrUb3JqHkxZ89ELFz3hvIjEuZoNQazwiGrU8U75\n\tOIitXb/n/4Za47CGwOuWWziqiawZMdPnapijzctw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZuomWRQ+\"; dkim-atps=neutral","Date":"Mon, 13 Jun 2022 12:48:55 +0300","To":"David Plowman ","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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 ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23414,"web_url":"https://patchwork.libcamera.org/comment/23414/","msgid":"","date":"2022-06-16T08:10:07","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Mon, Jun 13, 2022 at 12:48:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> On Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> > Hi Laurent, everyone\n> > \n> > Thanks for the suggestions. I'm fine to change the syntax to make\n> > things clearer, but I wonder if we could avoid breaking existing JSON\n> > files? There are probably low numbers of them out there beyond the\n> > ones that we've supplied, but you never quite know and backwards\n> > compatibility is still a nice thing. Do you think that's something we\n> > can arrange?\n> \n> This patch series guarantees ordering of entries in mapping (at the\n> expense of duplicated key storage in memory, but that could probably be\n> fixed), so there should be no breakage. The main drawback is that\n> YamlParser and YamlObject will then expose an order that is\n> implementation-specific and shouldn't be relied on by applications\n> according to the JSON and YAML specifications. There's thus a risk of\n> introducing new code that relies on mappings being ordered, which\n> wouldn't be the case if the implementation didn't guarantee ordering\n> (although one may argue that in the case the users of YamlObject may\n> still rely on a different implementation-specific order without\n> realizing it, we would need to randomize the order to avoid that, which\n> I don't think is a good idea).\n> \n> With versioned tuning files the IPA could fairly easily support both the\n> current format and the new format, so that shouldn't be a problem. I\n> think we should then provide a Python script to convert the old format\n> to the new one, and print a warning to the log. Would you expect the\n> Raspberry Pi IPA module to support the current format forever, or only\n> for a fixed duration to help users transition ?\n\nI'll post a v3 series of the YamlObject changes, and I'd like to decide\non which direction to take (if possible :-)). Could you share your\nthoughts on this ?\n\n> > On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > > >\n> > > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > > >\n> > > > > That's a very good point. [5] even mentions\n> > > > >\n> > > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > > representations to a sequential access medium, this serialization detail\n> > > > > must not be used to convey application level information.\"\n> > > > >\n> > > > > The same applies to JSON ([6]).\n> > > > >\n> > > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > > future :-S\n> > > >\n> > > > Given that the IPA does require ordering and we have been lucky with\n> > > > Boost preserving order in the JSON parser, I think we probably ought\n> > > > to specify ordering in the config file with a specific key.\n> > >\n> > > Sounds good to me.\n> > >\n> > > It could be done through a priority key in each algorithm, or by\n> > > converting the mapping to a list. In YAML format, that would be moving\n> > > from\n> > >\n> > > {\n> > > \"rpi.black_level\":\n> > > {\n> > > \"black_level\": 4096\n> > > },\n> > > \"rpi.noise\":\n> > > {\n> > > \"reference_constant\": 0,\n> > > \"reference_slope\": 3.67\n> > > },\n> > > }\n> > >\n> > > to\n> > >\n> > > [\n> > > {\n> > > \"rpi.black_level\":\n> > > {\n> > > \"black_level\": 4096\n> > > },\n> > > },\n> > > {\n> > > \"rpi.noise\":\n> > > {\n> > > \"reference_constant\": 0,\n> > > \"reference_slope\": 3.67\n> > > },\n> > > }\n> > > ]\n> > >\n> > > In YAML format, it would translate as a move from\n> > >\n> > > rpi.black_level:\n> > > black_level: 4096\n> > > rpi.noise:\n> > > reference_constant: 0\n> > > reference_slope: 3.67\n> > >\n> > > to\n> > >\n> > > - rpi.black_level:\n> > > black_level: 4096\n> > > - rpi.noise:\n> > > reference_constant: 0\n> > > reference_slope: 3.67\n> > >\n> > > As we have to change the tuning files, I'd like to take this as an\n> > > opportunity to add a format version. Something along those lines maybe ?\n> > >\n> > > version: 1.0\n> > > algorithms:\n> > > - rpi.black_level:\n> > > black_level: 4096\n> > > - rpi.noise:\n> > > reference_constant: 0\n> > > reference_slope: 3.67\n> > >\n> > > And while we're discussing this, does someone know about best practice\n> > > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > > if the following grammar would have any advantage:\n> > >\n> > > version: 1.0\n> > > algorithms:\n> > > - name: rpi.black_level\n> > > data:\n> > > black_level: 4096\n> > > - name: rpi.noise:\n> > > data:\n> > > reference_constant: 0\n> > > reference_slope: 3.67\n> > >\n> > > > When I get a\n> > > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > > and this series ought to \"just work\" after that.\n> > > >\n> > > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > > >\n> > > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > > [6] https://www.json.org/json-en.html\n> > > > >\n> > > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > > commit.\n> > > > > > >\n> > > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > > ---\n> > > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > > >\n> > > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > > --- a/test/yaml-parser.cpp\n> > > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > > \" - Mary\\n\"\n> > > > > > > \"dictionary:\\n\"\n> > > > > > > \" a: 1\\n\"\n> > > > > > > - \" b: 2\\n\"\n> > > > > > > \" c: 3\\n\"\n> > > > > > > + \" b: 2\\n\"\n> > > > > > > \"level1:\\n\"\n> > > > > > > \" level2:\\n\"\n> > > > > > > \" - [1, 2]\\n\"\n> > > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > > }\n> > > > > > >\n> > > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > > >\n> > > > > > > if (memeberNames.size() != 3) {\n> > > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > > }\n> > > > > > >\n> > > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > > - memeberNames[2] != \"c\") {\n> > > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > > + memeberNames[2] != \"b\") {\n> > > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > > return TestFail;\n> > > > > > > }","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 B971BBD808\n\tfor ;\n\tThu, 16 Jun 2022 08:10:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0713A65635;\n\tThu, 16 Jun 2022 10:10:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C4AEF601F1\n\tfor ;\n\tThu, 16 Jun 2022 10:10:18 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3600130B;\n\tThu, 16 Jun 2022 10:10:18 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655367020;\n\tbh=7LmtRwrZrv0/rbiA8c5sUTx2ovkMyJW2KogsLKQ+0FE=;\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=Z45ml8cRuPVBDGnIKB2EDz9SpO5Q45k85oDvalLyBg8d+gufvJB94N/jw9NU9ZY6v\n\t6+uO8+APKQQ73ojFnkFBL3pqYSUnn3KcRiveiFioUdm/8Ail7/QbWf1DHsuWXH8fI+\n\tkUu13wh1I/NihMeGzezvsvjwYVjowFM+3x3RvK5sB4rKIQNlSgFK1bkKuxF1eEGnsc\n\ttwkbnbbEgA+kOC4G6hoIjDFBC/tz2T5JC1KKgnzN5eWDzQlKPtU9mnAIQ6kqybCEEY\n\tVjN2zjmlfU5A0Z6XDAJ2WBSOC/LbyCG047ngM6S9DniQ3hA/O3moUjzA91rTRuXAGY\n\tahJ3OlyXarTtg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655367018;\n\tbh=7LmtRwrZrv0/rbiA8c5sUTx2ovkMyJW2KogsLKQ+0FE=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=TTz8XlTIAQbY1YhoiS3PArWrXfz1LjZU+57q0NP1Jop5HkePMc9ePNtP9ifr5K9b9\n\tf0Hs95S0JzJ6+qkV/8uijcoyBUyLM14qMTqavH+sdBM4JGjAxGHcVn0kTUJSDm7haA\n\tQzlJBUjcVrou/FjvFz8qwKtxbBeb8D9Doa6Jaj4M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TTz8XlTI\"; dkim-atps=neutral","Date":"Thu, 16 Jun 2022 11:10:07 +0300","To":"David Plowman ,\n\tlibcamera devel ","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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\" "}},{"id":23416,"web_url":"https://patchwork.libcamera.org/comment/23416/","msgid":"","date":"2022-06-16T08:32:21","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nSorry for the delay!\n\nOn Thu, 16 Jun 2022 at 09:10, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> On Mon, Jun 13, 2022 at 12:48:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > On Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> > > Hi Laurent, everyone\n> > >\n> > > Thanks for the suggestions. I'm fine to change the syntax to make\n> > > things clearer, but I wonder if we could avoid breaking existing JSON\n> > > files? There are probably low numbers of them out there beyond the\n> > > ones that we've supplied, but you never quite know and backwards\n> > > compatibility is still a nice thing. Do you think that's something we\n> > > can arrange?\n> >\n> > This patch series guarantees ordering of entries in mapping (at the\n> > expense of duplicated key storage in memory, but that could probably be\n> > fixed), so there should be no breakage. The main drawback is that\n> > YamlParser and YamlObject will then expose an order that is\n> > implementation-specific and shouldn't be relied on by applications\n> > according to the JSON and YAML specifications. There's thus a risk of\n> > introducing new code that relies on mappings being ordered, which\n> > wouldn't be the case if the implementation didn't guarantee ordering\n> > (although one may argue that in the case the users of YamlObject may\n> > still rely on a different implementation-specific order without\n> > realizing it, we would need to randomize the order to avoid that, which\n> > I don't think is a good idea).\n> >\n> > With versioned tuning files the IPA could fairly easily support both the\n> > current format and the new format, so that shouldn't be a problem. I\n> > think we should then provide a Python script to convert the old format\n> > to the new one, and print a warning to the log. Would you expect the\n> > Raspberry Pi IPA module to support the current format forever, or only\n> > for a fixed duration to help users transition ?\n>\n> I'll post a v3 series of the YamlObject changes, and I'd like to decide\n> on which direction to take (if possible :-)). Could you share your\n> thoughts on this ?\n\nI wasn't totally sure what the precise question is, but here are a few\nrandom answers:\n\n- I'd like existing turning files to continue working, could they\nimplicitly be regarded as \"version 0\" or something if they don't say\notherwise? It also means not having to touch the tuning tool just yet.\n\n- I don't like having different priorities stored with different\nalgorithms. You'll forever be hunting through the file reverse\nengineering the actual order. I'd rather have an \"order\" field (or\nsomething) that simply lists the correct order.\n\n- The \"order\" field would default to the \"standard order\" if not\npresent. The order can list algorithms that aren't present and that\nwould just be ignored. It would be common simply not to have all\nalgorithms, or to comment some out for debugging.\n\nI think there is some complexity in the order matching. For example,\nI'd want \"rpi.awb\" to match just \"awb\". But if \"rpi.awb\" is listed in\nthe order, that would take precedence.\n\nThe idea is that we can list the standard order like \"black_level\",\n\"dpc\", \"lux\", \"noise\", \"geq\", \"sdn\", \"awb\", and so on. But if someone\nwrites an algorithm \"foo.awb\" they can force it to go somewhere else\nif they have too, but by default it would take up the standard \"awb\"\nposition.\n\nHmm, that's starting to get a bit annoying, isn't it? Storing\npriorities in the algorithm would be less of a problem in this\nrespect, but I really don't like it...\n\n- Don't want to lose json, but happy to have both with the appropriate\nfile extension.\n\nDid I answer everything or was there anything I overlooked?\n\nThanks!\nDavid\n\n>\n> > > On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > > > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > > > >\n> > > > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > > > >\n> > > > > > That's a very good point. [5] even mentions\n> > > > > >\n> > > > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > > > representations to a sequential access medium, this serialization detail\n> > > > > > must not be used to convey application level information.\"\n> > > > > >\n> > > > > > The same applies to JSON ([6]).\n> > > > > >\n> > > > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > > > future :-S\n> > > > >\n> > > > > Given that the IPA does require ordering and we have been lucky with\n> > > > > Boost preserving order in the JSON parser, I think we probably ought\n> > > > > to specify ordering in the config file with a specific key.\n> > > >\n> > > > Sounds good to me.\n> > > >\n> > > > It could be done through a priority key in each algorithm, or by\n> > > > converting the mapping to a list. In YAML format, that would be moving\n> > > > from\n> > > >\n> > > > {\n> > > > \"rpi.black_level\":\n> > > > {\n> > > > \"black_level\": 4096\n> > > > },\n> > > > \"rpi.noise\":\n> > > > {\n> > > > \"reference_constant\": 0,\n> > > > \"reference_slope\": 3.67\n> > > > },\n> > > > }\n> > > >\n> > > > to\n> > > >\n> > > > [\n> > > > {\n> > > > \"rpi.black_level\":\n> > > > {\n> > > > \"black_level\": 4096\n> > > > },\n> > > > },\n> > > > {\n> > > > \"rpi.noise\":\n> > > > {\n> > > > \"reference_constant\": 0,\n> > > > \"reference_slope\": 3.67\n> > > > },\n> > > > }\n> > > > ]\n> > > >\n> > > > In YAML format, it would translate as a move from\n> > > >\n> > > > rpi.black_level:\n> > > > black_level: 4096\n> > > > rpi.noise:\n> > > > reference_constant: 0\n> > > > reference_slope: 3.67\n> > > >\n> > > > to\n> > > >\n> > > > - rpi.black_level:\n> > > > black_level: 4096\n> > > > - rpi.noise:\n> > > > reference_constant: 0\n> > > > reference_slope: 3.67\n> > > >\n> > > > As we have to change the tuning files, I'd like to take this as an\n> > > > opportunity to add a format version. Something along those lines maybe ?\n> > > >\n> > > > version: 1.0\n> > > > algorithms:\n> > > > - rpi.black_level:\n> > > > black_level: 4096\n> > > > - rpi.noise:\n> > > > reference_constant: 0\n> > > > reference_slope: 3.67\n> > > >\n> > > > And while we're discussing this, does someone know about best practice\n> > > > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > > > if the following grammar would have any advantage:\n> > > >\n> > > > version: 1.0\n> > > > algorithms:\n> > > > - name: rpi.black_level\n> > > > data:\n> > > > black_level: 4096\n> > > > - name: rpi.noise:\n> > > > data:\n> > > > reference_constant: 0\n> > > > reference_slope: 3.67\n> > > >\n> > > > > When I get a\n> > > > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > > > and this series ought to \"just work\" after that.\n> > > > >\n> > > > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > > > >\n> > > > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > > > [6] https://www.json.org/json-en.html\n> > > > > >\n> > > > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > > > commit.\n> > > > > > > >\n> > > > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > > > ---\n> > > > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > > > >\n> > > > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > > > --- a/test/yaml-parser.cpp\n> > > > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > > > \" - Mary\\n\"\n> > > > > > > > \"dictionary:\\n\"\n> > > > > > > > \" a: 1\\n\"\n> > > > > > > > - \" b: 2\\n\"\n> > > > > > > > \" c: 3\\n\"\n> > > > > > > > + \" b: 2\\n\"\n> > > > > > > > \"level1:\\n\"\n> > > > > > > > \" level2:\\n\"\n> > > > > > > > \" - [1, 2]\\n\"\n> > > > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > > > >\n> > > > > > > > if (memeberNames.size() != 3) {\n> > > > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > > > }\n> > > > > > > >\n> > > > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > > > - memeberNames[2] != \"c\") {\n> > > > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > > > + memeberNames[2] != \"b\") {\n> > > > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > > > return TestFail;\n> > > > > > > > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 011D3BD161\n\tfor ;\n\tThu, 16 Jun 2022 08:32:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4DB8765631;\n\tThu, 16 Jun 2022 10:32:35 +0200 (CEST)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C91EC601F1\n\tfor ;\n\tThu, 16 Jun 2022 10:32:32 +0200 (CEST)","by mail-ed1-x536.google.com with SMTP id x5so1173301edi.2\n\tfor ;\n\tThu, 16 Jun 2022 01:32:32 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655368355;\n\tbh=FJxO5lPgOs4nHLKKcFBJJxloFHl6uh3ImRTwKGwAkp4=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Bwxw1bNgrDQN04x+/rVe4d8+DAETin5kOZ/NPuA3fLXHYsceFZrLQ0N5XqziSlREA\n\t3lK3HDmS5GaIFnVzZMEYA4EQ5xbboslWEEsQ0f+zb3UqSuDVYH70md9dMNghrhWiJz\n\tyQoMF+RllcFU13uk9hyMZuYvCfQxA4qF2IxJGlxNkZisiGiDBqx6zHLK5mTWwppKI+\n\tLMlH1k9qJYAIfSr347KGNk0R5s8rEtylFEu5XhQFQAp3kall69uy0IKNMbED6Oy97J\n\tViiJXP+sPDsx+MdleNmtEsscA6abgto5FUeVcjlVkYWIN0pmKF0v+MYn1SW6nX5Rpf\n\toDVQS+6Bh1C1Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=fVCxTWZ+8vb93x0T8tOogxFz/fqU9Hja5ipFIsEii48=;\n\tb=oCymgOduvZ/wRq61KGwpYVRtmWIvatSTAUgoq4w6pfglwnhKOUXmC9eI9AEvq/LDrP\n\tEiwx906SSd4UUfj/zNeoylAg7W1htBl6FDVhaIiN+reGu9KFtD9sjeeUKe7/pSkpfXqh\n\tK38rBo0GLDSGIvTlK7QGUQ9YOzwdmHKIolL4Hl82LOreaTdctwWXLqD5b5qH960vzaMP\n\t4xQ0N6QvG7nlseEq9NkFr6q8eHLe5Xfma5uEVx/eAvbMuyTbLvN9XQKGiNji6k6PFf71\n\tSw6JF6t4mzBzxN7iFx/YnFdr4u89FDPpRox0+Fdvv4qPZGk9rP9xWmY3WOjiE1Zz1DAR\n\tbBhA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"oCymgOdu\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=fVCxTWZ+8vb93x0T8tOogxFz/fqU9Hja5ipFIsEii48=;\n\tb=3VQqPbC7fNI72P/878NTJLY34fxeN1oUcUqmPAFOnnSUIuZBK4EAbCzhGFzywsndzu\n\tNlBwlbHLlrjHeMU213bZWKIyo0+YLYZEZi1voFOzZHOO7s7BpnXBysY/nt1NE45VT7OR\n\tPIZVj/02pTfbP2lQYNZaQak5x3zSVFA6FATXr01EPngFOVWyXyAvGekz7kywKAjcu4qW\n\tr1lEH9PZbR85DANisVRbn/Xmo/QRZo0TBMHMucFFRSvCFC2dkSi1jzqFpC3jUYpxHD9s\n\tcIFQtdJ3QZ9av0Dhne7esEX68CVHDq1Kd6HlzbyheTT2ZU67E6CNhnr+zLyZMA8J3/Ol\n\tuDcw==","X-Gm-Message-State":"AJIora9OCHbPMw3ERT2jXQBjP359VeYUvBCndQ63PCAjpOg3AQEWtY/B\n\tVO14oGdpQmKoMBRYuJ6bBmmLHe1Y5XZHwSlHIcMu5L36qWNcnw==","X-Google-Smtp-Source":"AGRyM1uf84C11yhObtRIx7L34ijeimbDnOojUsVUT+B3X2wYvXok4vqqGWyVmlaTd1KeWE7UVayDbq9oTykn+ib8t3Y=","X-Received":"by 2002:a05:6402:3482:b0:42d:e063:7c1d with SMTP id\n\tv2-20020a056402348200b0042de0637c1dmr5035755edc.40.1655368352404;\n\tThu, 16 Jun 2022 01:32:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Thu, 16 Jun 2022 09:32:21 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23417,"web_url":"https://patchwork.libcamera.org/comment/23417/","msgid":"","date":"2022-06-16T08:48:24","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Thu, Jun 16, 2022 at 09:32:21AM +0100, David Plowman wrote:\n> Hi Laurent\n> \n> Sorry for the delay!\n\nThanks for the quick reply :-)\n\n> On Thu, 16 Jun 2022 at 09:10, Laurent Pinchart wrote:\n> > On Mon, Jun 13, 2022 at 12:48:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > On Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> > > > Hi Laurent, everyone\n> > > >\n> > > > Thanks for the suggestions. I'm fine to change the syntax to make\n> > > > things clearer, but I wonder if we could avoid breaking existing JSON\n> > > > files? There are probably low numbers of them out there beyond the\n> > > > ones that we've supplied, but you never quite know and backwards\n> > > > compatibility is still a nice thing. Do you think that's something we\n> > > > can arrange?\n> > >\n> > > This patch series guarantees ordering of entries in mapping (at the\n> > > expense of duplicated key storage in memory, but that could probably be\n> > > fixed), so there should be no breakage. The main drawback is that\n> > > YamlParser and YamlObject will then expose an order that is\n> > > implementation-specific and shouldn't be relied on by applications\n> > > according to the JSON and YAML specifications. There's thus a risk of\n> > > introducing new code that relies on mappings being ordered, which\n> > > wouldn't be the case if the implementation didn't guarantee ordering\n> > > (although one may argue that in the case the users of YamlObject may\n> > > still rely on a different implementation-specific order without\n> > > realizing it, we would need to randomize the order to avoid that, which\n> > > I don't think is a good idea).\n> > >\n> > > With versioned tuning files the IPA could fairly easily support both the\n> > > current format and the new format, so that shouldn't be a problem. I\n> > > think we should then provide a Python script to convert the old format\n> > > to the new one, and print a warning to the log. Would you expect the\n> > > Raspberry Pi IPA module to support the current format forever, or only\n> > > for a fixed duration to help users transition ?\n> >\n> > I'll post a v3 series of the YamlObject changes, and I'd like to decide\n> > on which direction to take (if possible :-)). Could you share your\n> > thoughts on this ?\n> \n> I wasn't totally sure what the precise question is, but here are a few\n> random answers:\n> \n> - I'd like existing turning files to continue working, could they\n> implicitly be regarded as \"version 0\" or something if they don't say\n> otherwise? It also means not having to touch the tuning tool just yet.\n\nWe can, the v2 I've posted can handle this. The only drawback is that it\nrisks introducing another dependency on YAML mapping ordering, but I\nsuppose that's worth it to avoid breaking all existing tuning files.\n\nWould you consider dropping support for v0 at some point ? If so, any\nidea when ?\n\n> - I don't like having different priorities stored with different\n> algorithms. You'll forever be hunting through the file reverse\n> engineering the actual order. I'd rather have an \"order\" field (or\n> something) that simply lists the correct order.\n> \n> - The \"order\" field would default to the \"standard order\" if not\n> present. The order can list algorithms that aren't present and that\n> would just be ignored. It would be common simply not to have all\n> algorithms, or to comment some out for debugging.\n> \n> I think there is some complexity in the order matching. For example,\n> I'd want \"rpi.awb\" to match just \"awb\". But if \"rpi.awb\" is listed in\n> the order, that would take precedence.\n> \n> The idea is that we can list the standard order like \"black_level\",\n> \"dpc\", \"lux\", \"noise\", \"geq\", \"sdn\", \"awb\", and so on. But if someone\n> writes an algorithm \"foo.awb\" they can force it to go somewhere else\n> if they have too, but by default it would take up the standard \"awb\"\n> position.\n> \n> Hmm, that's starting to get a bit annoying, isn't it? Storing\n> priorities in the algorithm would be less of a problem in this\n> respect, but I really don't like it...\n\nIt's getting complex indeed. I have no issue keeping the existing\nmechanism working for as long as we support v0. In v1 it would work with\nthe modification to the JSON file explained below. The only cost will be\none more level in the hierachy. I've also shown how it looks like in\nYAML as it's less verbose and thus easier to discuss by e-mail, but that\ndoesn't mean I want to convert existing files to YAML.\n\n> - Don't want to lose json, but happy to have both with the appropriate\n> file extension.\n\nThe YamlParser supports JSON just fine :-) The only caveat is that\nspaces have to be used for indentation, not tabs.\n\n> Did I answer everything or was there anything I overlooked?\n\nThere's the question of how the v1 format should look like. I'd propose\nthe following.\n\n{\n \"version\": 1\n \"algorithms\": [\n {\n \"rpi.black_level\":\n {\n \"black_level\": 4096\n },\n },\n {\n \"rpi.noise\":\n {\n \"reference_constant\": 0,\n \"reference_slope\": 3.67\n },\n }\n ]\n}\n\nAs the contents of the \"algorithms\" key is a list, it is ordered, so we\nkeep the current feature of algorithm ordering without abusing a mapping\norder.\n\nAnother option would be\n\n{\n \"version\": 1\n \"algorithms\": [\n {\n \"name\": \"rpi.black_level\",\n \"data\": {\n \"black_level\": 4096\n },\n },\n {\n \"name\": \"rpi.noise\",\n \"data\": {\n \"reference_constant\": 0,\n \"reference_slope\": 3.67\n },\n }\n ]\n}\n\nI don't know what's better, if it's a good practice to make keys well\nknown (as in the second option, where an algorithm entry has two keys\nthat are known at compile time, \"name\" and \"data\") or if it's fine (or\neven better) to make keys more dynamic (which then requires iterating\nover the parsed object, as we don't know the key name in advance).\n\n> > > > On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > > > > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > > > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > > > > >\n> > > > > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > > > > >\n> > > > > > > That's a very good point. [5] even mentions\n> > > > > > >\n> > > > > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > > > > representations to a sequential access medium, this serialization detail\n> > > > > > > must not be used to convey application level information.\"\n> > > > > > >\n> > > > > > > The same applies to JSON ([6]).\n> > > > > > >\n> > > > > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > > > > future :-S\n> > > > > >\n> > > > > > Given that the IPA does require ordering and we have been lucky with\n> > > > > > Boost preserving order in the JSON parser, I think we probably ought\n> > > > > > to specify ordering in the config file with a specific key.\n> > > > >\n> > > > > Sounds good to me.\n> > > > >\n> > > > > It could be done through a priority key in each algorithm, or by\n> > > > > converting the mapping to a list. In YAML format, that would be moving\n> > > > > from\n> > > > >\n> > > > > {\n> > > > > \"rpi.black_level\":\n> > > > > {\n> > > > > \"black_level\": 4096\n> > > > > },\n> > > > > \"rpi.noise\":\n> > > > > {\n> > > > > \"reference_constant\": 0,\n> > > > > \"reference_slope\": 3.67\n> > > > > },\n> > > > > }\n> > > > >\n> > > > > to\n> > > > >\n> > > > > [\n> > > > > {\n> > > > > \"rpi.black_level\":\n> > > > > {\n> > > > > \"black_level\": 4096\n> > > > > },\n> > > > > },\n> > > > > {\n> > > > > \"rpi.noise\":\n> > > > > {\n> > > > > \"reference_constant\": 0,\n> > > > > \"reference_slope\": 3.67\n> > > > > },\n> > > > > }\n> > > > > ]\n> > > > >\n> > > > > In YAML format, it would translate as a move from\n> > > > >\n> > > > > rpi.black_level:\n> > > > > black_level: 4096\n> > > > > rpi.noise:\n> > > > > reference_constant: 0\n> > > > > reference_slope: 3.67\n> > > > >\n> > > > > to\n> > > > >\n> > > > > - rpi.black_level:\n> > > > > black_level: 4096\n> > > > > - rpi.noise:\n> > > > > reference_constant: 0\n> > > > > reference_slope: 3.67\n> > > > >\n> > > > > As we have to change the tuning files, I'd like to take this as an\n> > > > > opportunity to add a format version. Something along those lines maybe ?\n> > > > >\n> > > > > version: 1.0\n> > > > > algorithms:\n> > > > > - rpi.black_level:\n> > > > > black_level: 4096\n> > > > > - rpi.noise:\n> > > > > reference_constant: 0\n> > > > > reference_slope: 3.67\n> > > > >\n> > > > > And while we're discussing this, does someone know about best practice\n> > > > > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > > > > if the following grammar would have any advantage:\n> > > > >\n> > > > > version: 1.0\n> > > > > algorithms:\n> > > > > - name: rpi.black_level\n> > > > > data:\n> > > > > black_level: 4096\n> > > > > - name: rpi.noise:\n> > > > > data:\n> > > > > reference_constant: 0\n> > > > > reference_slope: 3.67\n> > > > >\n> > > > > > When I get a\n> > > > > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > > > > and this series ought to \"just work\" after that.\n> > > > > >\n> > > > > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > > > > >\n> > > > > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > > > > [6] https://www.json.org/json-en.html\n> > > > > > >\n> > > > > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > > > > commit.\n> > > > > > > > >\n> > > > > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > > > > ---\n> > > > > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > > > > >\n> > > > > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > > > > --- a/test/yaml-parser.cpp\n> > > > > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > > > > \" - Mary\\n\"\n> > > > > > > > > \"dictionary:\\n\"\n> > > > > > > > > \" a: 1\\n\"\n> > > > > > > > > - \" b: 2\\n\"\n> > > > > > > > > \" c: 3\\n\"\n> > > > > > > > > + \" b: 2\\n\"\n> > > > > > > > > \"level1:\\n\"\n> > > > > > > > > \" level2:\\n\"\n> > > > > > > > > \" - [1, 2]\\n\"\n> > > > > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > > > > >\n> > > > > > > > > if (memeberNames.size() != 3) {\n> > > > > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > > > > }\n> > > > > > > > >\n> > > > > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > > > > - memeberNames[2] != \"c\") {\n> > > > > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > > > > + memeberNames[2] != \"b\") {\n> > > > > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > > > > return TestFail;\n> > > > > > > > > }","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 54F0CBD808\n\tfor ;\n\tThu, 16 Jun 2022 08:48:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3E5C65635;\n\tThu, 16 Jun 2022 10:48:37 +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 7E83E601F1\n\tfor ;\n\tThu, 16 Jun 2022 10:48:35 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C268415;\n\tThu, 16 Jun 2022 10:48:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655369317;\n\tbh=L5u3S7987shHma+8kgCSwUYFbnIN/bnBaoTGoBYoo5s=;\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:Cc:\n\tFrom;\n\tb=1Hi5WHR3EZFwcc1+zBeCgYudfc8z5KAJ8B/5FOqgx7YKMlNVw7Gzildi3nNU6C/jO\n\tUxxWHAzZtbKQnhXVGhnsHZuKP7P6aqRwtQ/7Ye6Ou7sdtCQzDwDqG0PPN1I08PuqPI\n\tRiHxzxberrHQt3QR31xUT04ZfitpPZybx7I5IVAcT8kHD9PWBtBsfyJM5zEm3KU/Oe\n\tJKlzbWh3ZZb+KXRBtUXDerUs5kxycLasWz76pGmRl5DmfNALhIty86AyIfhZRIPOPK\n\tiKSpWA2bfaQFjL0k9Tf3ZcZ24oKd/ZvyTs/PwfR9mDHwhX5L02hshQUY76CHnN4Loo\n\t+tfFbR74UEI0g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655369315;\n\tbh=L5u3S7987shHma+8kgCSwUYFbnIN/bnBaoTGoBYoo5s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hu/z2npaCThYWKlrrx9s51SQ4G2m9NdYc1aZIcyLjshJP+ZhL39r11dTJiH5lb8Ey\n\trFBCx8IX+H/X8UkyY6rtDGTGZZHKXw8stLWNtTfOZSm/JJYbJXLg2sORGesB0VnKo3\n\tyXfJCmmD7EEYZTKq2SBjo8rRn0Pbyr9OfUng3F+E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hu/z2npa\"; dkim-atps=neutral","Date":"Thu, 16 Jun 2022 11:48:24 +0300","To":"David Plowman ","Message-ID":"","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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 ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23420,"web_url":"https://patchwork.libcamera.org/comment/23420/","msgid":"","date":"2022-06-16T08:59:44","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nNot sure I quite answered all the right questions!\n\nOn Thu, 16 Jun 2022 at 09:48, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> On Thu, Jun 16, 2022 at 09:32:21AM +0100, David Plowman wrote:\n> > Hi Laurent\n> >\n> > Sorry for the delay!\n>\n> Thanks for the quick reply :-)\n>\n> > On Thu, 16 Jun 2022 at 09:10, Laurent Pinchart wrote:\n> > > On Mon, Jun 13, 2022 at 12:48:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > On Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> > > > > Hi Laurent, everyone\n> > > > >\n> > > > > Thanks for the suggestions. I'm fine to change the syntax to make\n> > > > > things clearer, but I wonder if we could avoid breaking existing JSON\n> > > > > files? There are probably low numbers of them out there beyond the\n> > > > > ones that we've supplied, but you never quite know and backwards\n> > > > > compatibility is still a nice thing. Do you think that's something we\n> > > > > can arrange?\n> > > >\n> > > > This patch series guarantees ordering of entries in mapping (at the\n> > > > expense of duplicated key storage in memory, but that could probably be\n> > > > fixed), so there should be no breakage. The main drawback is that\n> > > > YamlParser and YamlObject will then expose an order that is\n> > > > implementation-specific and shouldn't be relied on by applications\n> > > > according to the JSON and YAML specifications. There's thus a risk of\n> > > > introducing new code that relies on mappings being ordered, which\n> > > > wouldn't be the case if the implementation didn't guarantee ordering\n> > > > (although one may argue that in the case the users of YamlObject may\n> > > > still rely on a different implementation-specific order without\n> > > > realizing it, we would need to randomize the order to avoid that, which\n> > > > I don't think is a good idea).\n> > > >\n> > > > With versioned tuning files the IPA could fairly easily support both the\n> > > > current format and the new format, so that shouldn't be a problem. I\n> > > > think we should then provide a Python script to convert the old format\n> > > > to the new one, and print a warning to the log. Would you expect the\n> > > > Raspberry Pi IPA module to support the current format forever, or only\n> > > > for a fixed duration to help users transition ?\n> > >\n> > > I'll post a v3 series of the YamlObject changes, and I'd like to decide\n> > > on which direction to take (if possible :-)). Could you share your\n> > > thoughts on this ?\n> >\n> > I wasn't totally sure what the precise question is, but here are a few\n> > random answers:\n> >\n> > - I'd like existing turning files to continue working, could they\n> > implicitly be regarded as \"version 0\" or something if they don't say\n> > otherwise? It also means not having to touch the tuning tool just yet.\n>\n> We can, the v2 I've posted can handle this. The only drawback is that it\n> risks introducing another dependency on YAML mapping ordering, but I\n> suppose that's worth it to avoid breaking all existing tuning files.\n>\n> Would you consider dropping support for v0 at some point ? If so, any\n> idea when ?\n>\n> > - I don't like having different priorities stored with different\n> > algorithms. You'll forever be hunting through the file reverse\n> > engineering the actual order. I'd rather have an \"order\" field (or\n> > something) that simply lists the correct order.\n> >\n> > - The \"order\" field would default to the \"standard order\" if not\n> > present. The order can list algorithms that aren't present and that\n> > would just be ignored. It would be common simply not to have all\n> > algorithms, or to comment some out for debugging.\n> >\n> > I think there is some complexity in the order matching. For example,\n> > I'd want \"rpi.awb\" to match just \"awb\". But if \"rpi.awb\" is listed in\n> > the order, that would take precedence.\n> >\n> > The idea is that we can list the standard order like \"black_level\",\n> > \"dpc\", \"lux\", \"noise\", \"geq\", \"sdn\", \"awb\", and so on. But if someone\n> > writes an algorithm \"foo.awb\" they can force it to go somewhere else\n> > if they have too, but by default it would take up the standard \"awb\"\n> > position.\n> >\n> > Hmm, that's starting to get a bit annoying, isn't it? Storing\n> > priorities in the algorithm would be less of a problem in this\n> > respect, but I really don't like it...\n>\n> It's getting complex indeed. I have no issue keeping the existing\n> mechanism working for as long as we support v0. In v1 it would work with\n> the modification to the JSON file explained below. The only cost will be\n> one more level in the hierachy. I've also shown how it looks like in\n> YAML as it's less verbose and thus easier to discuss by e-mail, but that\n> doesn't mean I want to convert existing files to YAML.\n\nTrying to be less complicated...\n\nMaybe we leave the existing files as \"version 0\", and can we keep the\norder-in-the-file for \"version 0\". That would save any complications.\nI'd be happy for \"version 0\" to be json only if that helps.\n\n>\n> > - Don't want to lose json, but happy to have both with the appropriate\n> > file extension.\n>\n> The YamlParser supports JSON just fine :-) The only caveat is that\n> spaces have to be used for indentation, not tabs.\n>\n> > Did I answer everything or was there anything I overlooked?\n>\n> There's the question of how the v1 format should look like. I'd propose\n> the following.\n>\n> {\n> \"version\": 1\n> \"algorithms\": [\n> {\n> \"rpi.black_level\":\n> {\n> \"black_level\": 4096\n> },\n> },\n> {\n> \"rpi.noise\":\n> {\n> \"reference_constant\": 0,\n> \"reference_slope\": 3.67\n> },\n> }\n> ]\n> }\n>\n> As the contents of the \"algorithms\" key is a list, it is ordered, so we\n> keep the current feature of algorithm ordering without abusing a mapping\n> order.\n>\n> Another option would be\n>\n> {\n> \"version\": 1\n> \"algorithms\": [\n> {\n> \"name\": \"rpi.black_level\",\n> \"data\": {\n> \"black_level\": 4096\n> },\n> },\n> {\n> \"name\": \"rpi.noise\",\n> \"data\": {\n> \"reference_constant\": 0,\n> \"reference_slope\": 3.67\n> },\n> }\n> ]\n> }\n>\n> I don't know what's better, if it's a good practice to make keys well\n> known (as in the second option, where an algorithm entry has two keys\n> that are known at compile time, \"name\" and \"data\") or if it's fine (or\n> even better) to make keys more dynamic (which then requires iterating\n> over the parsed object, as we don't know the key name in advance).\n\nHmm, both seem to have (a little bit of) extra \"levels\" of syntax in\nthem. Would this work:\n\n...\n \"algorithms\": [\n {\n \"name\": \"rpi.black_level\",\n \"black_level\": 4096\n },\n...\n\nIf the json-reader just gives us a big blob maps/array then I'm\nguessing we can read the name first, make the correct C++ Algorithm\nobject and then pass this same blob to the read function?\n\nDavid\n\n>\n> > > > > On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > > > > > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > > > > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > > > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > > > > > >\n> > > > > > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > > > > > >\n> > > > > > > > That's a very good point. [5] even mentions\n> > > > > > > >\n> > > > > > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > > > > > representations to a sequential access medium, this serialization detail\n> > > > > > > > must not be used to convey application level information.\"\n> > > > > > > >\n> > > > > > > > The same applies to JSON ([6]).\n> > > > > > > >\n> > > > > > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > > > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > > > > > future :-S\n> > > > > > >\n> > > > > > > Given that the IPA does require ordering and we have been lucky with\n> > > > > > > Boost preserving order in the JSON parser, I think we probably ought\n> > > > > > > to specify ordering in the config file with a specific key.\n> > > > > >\n> > > > > > Sounds good to me.\n> > > > > >\n> > > > > > It could be done through a priority key in each algorithm, or by\n> > > > > > converting the mapping to a list. In YAML format, that would be moving\n> > > > > > from\n> > > > > >\n> > > > > > {\n> > > > > > \"rpi.black_level\":\n> > > > > > {\n> > > > > > \"black_level\": 4096\n> > > > > > },\n> > > > > > \"rpi.noise\":\n> > > > > > {\n> > > > > > \"reference_constant\": 0,\n> > > > > > \"reference_slope\": 3.67\n> > > > > > },\n> > > > > > }\n> > > > > >\n> > > > > > to\n> > > > > >\n> > > > > > [\n> > > > > > {\n> > > > > > \"rpi.black_level\":\n> > > > > > {\n> > > > > > \"black_level\": 4096\n> > > > > > },\n> > > > > > },\n> > > > > > {\n> > > > > > \"rpi.noise\":\n> > > > > > {\n> > > > > > \"reference_constant\": 0,\n> > > > > > \"reference_slope\": 3.67\n> > > > > > },\n> > > > > > }\n> > > > > > ]\n> > > > > >\n> > > > > > In YAML format, it would translate as a move from\n> > > > > >\n> > > > > > rpi.black_level:\n> > > > > > black_level: 4096\n> > > > > > rpi.noise:\n> > > > > > reference_constant: 0\n> > > > > > reference_slope: 3.67\n> > > > > >\n> > > > > > to\n> > > > > >\n> > > > > > - rpi.black_level:\n> > > > > > black_level: 4096\n> > > > > > - rpi.noise:\n> > > > > > reference_constant: 0\n> > > > > > reference_slope: 3.67\n> > > > > >\n> > > > > > As we have to change the tuning files, I'd like to take this as an\n> > > > > > opportunity to add a format version. Something along those lines maybe ?\n> > > > > >\n> > > > > > version: 1.0\n> > > > > > algorithms:\n> > > > > > - rpi.black_level:\n> > > > > > black_level: 4096\n> > > > > > - rpi.noise:\n> > > > > > reference_constant: 0\n> > > > > > reference_slope: 3.67\n> > > > > >\n> > > > > > And while we're discussing this, does someone know about best practice\n> > > > > > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > > > > > if the following grammar would have any advantage:\n> > > > > >\n> > > > > > version: 1.0\n> > > > > > algorithms:\n> > > > > > - name: rpi.black_level\n> > > > > > data:\n> > > > > > black_level: 4096\n> > > > > > - name: rpi.noise:\n> > > > > > data:\n> > > > > > reference_constant: 0\n> > > > > > reference_slope: 3.67\n> > > > > >\n> > > > > > > When I get a\n> > > > > > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > > > > > and this series ought to \"just work\" after that.\n> > > > > > >\n> > > > > > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > > > > > >\n> > > > > > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > > > > > [6] https://www.json.org/json-en.html\n> > > > > > > >\n> > > > > > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > > > > > commit.\n> > > > > > > > > >\n> > > > > > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > > > > > ---\n> > > > > > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > > > > > >\n> > > > > > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > > > > > --- a/test/yaml-parser.cpp\n> > > > > > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > > > > > \" - Mary\\n\"\n> > > > > > > > > > \"dictionary:\\n\"\n> > > > > > > > > > \" a: 1\\n\"\n> > > > > > > > > > - \" b: 2\\n\"\n> > > > > > > > > > \" c: 3\\n\"\n> > > > > > > > > > + \" b: 2\\n\"\n> > > > > > > > > > \"level1:\\n\"\n> > > > > > > > > > \" level2:\\n\"\n> > > > > > > > > > \" - [1, 2]\\n\"\n> > > > > > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > > > > > >\n> > > > > > > > > > if (memeberNames.size() != 3) {\n> > > > > > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > > > > > }\n> > > > > > > > > >\n> > > > > > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > > > > > - memeberNames[2] != \"c\") {\n> > > > > > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > > > > > + memeberNames[2] != \"b\") {\n> > > > > > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > > > > > return TestFail;\n> > > > > > > > > > }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 D430EBD808\n\tfor ;\n\tThu, 16 Jun 2022 08:59:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A16965637;\n\tThu, 16 Jun 2022 10:59:57 +0200 (CEST)","from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com\n\t[IPv6:2a00:1450:4864:20::52e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6928B601F1\n\tfor ;\n\tThu, 16 Jun 2022 10:59:55 +0200 (CEST)","by mail-ed1-x52e.google.com with SMTP id w27so1240365edl.7\n\tfor ;\n\tThu, 16 Jun 2022 01:59:55 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655369997;\n\tbh=yVkMlpasrvIB478BfFIb3+UzECtgQ9rpSwE6br61eiY=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=zUjRQy4g3IpdDy0jzezGWM9SsAL6tYZ+dIMh6MjVmhQhL/p0xjHJp2WwyYssUS9uV\n\tSIlz+bxQzSv5fjs7lCFeNpavQQrFHV2dUpI8MDN6AE0lGsQhlGMO2r51R52d5w21pI\n\tor2t3jQyzwRkORHFU1YyQRd/RIHdZ9GuFAy6FR9xx1whWGEhFABpQ7Il3pvqJLoWvz\n\tIhaBnSzzR6iLLqYys/keOg9hxCmL5HLzp/WN4q8Uz4gGhQcDx608hjZfNx7CEx+pxy\n\t68bLqJ09vvJblUokR5eVigy1ZyvsCOrc08dGRiCw+rPjbrmbffXI10eCigP1kyMz4D\n\tDes7XU92GGYfg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=1hkQWvjRkQyT8dMbpyN6lt5EM7RaVsIS5Z68sJ8OAxw=;\n\tb=JrY1u+gWPR3cRdl35wESbyB5Sfr9p6gBThSXkqiiWOmFoqNObgjC+VeVopNFsMuvsS\n\t7LlcOf0xYqSMyukWUBaBn075OzG+dsRuYUhjbPi+KQyGhypwJUMI2VK/E7ZW1U9OIfb+\n\tptv6G1R+hOdLS0hbCJL/USveEb7MIGI9mH3mu46dEc7P87GsBhwDvUGNM+abbO5Nzdnf\n\tEef1UzbR8rW1aQZyf1WgaxnxWPpmxpjaCKLm7M8lMcSNucmzdiwNErrwkLU0C+8jPeGw\n\tde8FS9cU+1JzzI7F1/oS3OGBorR4IB/nLCounDXIyc1W4snWIeC6jLF5AGRhWB/5X/dH\n\tS8kA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"JrY1u+gW\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=1hkQWvjRkQyT8dMbpyN6lt5EM7RaVsIS5Z68sJ8OAxw=;\n\tb=muK2gFprGpTE/ziyosrh6b9gliu7AH0o59bfzwnoLswnHUPuLy6D/aVzDL9vYTkGJm\n\tTOurWSCMy+LxuxY1HlFhUygs08dEGYZ1fgQNn6zXKfk0+vXnrE1yUqJNOUe/8++8c0Ak\n\ttjUz+xbZLsPWpvuN+UvzXr/qSTXO4QIxYKuz+HzfaxZQ9Bx8Ga7cJOxWMrQFxJckTfKa\n\tOKMzUjhm38yXZtVfrQnXxuB4seUdmlcxnmG2lTBeCtiGfySYeKjRvBY3FOFnUIRKHTYA\n\tqP1aI8JBAJw5mDk3sYi8akiHztfCMoiJvISy48wrevQGOGLvfIZPdNb2vDNe1wiSt8oz\n\tonZQ==","X-Gm-Message-State":"AJIora8XSzwTe2021e952URH5aQ7w2FqDBDOMNBbAxE51NHU8hUnuEbD\n\tOy2Pnlq8BSpy4L+r5HUwK++48qzmYGKjsOzn83DW/5kOqlwR3buY","X-Google-Smtp-Source":"AGRyM1uH0F0JtWjSrw+krBJVd3TZPbOTdSm/nL2o1eLzzYbbWz+e9FTffsrj8/mc9M1IiFL9Zcs0vrJZVDcNAI8vMY8=","X-Received":"by 2002:aa7:c492:0:b0:42d:ed6a:26e5 with SMTP id\n\tm18-20020aa7c492000000b0042ded6a26e5mr5021821edq.64.1655369994897;\n\tThu, 16 Jun 2022 01:59:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20220604185939.29163-1-laurent.pinchart@ideasonboard.com>\n\t<20220604185939.29163-6-laurent.pinchart@ideasonboard.com>\n\t<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Thu, 16 Jun 2022 09:59:44 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23425,"web_url":"https://patchwork.libcamera.org/comment/23425/","msgid":"","date":"2022-06-16T09:51:31","subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jun 16, 2022 at 09:59:44AM +0100, David Plowman wrote:\n> Hi Laurent\n> \n> Not sure I quite answered all the right questions!\n\nLet's see :-)\n\n> On Thu, 16 Jun 2022 at 09:48, Laurent Pinchart wrote:\n> > On Thu, Jun 16, 2022 at 09:32:21AM +0100, David Plowman wrote:\n> > > On Thu, 16 Jun 2022 at 09:10, Laurent Pinchart wrote:\n> > > > On Mon, Jun 13, 2022 at 12:48:55PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > > On Mon, Jun 13, 2022 at 10:25:15AM +0100, David Plowman wrote:\n> > > > > > Hi Laurent, everyone\n> > > > > >\n> > > > > > Thanks for the suggestions. I'm fine to change the syntax to make\n> > > > > > things clearer, but I wonder if we could avoid breaking existing JSON\n> > > > > > files? There are probably low numbers of them out there beyond the\n> > > > > > ones that we've supplied, but you never quite know and backwards\n> > > > > > compatibility is still a nice thing. Do you think that's something we\n> > > > > > can arrange?\n> > > > >\n> > > > > This patch series guarantees ordering of entries in mapping (at the\n> > > > > expense of duplicated key storage in memory, but that could probably be\n> > > > > fixed), so there should be no breakage. The main drawback is that\n> > > > > YamlParser and YamlObject will then expose an order that is\n> > > > > implementation-specific and shouldn't be relied on by applications\n> > > > > according to the JSON and YAML specifications. There's thus a risk of\n> > > > > introducing new code that relies on mappings being ordered, which\n> > > > > wouldn't be the case if the implementation didn't guarantee ordering\n> > > > > (although one may argue that in the case the users of YamlObject may\n> > > > > still rely on a different implementation-specific order without\n> > > > > realizing it, we would need to randomize the order to avoid that, which\n> > > > > I don't think is a good idea).\n> > > > >\n> > > > > With versioned tuning files the IPA could fairly easily support both the\n> > > > > current format and the new format, so that shouldn't be a problem. I\n> > > > > think we should then provide a Python script to convert the old format\n> > > > > to the new one, and print a warning to the log. Would you expect the\n> > > > > Raspberry Pi IPA module to support the current format forever, or only\n> > > > > for a fixed duration to help users transition ?\n> > > >\n> > > > I'll post a v3 series of the YamlObject changes, and I'd like to decide\n> > > > on which direction to take (if possible :-)). Could you share your\n> > > > thoughts on this ?\n> > >\n> > > I wasn't totally sure what the precise question is, but here are a few\n> > > random answers:\n> > >\n> > > - I'd like existing turning files to continue working, could they\n> > > implicitly be regarded as \"version 0\" or something if they don't say\n> > > otherwise? It also means not having to touch the tuning tool just yet.\n> >\n> > We can, the v2 I've posted can handle this. The only drawback is that it\n> > risks introducing another dependency on YAML mapping ordering, but I\n> > suppose that's worth it to avoid breaking all existing tuning files.\n> >\n> > Would you consider dropping support for v0 at some point ? If so, any\n> > idea when ?\n\nThis one went unanswered. I'd like to know if you foresee v0 being\nsupported forever, for a long time, or for a short transition period.\nI'm not pushing for one particular option, I'd just like to know in\norder to structure the YamlObject rework correctly.\n\n> > > - I don't like having different priorities stored with different\n> > > algorithms. You'll forever be hunting through the file reverse\n> > > engineering the actual order. I'd rather have an \"order\" field (or\n> > > something) that simply lists the correct order.\n> > >\n> > > - The \"order\" field would default to the \"standard order\" if not\n> > > present. The order can list algorithms that aren't present and that\n> > > would just be ignored. It would be common simply not to have all\n> > > algorithms, or to comment some out for debugging.\n> > >\n> > > I think there is some complexity in the order matching. For example,\n> > > I'd want \"rpi.awb\" to match just \"awb\". But if \"rpi.awb\" is listed in\n> > > the order, that would take precedence.\n> > >\n> > > The idea is that we can list the standard order like \"black_level\",\n> > > \"dpc\", \"lux\", \"noise\", \"geq\", \"sdn\", \"awb\", and so on. But if someone\n> > > writes an algorithm \"foo.awb\" they can force it to go somewhere else\n> > > if they have too, but by default it would take up the standard \"awb\"\n> > > position.\n> > >\n> > > Hmm, that's starting to get a bit annoying, isn't it? Storing\n> > > priorities in the algorithm would be less of a problem in this\n> > > respect, but I really don't like it...\n> >\n> > It's getting complex indeed. I have no issue keeping the existing\n> > mechanism working for as long as we support v0. In v1 it would work with\n> > the modification to the JSON file explained below. The only cost will be\n> > one more level in the hierachy. I've also shown how it looks like in\n> > YAML as it's less verbose and thus easier to discuss by e-mail, but that\n> > doesn't mean I want to convert existing files to YAML.\n> \n> Trying to be less complicated...\n> \n> Maybe we leave the existing files as \"version 0\", and can we keep the\n> order-in-the-file for \"version 0\". That would save any complications.\n\nYes, that's the idea, as you're concerned about breakages, the current\nformat will be considered as v0 and will continue operating exactly as\nit does today.\n\nFor v1, we can change the grammar of the JSON files to use lists instead\nof dictionnaries, thus giving us an order without infringing on the JSON\nspecification. We will thus still have the ordering controlled from the\ntuning file, same feature as today, just a different grammar.\n\n> I'd be happy for \"version 0\" to be json only if that helps.\n\nAs YamlParser supports JSON (with the caveat I've already mentioned),\nboth JSON and YAML are fine. It may be nice to standardize on one format\nthroughout libcamera if it doesn't cause any particular problem, but I'm\nfine with the Raspberry Pi tuning files continuing to use JSON.\n\n> > > - Don't want to lose json, but happy to have both with the appropriate\n> > > file extension.\n> >\n> > The YamlParser supports JSON just fine :-) The only caveat is that\n> > spaces have to be used for indentation, not tabs.\n> >\n> > > Did I answer everything or was there anything I overlooked?\n> >\n> > There's the question of how the v1 format should look like. I'd propose\n> > the following.\n> >\n> > {\n> > \"version\": 1\n> > \"algorithms\": [\n> > {\n> > \"rpi.black_level\":\n> > {\n> > \"black_level\": 4096\n> > },\n> > },\n> > {\n> > \"rpi.noise\":\n> > {\n> > \"reference_constant\": 0,\n> > \"reference_slope\": 3.67\n> > },\n> > }\n> > ]\n> > }\n> >\n> > As the contents of the \"algorithms\" key is a list, it is ordered, so we\n> > keep the current feature of algorithm ordering without abusing a mapping\n> > order.\n> >\n> > Another option would be\n> >\n> > {\n> > \"version\": 1\n> > \"algorithms\": [\n> > {\n> > \"name\": \"rpi.black_level\",\n> > \"data\": {\n> > \"black_level\": 4096\n> > },\n> > },\n> > {\n> > \"name\": \"rpi.noise\",\n> > \"data\": {\n> > \"reference_constant\": 0,\n> > \"reference_slope\": 3.67\n> > },\n> > }\n> > ]\n> > }\n> >\n> > I don't know what's better, if it's a good practice to make keys well\n> > known (as in the second option, where an algorithm entry has two keys\n> > that are known at compile time, \"name\" and \"data\") or if it's fine (or\n> > even better) to make keys more dynamic (which then requires iterating\n> > over the parsed object, as we don't know the key name in advance).\n> \n> Hmm, both seem to have (a little bit of) extra \"levels\" of syntax in\n> them.\n\nThat's right. Two new levels are introduced, one for versioning, and one\nto store algorithms in a list instead of a dictionnary (this is what\nwill give us ordering).\n\n> Would this work:\n> \n> ...\n> \"algorithms\": [\n> {\n> \"name\": \"rpi.black_level\",\n> \"black_level\": 4096\n> },\n> ...\n> \n> If the json-reader just gives us a big blob maps/array then I'm\n> guessing we can read the name first, make the correct C++ Algorithm\n> object and then pass this same blob to the read function?\n\nIt would work, but I don't like it much, it mixes two different levels\ninto one, and could result in name clashes (for instance an algorithm\ncouldn't use \"name\" as a key in its data, it may not be a big deal\nas-is, but it may be annoying if/when we introduce more common keys).\n\nI've been trying to find literature about best practices for JSON or\nYAML grammar design, but without luck :-(\n\n> > > > > > On Mon, 13 Jun 2022 at 09:46, Laurent Pinchart wrote:\n> > > > > > > On Mon, Jun 13, 2022 at 09:05:21AM +0100, Naushir Patuck wrote:\n> > > > > > > > On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart wrote:\n> > > > > > > > > On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > > > > > > > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart wrote:\n> > > > > > > > > > > The order of items in a YAML dictionary may matter. Update the test to\n> > > > > > > > > > > ensure that it is preserved. The test currently fails at the YamlParser\n> > > > > > > > > >\n> > > > > > > > > > My understanding is that YAML mappings are unordered [1] [2], and if\n> > > > > > > > > > order in the mapping is significant, then either a sequence of mappings\n> > > > > > > > > > [3] or flow mapping adjacent values [4] should be used.\n> > > > > > > > >\n> > > > > > > > > That's a very good point. [5] even mentions\n> > > > > > > > >\n> > > > > > > > > \"while imposing a order on mapping keys is necessary for flattening YAML\n> > > > > > > > > representations to a sequential access medium, this serialization detail\n> > > > > > > > > must not be used to convey application level information.\"\n> > > > > > > > >\n> > > > > > > > > The same applies to JSON ([6]).\n> > > > > > > > >\n> > > > > > > > > Fixing this would require changing the syntax of the tuning files. It's\n> > > > > > > > > inconvenient, but not doing so opens the door to more issues in the\n> > > > > > > > > future :-S\n> > > > > > > >\n> > > > > > > > Given that the IPA does require ordering and we have been lucky with\n> > > > > > > > Boost preserving order in the JSON parser, I think we probably ought\n> > > > > > > > to specify ordering in the config file with a specific key.\n> > > > > > >\n> > > > > > > Sounds good to me.\n> > > > > > >\n> > > > > > > It could be done through a priority key in each algorithm, or by\n> > > > > > > converting the mapping to a list. In YAML format, that would be moving\n> > > > > > > from\n> > > > > > >\n> > > > > > > {\n> > > > > > > \"rpi.black_level\":\n> > > > > > > {\n> > > > > > > \"black_level\": 4096\n> > > > > > > },\n> > > > > > > \"rpi.noise\":\n> > > > > > > {\n> > > > > > > \"reference_constant\": 0,\n> > > > > > > \"reference_slope\": 3.67\n> > > > > > > },\n> > > > > > > }\n> > > > > > >\n> > > > > > > to\n> > > > > > >\n> > > > > > > [\n> > > > > > > {\n> > > > > > > \"rpi.black_level\":\n> > > > > > > {\n> > > > > > > \"black_level\": 4096\n> > > > > > > },\n> > > > > > > },\n> > > > > > > {\n> > > > > > > \"rpi.noise\":\n> > > > > > > {\n> > > > > > > \"reference_constant\": 0,\n> > > > > > > \"reference_slope\": 3.67\n> > > > > > > },\n> > > > > > > }\n> > > > > > > ]\n> > > > > > >\n> > > > > > > In YAML format, it would translate as a move from\n> > > > > > >\n> > > > > > > rpi.black_level:\n> > > > > > > black_level: 4096\n> > > > > > > rpi.noise:\n> > > > > > > reference_constant: 0\n> > > > > > > reference_slope: 3.67\n> > > > > > >\n> > > > > > > to\n> > > > > > >\n> > > > > > > - rpi.black_level:\n> > > > > > > black_level: 4096\n> > > > > > > - rpi.noise:\n> > > > > > > reference_constant: 0\n> > > > > > > reference_slope: 3.67\n> > > > > > >\n> > > > > > > As we have to change the tuning files, I'd like to take this as an\n> > > > > > > opportunity to add a format version. Something along those lines maybe ?\n> > > > > > >\n> > > > > > > version: 1.0\n> > > > > > > algorithms:\n> > > > > > > - rpi.black_level:\n> > > > > > > black_level: 4096\n> > > > > > > - rpi.noise:\n> > > > > > > reference_constant: 0\n> > > > > > > reference_slope: 3.67\n> > > > > > >\n> > > > > > > And while we're discussing this, does someone know about best practice\n> > > > > > > rules to design JSON/YAML grammars ? I've been wondering for a long time\n> > > > > > > if the following grammar would have any advantage:\n> > > > > > >\n> > > > > > > version: 1.0\n> > > > > > > algorithms:\n> > > > > > > - name: rpi.black_level\n> > > > > > > data:\n> > > > > > > black_level: 4096\n> > > > > > > - name: rpi.noise:\n> > > > > > > data:\n> > > > > > > reference_constant: 0\n> > > > > > > reference_slope: 3.67\n> > > > > > >\n> > > > > > > > When I get a\n> > > > > > > > change, I'll look to add a patch to allow this on the existing codebase,\n> > > > > > > > and this series ought to \"just work\" after that.\n> > > > > > > >\n> > > > > > > > > > [1] https://yaml.org/spec/1.2.2/#mapping\n> > > > > > > > > > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order\n> > > > > > > > > > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings\n> > > > > > > > > > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values\n> > > > > > > > >\n> > > > > > > > > [5] https://yaml.org/spec/1.2.2/#32-information-models\n> > > > > > > > > [6] https://www.json.org/json-en.html\n> > > > > > > > >\n> > > > > > > > > > > doesn't correctly preserve the order, this will be fixed by the next\n> > > > > > > > > > > commit.\n> > > > > > > > > > >\n> > > > > > > > > > > Signed-off-by: Laurent Pinchart \n> > > > > > > > > > > ---\n> > > > > > > > > > > test/yaml-parser.cpp | 7 +++----\n> > > > > > > > > > > 1 file changed, 3 insertions(+), 4 deletions(-)\n> > > > > > > > > > >\n> > > > > > > > > > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp\n> > > > > > > > > > > index 5ff4c3236dbf..582c9caed836 100644\n> > > > > > > > > > > --- a/test/yaml-parser.cpp\n> > > > > > > > > > > +++ b/test/yaml-parser.cpp\n> > > > > > > > > > > @@ -29,8 +29,8 @@ static const string testYaml =\n> > > > > > > > > > > \" - Mary\\n\"\n> > > > > > > > > > > \"dictionary:\\n\"\n> > > > > > > > > > > \" a: 1\\n\"\n> > > > > > > > > > > - \" b: 2\\n\"\n> > > > > > > > > > > \" c: 3\\n\"\n> > > > > > > > > > > + \" b: 2\\n\"\n> > > > > > > > > > > \"level1:\\n\"\n> > > > > > > > > > > \" level2:\\n\"\n> > > > > > > > > > > \" - [1, 2]\\n\"\n> > > > > > > > > > > @@ -428,7 +428,6 @@ protected:\n> > > > > > > > > > > }\n> > > > > > > > > > >\n> > > > > > > > > > > auto memeberNames = dictObj.memberNames();\n> > > > > > > > > > > - sort(memeberNames.begin(), memeberNames.end());\n> > > > > > > > > > >\n> > > > > > > > > > > if (memeberNames.size() != 3) {\n> > > > > > > > > > > cerr << \"Dictionary object fail to extra member names\" << std::endl;\n> > > > > > > > > > > @@ -436,8 +435,8 @@ protected:\n> > > > > > > > > > > }\n> > > > > > > > > > >\n> > > > > > > > > > > if (memeberNames[0] != \"a\" ||\n> > > > > > > > > > > - memeberNames[1] != \"b\" ||\n> > > > > > > > > > > - memeberNames[2] != \"c\") {\n> > > > > > > > > > > + memeberNames[1] != \"c\" ||\n> > > > > > > > > > > + memeberNames[2] != \"b\") {\n> > > > > > > > > > > cerr << \"Dictionary object fail to parse member names\" << std::endl;\n> > > > > > > > > > > return TestFail;\n> > > > > > > > > > > }","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 56FD4BD808\n\tfor ;\n\tThu, 16 Jun 2022 09:51:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9144365637;\n\tThu, 16 Jun 2022 11:51:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D27F601F1\n\tfor ;\n\tThu, 16 Jun 2022 11:51:42 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5EE3415;\n\tThu, 16 Jun 2022 11:51:41 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655373103;\n\tbh=WIEypYlIKHS9Bc8GoH2v6aywy+f+lU4+3j225jaMx8s=;\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:Cc:\n\tFrom;\n\tb=bd2Cgl6UM/w4Wm+hMZeFULECF566Rsve6Owqs2qVy9yZZe4qMUuleEPPqr9xmDNfs\n\t+xyYmgoh8X8FHD+yK28OIQ+t/uH9XHT5NH7eEjkQdE3mS9Lj98QvUZcJA8b9cGjYff\n\teeXqA5yntJllvlFlkX1pU90O+d6hHCrA73iZYH11otVifO/cFgpoClXx60QCu1Ydjc\n\tyWDn3KDkrICKKJX/ylh4zsx0rsbkv2pBMaPQfma+sjCGUktZi1BWVjhIX61ruhzHil\n\t81ZlipYP/dBm/ePvhPEJ+siKB2oVWuQxG5YCj5Kja7Q114TSyEeJ7V8NerwhiLYH9g\n\tdItwOSuCy1Zng==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655373101;\n\tbh=WIEypYlIKHS9Bc8GoH2v6aywy+f+lU4+3j225jaMx8s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IBemuwqKkdXV2CIVt+WNdzwvdr7JaHEtQyXcKKWzhVTYyAoUgFSLe33ws7wh+gawe\n\tl51U7qsp+YNRXVDhxTgc70cqqDRpGGQ1VUHIsgbpJOl+7rjQGALTFaQQ941SjRtVM2\n\tfoY+9ex5dGKZJpztWpOhhdUgoexRbmMXQzrgFIHw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IBemuwqK\"; dkim-atps=neutral","Date":"Thu, 16 Jun 2022 12:51:31 +0300","To":"David Plowman ","Message-ID":"","References":"<20220613042219.GG2369877@pyrite.rasen.tech>\n\t\n\t\n\t\n\t\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test\n\tdictionary items ordering","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 ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]