[{"id":13831,"web_url":"https://patchwork.libcamera.org/comment/13831/","msgid":"<4c7e7d50-5959-033c-6484-b8475fb10f89@ideasonboard.com>","date":"2020-11-23T16:50:34","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Kieran,\n\n\nOn 23/11/2020 16:43, Kieran Bingham wrote:\n> Provide an interface to support reading configuration files.\n> \n> Initial support is included for JSON formatted files, but extending this\n> with other configuration file formats is not excluded.\n> \n\nAhem, that was an earlier design intention, which now that this class\ndirectly exposes a json object - does not currently hold true.\n\nOf course all of this is internal, so things can be refactored later -\nbut right now - Configuration() supports only JSON.\n\n--\nKieran\n\n\n> Signed-off-by: Kieran Bingham \n> ---\n> README.rst | 2 +-\n> include/libcamera/internal/configuration.h | 37 +++++++++\n> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 130 insertions(+), 1 deletion(-)\n> create mode 100644 include/libcamera/internal/configuration.h\n> create mode 100644 src/libcamera/configuration.cpp\n> \n> diff --git a/README.rst b/README.rst\n> index 251291b77c62..c09e262fcc43 100644\n> --- a/README.rst\n> +++ b/README.rst\n> @@ -58,7 +58,7 @@ Meson Build system: [required]\n> pip3 install --user --upgrade meson\n> \n> for the libcamera core: [required]\n> - python3-yaml python3-ply python3-jinja2\n> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n> \n> for IPA module signing: [required]\n> libgnutls28-dev openssl\n> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n> new file mode 100644\n> index 000000000000..a89732f0210f\n> --- /dev/null\n> +++ b/include/libcamera/internal/configuration.h\n> @@ -0,0 +1,37 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * configuration.h - Parsing configuration files\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> +\n> +#include \n> +#include \n> +\n> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n> +#define JSON_NOEXCEPTION 1\n> +#include \n> +\n> +using json = nlohmann::json;\n> +\n> +namespace libcamera {\n> +\n> +class Configuration\n> +{\n> +public:\n> +\tint open(std::string filename);\n> +\n> +\tjson &data() { return json_; }\n> +\n> +private:\n> +\tstd::string findFile(std::string filename);\n> +\n> +\tjson json_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n> +\n> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n> new file mode 100644\n> index 000000000000..f849088bbc45\n> --- /dev/null\n> +++ b/src/libcamera/configuration.cpp\n> @@ -0,0 +1,91 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * configuration.cpp - Parsing configuration files\n> + */\n> +#include \"libcamera/internal/configuration.h\"\n> +\n> +#include \"libcamera/internal/file.h\"\n> +#include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/utils.h\"\n> +\n> +#include \n> +#include \n> +\n> +/**\n> + * \\file configuration.h\n> + * \\brief Read interface for configuration files\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Configuration)\n> +\n> +/*\n> + * Configuration files can be stored in system paths, which are identified\n> + * through the build configuration.\n> + *\n> + * However, when running uninstalled - the source location takes precedence.\n> + */\n> +std::string Configuration::findFile(std::string filename)\n> +{\n> +\tstatic std::array searchPaths = {\n> +\t\tLIBCAMERA_SYSCONF_DIR,\n> +\t\tLIBCAMERA_DATA_DIR,\n> +\t};\n> +\n> +\tstd::string root = utils::libcameraSourcePath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n> +\n> +\t\tif (File::exists(configurationPath))\n> +\t\t\treturn configurationPath;\n> +\t}\n> +\n> +\tfor (std::string &path : searchPaths) {\n> +\t\tstd::string configurationPath = path + \"/\" + filename;\n> +\t\tif (File::exists(configurationPath))\n> +\t\t\treturn configurationPath;\n> +\t}\n> +\n> +\treturn \"\";\n> +}\n> +\n> +/**\n> + * \\brief Open and parse a configuration file.\n> + *\n> + * The filename will be searched for on the libcamera configuration and paths,\n> + * and then parsed.\n> + *\n> + * Successfully parsed files will present the data contained therein through the\n> + * json object exposed from data();\n> + */\n> +int Configuration::open(std::string filename)\n> +{\n> +\tstd::string data = findFile(filename);\n> +\tif (data.empty()) {\n> +\t\tLOG(Configuration, Warning)\n> +\t\t\t<< \"file: \\\"\" << filename\n> +\t\t\t<< \"\\\" was not found.\";\n> +\t\treturn -ENOENT;\n> +\t}\n> +\n> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n> +\n> +\t/* Parse with no error callbacks and exceptions disabled. */\n> +\tstd::ifstream input(data);\n> +\tjson j = json::parse(input, nullptr, false);\n> +\tif (j.is_discarded()) {\n> +\t\tLOG(Configuration, Error)\n> +\t\t\t<< \"file: \\\"\" << data\n> +\t\t\t<< \"\\\" was not parsable.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tjson_ = std::move(j);\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 387d5d88ecae..5d655c87a7a0 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -9,6 +9,7 @@ libcamera_sources = files([\n> 'camera_controls.cpp',\n> 'camera_manager.cpp',\n> 'camera_sensor.cpp',\n> + 'configuration.cpp',\n> 'controls.cpp',\n> 'control_serializer.cpp',\n> 'control_validator.cpp',\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 BE4EBBE176\n\tfor ;\n\tMon, 23 Nov 2020 16:50:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 52DD2633F4;\n\tMon, 23 Nov 2020 17:50:40 +0100 (CET)","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 6D221633F0\n\tfor ;\n\tMon, 23 Nov 2020 17:50:38 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CDC9B71;\n\tMon, 23 Nov 2020 17:50:37 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PQBM0i9P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606150237;\n\tbh=RX922DHqURtQVl/uFYkORgqj57QVWxVUQCosbDkHja4=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=PQBM0i9PGaK6pKxM6I0ZlB4jOL9lYEbKifuWkrFv27ONzTI3+beEag5omX7rq4lr+\n\tcFAVCpULvWeIWJEJF/Qu55Bl2g9XQ9kKoFJoAG4g2vrdHypEi68BMhkdOxvVxrRVRK\n\tUAvbY/fijD5FwIQNmk+drJnUPfym4BGoqElSlmnw=","To":"Kieran Bingham ,\n\tLibCamera Devel ","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<4c7e7d50-5959-033c-6484-b8475fb10f89@ideasonboard.com>","Date":"Mon, 23 Nov 2020 16:50:34 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201123164319.152742-7-kieran.bingham@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13869,"web_url":"https://patchwork.libcamera.org/comment/13869/","msgid":"<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>","date":"2020-11-25T08:52:57","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n> Provide an interface to support reading configuration files.\n>\n> Initial support is included for JSON formatted files, but extending this\n> with other configuration file formats is not excluded.\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> README.rst | 2 +-\n> include/libcamera/internal/configuration.h | 37 +++++++++\n> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 130 insertions(+), 1 deletion(-)\n> create mode 100644 include/libcamera/internal/configuration.h\n> create mode 100644 src/libcamera/configuration.cpp\n>\n> diff --git a/README.rst b/README.rst\n> index 251291b77c62..c09e262fcc43 100644\n> --- a/README.rst\n> +++ b/README.rst\n> @@ -58,7 +58,7 @@ Meson Build system: [required]\n> pip3 install --user --upgrade meson\n>\n> for the libcamera core: [required]\n> - python3-yaml python3-ply python3-jinja2\n> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n\nFirst question I have is about the dependency on this package.\n\nI have no real reasons to doubt about the availability of a packaged\nversion in most linux distro, nor about their stability and update\nrate. It might be that projects named after a single person always\nsound fragile to me, might be the rather low bus factor :)\n\nJoking aside this seems to be packaged in most distros\nhttps://repology.org/project/nlohmann-json/versions\nwith rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\nwhich is an LTS if I'm not mistaken).\n\nThere are notable exception though, in example Linux Mint.\n\nIn ChromiumOS R87 I don't see it packaged, my equery skills are very\nlow so I might have missed it.\n\nThe real question is:\n- Are we happy to have this as externally packaged dependency ?\n\nIf I look at the Arch package content:\nhttps://www.archlinux.org/packages/community/any/nlohmann-json/files/\n\nThis seems to be an header-only library. I wonder if we should not\nrope it in during the build. There seems to be built-in support in\nmeson for that:\nhttps://github.com/nlohmann/json#package-managers\nhttps://wrapdb.mesonbuild.com/nlohmann_json\n\nOr include ourselves a version of the library in our source tree,\nwhich opens a different question: how do we feel about distributing MIT\nlicensed code ? It should be no issue, but IANAL so it's better to\ngive this a thought.\n\nThanks\n j\n\n>\n> for IPA module signing: [required]\n> libgnutls28-dev openssl\n> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n> new file mode 100644\n> index 000000000000..a89732f0210f\n> --- /dev/null\n> +++ b/include/libcamera/internal/configuration.h\n> @@ -0,0 +1,37 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * configuration.h - Parsing configuration files\n> + */\n> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> +\n> +#include \n> +#include \n> +\n> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n> +#define JSON_NOEXCEPTION 1\n> +#include \n> +\n> +using json = nlohmann::json;\n> +\n> +namespace libcamera {\n> +\n> +class Configuration\n> +{\n> +public:\n> +\tint open(std::string filename);\n> +\n> +\tjson &data() { return json_; }\n> +\n> +private:\n> +\tstd::string findFile(std::string filename);\n> +\n> +\tjson json_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n> +\n> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n> new file mode 100644\n> index 000000000000..f849088bbc45\n> --- /dev/null\n> +++ b/src/libcamera/configuration.cpp\n> @@ -0,0 +1,91 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * configuration.cpp - Parsing configuration files\n> + */\n> +#include \"libcamera/internal/configuration.h\"\n> +\n> +#include \"libcamera/internal/file.h\"\n> +#include \"libcamera/internal/log.h\"\n> +#include \"libcamera/internal/utils.h\"\n> +\n> +#include \n> +#include \n> +\n> +/**\n> + * \\file configuration.h\n> + * \\brief Read interface for configuration files\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Configuration)\n> +\n> +/*\n> + * Configuration files can be stored in system paths, which are identified\n> + * through the build configuration.\n> + *\n> + * However, when running uninstalled - the source location takes precedence.\n> + */\n> +std::string Configuration::findFile(std::string filename)\n> +{\n> +\tstatic std::array searchPaths = {\n> +\t\tLIBCAMERA_SYSCONF_DIR,\n> +\t\tLIBCAMERA_DATA_DIR,\n> +\t};\n> +\n> +\tstd::string root = utils::libcameraSourcePath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n> +\n> +\t\tif (File::exists(configurationPath))\n> +\t\t\treturn configurationPath;\n> +\t}\n> +\n> +\tfor (std::string &path : searchPaths) {\n> +\t\tstd::string configurationPath = path + \"/\" + filename;\n> +\t\tif (File::exists(configurationPath))\n> +\t\t\treturn configurationPath;\n> +\t}\n> +\n> +\treturn \"\";\n> +}\n> +\n> +/**\n> + * \\brief Open and parse a configuration file.\n> + *\n> + * The filename will be searched for on the libcamera configuration and paths,\n> + * and then parsed.\n> + *\n> + * Successfully parsed files will present the data contained therein through the\n> + * json object exposed from data();\n> + */\n> +int Configuration::open(std::string filename)\n> +{\n> +\tstd::string data = findFile(filename);\n> +\tif (data.empty()) {\n> +\t\tLOG(Configuration, Warning)\n> +\t\t\t<< \"file: \\\"\" << filename\n> +\t\t\t<< \"\\\" was not found.\";\n> +\t\treturn -ENOENT;\n> +\t}\n> +\n> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n> +\n> +\t/* Parse with no error callbacks and exceptions disabled. */\n> +\tstd::ifstream input(data);\n> +\tjson j = json::parse(input, nullptr, false);\n> +\tif (j.is_discarded()) {\n> +\t\tLOG(Configuration, Error)\n> +\t\t\t<< \"file: \\\"\" << data\n> +\t\t\t<< \"\\\" was not parsable.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tjson_ = std::move(j);\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 387d5d88ecae..5d655c87a7a0 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -9,6 +9,7 @@ libcamera_sources = files([\n> 'camera_controls.cpp',\n> 'camera_manager.cpp',\n> 'camera_sensor.cpp',\n> + 'configuration.cpp',\n> 'controls.cpp',\n> 'control_serializer.cpp',\n> 'control_validator.cpp',\n> --\n> 2.25.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 6AC6CBE176\n\tfor ;\n\tWed, 25 Nov 2020 08:52:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0193663409;\n\tWed, 25 Nov 2020 09:52:55 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBC1E632EE\n\tfor ;\n\tWed, 25 Nov 2020 09:52:53 +0100 (CET)","from uno.localdomain (host-80-104-176-17.retail.telecomitalia.it\n\t[80.104.176.17]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D896CFF80F;\n\tWed, 25 Nov 2020 08:52:52 +0000 (UTC)"],"X-Originating-IP":"80.104.176.17","Date":"Wed, 25 Nov 2020 09:52:57 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201123164319.152742-7-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13870,"web_url":"https://patchwork.libcamera.org/comment/13870/","msgid":"<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>","date":"2020-11-25T09:11:10","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 25/11/2020 08:52, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n>> Provide an interface to support reading configuration files.\n>>\n>> Initial support is included for JSON formatted files, but extending this\n>> with other configuration file formats is not excluded.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> README.rst | 2 +-\n>> include/libcamera/internal/configuration.h | 37 +++++++++\n>> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n>> src/libcamera/meson.build | 1 +\n>> 4 files changed, 130 insertions(+), 1 deletion(-)\n>> create mode 100644 include/libcamera/internal/configuration.h\n>> create mode 100644 src/libcamera/configuration.cpp\n>>\n>> diff --git a/README.rst b/README.rst\n>> index 251291b77c62..c09e262fcc43 100644\n>> --- a/README.rst\n>> +++ b/README.rst\n>> @@ -58,7 +58,7 @@ Meson Build system: [required]\n>> pip3 install --user --upgrade meson\n>>\n>> for the libcamera core: [required]\n>> - python3-yaml python3-ply python3-jinja2\n>> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n> \n> First question I have is about the dependency on this package.\n> \n> I have no real reasons to doubt about the availability of a packaged\n> version in most linux distro, nor about their stability and update\n> rate. It might be that projects named after a single person always\n> sound fragile to me, might be the rather low bus factor :)\n\n\nYes, I find the 'named after me' packaging a little ... awkward. But\nthere's really two choices for json libraries if we don't want boost.\nnlohmann-json and json-cpp.\n\nFor bus factor, nlohmann-json has 178 contributors ... so I don't think\nit's just 'one guy'.\n\nI had planned to try both in earnest, and subclass Configuration so that\nthey could be swapped at will, but the nlohmann was seemingly easier to\nget started with in the end, and subclassing to swap would mean wrapping\nthe nlohmann generic container types in a generic container type and\ncreating my own tree structures.\n\nWe can do that of course, but that will take longer, and I honestly\ncouldn't see the point of wrapping it for the internal usages (yet).\n\nIf this was public API then it would be different.\n\nAnd maybe we'll update/change later - but I wanted to get things\nactually going so we can start figuring out what and where we store data\nin the first place.\n\nWho knows, maybe Configuration() will sometime need to parse yaml files\nso it will build a ConfigurationJSON or a ConfigurationYAML depending on\nthe data - but I don't want to over engineer just to be able to read\nsome key-values at this stage.\n\n> Joking aside this seems to be packaged in most distros\n> https://repology.org/project/nlohmann-json/versions\n> with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n> which is an LTS if I'm not mistaken).\n\nIt did seem to be quite well packaged, but I would probably lean towards\nbringing the headers in under third_party/ or such as it's header only\nto prevent any potential issues.\n\n\n> There are notable exception though, in example Linux Mint.\n> \n> In ChromiumOS R87 I don't see it packaged, my equery skills are very\n> low so I might have missed it.\n\nAlso, while available in the Raspbian OS distribution - it's an older\nversion which didn't have a feature I was using \"j.contains()\".\n\nI thought that was actually a nicer way to implement the code rather\nthan using iterators and j.find(), even if the .find() is perhaps more\nefficient as it will only do one look up - I think the code just doesn't\nflow quite as nicely. (Having to suddenly dereference iterators to a new\ntype etc...)\n\n\n> The real question is:\n> - Are we happy to have this as externally packaged dependency ?\n> \n> If I look at the Arch package content:\n> https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n> \n> This seems to be an header-only library. I wonder if we should not\n> rope it in during the build. There seems to be built-in support in\n> meson for that:\n> https://github.com/nlohmann/json#package-managers\n> https://wrapdb.mesonbuild.com/nlohmann_json\n\nIt is in wrapdb - but it's out of date.\n\nWe can either include the headers in libcamera, or we can update the\nwrapdb ;-)\n\nI'd probably lean towards updating the wrapdb as that's more beneficial\neverywhere.\n\nThe meson wraps seem to work quite well - and will seemlessly download\nthe package if it's missing and build/use that.\n\nPart of me is then weary of requiring an internet connection for that -\nbut I really don't' think that's an issue anymore, as if you've cloned\nthe repo - we can expect that there is a connection.\n\n\n\n> Or include ourselves a version of the library in our source tree,\n> which opens a different question: how do we feel about distributing MIT\n> licensed code ? It should be no issue, but IANAL so it's better to\n> give this a thought.\n\nThat's also what makes me lean towards updating and using the wrapdb ;-)\n\nAlternatively of course we can rewrite all this with json-cpp\n(https://github.com/open-source-parsers/jsoncpp) - but then we'll have\nexactly the same discussion on that library/package too ;-)\n\nKieran\n\n\n> Thanks\n> j\n> \n>>\n>> for IPA module signing: [required]\n>> libgnutls28-dev openssl\n>> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n>> new file mode 100644\n>> index 000000000000..a89732f0210f\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/configuration.h\n>> @@ -0,0 +1,37 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * configuration.h - Parsing configuration files\n>> + */\n>> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>> +\n>> +#include \n>> +#include \n>> +\n>> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n>> +#define JSON_NOEXCEPTION 1\n>> +#include \n>> +\n>> +using json = nlohmann::json;\n>> +\n>> +namespace libcamera {\n>> +\n>> +class Configuration\n>> +{\n>> +public:\n>> +\tint open(std::string filename);\n>> +\n>> +\tjson &data() { return json_; }\n>> +\n>> +private:\n>> +\tstd::string findFile(std::string filename);\n>> +\n>> +\tjson json_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n>> +\n>> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n>> new file mode 100644\n>> index 000000000000..f849088bbc45\n>> --- /dev/null\n>> +++ b/src/libcamera/configuration.cpp\n>> @@ -0,0 +1,91 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * configuration.cpp - Parsing configuration files\n>> + */\n>> +#include \"libcamera/internal/configuration.h\"\n>> +\n>> +#include \"libcamera/internal/file.h\"\n>> +#include \"libcamera/internal/log.h\"\n>> +#include \"libcamera/internal/utils.h\"\n>> +\n>> +#include \n>> +#include \n>> +\n>> +/**\n>> + * \\file configuration.h\n>> + * \\brief Read interface for configuration files\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(Configuration)\n>> +\n>> +/*\n>> + * Configuration files can be stored in system paths, which are identified\n>> + * through the build configuration.\n>> + *\n>> + * However, when running uninstalled - the source location takes precedence.\n>> + */\n>> +std::string Configuration::findFile(std::string filename)\n>> +{\n>> +\tstatic std::array searchPaths = {\n>> +\t\tLIBCAMERA_SYSCONF_DIR,\n>> +\t\tLIBCAMERA_DATA_DIR,\n>> +\t};\n>> +\n>> +\tstd::string root = utils::libcameraSourcePath();\n>> +\tif (!root.empty()) {\n>> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n>> +\n>> +\t\tif (File::exists(configurationPath))\n>> +\t\t\treturn configurationPath;\n>> +\t}\n>> +\n>> +\tfor (std::string &path : searchPaths) {\n>> +\t\tstd::string configurationPath = path + \"/\" + filename;\n>> +\t\tif (File::exists(configurationPath))\n>> +\t\t\treturn configurationPath;\n>> +\t}\n>> +\n>> +\treturn \"\";\n>> +}\n>> +\n>> +/**\n>> + * \\brief Open and parse a configuration file.\n>> + *\n>> + * The filename will be searched for on the libcamera configuration and paths,\n>> + * and then parsed.\n>> + *\n>> + * Successfully parsed files will present the data contained therein through the\n>> + * json object exposed from data();\n>> + */\n>> +int Configuration::open(std::string filename)\n>> +{\n>> +\tstd::string data = findFile(filename);\n>> +\tif (data.empty()) {\n>> +\t\tLOG(Configuration, Warning)\n>> +\t\t\t<< \"file: \\\"\" << filename\n>> +\t\t\t<< \"\\\" was not found.\";\n>> +\t\treturn -ENOENT;\n>> +\t}\n>> +\n>> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n>> +\n>> +\t/* Parse with no error callbacks and exceptions disabled. */\n>> +\tstd::ifstream input(data);\n>> +\tjson j = json::parse(input, nullptr, false);\n>> +\tif (j.is_discarded()) {\n>> +\t\tLOG(Configuration, Error)\n>> +\t\t\t<< \"file: \\\"\" << data\n>> +\t\t\t<< \"\\\" was not parsable.\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tjson_ = std::move(j);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 387d5d88ecae..5d655c87a7a0 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -9,6 +9,7 @@ libcamera_sources = files([\n>> 'camera_controls.cpp',\n>> 'camera_manager.cpp',\n>> 'camera_sensor.cpp',\n>> + 'configuration.cpp',\n>> 'controls.cpp',\n>> 'control_serializer.cpp',\n>> 'control_validator.cpp',\n>> --\n>> 2.25.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","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 1BD47BE08A\n\tfor ;\n\tWed, 25 Nov 2020 09:11:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8EC676340B;\n\tWed, 25 Nov 2020 10:11:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74427632EE\n\tfor ;\n\tWed, 25 Nov 2020 10:11:14 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CC3DAA1C;\n\tWed, 25 Nov 2020 10:11:13 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L+G+rb3Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606295474;\n\tbh=DC020+odBpzhKoP7HUr8JotxF6pJ2dA1uadDbeIYNIg=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=L+G+rb3Y9ZAoMtLO3yODzNRchDKk+UebtIXW+DNrOR+8gkLVeeUjKJRelOOQ72b7h\n\tAnTvrdOEDs3zBt3sKCIbrpuImxW43RKBZhloRyjWQXr23gQ6sXZRD4ypscvloaeoYt\n\tCJnNPh3vPCC/eTBTVzPClKfZEYECeVeLMbGK1MVU=","To":"Jacopo Mondi ","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>","Date":"Wed, 25 Nov 2020 09:11:10 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13873,"web_url":"https://patchwork.libcamera.org/comment/13873/","msgid":"<20201125101826.wutdjfogckbemqmz@uno.localdomain>","date":"2020-11-25T10:18:26","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Wed, Nov 25, 2020 at 09:11:10AM +0000, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 25/11/2020 08:52, Jacopo Mondi wrote:\n> > Hi Kieran,\n> >\n> > On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n> >> Provide an interface to support reading configuration files.\n> >>\n> >> Initial support is included for JSON formatted files, but extending this\n> >> with other configuration file formats is not excluded.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> README.rst | 2 +-\n> >> include/libcamera/internal/configuration.h | 37 +++++++++\n> >> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n> >> src/libcamera/meson.build | 1 +\n> >> 4 files changed, 130 insertions(+), 1 deletion(-)\n> >> create mode 100644 include/libcamera/internal/configuration.h\n> >> create mode 100644 src/libcamera/configuration.cpp\n> >>\n> >> diff --git a/README.rst b/README.rst\n> >> index 251291b77c62..c09e262fcc43 100644\n> >> --- a/README.rst\n> >> +++ b/README.rst\n> >> @@ -58,7 +58,7 @@ Meson Build system: [required]\n> >> pip3 install --user --upgrade meson\n> >>\n> >> for the libcamera core: [required]\n> >> - python3-yaml python3-ply python3-jinja2\n> >> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n> >\n> > First question I have is about the dependency on this package.\n> >\n> > I have no real reasons to doubt about the availability of a packaged\n> > version in most linux distro, nor about their stability and update\n> > rate. It might be that projects named after a single person always\n> > sound fragile to me, might be the rather low bus factor :)\n>\n>\n> Yes, I find the 'named after me' packaging a little ... awkward. But\n> there's really two choices for json libraries if we don't want boost.\n> nlohmann-json and json-cpp.\n>\n> For bus factor, nlohmann-json has 178 contributors ... so I don't think\n> it's just 'one guy'.\n>\n> I had planned to try both in earnest, and subclass Configuration so that\n> they could be swapped at will, but the nlohmann was seemingly easier to\n> get started with in the end, and subclassing to swap would mean wrapping\n> the nlohmann generic container types in a generic container type and\n> creating my own tree structures.\n>\n> We can do that of course, but that will take longer, and I honestly\n> couldn't see the point of wrapping it for the internal usages (yet).\n>\n> If this was public API then it would be different.\n>\n> And maybe we'll update/change later - but I wanted to get things\n> actually going so we can start figuring out what and where we store data\n> in the first place.\n>\n> Who knows, maybe Configuration() will sometime need to parse yaml files\n> so it will build a ConfigurationJSON or a ConfigurationYAML depending on\n> the data - but I don't want to over engineer just to be able to read\n> some key-values at this stage.\n>\n> > Joking aside this seems to be packaged in most distros\n> > https://repology.org/project/nlohmann-json/versions\n> > with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n> > which is an LTS if I'm not mistaken).\n>\n> It did seem to be quite well packaged, but I would probably lean towards\n> bringing the headers in under third_party/ or such as it's header only\n> to prevent any potential issues.\n>\n\nI would go in that direction too probably\n\n>\n> > There are notable exception though, in example Linux Mint.\n> >\n> > In ChromiumOS R87 I don't see it packaged, my equery skills are very\n> > low so I might have missed it.\n>\n> Also, while available in the Raspbian OS distribution - it's an older\n> version which didn't have a feature I was using \"j.contains()\".\n>\n> I thought that was actually a nicer way to implement the code rather\n> than using iterators and j.find(), even if the .find() is perhaps more\n> efficient as it will only do one look up - I think the code just doesn't\n> flow quite as nicely. (Having to suddenly dereference iterators to a new\n> type etc...)\n>\n\nIn general, knowing we can rely on known version would bring quite\nsome peace of mind in regard to versioning and available features\n\n>\n> > The real question is:\n> > - Are we happy to have this as externally packaged dependency ?\n> >\n> > If I look at the Arch package content:\n> > https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n> >\n> > This seems to be an header-only library. I wonder if we should not\n> > rope it in during the build. There seems to be built-in support in\n> > meson for that:\n> > https://github.com/nlohmann/json#package-managers\n> > https://wrapdb.mesonbuild.com/nlohmann_json\n>\n> It is in wrapdb - but it's out of date.\n>\n\nRight, 3.7.0 as I read it\n\n> We can either include the headers in libcamera, or we can update the\n> wrapdb ;-)\n>\n> I'd probably lean towards updating the wrapdb as that's more beneficial\n> everywhere.\n>\n\nIndeed, I'm not aware of what the process is to do so though\n\n> The meson wraps seem to work quite well - and will seemlessly download\n> the package if it's missing and build/use that.\n>\n> Part of me is then weary of requiring an internet connection for that -\n> but I really don't' think that's an issue anymore, as if you've cloned\n> the repo - we can expect that there is a connection.\n\nProbably so. I'm thinking of a use case where all the dependencies and\nlibcamera sources are packaged on a rootfs and -then- libcamera is\nbuilt on a live target, but I hardly find one.\n\n>\n>\n>\n> > Or include ourselves a version of the library in our source tree,\n> > which opens a different question: how do we feel about distributing MIT\n> > licensed code ? It should be no issue, but IANAL so it's better to\n> > give this a thought.\n>\n> That's also what makes me lean towards updating and using the wrapdb ;-)\n>\n> Alternatively of course we can rewrite all this with json-cpp\n> (https://github.com/open-source-parsers/jsoncpp) - but then we'll have\n> exactly the same discussion on that library/package too ;-)\n\nYou've done the background investigations, so I assume you chose\nnlohmann_json for valid reasons. Also, jsoncpp seems not to be an\nheader only library, so integration might become more problematic.\nThey're both MIT-licensed project, in one case we will need to only\ninclude un-touched MIT-licensed code headers in LGPL code. If I'm not\nmistaken with json-cpp we would need to link against it, so either\nbuild it as a .so or rely on pre-packaged versions (unless we go the\n'amalgamated' way:\nhttps://github.com/open-source-parsers/jsoncpp/wiki/Amalgamated-(Possibly-outdated)\nbut we would have to build it ourselves to obtain the amalgamated\nversion)\n\nIs this your understanding as well ?\n\nThanks\n j\n\n>\n> Kieran\n>\n>\n> > Thanks\n> > j\n> >\n> >>\n> >> for IPA module signing: [required]\n> >> libgnutls28-dev openssl\n> >> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n> >> new file mode 100644\n> >> index 000000000000..a89732f0210f\n> >> --- /dev/null\n> >> +++ b/include/libcamera/internal/configuration.h\n> >> @@ -0,0 +1,37 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Google Inc.\n> >> + *\n> >> + * configuration.h - Parsing configuration files\n> >> + */\n> >> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >> +\n> >> +#include \n> >> +#include \n> >> +\n> >> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n> >> +#define JSON_NOEXCEPTION 1\n> >> +#include \n> >> +\n> >> +using json = nlohmann::json;\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +class Configuration\n> >> +{\n> >> +public:\n> >> +\tint open(std::string filename);\n> >> +\n> >> +\tjson &data() { return json_; }\n> >> +\n> >> +private:\n> >> +\tstd::string findFile(std::string filename);\n> >> +\n> >> +\tjson json_;\n> >> +};\n> >> +\n> >> +} /* namespace libcamera */\n> >> +\n> >> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n> >> +\n> >> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n> >> new file mode 100644\n> >> index 000000000000..f849088bbc45\n> >> --- /dev/null\n> >> +++ b/src/libcamera/configuration.cpp\n> >> @@ -0,0 +1,91 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2020, Google Inc.\n> >> + *\n> >> + * configuration.cpp - Parsing configuration files\n> >> + */\n> >> +#include \"libcamera/internal/configuration.h\"\n> >> +\n> >> +#include \"libcamera/internal/file.h\"\n> >> +#include \"libcamera/internal/log.h\"\n> >> +#include \"libcamera/internal/utils.h\"\n> >> +\n> >> +#include \n> >> +#include \n> >> +\n> >> +/**\n> >> + * \\file configuration.h\n> >> + * \\brief Read interface for configuration files\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +LOG_DEFINE_CATEGORY(Configuration)\n> >> +\n> >> +/*\n> >> + * Configuration files can be stored in system paths, which are identified\n> >> + * through the build configuration.\n> >> + *\n> >> + * However, when running uninstalled - the source location takes precedence.\n> >> + */\n> >> +std::string Configuration::findFile(std::string filename)\n> >> +{\n> >> +\tstatic std::array searchPaths = {\n> >> +\t\tLIBCAMERA_SYSCONF_DIR,\n> >> +\t\tLIBCAMERA_DATA_DIR,\n> >> +\t};\n> >> +\n> >> +\tstd::string root = utils::libcameraSourcePath();\n> >> +\tif (!root.empty()) {\n> >> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n> >> +\n> >> +\t\tif (File::exists(configurationPath))\n> >> +\t\t\treturn configurationPath;\n> >> +\t}\n> >> +\n> >> +\tfor (std::string &path : searchPaths) {\n> >> +\t\tstd::string configurationPath = path + \"/\" + filename;\n> >> +\t\tif (File::exists(configurationPath))\n> >> +\t\t\treturn configurationPath;\n> >> +\t}\n> >> +\n> >> +\treturn \"\";\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Open and parse a configuration file.\n> >> + *\n> >> + * The filename will be searched for on the libcamera configuration and paths,\n> >> + * and then parsed.\n> >> + *\n> >> + * Successfully parsed files will present the data contained therein through the\n> >> + * json object exposed from data();\n> >> + */\n> >> +int Configuration::open(std::string filename)\n> >> +{\n> >> +\tstd::string data = findFile(filename);\n> >> +\tif (data.empty()) {\n> >> +\t\tLOG(Configuration, Warning)\n> >> +\t\t\t<< \"file: \\\"\" << filename\n> >> +\t\t\t<< \"\\\" was not found.\";\n> >> +\t\treturn -ENOENT;\n> >> +\t}\n> >> +\n> >> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n> >> +\n> >> +\t/* Parse with no error callbacks and exceptions disabled. */\n> >> +\tstd::ifstream input(data);\n> >> +\tjson j = json::parse(input, nullptr, false);\n> >> +\tif (j.is_discarded()) {\n> >> +\t\tLOG(Configuration, Error)\n> >> +\t\t\t<< \"file: \\\"\" << data\n> >> +\t\t\t<< \"\\\" was not parsable.\";\n> >> +\t\treturn -EINVAL;\n> >> +\t}\n> >> +\n> >> +\tjson_ = std::move(j);\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >> index 387d5d88ecae..5d655c87a7a0 100644\n> >> --- a/src/libcamera/meson.build\n> >> +++ b/src/libcamera/meson.build\n> >> @@ -9,6 +9,7 @@ libcamera_sources = files([\n> >> 'camera_controls.cpp',\n> >> 'camera_manager.cpp',\n> >> 'camera_sensor.cpp',\n> >> + 'configuration.cpp',\n> >> 'controls.cpp',\n> >> 'control_serializer.cpp',\n> >> 'control_validator.cpp',\n> >> --\n> >> 2.25.1\n> >>\n> >> _______________________________________________\n> >> libcamera-devel mailing list\n> >> libcamera-devel@lists.libcamera.org\n> >> https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards\n> --\n> Kieran","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 8CE0CBE176\n\tfor ;\n\tWed, 25 Nov 2020 10:18:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3104963407;\n\tWed, 25 Nov 2020 11:18:24 +0100 (CET)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 940F9632EE\n\tfor ;\n\tWed, 25 Nov 2020 11:18:22 +0100 (CET)","from uno.localdomain (host-80-104-176-17.retail.telecomitalia.it\n\t[80.104.176.17]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 700FB240008;\n\tWed, 25 Nov 2020 10:18:21 +0000 (UTC)"],"X-Originating-IP":"80.104.176.17","Date":"Wed, 25 Nov 2020 11:18:26 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20201125101826.wutdjfogckbemqmz@uno.localdomain>","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>\n\t<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13875,"web_url":"https://patchwork.libcamera.org/comment/13875/","msgid":"<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>","date":"2020-11-25T10:37:15","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 25/11/2020 10:18, Jacopo Mondi wrote:\n> On Wed, Nov 25, 2020 at 09:11:10AM +0000, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 25/11/2020 08:52, Jacopo Mondi wrote:\n>>> Hi Kieran,\n>>>\n>>> On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n>>>> Provide an interface to support reading configuration files.\n>>>>\n>>>> Initial support is included for JSON formatted files, but extending this\n>>>> with other configuration file formats is not excluded.\n>>>>\n>>>> Signed-off-by: Kieran Bingham \n>>>> ---\n>>>> README.rst | 2 +-\n>>>> include/libcamera/internal/configuration.h | 37 +++++++++\n>>>> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n>>>> src/libcamera/meson.build | 1 +\n>>>> 4 files changed, 130 insertions(+), 1 deletion(-)\n>>>> create mode 100644 include/libcamera/internal/configuration.h\n>>>> create mode 100644 src/libcamera/configuration.cpp\n>>>>\n>>>> diff --git a/README.rst b/README.rst\n>>>> index 251291b77c62..c09e262fcc43 100644\n>>>> --- a/README.rst\n>>>> +++ b/README.rst\n>>>> @@ -58,7 +58,7 @@ Meson Build system: [required]\n>>>> pip3 install --user --upgrade meson\n>>>>\n>>>> for the libcamera core: [required]\n>>>> - python3-yaml python3-ply python3-jinja2\n>>>> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n>>>\n>>> First question I have is about the dependency on this package.\n>>>\n>>> I have no real reasons to doubt about the availability of a packaged\n>>> version in most linux distro, nor about their stability and update\n>>> rate. It might be that projects named after a single person always\n>>> sound fragile to me, might be the rather low bus factor :)\n>>\n>>\n>> Yes, I find the 'named after me' packaging a little ... awkward. But\n>> there's really two choices for json libraries if we don't want boost.\n>> nlohmann-json and json-cpp.\n>>\n>> For bus factor, nlohmann-json has 178 contributors ... so I don't think\n>> it's just 'one guy'.\n>>\n>> I had planned to try both in earnest, and subclass Configuration so that\n>> they could be swapped at will, but the nlohmann was seemingly easier to\n>> get started with in the end, and subclassing to swap would mean wrapping\n>> the nlohmann generic container types in a generic container type and\n>> creating my own tree structures.\n>>\n>> We can do that of course, but that will take longer, and I honestly\n>> couldn't see the point of wrapping it for the internal usages (yet).\n>>\n>> If this was public API then it would be different.\n>>\n>> And maybe we'll update/change later - but I wanted to get things\n>> actually going so we can start figuring out what and where we store data\n>> in the first place.\n>>\n>> Who knows, maybe Configuration() will sometime need to parse yaml files\n>> so it will build a ConfigurationJSON or a ConfigurationYAML depending on\n>> the data - but I don't want to over engineer just to be able to read\n>> some key-values at this stage.\n>>\n>>> Joking aside this seems to be packaged in most distros\n>>> https://repology.org/project/nlohmann-json/versions\n>>> with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n>>> which is an LTS if I'm not mistaken).\n>>\n>> It did seem to be quite well packaged, but I would probably lean towards\n>> bringing the headers in under third_party/ or such as it's header only\n>> to prevent any potential issues.\n>>\n> \n> I would go in that direction too probably\n> \n>>\n>>> There are notable exception though, in example Linux Mint.\n>>>\n>>> In ChromiumOS R87 I don't see it packaged, my equery skills are very\n>>> low so I might have missed it.\n>>\n>> Also, while available in the Raspbian OS distribution - it's an older\n>> version which didn't have a feature I was using \"j.contains()\".\n>>\n>> I thought that was actually a nicer way to implement the code rather\n>> than using iterators and j.find(), even if the .find() is perhaps more\n>> efficient as it will only do one look up - I think the code just doesn't\n>> flow quite as nicely. (Having to suddenly dereference iterators to a new\n>> type etc...)\n>>\n> \n> In general, knowing we can rely on known version would bring quite\n> some peace of mind in regard to versioning and available features\n> \n>>\n>>> The real question is:\n>>> - Are we happy to have this as externally packaged dependency ?\n>>>\n>>> If I look at the Arch package content:\n>>> https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n>>>\n>>> This seems to be an header-only library. I wonder if we should not\n>>> rope it in during the build. There seems to be built-in support in\n>>> meson for that:\n>>> https://github.com/nlohmann/json#package-managers\n>>> https://wrapdb.mesonbuild.com/nlohmann_json\n>>\n>> It is in wrapdb - but it's out of date.\n>>\n> \n> Right, 3.7.0 as I read it\n> \n>> We can either include the headers in libcamera, or we can update the\n>> wrapdb ;-)\n>>\n>> I'd probably lean towards updating the wrapdb as that's more beneficial\n>> everywhere.\n>>\n> \n> Indeed, I'm not aware of what the process is to do so though\n> \n>> The meson wraps seem to work quite well - and will seemlessly download\n>> the package if it's missing and build/use that.\n>>\n>> Part of me is then weary of requiring an internet connection for that -\n>> but I really don't' think that's an issue anymore, as if you've cloned\n>> the repo - we can expect that there is a connection.\n> \n> Probably so. I'm thinking of a use case where all the dependencies and\n> libcamera sources are packaged on a rootfs and -then- libcamera is\n> built on a live target, but I hardly find one.\n> \n>>\n>>\n>>\n>>> Or include ourselves a version of the library in our source tree,\n>>> which opens a different question: how do we feel about distributing MIT\n>>> licensed code ? It should be no issue, but IANAL so it's better to\n>>> give this a thought.\n>>\n>> That's also what makes me lean towards updating and using the wrapdb ;-)\n>>\n>> Alternatively of course we can rewrite all this with json-cpp\n>> (https://github.com/open-source-parsers/jsoncpp) - but then we'll have\n>> exactly the same discussion on that library/package too ;-)\n> \n> You've done the background investigations, so I assume you chose\n> nlohmann_json for valid reasons. Also, jsoncpp seems not to be an\n\nPart of the reasons were that nlohmann is also reported to have better\nperformances.\n\n> header only library, so integration might become more problematic.\n> They're both MIT-licensed project, in one case we will need to only\n> include un-touched MIT-licensed code headers in LGPL code. If I'm not\n> mistaken with json-cpp we would need to link against it, so either\n> build it as a .so or rely on pre-packaged versions (unless we go the\n> 'amalgamated' way:\n> https://github.com/open-source-parsers/jsoncpp/wiki/Amalgamated-(Possibly-outdated)\n> but we would have to build it ourselves to obtain the amalgamated\n> version)\n\nI hadn't looked at any of the amalgamated options. I assumed we'd have\nto link against it.\n\nFrom a license point of view, I don't think there's any conflict for\nusing an MIT licences either by linking, or by including the headers.\n\n> Is this your understanding as well ?\n\nClose enough.\n\n> \n> Thanks\n> j\n> \n>>\n>> Kieran\n>>\n>>\n>>> Thanks\n>>> j\n>>>\n>>>>\n>>>> for IPA module signing: [required]\n>>>> libgnutls28-dev openssl\n>>>> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n>>>> new file mode 100644\n>>>> index 000000000000..a89732f0210f\n>>>> --- /dev/null\n>>>> +++ b/include/libcamera/internal/configuration.h\n>>>> @@ -0,0 +1,37 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2020, Google Inc.\n>>>> + *\n>>>> + * configuration.h - Parsing configuration files\n>>>> + */\n>>>> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>>>> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>>>> +\n>>>> +#include \n>>>> +#include \n>>>> +\n>>>> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n>>>> +#define JSON_NOEXCEPTION 1\n>>>> +#include \n>>>> +\n>>>> +using json = nlohmann::json;\n>>>> +\n>>>> +namespace libcamera {\n>>>> +\n>>>> +class Configuration\n>>>> +{\n>>>> +public:\n>>>> +\tint open(std::string filename);\n>>>> +\n>>>> +\tjson &data() { return json_; }\n>>>> +\n>>>> +private:\n>>>> +\tstd::string findFile(std::string filename);\n>>>> +\n>>>> +\tjson json_;\n>>>> +};\n>>>> +\n>>>> +} /* namespace libcamera */\n>>>> +\n>>>> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n>>>> +\n>>>> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n>>>> new file mode 100644\n>>>> index 000000000000..f849088bbc45\n>>>> --- /dev/null\n>>>> +++ b/src/libcamera/configuration.cpp\n>>>> @@ -0,0 +1,91 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2020, Google Inc.\n>>>> + *\n>>>> + * configuration.cpp - Parsing configuration files\n>>>> + */\n>>>> +#include \"libcamera/internal/configuration.h\"\n>>>> +\n>>>> +#include \"libcamera/internal/file.h\"\n>>>> +#include \"libcamera/internal/log.h\"\n>>>> +#include \"libcamera/internal/utils.h\"\n>>>> +\n>>>> +#include \n>>>> +#include \n>>>> +\n>>>> +/**\n>>>> + * \\file configuration.h\n>>>> + * \\brief Read interface for configuration files\n>>>> + */\n>>>> +\n>>>> +namespace libcamera {\n>>>> +\n>>>> +LOG_DEFINE_CATEGORY(Configuration)\n>>>> +\n>>>> +/*\n>>>> + * Configuration files can be stored in system paths, which are identified\n>>>> + * through the build configuration.\n>>>> + *\n>>>> + * However, when running uninstalled - the source location takes precedence.\n>>>> + */\n>>>> +std::string Configuration::findFile(std::string filename)\n>>>> +{\n>>>> +\tstatic std::array searchPaths = {\n>>>> +\t\tLIBCAMERA_SYSCONF_DIR,\n>>>> +\t\tLIBCAMERA_DATA_DIR,\n>>>> +\t};\n>>>> +\n>>>> +\tstd::string root = utils::libcameraSourcePath();\n>>>> +\tif (!root.empty()) {\n>>>> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n>>>> +\n>>>> +\t\tif (File::exists(configurationPath))\n>>>> +\t\t\treturn configurationPath;\n>>>> +\t}\n>>>> +\n\nWe might also want to expose an environment variable to add to the paths\nhere, as we do for IPA configuration files (LIBCAMERA_IPA_CONFIG_PATH)\n\n\nHrm ... I should tackle the IPA configuration files as part of this\nseries too.\n\nIt looks like that code path isn't really being used much yet though -\nexcept for a dummy file at src/ipa/vimc/data/vimc.conf\n\nMore things for me to look at ;-)\n\n--\nKieran\n\n\n>>>> +\tfor (std::string &path : searchPaths) {\n>>>> +\t\tstd::string configurationPath = path + \"/\" + filename;\n>>>> +\t\tif (File::exists(configurationPath))\n>>>> +\t\t\treturn configurationPath;\n>>>> +\t}\n>>>> +\n>>>> +\treturn \"\";\n>>>> +}\n>>>> +\n>>>> +/**\n>>>> + * \\brief Open and parse a configuration file.\n>>>> + *\n>>>> + * The filename will be searched for on the libcamera configuration and paths,\n>>>> + * and then parsed.\n>>>> + *\n>>>> + * Successfully parsed files will present the data contained therein through the\n>>>> + * json object exposed from data();\n>>>> + */\n>>>> +int Configuration::open(std::string filename)\n>>>> +{\n>>>> +\tstd::string data = findFile(filename);\n>>>> +\tif (data.empty()) {\n>>>> +\t\tLOG(Configuration, Warning)\n>>>> +\t\t\t<< \"file: \\\"\" << filename\n>>>> +\t\t\t<< \"\\\" was not found.\";\n>>>> +\t\treturn -ENOENT;\n>>>> +\t}\n>>>> +\n>>>> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n>>>> +\n>>>> +\t/* Parse with no error callbacks and exceptions disabled. */\n>>>> +\tstd::ifstream input(data);\n>>>> +\tjson j = json::parse(input, nullptr, false);\n>>>> +\tif (j.is_discarded()) {\n>>>> +\t\tLOG(Configuration, Error)\n>>>> +\t\t\t<< \"file: \\\"\" << data\n>>>> +\t\t\t<< \"\\\" was not parsable.\";\n>>>> +\t\treturn -EINVAL;\n>>>> +\t}\n>>>> +\n>>>> +\tjson_ = std::move(j);\n>>>> +\n>>>> +\treturn 0;\n>>>> +}\n>>>> +\n>>>> +} /* namespace libcamera */\n>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>>> index 387d5d88ecae..5d655c87a7a0 100644\n>>>> --- a/src/libcamera/meson.build\n>>>> +++ b/src/libcamera/meson.build\n>>>> @@ -9,6 +9,7 @@ libcamera_sources = files([\n>>>> 'camera_controls.cpp',\n>>>> 'camera_manager.cpp',\n>>>> 'camera_sensor.cpp',\n>>>> + 'configuration.cpp',\n>>>> 'controls.cpp',\n>>>> 'control_serializer.cpp',\n>>>> 'control_validator.cpp',\n>>>> --\n>>>> 2.25.1\n>>>>\n>>>> _______________________________________________\n>>>> libcamera-devel mailing list\n>>>> libcamera-devel@lists.libcamera.org\n>>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 76384BE176\n\tfor ;\n\tWed, 25 Nov 2020 10:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 10DCD6340B;\n\tWed, 25 Nov 2020 11:37:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1828D632EE\n\tfor ;\n\tWed, 25 Nov 2020 11:37:18 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7235E292;\n\tWed, 25 Nov 2020 11:37:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"T/qzCx3R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606300637;\n\tbh=fubH9Z7FUKBUdwnPHOLThJTVfRdQj71VDrGWIB8Lw1g=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=T/qzCx3Rk7L26wg2WTEW1RJGJ6ADERiirT8LwlPxQg8sfYkt6OJlM0601NVzlRDa9\n\tD8yyBMih2NMQaz4FktSIaS6hhs2XhXgRjdlgWSEQADgOx5Zk1b0kPpZidOmkANw4Ia\n\t/etsdbJT8ZgunJTsVx9ILJfNWd2YPOj8pgy56cd0=","To":"Jacopo Mondi ","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>\n\t<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>\n\t<20201125101826.wutdjfogckbemqmz@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>","Date":"Wed, 25 Nov 2020 10:37:15 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201125101826.wutdjfogckbemqmz@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14018,"web_url":"https://patchwork.libcamera.org/comment/14018/","msgid":"<20201201180230.GR4569@pendragon.ideasonboard.com>","date":"2020-12-01T18:02:30","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Wed, Nov 25, 2020 at 10:37:15AM +0000, Kieran Bingham wrote:\n> On 25/11/2020 10:18, Jacopo Mondi wrote:\n> > On Wed, Nov 25, 2020 at 09:11:10AM +0000, Kieran Bingham wrote:\n> >> On 25/11/2020 08:52, Jacopo Mondi wrote:\n> >>> On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n> >>>> Provide an interface to support reading configuration files.\n> >>>>\n> >>>> Initial support is included for JSON formatted files, but extending this\n> >>>> with other configuration file formats is not excluded.\n> >>>>\n> >>>> Signed-off-by: Kieran Bingham \n> >>>> ---\n> >>>> README.rst | 2 +-\n> >>>> include/libcamera/internal/configuration.h | 37 +++++++++\n> >>>> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n> >>>> src/libcamera/meson.build | 1 +\n> >>>> 4 files changed, 130 insertions(+), 1 deletion(-)\n> >>>> create mode 100644 include/libcamera/internal/configuration.h\n> >>>> create mode 100644 src/libcamera/configuration.cpp\n> >>>>\n> >>>> diff --git a/README.rst b/README.rst\n> >>>> index 251291b77c62..c09e262fcc43 100644\n> >>>> --- a/README.rst\n> >>>> +++ b/README.rst\n> >>>> @@ -58,7 +58,7 @@ Meson Build system: [required]\n> >>>> pip3 install --user --upgrade meson\n> >>>>\n> >>>> for the libcamera core: [required]\n> >>>> - python3-yaml python3-ply python3-jinja2\n> >>>> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n> >>>\n> >>> First question I have is about the dependency on this package.\n> >>>\n> >>> I have no real reasons to doubt about the availability of a packaged\n> >>> version in most linux distro, nor about their stability and update\n> >>> rate. It might be that projects named after a single person always\n> >>> sound fragile to me, might be the rather low bus factor :)\n> >>\n> >> Yes, I find the 'named after me' packaging a little ... awkward. But\n> >> there's really two choices for json libraries if we don't want boost.\n> >> nlohmann-json and json-cpp.\n\nWe also have the option of using a C library, as we wrap it in a custom\nclass anyway. https://github.com/json-c/json-c/wiki and\nhttps://www.digip.org/jansson/ come to mind, there are probably other\noptions. I'm not advocating switching to a different library, just\nwanted to make sure that all options are known by everybody.\n\n> >> For bus factor, nlohmann-json has 178 contributors ... so I don't think\n> >> it's just 'one guy'.\n> >>\n> >> I had planned to try both in earnest, and subclass Configuration so that\n> >> they could be swapped at will, but the nlohmann was seemingly easier to\n> >> get started with in the end, and subclassing to swap would mean wrapping\n> >> the nlohmann generic container types in a generic container type and\n> >> creating my own tree structures.\n> >>\n> >> We can do that of course, but that will take longer, and I honestly\n> >> couldn't see the point of wrapping it for the internal usages (yet).\n> >>\n> >> If this was public API then it would be different.\n> >>\n> >> And maybe we'll update/change later - but I wanted to get things\n> >> actually going so we can start figuring out what and where we store data\n> >> in the first place.\n> >>\n> >> Who knows, maybe Configuration() will sometime need to parse yaml files\n> >> so it will build a ConfigurationJSON or a ConfigurationYAML depending on\n> >> the data - but I don't want to over engineer just to be able to read\n> >> some key-values at this stage.\n\nYAML is more complicated, otherwise I would have proposed going directly\nfor it (I *really* dislike how JSON doesn't support comments).\n\n> >>> Joking aside this seems to be packaged in most distros\n> >>> https://repology.org/project/nlohmann-json/versions\n> >>> with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n> >>> which is an LTS if I'm not mistaken).\n> >>\n> >> It did seem to be quite well packaged, but I would probably lean towards\n> >> bringing the headers in under third_party/ or such as it's header only\n> >> to prevent any potential issues.\n> > \n> > I would go in that direction too probably\n\nThat or using a wrap, yes. Relying on the distro version is likely not a\ngood idea if we can't rely on a recent enough version. It however also\nmeans that we'll be responsible for tracking upstream changes\n(especially security fixes) and bringing them in.\n\n> >>> There are notable exception though, in example Linux Mint.\n> >>>\n> >>> In ChromiumOS R87 I don't see it packaged, my equery skills are very\n> >>> low so I might have missed it.\n> >>\n> >> Also, while available in the Raspbian OS distribution - it's an older\n> >> version which didn't have a feature I was using \"j.contains()\".\n> >>\n> >> I thought that was actually a nicer way to implement the code rather\n> >> than using iterators and j.find(), even if the .find() is perhaps more\n> >> efficient as it will only do one look up - I think the code just doesn't\n> >> flow quite as nicely. (Having to suddenly dereference iterators to a new\n> >> type etc...)\n> > \n> > In general, knowing we can rely on known version would bring quite\n> > some peace of mind in regard to versioning and available features\n> > \n> >>> The real question is:\n> >>> - Are we happy to have this as externally packaged dependency ?\n> >>>\n> >>> If I look at the Arch package content:\n> >>> https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n> >>>\n> >>> This seems to be an header-only library. I wonder if we should not\n> >>> rope it in during the build. There seems to be built-in support in\n> >>> meson for that:\n> >>> https://github.com/nlohmann/json#package-managers\n> >>> https://wrapdb.mesonbuild.com/nlohmann_json\n> >>\n> >> It is in wrapdb - but it's out of date.\n> > \n> > Right, 3.7.0 as I read it\n> > \n> >> We can either include the headers in libcamera, or we can update the\n> >> wrapdb ;-)\n> >>\n> >> I'd probably lean towards updating the wrapdb as that's more beneficial\n> >> everywhere.\n> > \n> > Indeed, I'm not aware of what the process is to do so though\n> > \n> >> The meson wraps seem to work quite well - and will seemlessly download\n> >> the package if it's missing and build/use that.\n> >>\n> >> Part of me is then weary of requiring an internet connection for that -\n> >> but I really don't' think that's an issue anymore, as if you've cloned\n> >> the repo - we can expect that there is a connection.\n> > \n> > Probably so. I'm thinking of a use case where all the dependencies and\n> > libcamera sources are packaged on a rootfs and -then- libcamera is\n> > built on a live target, but I hardly find one.\n\nI believe meson wrap can be pointed to a cache directory with\npre-downloaded zip files. That would be my preference, if we go for a\nwrap.\n\n> >>> Or include ourselves a version of the library in our source tree,\n> >>> which opens a different question: how do we feel about distributing MIT\n> >>> licensed code ? It should be no issue, but IANAL so it's better to\n> >>> give this a thought.\n> >>\n> >> That's also what makes me lean towards updating and using the wrapdb ;-)\n\nThat won't change much though, we'll still link against MIT code,\nregardless of whether we carry a copy of it or not. The MIT license is\ncompatible with the LGPL as far as I can tell, so that's not an issue.\n\nIf we want to use an LGPL JSON parser, there's json-glib, but I don't\nthink that's a good idea :-) I'll also mention simdjson, just to share\nthe \"interesting\" idea of SIMD-accelerated JSON parsing.\n\n> >> Alternatively of course we can rewrite all this with json-cpp\n> >> (https://github.com/open-source-parsers/jsoncpp) - but then we'll have\n> >> exactly the same discussion on that library/package too ;-)\n> > \n> > You've done the background investigations, so I assume you chose\n> > nlohmann_json for valid reasons. Also, jsoncpp seems not to be an\n> \n> Part of the reasons were that nlohmann is also reported to have better\n> performances.\n> \n> > header only library, so integration might become more problematic.\n> > They're both MIT-licensed project, in one case we will need to only\n> > include un-touched MIT-licensed code headers in LGPL code. If I'm not\n> > mistaken with json-cpp we would need to link against it, so either\n> > build it as a .so or rely on pre-packaged versions (unless we go the\n> > 'amalgamated' way:\n> > https://github.com/open-source-parsers/jsoncpp/wiki/Amalgamated-(Possibly-outdated)\n> > but we would have to build it ourselves to obtain the amalgamated\n> > version)\n> \n> I hadn't looked at any of the amalgamated options. I assumed we'd have\n> to link against it.\n> \n> From a license point of view, I don't think there's any conflict for\n> using an MIT licences either by linking, or by including the headers.\n> \n> > Is this your understanding as well ?\n> \n> Close enough.\n> \n> >>>> for IPA module signing: [required]\n> >>>> libgnutls28-dev openssl\n> >>>> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n> >>>> new file mode 100644\n> >>>> index 000000000000..a89732f0210f\n> >>>> --- /dev/null\n> >>>> +++ b/include/libcamera/internal/configuration.h\n> >>>> @@ -0,0 +1,37 @@\n> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>> +/*\n> >>>> + * Copyright (C) 2020, Google Inc.\n> >>>> + *\n> >>>> + * configuration.h - Parsing configuration files\n> >>>> + */\n> >>>> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >>>> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >>>> +\n> >>>> +#include \n\nIs this needed ?\n\n> >>>> +#include \n> >>>> +\n> >>>> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n> >>>> +#define JSON_NOEXCEPTION 1\n> >>>> +#include \n\nThis is what I was hoping to avoid :-S We're pulling nearly 1MB of\nheader in every file that includes configuration.h. As everything is\ninline in headers, it will mean quite a bit of code duplication. At\nleast the parser is isolated in a .cpp file and won't get duplicated\n(but will still get parsed by the compiler, with some impact on build\ntime), but access to the data will always be inlined :-(\n\n> >>>> +\n> >>>> +using json = nlohmann::json;\n> >>>> +\n> >>>> +namespace libcamera {\n> >>>> +\n> >>>> +class Configuration\n\nThe name Configuration is very generic, especially given\nCameraConfiguration that is often referred to as just \"configuration\".\nHow about ConfigurationFile ?\n\n> >>>> +{\n> >>>> +public:\n> >>>> +\tint open(std::string filename);\n\nconst &\n\ns/open/parse/ ? I also wonder if we couldn't just have a static parse()\nfunction that would return a json object.\n\n> >>>> +\n> >>>> +\tjson &data() { return json_; }\n\nOoohhhh so json is a class and contains member types. It's not very nice\nto write json without a prefix, and json::parse() or json::iterator that\nlooks like namespace-qualified names :-S\n\n> >>>> +\n> >>>> +private:\n> >>>> +\tstd::string findFile(std::string filename);\n\nconst &\n\n> >>>> +\n> >>>> +\tjson json_;\n> >>>> +};\n> >>>> +\n> >>>> +} /* namespace libcamera */\n> >>>> +\n> >>>> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n> >>>> +\n> >>>> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n> >>>> new file mode 100644\n> >>>> index 000000000000..f849088bbc45\n> >>>> --- /dev/null\n> >>>> +++ b/src/libcamera/configuration.cpp\n> >>>> @@ -0,0 +1,91 @@\n> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>> +/*\n> >>>> + * Copyright (C) 2020, Google Inc.\n> >>>> + *\n> >>>> + * configuration.cpp - Parsing configuration files\n> >>>> + */\n> >>>> +#include \"libcamera/internal/configuration.h\"\n> >>>> +\n> >>>> +#include \"libcamera/internal/file.h\"\n> >>>> +#include \"libcamera/internal/log.h\"\n> >>>> +#include \"libcamera/internal/utils.h\"\n> >>>> +\n> >>>> +#include \n> >>>> +#include \n\nThis should go above the previous three headers.\n\n> >>>> +\n> >>>> +/**\n> >>>> + * \\file configuration.h\n> >>>> + * \\brief Read interface for configuration files\n> >>>> + */\n> >>>> +\n> >>>> +namespace libcamera {\n> >>>> +\n> >>>> +LOG_DEFINE_CATEGORY(Configuration)\n> >>>> +\n> >>>> +/*\n> >>>> + * Configuration files can be stored in system paths, which are identified\n> >>>> + * through the build configuration.\n> >>>> + *\n> >>>> + * However, when running uninstalled - the source location takes precedence.\n> >>>> + */\n\nI don't think thins should be part of this class. I'd rather have a\nJSON parser that only parses files, and deal with paths externally,\nespecially given that different types of configuration files may be\nstored in different locations. For instance I expect we'll have a global\nconfiguration file stored in /etc/libcamera/ with an override in\n$HOME/.libcamera/, IPA configuration files in /usr/share/libcamera/\n(likely with overrides, including through an environment variable), and\nprobably more. The mechanisms to locate files will differ, and shouldn't\nbe in scope for the parser.\n\n> >>>> +std::string Configuration::findFile(std::string filename)\n> >>>> +{\n> >>>> +\tstatic std::array searchPaths = {\n> >>>> +\t\tLIBCAMERA_SYSCONF_DIR,\n> >>>> +\t\tLIBCAMERA_DATA_DIR,\n> >>>> +\t};\n> >>>> +\n> >>>> +\tstd::string root = utils::libcameraSourcePath();\n> >>>> +\tif (!root.empty()) {\n> >>>> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n> >>>> +\n> >>>> +\t\tif (File::exists(configurationPath))\n> >>>> +\t\t\treturn configurationPath;\n> >>>> +\t}\n> >>>> +\n> \n> We might also want to expose an environment variable to add to the paths\n> here, as we do for IPA configuration files (LIBCAMERA_IPA_CONFIG_PATH)\n> \n> \n> Hrm ... I should tackle the IPA configuration files as part of this\n> series too.\n> \n> It looks like that code path isn't really being used much yet though -\n> except for a dummy file at src/ipa/vimc/data/vimc.conf\n> \n> More things for me to look at ;-)\n> \n> >>>> +\tfor (std::string &path : searchPaths) {\n> >>>> +\t\tstd::string configurationPath = path + \"/\" + filename;\n> >>>> +\t\tif (File::exists(configurationPath))\n> >>>> +\t\t\treturn configurationPath;\n> >>>> +\t}\n> >>>> +\n> >>>> +\treturn \"\";\n> >>>> +}\n> >>>> +\n> >>>> +/**\n> >>>> + * \\brief Open and parse a configuration file.\n> >>>> + *\n> >>>> + * The filename will be searched for on the libcamera configuration and paths,\n> >>>> + * and then parsed.\n> >>>> + *\n> >>>> + * Successfully parsed files will present the data contained therein through the\n> >>>> + * json object exposed from data();\n> >>>> + */\n> >>>> +int Configuration::open(std::string filename)\n> >>>> +{\n> >>>> +\tstd::string data = findFile(filename);\n> >>>> +\tif (data.empty()) {\n> >>>> +\t\tLOG(Configuration, Warning)\n> >>>> +\t\t\t<< \"file: \\\"\" << filename\n> >>>> +\t\t\t<< \"\\\" was not found.\";\n> >>>> +\t\treturn -ENOENT;\n> >>>> +\t}\n> >>>> +\n> >>>> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n> >>>> +\n> >>>> +\t/* Parse with no error callbacks and exceptions disabled. */\n> >>>> +\tstd::ifstream input(data);\n> >>>> +\tjson j = json::parse(input, nullptr, false);\n> >>>> +\tif (j.is_discarded()) {\n> >>>> +\t\tLOG(Configuration, Error)\n> >>>> +\t\t\t<< \"file: \\\"\" << data\n> >>>> +\t\t\t<< \"\\\" was not parsable.\";\n> >>>> +\t\treturn -EINVAL;\n> >>>> +\t}\n> >>>> +\n> >>>> +\tjson_ = std::move(j);\n> >>>> +\n> >>>> +\treturn 0;\n> >>>> +}\n> >>>> +\n> >>>> +} /* namespace libcamera */\n> >>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>> index 387d5d88ecae..5d655c87a7a0 100644\n> >>>> --- a/src/libcamera/meson.build\n> >>>> +++ b/src/libcamera/meson.build\n> >>>> @@ -9,6 +9,7 @@ libcamera_sources = files([\n> >>>> 'camera_controls.cpp',\n> >>>> 'camera_manager.cpp',\n> >>>> 'camera_sensor.cpp',\n> >>>> + 'configuration.cpp',\n> >>>> 'controls.cpp',\n> >>>> 'control_serializer.cpp',\n> >>>> 'control_validator.cpp',","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 4BB5FBE177\n\tfor ;\n\tTue, 1 Dec 2020 18:02:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB5DE63505;\n\tTue, 1 Dec 2020 19:02:40 +0100 (CET)","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 0139E63460\n\tfor ;\n\tTue, 1 Dec 2020 19:02:39 +0100 (CET)","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 CC35ADBD;\n\tTue, 1 Dec 2020 19:02:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JPG/b0R2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606845759;\n\tbh=sQZH9h+i6RNcwIjYY9DZqn9BNXLcmTBcWxWs3UhEIn8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JPG/b0R2IydhDtJdBOHNrCZ6sJGgIresCJ2HOxtpoLwANH8KEmUrc0RMmfhBfPU22\n\tffy0Hh7Uanf5Icw5SnSfzHP7vY2n3h8oXfSCaA1AESojxbJy6lmiJBqoONnAgiQdbL\n\tQlrdnt+Wdzpn+DUCW+oJD2CJba/7Jt/PJ8v8/Me8=","Date":"Tue, 1 Dec 2020 20:02:30 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20201201180230.GR4569@pendragon.ideasonboard.com>","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>\n\t<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>\n\t<20201125101826.wutdjfogckbemqmz@uno.localdomain>\n\t<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14104,"web_url":"https://patchwork.libcamera.org/comment/14104/","msgid":"<25112de2-f25d-f3d3-b27b-37c594093c01@ideasonboard.com>","date":"2020-12-07T16:32:38","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 01/12/2020 18:02, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Wed, Nov 25, 2020 at 10:37:15AM +0000, Kieran Bingham wrote:\n>> On 25/11/2020 10:18, Jacopo Mondi wrote:\n>>> On Wed, Nov 25, 2020 at 09:11:10AM +0000, Kieran Bingham wrote:\n>>>> On 25/11/2020 08:52, Jacopo Mondi wrote:\n>>>>> On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n>>>>>> Provide an interface to support reading configuration files.\n>>>>>>\n>>>>>> Initial support is included for JSON formatted files, but extending this\n>>>>>> with other configuration file formats is not excluded.\n>>>>>>\n>>>>>> Signed-off-by: Kieran Bingham \n>>>>>> ---\n>>>>>> README.rst | 2 +-\n>>>>>> include/libcamera/internal/configuration.h | 37 +++++++++\n>>>>>> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n>>>>>> src/libcamera/meson.build | 1 +\n>>>>>> 4 files changed, 130 insertions(+), 1 deletion(-)\n>>>>>> create mode 100644 include/libcamera/internal/configuration.h\n>>>>>> create mode 100644 src/libcamera/configuration.cpp\n>>>>>>\n>>>>>> diff --git a/README.rst b/README.rst\n>>>>>> index 251291b77c62..c09e262fcc43 100644\n>>>>>> --- a/README.rst\n>>>>>> +++ b/README.rst\n>>>>>> @@ -58,7 +58,7 @@ Meson Build system: [required]\n>>>>>> pip3 install --user --upgrade meson\n>>>>>>\n>>>>>> for the libcamera core: [required]\n>>>>>> - python3-yaml python3-ply python3-jinja2\n>>>>>> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n>>>>>\n>>>>> First question I have is about the dependency on this package.\n>>>>>\n>>>>> I have no real reasons to doubt about the availability of a packaged\n>>>>> version in most linux distro, nor about their stability and update\n>>>>> rate. It might be that projects named after a single person always\n>>>>> sound fragile to me, might be the rather low bus factor :)\n>>>>\n>>>> Yes, I find the 'named after me' packaging a little ... awkward. But\n>>>> there's really two choices for json libraries if we don't want boost.\n>>>> nlohmann-json and json-cpp.\n> \n> We also have the option of using a C library, as we wrap it in a custom\n> class anyway. https://github.com/json-c/json-c/wiki and\n> https://www.digip.org/jansson/ come to mind, there are probably other\n> options. I'm not advocating switching to a different library, just\n> wanted to make sure that all options are known by everybody.\n\n\nYou know, I hadn't actually considered starting at a c-library (because\nwe're in C++), so I hadn't looked at those.\n\n\n\n>>>> For bus factor, nlohmann-json has 178 contributors ... so I don't think\n>>>> it's just 'one guy'.\n>>>>\n>>>> I had planned to try both in earnest, and subclass Configuration so that\n>>>> they could be swapped at will, but the nlohmann was seemingly easier to\n>>>> get started with in the end, and subclassing to swap would mean wrapping\n>>>> the nlohmann generic container types in a generic container type and\n>>>> creating my own tree structures.\n>>>>\n>>>> We can do that of course, but that will take longer, and I honestly\n>>>> couldn't see the point of wrapping it for the internal usages (yet).\n>>>>\n>>>> If this was public API then it would be different.\n>>>>\n>>>> And maybe we'll update/change later - but I wanted to get things\n>>>> actually going so we can start figuring out what and where we store data\n>>>> in the first place.\n>>>>\n>>>> Who knows, maybe Configuration() will sometime need to parse yaml files\n>>>> so it will build a ConfigurationJSON or a ConfigurationYAML depending on\n>>>> the data - but I don't want to over engineer just to be able to read\n>>>> some key-values at this stage.\n> \n> YAML is more complicated, otherwise I would have proposed going directly\n> for it (I *really* dislike how JSON doesn't support comments).\n\nComments are always helpful anywhere.\nRestricting them is certainly frustrating.\n\n\n\n>>>>> Joking aside this seems to be packaged in most distros\n>>>>> https://repology.org/project/nlohmann-json/versions\n>>>>> with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n>>>>> which is an LTS if I'm not mistaken).\n>>>>\n>>>> It did seem to be quite well packaged, but I would probably lean towards\n>>>> bringing the headers in under third_party/ or such as it's header only\n>>>> to prevent any potential issues.\n>>>\n>>> I would go in that direction too probably\n> \n> That or using a wrap, yes. Relying on the distro version is likely not a\n> good idea if we can't rely on a recent enough version. It however also\n> means that we'll be responsible for tracking upstream changes\n> (especially security fixes) and bringing them in.\n> \n>>>>> There are notable exception though, in example Linux Mint.\n>>>>>\n>>>>> In ChromiumOS R87 I don't see it packaged, my equery skills are very\n>>>>> low so I might have missed it.\n>>>>\n>>>> Also, while available in the Raspbian OS distribution - it's an older\n>>>> version which didn't have a feature I was using \"j.contains()\".\n>>>>\n>>>> I thought that was actually a nicer way to implement the code rather\n>>>> than using iterators and j.find(), even if the .find() is perhaps more\n>>>> efficient as it will only do one look up - I think the code just doesn't\n>>>> flow quite as nicely. (Having to suddenly dereference iterators to a new\n>>>> type etc...)\n>>>\n>>> In general, knowing we can rely on known version would bring quite\n>>> some peace of mind in regard to versioning and available features\n>>>\n>>>>> The real question is:\n>>>>> - Are we happy to have this as externally packaged dependency ?\n>>>>>\n>>>>> If I look at the Arch package content:\n>>>>> https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n>>>>>\n>>>>> This seems to be an header-only library. I wonder if we should not\n>>>>> rope it in during the build. There seems to be built-in support in\n>>>>> meson for that:\n>>>>> https://github.com/nlohmann/json#package-managers\n>>>>> https://wrapdb.mesonbuild.com/nlohmann_json\n>>>>\n>>>> It is in wrapdb - but it's out of date.\n>>>\n>>> Right, 3.7.0 as I read it\n>>>\n>>>> We can either include the headers in libcamera, or we can update the\n>>>> wrapdb ;-)\n>>>>\n>>>> I'd probably lean towards updating the wrapdb as that's more beneficial\n>>>> everywhere.\n>>>\n>>> Indeed, I'm not aware of what the process is to do so though\n>>>\n>>>> The meson wraps seem to work quite well - and will seemlessly download\n>>>> the package if it's missing and build/use that.\n>>>>\n>>>> Part of me is then weary of requiring an internet connection for that -\n>>>> but I really don't' think that's an issue anymore, as if you've cloned\n>>>> the repo - we can expect that there is a connection.\n>>>\n>>> Probably so. I'm thinking of a use case where all the dependencies and\n>>> libcamera sources are packaged on a rootfs and -then- libcamera is\n>>> built on a live target, but I hardly find one.\n> \n> I believe meson wrap can be pointed to a cache directory with\n> pre-downloaded zip files. That would be my preference, if we go for a\n> wrap.\n> \n>>>>> Or include ourselves a version of the library in our source tree,\n>>>>> which opens a different question: how do we feel about distributing MIT\n>>>>> licensed code ? It should be no issue, but IANAL so it's better to\n>>>>> give this a thought.\n>>>>\n>>>> That's also what makes me lean towards updating and using the wrapdb ;-)\n> \n> That won't change much though, we'll still link against MIT code,\n> regardless of whether we carry a copy of it or not. The MIT license is\n> compatible with the LGPL as far as I can tell, so that's not an issue.\n> \n> If we want to use an LGPL JSON parser, there's json-glib, but I don't\n> think that's a good idea :-) I'll also mention simdjson, just to share\n> the \"interesting\" idea of SIMD-accelerated JSON parsing.\n\n\nhttps://github.com/zserge/jsmn/\n\n(jasmine == Json Minimalist) also looks quite reasonable as a c-parser\n\nBut we could pull in any of a hundred external libraries. The fact is,\nwhat we want isn't provided by the standard libraries, so we pull in\nsomething else or write our own. And I don't intend to write a full json\nparser.\n\n\nThough I must say, jsmn does it in 400 lines of header, so JSON really\nis quite simple ;-)\n\n>>>> Alternatively of course we can rewrite all this with json-cpp\n>>>> (https://github.com/open-source-parsers/jsoncpp) - but then we'll have\n>>>> exactly the same discussion on that library/package too ;-)\n>>>\n>>> You've done the background investigations, so I assume you chose\n>>> nlohmann_json for valid reasons. Also, jsoncpp seems not to be an\n>>\n>> Part of the reasons were that nlohmann is also reported to have better\n>> performances.\n>>\n>>> header only library, so integration might become more problematic.\n>>> They're both MIT-licensed project, in one case we will need to only\n>>> include un-touched MIT-licensed code headers in LGPL code. If I'm not\n>>> mistaken with json-cpp we would need to link against it, so either\n>>> build it as a .so or rely on pre-packaged versions (unless we go the\n>>> 'amalgamated' way:\n>>> https://github.com/open-source-parsers/jsoncpp/wiki/Amalgamated-(Possibly-outdated)\n>>> but we would have to build it ourselves to obtain the amalgamated\n>>> version)\n>>\n>> I hadn't looked at any of the amalgamated options. I assumed we'd have\n>> to link against it.\n>>\n>> From a license point of view, I don't think there's any conflict for\n>> using an MIT licences either by linking, or by including the headers.\n>>\n>>> Is this your understanding as well ?\n>>\n>> Close enough.\n>>\n>>>>>> for IPA module signing: [required]\n>>>>>> libgnutls28-dev openssl\n>>>>>> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n>>>>>> new file mode 100644\n>>>>>> index 000000000000..a89732f0210f\n>>>>>> --- /dev/null\n>>>>>> +++ b/include/libcamera/internal/configuration.h\n>>>>>> @@ -0,0 +1,37 @@\n>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>>> +/*\n>>>>>> + * Copyright (C) 2020, Google Inc.\n>>>>>> + *\n>>>>>> + * configuration.h - Parsing configuration files\n>>>>>> + */\n>>>>>> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>>>>>> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n>>>>>> +\n>>>>>> +#include \n> \n> Is this needed ?\n> \n\nNot anymore,\n\n>>>>>> +#include \n>>>>>> +\n>>>>>> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n>>>>>> +#define JSON_NOEXCEPTION 1\n>>>>>> +#include \n> \n> This is what I was hoping to avoid :-S We're pulling nearly 1MB of\n> header in every file that includes configuration.h. As everything is\n> inline in headers, it will mean quite a bit of code duplication. At\n> least the parser is isolated in a .cpp file and won't get duplicated\n> (but will still get parsed by the compiler, with some impact on build\n> time), but access to the data will always be inlined :-(\n\nOk, so then we'll have to create our own 'types' or structures to pass\naround or store data tokens.\n\nI went this route to avoid exactly that in the end. But I guess I'm back\nto the other path.\n\n\n>>>>>> +\n>>>>>> +using json = nlohmann::json;\n>>>>>> +\n>>>>>> +namespace libcamera {\n>>>>>> +\n>>>>>> +class Configuration\n> \n> The name Configuration is very generic, especially given\n> CameraConfiguration that is often referred to as just \"configuration\".\n> How about ConfigurationFile ?\n\nConfigurationFile sounds good.\n\n\n> \n>>>>>> +{\n>>>>>> +public:\n>>>>>> +\tint open(std::string filename);\n> \n> const &\n> \n> s/open/parse/ ? I also wonder if we couldn't just have a static parse()\n> function that would return a json object.\n\nparse sounds good actually. I thought the open/then data() felt a bit\nawkward.\n\n\n> \n>>>>>> +\n>>>>>> +\tjson &data() { return json_; }\n> \n> Ooohhhh so json is a class and contains member types. It's not very nice\n> to write json without a prefix, and json::parse() or json::iterator that\n> looks like namespace-qualified names :-S\n\nWell actually its:\n\nnlohmann_json::json::iterator\n\nI shorten it above with:\n\nusing json = nlohmann::json;\n\nas prefixing nlohmann everywhere felt quite heavyweight.\n\n>>>>>> +\n>>>>>> +private:\n>>>>>> +\tstd::string findFile(std::string filename);\n> \n> const &\n> \n>>>>>> +\n>>>>>> +\tjson json_;\n>>>>>> +};\n>>>>>> +\n>>>>>> +} /* namespace libcamera */\n>>>>>> +\n>>>>>> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n>>>>>> +\n>>>>>> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n>>>>>> new file mode 100644\n>>>>>> index 000000000000..f849088bbc45\n>>>>>> --- /dev/null\n>>>>>> +++ b/src/libcamera/configuration.cpp\n>>>>>> @@ -0,0 +1,91 @@\n>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>>> +/*\n>>>>>> + * Copyright (C) 2020, Google Inc.\n>>>>>> + *\n>>>>>> + * configuration.cpp - Parsing configuration files\n>>>>>> + */\n>>>>>> +#include \"libcamera/internal/configuration.h\"\n>>>>>> +\n>>>>>> +#include \"libcamera/internal/file.h\"\n>>>>>> +#include \"libcamera/internal/log.h\"\n>>>>>> +#include \"libcamera/internal/utils.h\"\n>>>>>> +\n>>>>>> +#include \n>>>>>> +#include \n> \n> This should go above the previous three headers.\n> \n>>>>>> +\n>>>>>> +/**\n>>>>>> + * \\file configuration.h\n>>>>>> + * \\brief Read interface for configuration files\n>>>>>> + */\n>>>>>> +\n>>>>>> +namespace libcamera {\n>>>>>> +\n>>>>>> +LOG_DEFINE_CATEGORY(Configuration)\n>>>>>> +\n>>>>>> +/*\n>>>>>> + * Configuration files can be stored in system paths, which are identified\n>>>>>> + * through the build configuration.\n>>>>>> + *\n>>>>>> + * However, when running uninstalled - the source location takes precedence.\n>>>>>> + */\n> \n> I don't think thins should be part of this class. I'd rather have a\n> JSON parser that only parses files, and deal with paths externally,\n> especially given that different types of configuration files may be\n\nOk - so I think that's the split.\n\nTo me this Configuration class was dealing with obtaining the file\nlocations, and the json library was dealing with the parsing (as it was\nincluded in the components that use it).\n\n\n> stored in different locations. For instance I expect we'll have a global\n> configuration file stored in /etc/libcamera/ with an override in\n> $HOME/.libcamera/, IPA configuration files in /usr/share/libcamera/\n> (likely with overrides, including through an environment variable), and\n> probably more. The mechanisms to locate files will differ, and shouldn't\n> be in scope for the parser.\n\nNo - but that is precisely what is being handled by findFile().\n\nSo really, the semantics you're saying is that findFile should be separated.\n\nIf so - where would you like it to be ?\n\n\n> \n>>>>>> +std::string Configuration::findFile(std::string filename)\n>>>>>> +{\n>>>>>> +\tstatic std::array searchPaths = {\n>>>>>> +\t\tLIBCAMERA_SYSCONF_DIR,\n>>>>>> +\t\tLIBCAMERA_DATA_DIR,\n>>>>>> +\t};\n>>>>>> +\n>>>>>> +\tstd::string root = utils::libcameraSourcePath();\n>>>>>> +\tif (!root.empty()) {\n>>>>>> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n>>>>>> +\n>>>>>> +\t\tif (File::exists(configurationPath))\n>>>>>> +\t\t\treturn configurationPath;\n>>>>>> +\t}\n>>>>>> +\n>>\n>> We might also want to expose an environment variable to add to the paths\n>> here, as we do for IPA configuration files (LIBCAMERA_IPA_CONFIG_PATH)\n>>\n>>\n>> Hrm ... I should tackle the IPA configuration files as part of this\n>> series too.\n>>\n>> It looks like that code path isn't really being used much yet though -\n>> except for a dummy file at src/ipa/vimc/data/vimc.conf\n>>\n>> More things for me to look at ;-)\n>>\n>>>>>> +\tfor (std::string &path : searchPaths) {\n>>>>>> +\t\tstd::string configurationPath = path + \"/\" + filename;\n>>>>>> +\t\tif (File::exists(configurationPath))\n>>>>>> +\t\t\treturn configurationPath;\n>>>>>> +\t}\n>>>>>> +\n>>>>>> +\treturn \"\";\n>>>>>> +}\n>>>>>> +\n>>>>>> +/**\n>>>>>> + * \\brief Open and parse a configuration file.\n>>>>>> + *\n>>>>>> + * The filename will be searched for on the libcamera configuration and paths,\n>>>>>> + * and then parsed.\n>>>>>> + *\n>>>>>> + * Successfully parsed files will present the data contained therein through the\n>>>>>> + * json object exposed from data();\n>>>>>> + */\n>>>>>> +int Configuration::open(std::string filename)\n>>>>>> +{\n>>>>>> +\tstd::string data = findFile(filename);\n>>>>>> +\tif (data.empty()) {\n>>>>>> +\t\tLOG(Configuration, Warning)\n>>>>>> +\t\t\t<< \"file: \\\"\" << filename\n>>>>>> +\t\t\t<< \"\\\" was not found.\";\n>>>>>> +\t\treturn -ENOENT;\n>>>>>> +\t}\n>>>>>> +\n>>>>>> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n>>>>>> +\n>>>>>> +\t/* Parse with no error callbacks and exceptions disabled. */\n>>>>>> +\tstd::ifstream input(data);\n>>>>>> +\tjson j = json::parse(input, nullptr, false);\n>>>>>> +\tif (j.is_discarded()) {\n>>>>>> +\t\tLOG(Configuration, Error)\n>>>>>> +\t\t\t<< \"file: \\\"\" << data\n>>>>>> +\t\t\t<< \"\\\" was not parsable.\";\n>>>>>> +\t\treturn -EINVAL;\n>>>>>> +\t}\n>>>>>> +\n>>>>>> +\tjson_ = std::move(j);\n>>>>>> +\n>>>>>> +\treturn 0;\n>>>>>> +}\n>>>>>> +\n>>>>>> +} /* namespace libcamera */\n>>>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>>>>> index 387d5d88ecae..5d655c87a7a0 100644\n>>>>>> --- a/src/libcamera/meson.build\n>>>>>> +++ b/src/libcamera/meson.build\n>>>>>> @@ -9,6 +9,7 @@ libcamera_sources = files([\n>>>>>> 'camera_controls.cpp',\n>>>>>> 'camera_manager.cpp',\n>>>>>> 'camera_sensor.cpp',\n>>>>>> + 'configuration.cpp',\n>>>>>> 'controls.cpp',\n>>>>>> 'control_serializer.cpp',\n>>>>>> 'control_validator.cpp',\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 37ACCBDB1F\n\tfor ;\n\tMon, 7 Dec 2020 16:32:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9791867E6E;\n\tMon, 7 Dec 2020 17:32:43 +0100 (CET)","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 C4AF2635A0\n\tfor ;\n\tMon, 7 Dec 2020 17:32:41 +0100 (CET)","from [192.168.0.217]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 165A78D;\n\tMon, 7 Dec 2020 17:32:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BomzNuCf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607358761;\n\tbh=8ATb8ZcESWnTCbwBpofoNr3xRBgjYqvwPYVtxGytv9A=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=BomzNuCfz0LpG7mzqcuMBaGrcgsd7MaZJGQ5OEGwNfuIJ354cI1KmwS8/YqIZrZDq\n\t/nP7HdMRK3/tKOhA3jDK0//jCMg0bGZFF9TqkfLdJujJvey36q7FmsJP4Az6AOxxeE\n\teDaZqWpntAx22N5z9A+9Fj0UcDVMltPUa3OIcqjQ=","To":"Laurent Pinchart ","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>\n\t<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>\n\t<20201125101826.wutdjfogckbemqmz@uno.localdomain>\n\t<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>\n\t<20201201180230.GR4569@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<25112de2-f25d-f3d3-b27b-37c594093c01@ideasonboard.com>","Date":"Mon, 7 Dec 2020 16:32:38 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201201180230.GR4569@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14106,"web_url":"https://patchwork.libcamera.org/comment/14106/","msgid":"","date":"2020-12-07T18:38:14","subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Dec 07, 2020 at 04:32:38PM +0000, Kieran Bingham wrote:\n> On 01/12/2020 18:02, Laurent Pinchart wrote:\n> > On Wed, Nov 25, 2020 at 10:37:15AM +0000, Kieran Bingham wrote:\n> >> On 25/11/2020 10:18, Jacopo Mondi wrote:\n> >>> On Wed, Nov 25, 2020 at 09:11:10AM +0000, Kieran Bingham wrote:\n> >>>> On 25/11/2020 08:52, Jacopo Mondi wrote:\n> >>>>> On Mon, Nov 23, 2020 at 04:43:17PM +0000, Kieran Bingham wrote:\n> >>>>>> Provide an interface to support reading configuration files.\n> >>>>>>\n> >>>>>> Initial support is included for JSON formatted files, but extending this\n> >>>>>> with other configuration file formats is not excluded.\n> >>>>>>\n> >>>>>> Signed-off-by: Kieran Bingham \n> >>>>>> ---\n> >>>>>> README.rst | 2 +-\n> >>>>>> include/libcamera/internal/configuration.h | 37 +++++++++\n> >>>>>> src/libcamera/configuration.cpp | 91 ++++++++++++++++++++++\n> >>>>>> src/libcamera/meson.build | 1 +\n> >>>>>> 4 files changed, 130 insertions(+), 1 deletion(-)\n> >>>>>> create mode 100644 include/libcamera/internal/configuration.h\n> >>>>>> create mode 100644 src/libcamera/configuration.cpp\n> >>>>>>\n> >>>>>> diff --git a/README.rst b/README.rst\n> >>>>>> index 251291b77c62..c09e262fcc43 100644\n> >>>>>> --- a/README.rst\n> >>>>>> +++ b/README.rst\n> >>>>>> @@ -58,7 +58,7 @@ Meson Build system: [required]\n> >>>>>> pip3 install --user --upgrade meson\n> >>>>>>\n> >>>>>> for the libcamera core: [required]\n> >>>>>> - python3-yaml python3-ply python3-jinja2\n> >>>>>> + python3-yaml python3-ply python3-jinja2 nlohmann-json3-dev\n> >>>>>\n> >>>>> First question I have is about the dependency on this package.\n> >>>>>\n> >>>>> I have no real reasons to doubt about the availability of a packaged\n> >>>>> version in most linux distro, nor about their stability and update\n> >>>>> rate. It might be that projects named after a single person always\n> >>>>> sound fragile to me, might be the rather low bus factor :)\n> >>>>\n> >>>> Yes, I find the 'named after me' packaging a little ... awkward. But\n> >>>> there's really two choices for json libraries if we don't want boost.\n> >>>> nlohmann-json and json-cpp.\n> > \n> > We also have the option of using a C library, as we wrap it in a custom\n> > class anyway. https://github.com/json-c/json-c/wiki and\n> > https://www.digip.org/jansson/ come to mind, there are probably other\n> > options. I'm not advocating switching to a different library, just\n> > wanted to make sure that all options are known by everybody.\n> \n> You know, I hadn't actually considered starting at a c-library (because\n> we're in C++), so I hadn't looked at those.\n\nI would have gone for a C++ library too, it certainly makes sense. I\nwonder why none of the available libraries seem to have been developed\nin a sensical way though. And I also wonder if people are telling the\nsame about libcamera :-)\n\n> >>>> For bus factor, nlohmann-json has 178 contributors ... so I don't think\n> >>>> it's just 'one guy'.\n> >>>>\n> >>>> I had planned to try both in earnest, and subclass Configuration so that\n> >>>> they could be swapped at will, but the nlohmann was seemingly easier to\n> >>>> get started with in the end, and subclassing to swap would mean wrapping\n> >>>> the nlohmann generic container types in a generic container type and\n> >>>> creating my own tree structures.\n> >>>>\n> >>>> We can do that of course, but that will take longer, and I honestly\n> >>>> couldn't see the point of wrapping it for the internal usages (yet).\n> >>>>\n> >>>> If this was public API then it would be different.\n> >>>>\n> >>>> And maybe we'll update/change later - but I wanted to get things\n> >>>> actually going so we can start figuring out what and where we store data\n> >>>> in the first place.\n> >>>>\n> >>>> Who knows, maybe Configuration() will sometime need to parse yaml files\n> >>>> so it will build a ConfigurationJSON or a ConfigurationYAML depending on\n> >>>> the data - but I don't want to over engineer just to be able to read\n> >>>> some key-values at this stage.\n> > \n> > YAML is more complicated, otherwise I would have proposed going directly\n> > for it (I *really* dislike how JSON doesn't support comments).\n> \n> Comments are always helpful anywhere.\n> Restricting them is certainly frustrating.\n\nI wonder if some parsers may support comments, even if they're not\nstandard. I guess we could preprocess too (for instance if a parser\ntakes its input from an istream, we could create an ifstream that opens\nthe file, and wraps it in an istream that filters comments out).\n\n> >>>>> Joking aside this seems to be packaged in most distros\n> >>>>> https://repology.org/project/nlohmann-json/versions\n> >>>>> with rather different versions (3.9.1 on Arch, 2.1.1 on Ubuntu 18.04\n> >>>>> which is an LTS if I'm not mistaken).\n> >>>>\n> >>>> It did seem to be quite well packaged, but I would probably lean towards\n> >>>> bringing the headers in under third_party/ or such as it's header only\n> >>>> to prevent any potential issues.\n> >>>\n> >>> I would go in that direction too probably\n> > \n> > That or using a wrap, yes. Relying on the distro version is likely not a\n> > good idea if we can't rely on a recent enough version. It however also\n> > means that we'll be responsible for tracking upstream changes\n> > (especially security fixes) and bringing them in.\n> > \n> >>>>> There are notable exception though, in example Linux Mint.\n> >>>>>\n> >>>>> In ChromiumOS R87 I don't see it packaged, my equery skills are very\n> >>>>> low so I might have missed it.\n> >>>>\n> >>>> Also, while available in the Raspbian OS distribution - it's an older\n> >>>> version which didn't have a feature I was using \"j.contains()\".\n> >>>>\n> >>>> I thought that was actually a nicer way to implement the code rather\n> >>>> than using iterators and j.find(), even if the .find() is perhaps more\n> >>>> efficient as it will only do one look up - I think the code just doesn't\n> >>>> flow quite as nicely. (Having to suddenly dereference iterators to a new\n> >>>> type etc...)\n> >>>\n> >>> In general, knowing we can rely on known version would bring quite\n> >>> some peace of mind in regard to versioning and available features\n> >>>\n> >>>>> The real question is:\n> >>>>> - Are we happy to have this as externally packaged dependency ?\n> >>>>>\n> >>>>> If I look at the Arch package content:\n> >>>>> https://www.archlinux.org/packages/community/any/nlohmann-json/files/\n> >>>>>\n> >>>>> This seems to be an header-only library. I wonder if we should not\n> >>>>> rope it in during the build. There seems to be built-in support in\n> >>>>> meson for that:\n> >>>>> https://github.com/nlohmann/json#package-managers\n> >>>>> https://wrapdb.mesonbuild.com/nlohmann_json\n> >>>>\n> >>>> It is in wrapdb - but it's out of date.\n> >>>\n> >>> Right, 3.7.0 as I read it\n> >>>\n> >>>> We can either include the headers in libcamera, or we can update the\n> >>>> wrapdb ;-)\n> >>>>\n> >>>> I'd probably lean towards updating the wrapdb as that's more beneficial\n> >>>> everywhere.\n> >>>\n> >>> Indeed, I'm not aware of what the process is to do so though\n> >>>\n> >>>> The meson wraps seem to work quite well - and will seemlessly download\n> >>>> the package if it's missing and build/use that.\n> >>>>\n> >>>> Part of me is then weary of requiring an internet connection for that -\n> >>>> but I really don't' think that's an issue anymore, as if you've cloned\n> >>>> the repo - we can expect that there is a connection.\n> >>>\n> >>> Probably so. I'm thinking of a use case where all the dependencies and\n> >>> libcamera sources are packaged on a rootfs and -then- libcamera is\n> >>> built on a live target, but I hardly find one.\n> > \n> > I believe meson wrap can be pointed to a cache directory with\n> > pre-downloaded zip files. That would be my preference, if we go for a\n> > wrap.\n> > \n> >>>>> Or include ourselves a version of the library in our source tree,\n> >>>>> which opens a different question: how do we feel about distributing MIT\n> >>>>> licensed code ? It should be no issue, but IANAL so it's better to\n> >>>>> give this a thought.\n> >>>>\n> >>>> That's also what makes me lean towards updating and using the wrapdb ;-)\n> > \n> > That won't change much though, we'll still link against MIT code,\n> > regardless of whether we carry a copy of it or not. The MIT license is\n> > compatible with the LGPL as far as I can tell, so that's not an issue.\n> > \n> > If we want to use an LGPL JSON parser, there's json-glib, but I don't\n> > think that's a good idea :-) I'll also mention simdjson, just to share\n> > the \"interesting\" idea of SIMD-accelerated JSON parsing.\n> \n> https://github.com/zserge/jsmn/\n> \n> (jasmine == Json Minimalist) also looks quite reasonable as a c-parser\n> \n> But we could pull in any of a hundred external libraries. The fact is,\n> what we want isn't provided by the standard libraries, so we pull in\n> something else or write our own. And I don't intend to write a full json\n> parser.\n> \n> Though I must say, jsmn does it in 400 lines of header, so JSON really\n> is quite simple ;-)\n\nMy go for a 400 lines implementation when there's a 1MB header available\nto do the same :-)\n\n> >>>> Alternatively of course we can rewrite all this with json-cpp\n> >>>> (https://github.com/open-source-parsers/jsoncpp) - but then we'll have\n> >>>> exactly the same discussion on that library/package too ;-)\n> >>>\n> >>> You've done the background investigations, so I assume you chose\n> >>> nlohmann_json for valid reasons. Also, jsoncpp seems not to be an\n> >>\n> >> Part of the reasons were that nlohmann is also reported to have better\n> >> performances.\n> >>\n> >>> header only library, so integration might become more problematic.\n> >>> They're both MIT-licensed project, in one case we will need to only\n> >>> include un-touched MIT-licensed code headers in LGPL code. If I'm not\n> >>> mistaken with json-cpp we would need to link against it, so either\n> >>> build it as a .so or rely on pre-packaged versions (unless we go the\n> >>> 'amalgamated' way:\n> >>> https://github.com/open-source-parsers/jsoncpp/wiki/Amalgamated-(Possibly-outdated)\n> >>> but we would have to build it ourselves to obtain the amalgamated\n> >>> version)\n> >>\n> >> I hadn't looked at any of the amalgamated options. I assumed we'd have\n> >> to link against it.\n> >>\n> >> From a license point of view, I don't think there's any conflict for\n> >> using an MIT licences either by linking, or by including the headers.\n> >>\n> >>> Is this your understanding as well ?\n> >>\n> >> Close enough.\n> >>\n> >>>>>> for IPA module signing: [required]\n> >>>>>> libgnutls28-dev openssl\n> >>>>>> diff --git a/include/libcamera/internal/configuration.h b/include/libcamera/internal/configuration.h\n> >>>>>> new file mode 100644\n> >>>>>> index 000000000000..a89732f0210f\n> >>>>>> --- /dev/null\n> >>>>>> +++ b/include/libcamera/internal/configuration.h\n> >>>>>> @@ -0,0 +1,37 @@\n> >>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>>>> +/*\n> >>>>>> + * Copyright (C) 2020, Google Inc.\n> >>>>>> + *\n> >>>>>> + * configuration.h - Parsing configuration files\n> >>>>>> + */\n> >>>>>> +#ifndef __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >>>>>> +#define __LIBCAMERA_INTERNAL_CONFIGURATION_H__\n> >>>>>> +\n> >>>>>> +#include \n> > \n> > Is this needed ?\n> \n> Not anymore,\n> \n> >>>>>> +#include \n> >>>>>> +\n> >>>>>> +/* https://nlohmann.github.io/json/home/exceptions/#switch-off-exceptions */\n> >>>>>> +#define JSON_NOEXCEPTION 1\n> >>>>>> +#include \n> > \n> > This is what I was hoping to avoid :-S We're pulling nearly 1MB of\n> > header in every file that includes configuration.h. As everything is\n> > inline in headers, it will mean quite a bit of code duplication. At\n> > least the parser is isolated in a .cpp file and won't get duplicated\n> > (but will still get parsed by the compiler, with some impact on build\n> > time), but access to the data will always be inlined :-(\n> \n> Ok, so then we'll have to create our own 'types' or structures to pass\n> around or store data tokens.\n> \n> I went this route to avoid exactly that in the end. But I guess I'm back\n> to the other path.\n\nThat's the annoying part, I agree. We have done the same for\nControlValue, and at some point I wonder if we'll end up with a Value\nclass that will be used in different parts of libcamera. It's not\nsomething to worry about now in any case.\n\n> >>>>>> +\n> >>>>>> +using json = nlohmann::json;\n> >>>>>> +\n> >>>>>> +namespace libcamera {\n> >>>>>> +\n> >>>>>> +class Configuration\n> > \n> > The name Configuration is very generic, especially given\n> > CameraConfiguration that is often referred to as just \"configuration\".\n> > How about ConfigurationFile ?\n> \n> ConfigurationFile sounds good.\n> \n> >>>>>> +{\n> >>>>>> +public:\n> >>>>>> +\tint open(std::string filename);\n> > \n> > const &\n> > \n> > s/open/parse/ ? I also wonder if we couldn't just have a static parse()\n> > function that would return a json object.\n> \n> parse sounds good actually. I thought the open/then data() felt a bit\n> awkward.\n> \n> >>>>>> +\n> >>>>>> +\tjson &data() { return json_; }\n> > \n> > Ooohhhh so json is a class and contains member types. It's not very nice\n> > to write json without a prefix, and json::parse() or json::iterator that\n> > looks like namespace-qualified names :-S\n> \n> Well actually its:\n> \n> nlohmann_json::json::iterator\n> \n> I shorten it above with:\n> \n> using json = nlohmann::json;\n> \n> as prefixing nlohmann everywhere felt quite heavyweight.\n\nI wish the nlohmann parser were split from the property tree\nimplementation :-S\n\n> >>>>>> +\n> >>>>>> +private:\n> >>>>>> +\tstd::string findFile(std::string filename);\n> > \n> > const &\n> > \n> >>>>>> +\n> >>>>>> +\tjson json_;\n> >>>>>> +};\n> >>>>>> +\n> >>>>>> +} /* namespace libcamera */\n> >>>>>> +\n> >>>>>> +#endif /* __LIBCAMERA_INTERNAL_CONFIGURATION_H__ */\n> >>>>>> +\n> >>>>>> diff --git a/src/libcamera/configuration.cpp b/src/libcamera/configuration.cpp\n> >>>>>> new file mode 100644\n> >>>>>> index 000000000000..f849088bbc45\n> >>>>>> --- /dev/null\n> >>>>>> +++ b/src/libcamera/configuration.cpp\n> >>>>>> @@ -0,0 +1,91 @@\n> >>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>>>> +/*\n> >>>>>> + * Copyright (C) 2020, Google Inc.\n> >>>>>> + *\n> >>>>>> + * configuration.cpp - Parsing configuration files\n> >>>>>> + */\n> >>>>>> +#include \"libcamera/internal/configuration.h\"\n> >>>>>> +\n> >>>>>> +#include \"libcamera/internal/file.h\"\n> >>>>>> +#include \"libcamera/internal/log.h\"\n> >>>>>> +#include \"libcamera/internal/utils.h\"\n> >>>>>> +\n> >>>>>> +#include \n> >>>>>> +#include \n> > \n> > This should go above the previous three headers.\n> > \n> >>>>>> +\n> >>>>>> +/**\n> >>>>>> + * \\file configuration.h\n> >>>>>> + * \\brief Read interface for configuration files\n> >>>>>> + */\n> >>>>>> +\n> >>>>>> +namespace libcamera {\n> >>>>>> +\n> >>>>>> +LOG_DEFINE_CATEGORY(Configuration)\n> >>>>>> +\n> >>>>>> +/*\n> >>>>>> + * Configuration files can be stored in system paths, which are identified\n> >>>>>> + * through the build configuration.\n> >>>>>> + *\n> >>>>>> + * However, when running uninstalled - the source location takes precedence.\n> >>>>>> + */\n> > \n> > I don't think thins should be part of this class. I'd rather have a\n> > JSON parser that only parses files, and deal with paths externally,\n> > especially given that different types of configuration files may be\n> \n> Ok - so I think that's the split.\n> \n> To me this Configuration class was dealing with obtaining the file\n> locations, and the json library was dealing with the parsing (as it was\n> included in the components that use it).\n> \n> > stored in different locations. For instance I expect we'll have a global\n> > configuration file stored in /etc/libcamera/ with an override in\n> > $HOME/.libcamera/, IPA configuration files in /usr/share/libcamera/\n> > (likely with overrides, including through an environment variable), and\n> > probably more. The mechanisms to locate files will differ, and shouldn't\n> > be in scope for the parser.\n> \n> No - but that is precisely what is being handled by findFile().\n> \n> So really, the semantics you're saying is that findFile should be separated.\n> \n> If so - where would you like it to be ?\n\nVery good question :-) I see two options:\n\n- We have some logic in IPAProxy::configurationFile() already, and we\n could extend other classes to support a similar function. For a global\n configuration file, that would likely be the CameraManager. A HAL\n configuration file would be handled in the HAL itself.\n\n- Thinking more about this, maybe this class isn't such a bad location\n to store this code, in which case we would need to move\n IPAProxy::configurationFile() here, and likely add a configuration\n file type enum parameter to the findFile() function.\n\nThe latter could actually allow us to implement configuration file\nmerging, where asking the ConfigurationFile class to parse, for\ninstance, the global libcamera.json, would look for it in\n/etc/libcamera/ first, and then would overwrite values with\n$HOME/.libcamera/libcamera.json. I'm not sure if that would end up being\nuseful, but it seems like a nice feature.\n\n> >>>>>> +std::string Configuration::findFile(std::string filename)\n> >>>>>> +{\n> >>>>>> +\tstatic std::array searchPaths = {\n> >>>>>> +\t\tLIBCAMERA_SYSCONF_DIR,\n> >>>>>> +\t\tLIBCAMERA_DATA_DIR,\n> >>>>>> +\t};\n> >>>>>> +\n> >>>>>> +\tstd::string root = utils::libcameraSourcePath();\n> >>>>>> +\tif (!root.empty()) {\n> >>>>>> +\t\tstd::string configurationPath = root + \"data/\" + filename;\n> >>>>>> +\n> >>>>>> +\t\tif (File::exists(configurationPath))\n> >>>>>> +\t\t\treturn configurationPath;\n> >>>>>> +\t}\n> >>>>>> +\n> >>\n> >> We might also want to expose an environment variable to add to the paths\n> >> here, as we do for IPA configuration files (LIBCAMERA_IPA_CONFIG_PATH)\n> >>\n> >> Hrm ... I should tackle the IPA configuration files as part of this\n> >> series too.\n> >>\n> >> It looks like that code path isn't really being used much yet though -\n> >> except for a dummy file at src/ipa/vimc/data/vimc.conf\n> >>\n> >> More things for me to look at ;-)\n> >>\n> >>>>>> +\tfor (std::string &path : searchPaths) {\n> >>>>>> +\t\tstd::string configurationPath = path + \"/\" + filename;\n> >>>>>> +\t\tif (File::exists(configurationPath))\n> >>>>>> +\t\t\treturn configurationPath;\n> >>>>>> +\t}\n> >>>>>> +\n> >>>>>> +\treturn \"\";\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> +/**\n> >>>>>> + * \\brief Open and parse a configuration file.\n> >>>>>> + *\n> >>>>>> + * The filename will be searched for on the libcamera configuration and paths,\n> >>>>>> + * and then parsed.\n> >>>>>> + *\n> >>>>>> + * Successfully parsed files will present the data contained therein through the\n> >>>>>> + * json object exposed from data();\n> >>>>>> + */\n> >>>>>> +int Configuration::open(std::string filename)\n> >>>>>> +{\n> >>>>>> +\tstd::string data = findFile(filename);\n> >>>>>> +\tif (data.empty()) {\n> >>>>>> +\t\tLOG(Configuration, Warning)\n> >>>>>> +\t\t\t<< \"file: \\\"\" << filename\n> >>>>>> +\t\t\t<< \"\\\" was not found.\";\n> >>>>>> +\t\treturn -ENOENT;\n> >>>>>> +\t}\n> >>>>>> +\n> >>>>>> +\tLOG(Configuration, Debug) << \"Reading configuration from \" << data;\n> >>>>>> +\n> >>>>>> +\t/* Parse with no error callbacks and exceptions disabled. */\n> >>>>>> +\tstd::ifstream input(data);\n> >>>>>> +\tjson j = json::parse(input, nullptr, false);\n> >>>>>> +\tif (j.is_discarded()) {\n> >>>>>> +\t\tLOG(Configuration, Error)\n> >>>>>> +\t\t\t<< \"file: \\\"\" << data\n> >>>>>> +\t\t\t<< \"\\\" was not parsable.\";\n> >>>>>> +\t\treturn -EINVAL;\n> >>>>>> +\t}\n> >>>>>> +\n> >>>>>> +\tjson_ = std::move(j);\n> >>>>>> +\n> >>>>>> +\treturn 0;\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> +} /* namespace libcamera */\n> >>>>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >>>>>> index 387d5d88ecae..5d655c87a7a0 100644\n> >>>>>> --- a/src/libcamera/meson.build\n> >>>>>> +++ b/src/libcamera/meson.build\n> >>>>>> @@ -9,6 +9,7 @@ libcamera_sources = files([\n> >>>>>> 'camera_controls.cpp',\n> >>>>>> 'camera_manager.cpp',\n> >>>>>> 'camera_sensor.cpp',\n> >>>>>> + 'configuration.cpp',\n> >>>>>> 'controls.cpp',\n> >>>>>> 'control_serializer.cpp',\n> >>>>>> 'control_validator.cpp',","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 05FECBDB20\n\tfor ;\n\tMon, 7 Dec 2020 18:38:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B4ED67E72;\n\tMon, 7 Dec 2020 19:38:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F9B867E6A\n\tfor ;\n\tMon, 7 Dec 2020 19:38:18 +0100 (CET)","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 D8A258D;\n\tMon, 7 Dec 2020 19:38:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QTuKiRPj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607366298;\n\tbh=JdpkpmdUxxGczZoOySK31Ma2g4OVX7rmJLwxnRULauw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QTuKiRPjIszcIK7WjJSUm3EGvNjgFXOT4kVS5ZZqka9F5wC/iyvrNxqTa0dbMEkLt\n\tpYjGUbVhP0SdK4C0sIVSAHkSEEvBXsscDFWmOlhvtWF5Pve9wFNTwQ4rEpvsQKGVpD\n\tN9z5B9V5CJERBYxC/dcB5hfj9c6IGR7ADRBI0BYs=","Date":"Mon, 7 Dec 2020 20:38:14 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20201123164319.152742-1-kieran.bingham@ideasonboard.com>\n\t<20201123164319.152742-7-kieran.bingham@ideasonboard.com>\n\t<20201125085257.gsmpldpc3u4huj4u@uno.localdomain>\n\t<49ed2710-dc50-b05e-3194-583e51462b09@ideasonboard.com>\n\t<20201125101826.wutdjfogckbemqmz@uno.localdomain>\n\t<1900c8b7-9f6d-379a-d9f7-19052b1ff411@ideasonboard.com>\n\t<20201201180230.GR4569@pendragon.ideasonboard.com>\n\t<25112de2-f25d-f3d3-b27b-37c594093c01@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<25112de2-f25d-f3d3-b27b-37c594093c01@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 6/8] libcamera: Add configuration\n\tinterface","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]