[{"id":24063,"web_url":"https://patchwork.libcamera.org/comment/24063/","msgid":"","date":"2022-07-22T19:25:37","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> From: Laurent Pinchart \n> \n> The Raspberry Pi IPA module depends on boost only to parse the JSON\n> tuning data files. As libcamera depends on libyaml, use the YamlParser\n> class to parse those files and drop the dependency on boost.\n> \n> Signed-off-by: Laurent Pinchart \n\nI assume you're fine with this patch as you've posted it as part of your\nseries, but could you give it a review ? :-)\n\n> ---\n> README.rst | 6 --\n> src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n> src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> .../controller/rpi/black_level.cpp | 12 +--\n> .../controller/rpi/black_level.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> src/ipa/raspberrypi/meson.build | 1 -\n> src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> 32 files changed, 241 insertions(+), 240 deletions(-)","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 282A2BE173\n\tfor ;\n\tFri, 22 Jul 2022 19:25:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 502536330F;\n\tFri, 22 Jul 2022 21:25:43 +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 203BA601B8\n\tfor ;\n\tFri, 22 Jul 2022 21:25:41 +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 8295C6D5;\n\tFri, 22 Jul 2022 21:25:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658517943;\n\tbh=L1dVfgZvL6Wi6VNzimDF/YEpf52+5wq+zPMrUhwdFDE=;\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=iCQ/04BEsy2W/kF2a3wPnI9+EIs83QxSjqVP+V5Qo0y5BjrIt+rh+8SHuk8zoOgVP\n\tLgT68CS1K6fX4MeO2fqZCWLnY+QTTQgrQa1hE0AfNYIb0SeqgvvO65KCSAVjumeJRB\n\t2SOroNkvxT7ppUxEAN53pG/RdetvH453JckxXHO6GAdjdih3kqC2e3kdYTKvc59S1n\n\tajTinhEdDkKLMmV+36dI+HbgTNLp35RwlsXHsUNZlbKsqGf/HzSXHbfDYhLAz2HKFk\n\tuOewhwsxquOEK6wx2JmfPeY8YUNhnNDwFQ9rJzgOACf9DUizhT7r49TNK8WmHl3WO1\n\tyZtf+jysyw9vQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658517940;\n\tbh=L1dVfgZvL6Wi6VNzimDF/YEpf52+5wq+zPMrUhwdFDE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J5kFgoBG26c75yt0DN2LcUAhVH9/10nN+aEk/xtCTmufAsnLPcxgY5YDLUgC+pO8i\n\tGHFuDCt8z3wYqmfuWAbM1rrb4nqIv8yXaev7HL6Hyj2899ioFPLfhsB0mBcOeho/cd\n\tDQOPotvkOeDI9m9a7HWmDRkXmqcxWvwpDRNONJrU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"J5kFgoBG\"; dkim-atps=neutral","Date":"Fri, 22 Jul 2022 22:25:37 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220718081602.32535-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24093,"web_url":"https://patchwork.libcamera.org/comment/24093/","msgid":"","date":"2022-07-25T10:26:14","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\n\nOn Fri, 22 Jul 2022 at 20:25, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> > From: Laurent Pinchart \n> >\n> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n> > tuning data files. As libcamera depends on libyaml, use the YamlParser\n> > class to parse those files and drop the dependency on boost.\n> >\n> > Signed-off-by: Laurent Pinchart \n>\n> I assume you're fine with this patch as you've posted it as part of your\n> series, but could you give it a review ? :-)\n>\n\nI did have one outstanding query on this about default values.\n\nIf you recall, we discussed having YamlObject::get() not take in a default\nvalue\nand complain loudly if the key is not present, similar to what the Boost\nparser\ndoes now. This will help avoid possibly hard to debug issues if a critical\nparameter is missing from the tuning config. David, what are your thoughts?\n\nDo you think this is something that we can add easily to this patch?\n\nRegards,\nNaush\n\n\n> > ---\n> > README.rst | 6 --\n> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> > .../controller/rpi/black_level.cpp | 12 +--\n> > .../controller/rpi/black_level.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> > src/ipa/raspberrypi/meson.build | 1 -\n> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> > 32 files changed, 241 insertions(+), 240 deletions(-)\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 45C78C3275\n\tfor ;\n\tMon, 25 Jul 2022 10:26:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95A8963312;\n\tMon, 25 Jul 2022 12:26:32 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7BD66330A\n\tfor ;\n\tMon, 25 Jul 2022 12:26:30 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id bf9so17272731lfb.13\n\tfor ;\n\tMon, 25 Jul 2022 03:26:30 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658744792;\n\tbh=1fJIMYKMF/GGeHmrpyqRBjV6YfEVcStDL73UkHCV/dg=;\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=BQvIqjA9u5VkLy6vA8PylV0UjtZOn+umPcBCnR4QBdx7taod7dgq4J4WzMKz50rF0\n\tsXh1VAyrGrYMjxsR6g8hv23HnfwDeYbZirkj+UMiESImqy2DQnGAdrKkZWSMQUpYI0\n\t0aca4En61UdQLAfT98EAioriob4518jO77nYdcNs3auTuh20LRmbLIPU0e1MVpHTBZ\n\teyrhyiPdoh+Rqfy+yH0Do4eCsJD6KTI0KnQU8lmW8cxwHJDipysMRbCEzx4Z9RiB5K\n\tOtyP/37Q3JsJuY5jfwSXEv2dibgVhs8nEJwUJzI2koeluPxpTLeZclJ1LdrKJLuZmt\n\t4lQH6VdG60H6A==","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=5VAqQ1WT+kZhF72YyV8qjrhLpiwcMyECgA9SdrI9ttg=;\n\tb=EfTTSCXn7+kKzcekl4m0QYtAlBz+rxDSimwtdOwzT7bLSOm1Es1PSKFF7EZyjdciSV\n\tsolZvtiiTkxPskb/My+yxK4Ma0sh59dwVpQZIrZu364RcturBxvSSyjjHOKrh51rz5pB\n\tf7fwR0apBUzcTGt1GCMGnf5zY8edZSoqQWEP65FZg8nMKdY1KZ1+4aQH3hkwNZlxqd1G\n\tYf+cKN9JrLt6IS+Le6/QPj4DVPj+qbo8ci53hILIbj8OrMK7tiaGgpTjvKvwViejvi7N\n\tXmsSs3M8MZ3IMqjgNcYnNiGSXI3g/GxwSH3o+zcioJdxd2PYAUC0Vaxqd4aGTIjZmZIC\n\tRHKg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"EfTTSCXn\"; 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=5VAqQ1WT+kZhF72YyV8qjrhLpiwcMyECgA9SdrI9ttg=;\n\tb=D5igH7+kjlBN9NCgKgjnHtJ0ldOiKEg/BkCsOdE+7FhiC2YbY2SMk/W9P25s+WR5QT\n\tdm65ajywmJt6DQryN/5yy3laumfjB0aHegoge9ZaDgKh92zHnnxL4JdpwFC8Mpuvo1Mt\n\taFP4KQvIqVyIe9Ss9NRd8H7ASTJl3UJfx7KWxddWY+1icn5iQxU0YPH/DCaAx8hY+l4e\n\tVR6qoCfiSs8xa/L7gw1c1m0osydBw2RjUcOspMFGOJvsz8M1JPopDxw40Uw1eiO3k4jH\n\tyrDXfmJK/vEqf85AQbNYmpzoGZzafCr3R26xrXTfML8ROjEwcDOyL6jBZyChqwzWw7Fd\n\tK1lw==","X-Gm-Message-State":"AJIora9JqextR3maVBAhhU9dD8Dcm1HO72il7dTtxkIC6xRMHMMqdif9\n\tEUY8afGOx5Mp0o81AmBgAM2W5gl5jNJirnusX8xlhw==","X-Google-Smtp-Source":"AGRyM1uHyUvIJD3SN8PeMRXpHJJo2Xw8L7qEDiM/hTBgA8tIm5Rw/8eD5bpwmYeJE3zrWpM0uRRu6dqWpCbLPGFx1RU=","X-Received":"by 2002:ac2:5463:0:b0:48a:7f6f:4e47 with SMTP id\n\te3-20020ac25463000000b0048a7f6f4e47mr4178544lfn.356.1658744789803;\n\tMon, 25 Jul 2022 03:26:29 -0700 (PDT)","MIME-Version":"1.0","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Mon, 25 Jul 2022 11:26:14 +0100","Message-ID":"","To":"Laurent Pinchart , \n\tDavid Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000f838bb05e49e9d22\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24095,"web_url":"https://patchwork.libcamera.org/comment/24095/","msgid":"","date":"2022-07-25T10:50:06","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush, Laurent\n\nOn Mon, 25 Jul 2022 at 11:26, Naushir Patuck wrote:\n>\n> Hi Laurent,\n>\n>\n> On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart wrote:\n>>\n>> Hi Naush,\n>>\n>> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n>> > From: Laurent Pinchart \n>> >\n>> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n>> > tuning data files. As libcamera depends on libyaml, use the YamlParser\n>> > class to parse those files and drop the dependency on boost.\n>> >\n>> > Signed-off-by: Laurent Pinchart \n>>\n>> I assume you're fine with this patch as you've posted it as part of your\n>> series, but could you give it a review ? :-)\n>\n>\n> I did have one outstanding query on this about default values.\n>\n> If you recall, we discussed having YamlObject::get() not take in a default value\n> and complain loudly if the key is not present, similar to what the Boost parser\n> does now. This will help avoid possibly hard to debug issues if a critical\n> parameter is missing from the tuning config. David, what are your thoughts?\n\nI agree it would be nice to get some kind of error or at least warning\nif something that we really expected is not present. Possibly we could\nreview some of those and provide default values in more cases, but I'm\nguessing that's not the kind of thing to \"hide\" inside a patch that\nisn't meant to change the behaviour of any algorithms.\n\nThanks!\nDavid\n\n>\n> Do you think this is something that we can add easily to this patch?\n>\n> Regards,\n> Naush\n>\n>>\n>> > ---\n>> > README.rst | 6 --\n>> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n>> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n>> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n>> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n>> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n>> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n>> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n>> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n>> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n>> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n>> > .../controller/rpi/black_level.cpp | 12 +--\n>> > .../controller/rpi/black_level.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n>> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n>> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n>> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n>> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n>> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n>> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n>> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n>> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n>> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n>> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n>> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n>> > src/ipa/raspberrypi/meson.build | 1 -\n>> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n>> > 32 files changed, 241 insertions(+), 240 deletions(-)\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 8C040C3275\n\tfor ;\n\tMon, 25 Jul 2022 10:50:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 058B663312;\n\tMon, 25 Jul 2022 12:50:19 +0200 (CEST)","from mail-ed1-x530.google.com (mail-ed1-x530.google.com\n\t[IPv6:2a00:1450:4864:20::530])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD6FF6330A\n\tfor ;\n\tMon, 25 Jul 2022 12:50:17 +0200 (CEST)","by mail-ed1-x530.google.com with SMTP id m8so13338720edd.9\n\tfor ;\n\tMon, 25 Jul 2022 03:50:17 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658746219;\n\tbh=YSDKhutR2umgAVs4EhZFAStgv4MWnfJsykY+CzOwyuI=;\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=xif9ij+euUBG7uLe5YII9WfQnqz183GV0HgQBBcj/RzmX2ZAE9XHMu6jKvJVfdzCg\n\tmHNQ1nVv3+ChMF/Lcgh5wMAikxaSuq4E3/BGPRPYkl/OiDrulOK7dGjA6aJfJ7aPsS\n\tFF0d+rMXieLNUSD97l7ts8coK02caLzV3N7omr2HfjJbCOqMmuaCwLEb4//+XPTFmB\n\tgngYQLlC6vTyTUCuqM4nlL/iREQPGhDJDxE+zE61Qs7Pv6PzUXDLAyDyc73HJiCeze\n\tyFVwIJUVLwuD5xB3HNOGFvT2lA89ZLuDIQFy/H0KbYBW8ApLQmwWErhGessJGFQofB\n\txpLpduzgdvjTw==","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=QCYGdBHGjYAjAeyEdUVzb8XQY9TLhgqbRXhlX9sksEI=;\n\tb=ErzeEdjMQDFaIv/E0s2YZyI6epUfanRdoUC5Ygm/wZERUeiMU0UY5rHnrZqvjtadxM\n\tKlU2AqPyxz4yjpxn4OH4n66PNUp9VdKBDnPcSMPeRHdKvm3jhcvKieI2YY4Ktv7wYHTB\n\tdyAU1T5IEdqAcgvay+cDnso4sc3zyJa3rn2NppNE1lVa0fBMtjnoBvbdARWYng48JFfG\n\taUBzPxpWPu917RrMBVTNqmDgFu60EOwENoqQM82SkGFv8a0+0ABI3BaK4JF+RwKhjrCL\n\tm5Spm9+s+4qL2Ub47uckCtuh4oBDJboLRRim21KgHuZEyl8iUIR8M2LhsOTil/BGf4Va\n\t4Gkw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ErzeEdjM\"; 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=QCYGdBHGjYAjAeyEdUVzb8XQY9TLhgqbRXhlX9sksEI=;\n\tb=fiCVUpcDz1Bg9MBf9PSfY7iuRBsX2ZSC5dI26ti7DWLUgLBexdorod4nyATN6ja51d\n\t1vM8PuzsczuEjjg5u5A/o4EEmPM4SsfQiz+XFnnuiiwTrK7XIZkKylyUTUtWNLqpHmOb\n\tKWW1WMlzgTqAdyRI+9E8mKck3EIAPfiVaOub1l2FR1/Y40mK3iY1jryfZxiz92hOeDe4\n\tevAADRvawxe0bS3mVSpOznh2zilRsmjx2DY0FYQG5xtIAck8i6NYMXfMcBSgec+qdnQq\n\t+Du6h7HzXlFcjHHS/oO9/HIpOcNfyZb0HoHBqOQZeRUGj8xbGvN2aaIKTLt0PMb/IbNF\n\tM2CQ==","X-Gm-Message-State":"AJIora+yVWuqtPMEDT3jGe+UNQy9i6jBegI79URSSEw4SjxeuAIIDCOu\n\tTuKQ8V9An7WjhouhFdVqAZM+5Opc+n9chclV10yJ6StKbUPnQw==","X-Google-Smtp-Source":"AGRyM1tvUMuftLcSLoeO2TXL4jKUYLo+RLz1CHZVqP/3rh3fXL+EfHoWmKkDVRz2zU08byUOOyVqlQSbs0F/IEG3khw=","X-Received":"by 2002:a05:6402:344:b0:43b:e42b:fdf with SMTP id\n\tr4-20020a056402034400b0043be42b0fdfmr8949315edw.416.1658746217272;\n\tMon, 25 Jul 2022 03:50:17 -0700 (PDT)","MIME-Version":"1.0","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","Date":"Mon, 25 Jul 2022 11:50:06 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24099,"web_url":"https://patchwork.libcamera.org/comment/24099/","msgid":"","date":"2022-07-25T13:50:28","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Jul 25, 2022 at 11:50:06AM +0100, David Plowman wrote:\n> On Mon, 25 Jul 2022 at 11:26, Naushir Patuck wrote:\n> > On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart wrote:\n> >> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> >> > From: Laurent Pinchart \n> >> >\n> >> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n> >> > tuning data files. As libcamera depends on libyaml, use the YamlParser\n> >> > class to parse those files and drop the dependency on boost.\n> >> >\n> >> > Signed-off-by: Laurent Pinchart \n> >>\n> >> I assume you're fine with this patch as you've posted it as part of your\n> >> series, but could you give it a review ? :-)\n> >\n> > I did have one outstanding query on this about default values.\n> >\n> > If you recall, we discussed having YamlObject::get() not take in a default value\n> > and complain loudly if the key is not present, similar to what the Boost parser\n> > does now. This will help avoid possibly hard to debug issues if a critical\n> > parameter is missing from the tuning config. David, what are your thoughts?\n> \n> I agree it would be nice to get some kind of error or at least warning\n> if something that we really expected is not present. Possibly we could\n> review some of those and provide default values in more cases, but I'm\n> guessing that's not the kind of thing to \"hide\" inside a patch that\n> isn't meant to change the behaviour of any algorithms.\n\nI agree, that's a good point.\n\nThe boost json parser will throw an exception in this case, which I\ncan't replicate in YamlObject. I can update the Read() function to\nreturn an error, would it be enough to log a message at the top level,\nor would you like individual log statements in every location to tell\nwhich parameter was missing ?\n\n> > Do you think this is something that we can add easily to this patch?\n> >\n> >> > ---\n> >> > README.rst | 6 --\n> >> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> >> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> >> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> >> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> >> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n> >> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> >> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n> >> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n> >> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> >> > .../controller/rpi/black_level.cpp | 12 +--\n> >> > .../controller/rpi/black_level.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> >> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> >> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> >> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> >> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> >> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> >> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> >> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> >> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> >> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> >> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> >> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> >> > src/ipa/raspberrypi/meson.build | 1 -\n> >> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> >> > 32 files changed, 241 insertions(+), 240 deletions(-)","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 F1A90BE173\n\tfor ;\n\tMon, 25 Jul 2022 13:50:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8437563312;\n\tMon, 25 Jul 2022 15:50:35 +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 9EB476330A\n\tfor ;\n\tMon, 25 Jul 2022 15:50:33 +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 194256DD;\n\tMon, 25 Jul 2022 15:50:33 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658757035;\n\tbh=QVQ4nTeja4rO9bLWZ724twmANQPEXk0+RPWTEPVX4sI=;\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=Kx/LxXYU2vXRGxjUHuMm85oH+l7QaaRBOLSrc2Yxtu6Oc2TjyVTEidr6RByePFwXv\n\tiYc/9bwB6ONsf1WF0sIt5cJHmmmA3EyKfpX/fMZMuL17VWe4QfUx5wn2iWShVM06az\n\tLSy+F4ew8eeo/RSp4XB2WIT3usROXLKXuVRe68FEBjX/6PIdU1kVgjkbdz4dZFejP2\n\t2Pv/XcaWbwMe2dOPUlWVkiR0g0WRAqneAwdieXcC1KlJF0jkRMJ//XRZeq6InrnvcV\n\tOPlsi+Qf99nWQStUCmhb3yKwpIL33Sf2atPHum7z1OndqzB4XIVgvpectAgabK3ACY\n\tV0RREOtbnTCcg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658757033;\n\tbh=QVQ4nTeja4rO9bLWZ724twmANQPEXk0+RPWTEPVX4sI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=knEGJzAsW6sKOLfvRKuVrm+kxqUnPLvIBFlOEvkPm0lS0Ctf5RorlLitpipZecy9c\n\thDrgxJ9vTAOCXc47SgZQmrrZYBJHrlPSHCvqXCY0TEolH/VBMHkiKgnEyFaiBxbcY3\n\tlEvs9/JCcMlfWl5aaA5oi7UBJw9LoOEloxUaoNrQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"knEGJzAs\"; dkim-atps=neutral","Date":"Mon, 25 Jul 2022 16:50:28 +0300","To":"David Plowman ","Message-ID":"","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\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] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24100,"web_url":"https://patchwork.libcamera.org/comment/24100/","msgid":"","date":"2022-07-25T14:48:21","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Mon, 25 Jul 2022 at 14:50, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hello,\n>\n> On Mon, Jul 25, 2022 at 11:50:06AM +0100, David Plowman wrote:\n> > On Mon, 25 Jul 2022 at 11:26, Naushir Patuck \n> wrote:\n> > > On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> > >> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> > >> > From: Laurent Pinchart \n> > >> >\n> > >> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n> > >> > tuning data files. As libcamera depends on libyaml, use the\n> YamlParser\n> > >> > class to parse those files and drop the dependency on boost.\n> > >> >\n> > >> > Signed-off-by: Laurent Pinchart \n> > >>\n> > >> I assume you're fine with this patch as you've posted it as part of\n> your\n> > >> series, but could you give it a review ? :-)\n> > >\n> > > I did have one outstanding query on this about default values.\n> > >\n> > > If you recall, we discussed having YamlObject::get() not take in a\n> default value\n> > > and complain loudly if the key is not present, similar to what the\n> Boost parser\n> > > does now. This will help avoid possibly hard to debug issues if a\n> critical\n> > > parameter is missing from the tuning config. David, what are your\n> thoughts?\n> >\n> > I agree it would be nice to get some kind of error or at least warning\n> > if something that we really expected is not present. Possibly we could\n> > review some of those and provide default values in more cases, but I'm\n> > guessing that's not the kind of thing to \"hide\" inside a patch that\n> > isn't meant to change the behaviour of any algorithms.\n>\n> I agree, that's a good point.\n>\n> The boost json parser will throw an exception in this case, which I\n> can't replicate in YamlObject. I can update the Read() function to\n> return an error, would it be enough to log a message at the top level,\n> or would you like individual log statements in every location to tell\n> which parameter was missing ?\n>\n\nHaving read() return an error code - provided no default is set is a good\nidea.\nThen the top level can decide how to handle this - a LOG(Fatal) message\nin our case. David, what do you think?\n\nRegards,\nNaush\n\n\n> > > Do you think this is something that we can add easily to this patch?\n> > >\n> > >> > ---\n> > >> > README.rst | 6 --\n> > >> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> > >> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> > >> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> > >> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94\n> +++++++++----------\n> > >> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94\n> +++++++++----------\n> > >> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89\n> +++++++++---------\n> > >> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> > >> > .../controller/rpi/black_level.cpp | 12 +--\n> > >> > .../controller/rpi/black_level.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> > >> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> > >> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> > >> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> > >> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> > >> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> > >> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> > >> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> > >> > src/ipa/raspberrypi/meson.build | 1 -\n> > >> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> > >> > 32 files changed, 241 insertions(+), 240 deletions(-)\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 0ED82C3275\n\tfor ;\n\tMon, 25 Jul 2022 14:48:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 62DE963312;\n\tMon, 25 Jul 2022 16:48:41 +0200 (CEST)","from mail-lf1-x132.google.com (mail-lf1-x132.google.com\n\t[IPv6:2a00:1450:4864:20::132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33BC96330A\n\tfor ;\n\tMon, 25 Jul 2022 16:48:40 +0200 (CEST)","by mail-lf1-x132.google.com with SMTP id q23so5181241lfr.3\n\tfor ;\n\tMon, 25 Jul 2022 07:48:40 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658760521;\n\tbh=ZxJWsA/jUCngQ/VgHSlZzRO1aLvNhbgE6y3Y09zCGAc=;\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=OLESrfO0zpulgBOFfYMEpj34+0cfo8/M3uaJPLeBM91KMoSFfYm24229/iN0WSB43\n\t21uZH0UxOKaN46hpHdnIzjJ/pSsKd8Hvb2yGgDwX8jms+syKGCmBCSNNnC5A8WUqBt\n\tIf8G42lkAMWj/5CHbuC79CranDzRsJdEogwAi3jshN29JKMpWuIcUpsb94q8VIUNzw\n\tdqIPDi0kfK6gMzT56LmWnTT2tpYe0pM+ycykv1VviALwZzEUfK/DjF848fW2mX445Y\n\tziAtnL5AIqVa90u9XxReN9tIGJWMLzo8jxe8oMtAEclX9i63Z2/uZ8K+m08iR5oPXd\n\t9b9kP9j8rVfcA==","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=x754MyiPPIi/h3fEZpdkBMe3lB15kiHUWLhHA8bt83U=;\n\tb=rmBDofWuHxv4RWTRCkvrfA5XpzYlNm0+aV29L5XXcyg/YVYFOQu9/xzBKtqj2fSlso\n\tx3XUrSljxd9qopblHKb+12kMT+LKDzRofYbMnURPdN2j0SnhP/C5mTymFtwqf5FWlncy\n\todM0wim/sop3GMaEziCZNUYlnYme97dY31lJ1PxlnI49v/w5wPz10s4ToLeNY/U5MuWM\n\tKpS7uW8t6i+Juz15Dwj33wrvTr6lFVQ9tCIfSrQehn3X/9tAuioDvPxXBl44XAMjNDqv\n\tscwtgA693BbEFLQXsV34aSjTXHdmVrXjg0wO+GUObxGLrJnEe7m+fvEwglqQjRzQ+C3M\n\tzVew=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"rmBDofWu\"; 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=x754MyiPPIi/h3fEZpdkBMe3lB15kiHUWLhHA8bt83U=;\n\tb=FWs4g1+PY2xwCH6gQ+nu/TH+VjB7TwvUcoQyZvyahqqJHoFiwik5j/CGqjxUJ4+02z\n\tVGOv+42qiYQEurH20MUW8C0804KcvcB6plqCLvILBa+UhitmXQItgRaDZu2kaeE0hJjp\n\tXYO3mJrYEh0C9tWrWrGpZuAnkqbIAkbm/EUdoyT55/UfRfjR1knLGxvQ8Cqs278KK1yJ\n\tTZOI6eFfYzMF+hakzUKv8KAa++8x98t0TsD2kEuwgtP/02s1wKMtHLKcFFV5ik8GDQc1\n\tbSx3tTzqqZboCCsrS1q6Wa52yR7xErWx1tUblsmxqEjWyIvc6uKfw60h4aws3WSFe740\n\tc2mw==","X-Gm-Message-State":"AJIora+69CW14Km2AMbrVZoYIPA3rodHL8y16ttLG8/mrKrdVUOi5Cjm\n\tds20CWCJ44C5vSGA9j2mnLKvI3KPRUnESRoM0fS5xg==","X-Google-Smtp-Source":"AGRyM1vVBHat02rNZjz9ej8ybkxWpX4XwM3L1cHgokewD6+2yjosxDgfXGGVyjBu4Zx+s/0qvPeIXvuCSLzGjNlyha4=","X-Received":"by 2002:ac2:4c4d:0:b0:48a:7a96:470d with SMTP id\n\to13-20020ac24c4d000000b0048a7a96470dmr4551732lfk.682.1658760518482;\n\tMon, 25 Jul 2022 07:48:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Mon, 25 Jul 2022 15:48:21 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000078d0c705e4a24763\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24113,"web_url":"https://patchwork.libcamera.org/comment/24113/","msgid":"","date":"2022-07-25T15:48:57","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nOn Mon, 25 Jul 2022 at 15:48, Naushir Patuck wrote:\n>\n> Hi Laurent,\n>\n> On Mon, 25 Jul 2022 at 14:50, Laurent Pinchart wrote:\n>>\n>> Hello,\n>>\n>> On Mon, Jul 25, 2022 at 11:50:06AM +0100, David Plowman wrote:\n>> > On Mon, 25 Jul 2022 at 11:26, Naushir Patuck wrote:\n>> > > On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart wrote:\n>> > >> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n>> > >> > From: Laurent Pinchart \n>> > >> >\n>> > >> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n>> > >> > tuning data files. As libcamera depends on libyaml, use the YamlParser\n>> > >> > class to parse those files and drop the dependency on boost.\n>> > >> >\n>> > >> > Signed-off-by: Laurent Pinchart \n>> > >>\n>> > >> I assume you're fine with this patch as you've posted it as part of your\n>> > >> series, but could you give it a review ? :-)\n>> > >\n>> > > I did have one outstanding query on this about default values.\n>> > >\n>> > > If you recall, we discussed having YamlObject::get() not take in a default value\n>> > > and complain loudly if the key is not present, similar to what the Boost parser\n>> > > does now. This will help avoid possibly hard to debug issues if a critical\n>> > > parameter is missing from the tuning config. David, what are your thoughts?\n>> >\n>> > I agree it would be nice to get some kind of error or at least warning\n>> > if something that we really expected is not present. Possibly we could\n>> > review some of those and provide default values in more cases, but I'm\n>> > guessing that's not the kind of thing to \"hide\" inside a patch that\n>> > isn't meant to change the behaviour of any algorithms.\n>>\n>> I agree, that's a good point.\n>>\n>> The boost json parser will throw an exception in this case, which I\n>> can't replicate in YamlObject. I can update the Read() function to\n>> return an error, would it be enough to log a message at the top level,\n>> or would you like individual log statements in every location to tell\n>> which parameter was missing ?\n>\n>\n> Having read() return an error code - provided no default is set is a good idea.\n> Then the top level can decide how to handle this - a LOG(Fatal) message\n> in our case. David, what do you think?\n\nWorks for me!\n\nDavid\n\n>\n> Regards,\n> Naush\n>\n>>\n>> > > Do you think this is something that we can add easily to this patch?\n>> > >\n>> > >> > ---\n>> > >> > README.rst | 6 --\n>> > >> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n>> > >> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n>> > >> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n>> > >> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n>> > >> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n>> > >> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n>> > >> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n>> > >> > .../controller/rpi/black_level.cpp | 12 +--\n>> > >> > .../controller/rpi/black_level.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n>> > >> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n>> > >> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n>> > >> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n>> > >> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n>> > >> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n>> > >> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n>> > >> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n>> > >> > src/ipa/raspberrypi/meson.build | 1 -\n>> > >> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n>> > >> > 32 files changed, 241 insertions(+), 240 deletions(-)\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 ECB5DC3275\n\tfor ;\n\tMon, 25 Jul 2022 15:49:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B029863312;\n\tMon, 25 Jul 2022 17:49:10 +0200 (CEST)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 597C76330A\n\tfor ;\n\tMon, 25 Jul 2022 17:49:08 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id oy13so21366765ejb.1\n\tfor ;\n\tMon, 25 Jul 2022 08:49:08 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658764150;\n\tbh=tlcxN2nLKKc/QNiNRyDKBNC70k6vbTzxDdCoIeSWBVc=;\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=vP3L+uGtM4l32hBxiodYu5QMy+Eg87wan7wm1VcwUE6AE7X4EBiwwGQ3BLwtCayjk\n\txDhfNBDJJLyilgbug9e0XxhgYpfcTsaxTPdHPuj7BH/vg2ekZeKY1m7e9itXrKU1qy\n\tqmHE2POLjaTCCgloQ+YQtKbQw4TJ4WWSx2iggOwAqJ0vsAQN4QYqByI2S6uHF4QL+S\n\tlBhBWs508dxJ9DvkjB1qLXux//n8Q7e8kBYvfFtzvwdbFbJakjL3SHUCPmDdIpKrwk\n\tzzyTPEZzTxafdfHfDTliiuyW9Se6zgszpoU1nbGBYOoO5b9YMB3e/4ZvqbXHvBJ6+7\n\trDZG/PMF0RVRw==","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=vx7oWCADfa8yI2J/BReqB/Z4x7QRqg6sQ1vW5dqwNyY=;\n\tb=oQAIucgQiT56bfAFFo+9e0oNnJek4xcVro3lK9LBtHKwz+k8MzkkQDMYJxOV3b8npM\n\tFQYwwiNlv6XqzTaJu11VbzwSrZA5aZI+cZKcOZbABZYz3a80memimngYJGPSWvlh4i1g\n\t681GeqqBdYMPFJyCJLmNH6xe4XZrTaTqXbeIKDVjhBOg7USvR3vAIb8fqVfEyReyKITk\n\tOjvA/H6UW4WIHRA9aISoz7pC+0Y1T9F5Gh2RWJFzuBXpE8c8psvbTcFFgXZf7KRDPGQi\n\tLz9xB2dA+fCZrW66gj3I/I29ygnvL0tUorJtmk0fwAY5CZ3gKIUfLWhyNEYaV2dC8Dr+\n\teiyw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"oQAIucgQ\"; 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=vx7oWCADfa8yI2J/BReqB/Z4x7QRqg6sQ1vW5dqwNyY=;\n\tb=5MLHrke6Vo1Fvqyhee191oKyOVQdR/w4VqU+E2FihD2Zy+Bn2yXz/DMGMm0S7TEcq4\n\tsQjbseol8GcTpP9axl68Pr7SQfqFL4xmkIgHzvYPs5yXd/5/W/PB0SlR3qnJ/nhfI+It\n\taq1FKXalUx6wQeHEUl5mjqR2tDtGwcD0QxFjGjQtur8wn8DWOHgeqVv2lnu52FIBZ2wA\n\t60A3FbMd872KlSqYro70XcdhW6jeK9ox4TjG0EDWHWGyj+CJYOVfyHyC67KwAw/gQT/C\n\tT10CK5jqvHlHUMKsrjbpGclK+OnG6tKQtKTrRQChaQZED2sFSKGOxeDQtlzA37SqoK/F\n\ta+Mg==","X-Gm-Message-State":"AJIora/Fx+3v7bQo8kJg8TueRk1kX9O8h01JMPJMqjaCXk/3DY3ayytp\n\tyWAHqIM0v7sU1amTSg5/4VLjx2G/PCQYZto5IOQ2JQ==","X-Google-Smtp-Source":"AGRyM1t17VmZdjSyTZG3F3H6XzpQypa6N3Lly3puhIzctbiZMSf6o4OLaQUuxZueLi8LI6CeHqqkwRH0WdbZ9gkL+rU=","X-Received":"by 2002:a17:907:7f1a:b0:72e:f9c9:dfe2 with SMTP id\n\tqf26-20020a1709077f1a00b0072ef9c9dfe2mr10660151ejc.84.1658764147994;\n\tMon, 25 Jul 2022 08:49:07 -0700 (PDT)","MIME-Version":"1.0","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Mon, 25 Jul 2022 16:48:57 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24138,"web_url":"https://patchwork.libcamera.org/comment/24138/","msgid":"","date":"2022-07-26T07:04:57","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Mon, 25 Jul 2022 at 16:49, David Plowman \nwrote:\n\n> Hi everyone\n>\n> On Mon, 25 Jul 2022 at 15:48, Naushir Patuck \n> wrote:\n> >\n> > Hi Laurent,\n> >\n> > On Mon, 25 Jul 2022 at 14:50, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> >>\n> >> Hello,\n> >>\n> >> On Mon, Jul 25, 2022 at 11:50:06AM +0100, David Plowman wrote:\n> >> > On Mon, 25 Jul 2022 at 11:26, Naushir Patuck \n> wrote:\n> >> > > On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> >> > >> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> >> > >> > From: Laurent Pinchart \n> >> > >> >\n> >> > >> > The Raspberry Pi IPA module depends on boost only to parse the\n> JSON\n> >> > >> > tuning data files. As libcamera depends on libyaml, use the\n> YamlParser\n> >> > >> > class to parse those files and drop the dependency on boost.\n> >> > >> >\n> >> > >> > Signed-off-by: Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com>\n> >> > >>\n> >> > >> I assume you're fine with this patch as you've posted it as part\n> of your\n> >> > >> series, but could you give it a review ? :-)\n> >> > >\n> >> > > I did have one outstanding query on this about default values.\n> >> > >\n> >> > > If you recall, we discussed having YamlObject::get() not take in a\n> default value\n> >> > > and complain loudly if the key is not present, similar to what the\n> Boost parser\n> >> > > does now. This will help avoid possibly hard to debug issues if a\n> critical\n> >> > > parameter is missing from the tuning config. David, what are your\n> thoughts?\n> >> >\n> >> > I agree it would be nice to get some kind of error or at least warning\n> >> > if something that we really expected is not present. Possibly we could\n> >> > review some of those and provide default values in more cases, but I'm\n> >> > guessing that's not the kind of thing to \"hide\" inside a patch that\n> >> > isn't meant to change the behaviour of any algorithms.\n> >>\n> >> I agree, that's a good point.\n> >>\n> >> The boost json parser will throw an exception in this case, which I\n> >> can't replicate in YamlObject. I can update the Read() function to\n> >> return an error, would it be enough to log a message at the top level,\n> >> or would you like individual log statements in every location to tell\n> >> which parameter was missing ?\n> >\n> >\n> > Having read() return an error code - provided no default is set is a\n> good idea.\n> > Then the top level can decide how to handle this - a LOG(Fatal) message\n> > in our case. David, what do you think?\n>\n> Works for me!\n>\n\nWould it make sense to use std::optional as a return value from\nYamlObject::get() in\na similar way to what we now do for ControlList::get()?\n\nNaush\n\n\n>\n> David\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >>\n> >> > > Do you think this is something that we can add easily to this patch?\n> >> > >\n> >> > >> > ---\n> >> > >> > README.rst | 6 --\n> >> > >> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> >> > >> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> >> > >> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> >> > >> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94\n> +++++++++----------\n> >> > >> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94\n> +++++++++----------\n> >> > >> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89\n> +++++++++---------\n> >> > >> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> >> > >> > .../controller/rpi/black_level.cpp | 12 +--\n> >> > >> > .../controller/rpi/black_level.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> >> > >> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> >> > >> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> >> > >> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> >> > >> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> >> > >> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> >> > >> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> >> > >> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> >> > >> > src/ipa/raspberrypi/meson.build | 1 -\n> >> > >> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> >> > >> > 32 files changed, 241 insertions(+), 240 deletions(-)\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 ED602BE173\n\tfor ;\n\tTue, 26 Jul 2022 07:05:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2943A63312;\n\tTue, 26 Jul 2022 09:05:16 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EAD3A6330D\n\tfor ;\n\tTue, 26 Jul 2022 09:05:13 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id y11so21179225lfs.6\n\tfor ;\n\tTue, 26 Jul 2022 00:05:13 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658819116;\n\tbh=JDqeoQpbFEVD+Qzb1BpU87+wRAxDs2isR9QZU7EoWRw=;\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=NN2yOKnHp7Y0UWUTIm1znMDFy23wErzPT6CMhghuM+uDFvuDezzaVb1h6LbT/rkxB\n\t+XaH1h4UWmGzUAqcnt3mpIIHXFbYk1CwCPautHqyarz0VCFBof9w8djMxTApLGjIrS\n\tWAVyZt6GCK73c1StmeoZwECo4KmsBIo+xPq77wlMX3EVIYHk12i3VwM+W3aCYqWAgf\n\txfGW96WFXY1rLreATRu3Dae3mV5vxdRtgE33geA/9gkmHQ4seXgrctaZS4mbXfd84l\n\ta43WqoPCTI9+HIJd4RPAAo0XufL2z3fFzo3SamKWAh6WV/cXLp1TTVUEqNtks718ao\n\tBYEEC4Bu0xVAQ==","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=3pTzRZLSukLQlWEu8DoaJvEwPa9L8lMz8l9ApAQNUrw=;\n\tb=STZN5e7Pi5WpHlmGpp/LIPinCPxOAmlLfhG03nb4XvzaAinFFfkaSzZC8tpFgVH8NV\n\t896t1N3p3XnpLn56Zl4+lg+FhhG6ylC/JWQ7tuNIbM85udaOEsNVat34VsOZXPNjyfOr\n\tzt2wjoV/eCRHYVwzEuv+xYktX4V50eXR83qhvs/4BzvQUnknyAi9ZVjcJV3hbB/E8+Qd\n\tNhbE2XlhjQlAD9/2g19cS0dfkEy9DWdqdu2B7z7foWriyWhyPIucPSWH1YIV9qBdA8MB\n\t9atP7vKf02FPSgeXll8InT/qvFSHa2JFwENnrCIINkiSD1o8EQh++jbyqGJtpAPUe3aM\n\tQIKQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"STZN5e7P\"; 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=3pTzRZLSukLQlWEu8DoaJvEwPa9L8lMz8l9ApAQNUrw=;\n\tb=FcOkOvPC0xByN8pFrG0JDn5mR6Alp6h0rTgo7JsaoJr94v7hl1dotoe2clpGQicMCC\n\tJgMfh7eJCFfKo0ZdAlmfMdpl/wNfZR+2xBFeKFnzPIsNr9xZvdRsjzep5qoFds5czIUo\n\tnABiW0lsheKiQ2CWfFvtlDdlyRfto5aA80Ja5UQ++ySxf3JacgKv2LPfcZ5mOBIietYT\n\tNJTNJC1r6bQgnX+KQlPQ7hj09G5e0HSeeimMuZ9NItl3t6e+FqUvZC0i5LLX2b1wr4Ow\n\tFT+JfPs+Oog8WZEqS2uPOEciqaWeDvy2Cof4cZcPWeD9kumHeJGlp5SYkZNygkJPQ4/B\n\tad3Q==","X-Gm-Message-State":"AJIora8mCmFUKrjoN98cL77p2gsa0dO7eOdOe5ny6AxI42r+wJJd9qW6\n\tX4hYBmIVXBeeVL/k5Kt2ZU44NsD6ti2v0VqAQVm2rA==","X-Google-Smtp-Source":"AGRyM1tlSZJh0tL3ZHH9w2go9axiLo+YD/wmCOsVhQ5TN6wvg6A26Xfginrl1i7mOATrBkpYwDRg5xXzPyOAx6Eooyk=","X-Received":"by 2002:a05:6512:3c91:b0:48a:9b42:670b with SMTP id\n\th17-20020a0565123c9100b0048a9b42670bmr1800949lfv.38.1658819112726;\n\tTue, 26 Jul 2022 00:05:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","Date":"Tue, 26 Jul 2022 08:04:57 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000f6074705e4afebac\"","Subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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":24140,"web_url":"https://patchwork.libcamera.org/comment/24140/","msgid":"","date":"2022-07-26T08:15:08","subject":"Re: [libcamera-devel] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Tue, Jul 26, 2022 at 08:04:57AM +0100, Naushir Patuck wrote:\n> On Mon, 25 Jul 2022 at 16:49, David Plowman wrote:\n> > On Mon, 25 Jul 2022 at 15:48, Naushir Patuck wrote:\n> > > On Mon, 25 Jul 2022 at 14:50, Laurent Pinchart wrote:\n> > >> On Mon, Jul 25, 2022 at 11:50:06AM +0100, David Plowman wrote:\n> > >> > On Mon, 25 Jul 2022 at 11:26, Naushir Patuck wrote:\n> > >> > > On Fri, 22 Jul 2022 at 20:25, Laurent Pinchart wrote:\n> > >> > >> On Mon, Jul 18, 2022 at 09:15:58AM +0100, Naushir Patuck wrote:\n> > >> > >> > From: Laurent Pinchart \n> > >> > >> >\n> > >> > >> > The Raspberry Pi IPA module depends on boost only to parse the JSON\n> > >> > >> > tuning data files. As libcamera depends on libyaml, use the YamlParser\n> > >> > >> > class to parse those files and drop the dependency on boost.\n> > >> > >> >\n> > >> > >> > Signed-off-by: Laurent Pinchart \n> > >> > >>\n> > >> > >> I assume you're fine with this patch as you've posted it as part of your\n> > >> > >> series, but could you give it a review ? :-)\n> > >> > >\n> > >> > > I did have one outstanding query on this about default values.\n> > >> > >\n> > >> > > If you recall, we discussed having YamlObject::get() not take in a default value\n> > >> > > and complain loudly if the key is not present, similar to what the Boost parser\n> > >> > > does now. This will help avoid possibly hard to debug issues if a critical\n> > >> > > parameter is missing from the tuning config. David, what are your thoughts?\n> > >> >\n> > >> > I agree it would be nice to get some kind of error or at least warning\n> > >> > if something that we really expected is not present. Possibly we could\n> > >> > review some of those and provide default values in more cases, but I'm\n> > >> > guessing that's not the kind of thing to \"hide\" inside a patch that\n> > >> > isn't meant to change the behaviour of any algorithms.\n> > >>\n> > >> I agree, that's a good point.\n> > >>\n> > >> The boost json parser will throw an exception in this case, which I\n> > >> can't replicate in YamlObject. I can update the Read() function to\n> > >> return an error, would it be enough to log a message at the top level,\n> > >> or would you like individual log statements in every location to tell\n> > >> which parameter was missing ?\n> > >\n> > > Having read() return an error code - provided no default is set is a good idea.\n> > > Then the top level can decide how to handle this - a LOG(Fatal) message\n> > > in our case. David, what do you think?\n> >\n> > Works for me!\n> \n> Would it make sense to use std::optional as a return value from YamlObject::get() in\n> a similar way to what we now do for ControlList::get()?\n\nFunny you mention that, it took part of last night to implement it :-)\nI'll post patches soon.\n\n> > >> > > Do you think this is something that we can add easily to this patch?\n> > >> > >\n> > >> > >> > ---\n> > >> > >> > README.rst | 6 --\n> > >> > >> > src/ipa/raspberrypi/controller/algorithm.cpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/algorithm.hpp | 6 +-\n> > >> > >> > src/ipa/raspberrypi/controller/controller.cpp | 27 ++++--\n> > >> > >> > src/ipa/raspberrypi/controller/pwl.cpp | 12 ++-\n> > >> > >> > src/ipa/raspberrypi/controller/pwl.hpp | 5 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 94 +++++++++----------\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 10 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/alsc.cpp | 94 +++++++++----------\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 89 +++++++++---------\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 8 +-\n> > >> > >> > .../controller/rpi/black_level.cpp | 12 +--\n> > >> > >> > .../controller/rpi/black_level.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/ccm.cpp | 28 +++---\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/ccm.hpp | 4 +-\n> > >> > >> > .../raspberrypi/controller/rpi/contrast.cpp | 18 ++--\n> > >> > >> > .../raspberrypi/controller/rpi/contrast.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/dpc.cpp | 4 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/dpc.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/geq.cpp | 10 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/geq.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/lux.cpp | 12 +--\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/lux.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/noise.cpp | 6 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/sdn.cpp | 6 +-\n> > >> > >> > src/ipa/raspberrypi/controller/rpi/sdn.hpp | 2 +-\n> > >> > >> > .../raspberrypi/controller/rpi/sharpen.cpp | 8 +-\n> > >> > >> > .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-\n> > >> > >> > src/ipa/raspberrypi/meson.build | 1 -\n> > >> > >> > src/ipa/raspberrypi/raspberrypi.cpp | 1 +\n> > >> > >> > 32 files changed, 241 insertions(+), 240 deletions(-)","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 E0CB7BE173\n\tfor ;\n\tTue, 26 Jul 2022 08:15:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0FDF963312;\n\tTue, 26 Jul 2022 10:15:15 +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 737486330D\n\tfor ;\n\tTue, 26 Jul 2022 10:15:13 +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 DC319735;\n\tTue, 26 Jul 2022 10:15:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658823315;\n\tbh=VBfUw9O8MUf1AlJFlOXYxxidJhADGIGDXh+Eyb6nvZw=;\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=LqViQDMUULxFu7q40ErJ6otRCgLyv4ko5S9FCkZJJBBy+uiWnDmgNekqFZiEp37MP\n\turZxFOjHwUVWuFwenSSgJbMnYJ0/1mG8draPuMUQffIE5NIv6kt868WsRdjg3y0fn+\n\t8TgC4grsTzlburvcYDFWVNBsHGGdmQsfdPMEzZUBKLXxJFw+PF0QZkg2bj6qbCxZ0a\n\tfqF08dJg8LIE1623fOkq+4u0aPukgPYWgH5y0/YepRNr1q/lId7r0edfHy+jM8lDYI\n\tkw5oEuR7iqSyYJ1yUaPNOz53DX3zQQp35AHGC2FIWcSGA1UitRvDODwUV1YS2DM5BY\n\to6vXBDK1PQadQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658823313;\n\tbh=VBfUw9O8MUf1AlJFlOXYxxidJhADGIGDXh+Eyb6nvZw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I8rHOxB+7e8iO2sXn/JXX5z6UlIfGk+RzWxe1mMV9V2Mandh3YPpDXoiD2YRKkA/t\n\tg4xr/yP4kMOfu276FHelZlW8QFCUSKlTgdoGu2P1ZOjdFIBlupvl27oOOukQAbegfV\n\tQc7mNwG8P3a4uEbdDH3NDX/+2XgfcVFYR68Jrg/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=\"I8rHOxB+\"; dkim-atps=neutral","Date":"Tue, 26 Jul 2022 11:15:08 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220718081602.32535-1-naush@raspberrypi.com>\n\t<20220718081602.32535-5-naush@raspberrypi.com>\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] [PATCH v6 4/8] ipa: raspberrypi: Use\n\tYamlParser to replace dependency on boost","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\" "}}]