Patch Detail
Show a patch.
GET /api/patches/12822/?format=api
{ "id": 12822, "url": "https://patchwork.libcamera.org/api/patches/12822/?format=api", "web_url": "https://patchwork.libcamera.org/patch/12822/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210707021941.20804-10-laurent.pinchart@ideasonboard.com>", "date": "2021-07-07T02:19:20", "name": "[libcamera-devel,09/30] cam: options: Support parent-child relationship between options", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "0e00a3e872917cd108fa62521ea5c5a5af48636d", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/12822/mbox/", "series": [ { "id": 2213, "url": "https://patchwork.libcamera.org/api/series/2213/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2213", "date": "2021-07-07T02:19:12", "name": "Multi-camera support in the cam application", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2213/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/12822/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/12822/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 17B7DBD794\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 7 Jul 2021 02:20:44 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5EA9068508;\n\tWed, 7 Jul 2021 04:20:39 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BB3556850B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 7 Jul 2021 04:20:34 +0200 (CEST)", "from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 51F1F466\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 7 Jul 2021 04:20:34 +0200 (CEST)" ], "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=\"Telmcv1A\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1625624434;\n\tbh=0ottbuWo/y+fVqmSnUM4bxnkf4AhDbIHUgARJhcD0jk=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=Telmcv1AJhXQfRJCyLHwFAj62tgQI3y3YIjmlFJgrBkJSJL3C9HGpYoVSpNrpxAyd\n\tSdfpGWPHthaAD0nBPZcifI9/ED129WxCTlLNYTXeil96C9BGsX+aAEz7nd0Qae/pMB\n\tWDPCkuB2ObBH8+edhUzUMJfwXGk+YcmoumlcungU=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 7 Jul 2021 05:19:20 +0300", "Message-Id": "<20210707021941.20804-10-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210707021941.20804-1-laurent.pinchart@ideasonboard.com>", "References": "<20210707021941.20804-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 09/30] cam: options: Support parent-child\n\trelationship between options", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Add support for creating a tree-based hiearchy of options instead of a\nflat list. This is useful to support options that need to be interpreted\nin the context of a particular occurrence of an array option.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/cam/options.cpp | 217 ++++++++++++++++++++++++++++++++++++++------\n src/cam/options.h | 14 ++-\n 2 files changed, 199 insertions(+), 32 deletions(-)", "diff": "diff --git a/src/cam/options.cpp b/src/cam/options.cpp\nindex 4a88c38fb154..d7f9f741c731 100644\n--- a/src/cam/options.cpp\n+++ b/src/cam/options.cpp\n@@ -79,6 +79,12 @@\n * \\var Option::isArray\n * \\brief Whether the option can appear once or multiple times\n *\n+ * \\var Option::parent\n+ * \\brief The parent option\n+ *\n+ * \\var Option::children\n+ * \\brief List of child options, storing all options whose parent is this option\n+ *\n * \\fn Option::hasShortOption()\n * \\brief Tell if the option has a short option specifier (e.g. `-f`)\n * \\return True if the option has a short option specifier, false otherwise\n@@ -96,6 +102,8 @@ struct Option {\n \tconst char *help;\n \tKeyValueParser *keyValueParser;\n \tbool isArray;\n+\tOption *parent;\n+\tstd::list<Option> children;\n \n \tbool hasShortOption() const { return isalnum(opt); }\n \tbool hasLongOption() const { return name != nullptr; }\n@@ -336,7 +344,7 @@ bool KeyValueParser::addOption(const char *name, OptionType type,\n \t\treturn false;\n \n \toptionsMap_[name] = Option({ 0, type, name, argument, nullptr,\n-\t\t\t\t help, nullptr, false });\n+\t\t\t\t help, nullptr, false, nullptr, {} });\n \treturn true;\n }\n \n@@ -473,6 +481,11 @@ void KeyValueParser::usage(int indent)\n * option. It supports empty values, integers, strings, key-value lists, as well\n * as arrays of those types. For array values, all array elements shall have the\n * same type.\n+ *\n+ * OptionValue instances are organized in a tree-based structure that matches\n+ * the parent-child relationship of the options added to the parser. Children\n+ * are retrieve with the children() function, and are stored as an\n+ * OptionsBase<int>.\n */\n \n /**\n@@ -663,6 +676,15 @@ std::vector<OptionValue> OptionValue::toArray() const\n \treturn array_;\n }\n \n+/**\n+ * \\brief Retrieve the list of child values\n+ * \\return The list of child values\n+ */\n+const OptionsParser::Options &OptionValue::children() const\n+{\n+\treturn children_;\n+}\n+\n /* -----------------------------------------------------------------------------\n * OptionsParser\n */\n@@ -725,6 +747,32 @@ std::vector<OptionValue> OptionValue::toArray() const\n * accept an argument, the option value can be access by Options::operator[]()\n * using the option identifier as the key. The order in which different options\n * are specified on the command line isn't preserved.\n+ *\n+ * Options can be created with parent-child relationships to organize them as a\n+ * tree instead of a flat list. When parsing a command line, the child options\n+ * are considered related to the parent option that precedes them. This is\n+ * useful when the parent is an array option. The Options values list generated\n+ * by the parser then turns into a tree, which each parent value storing the\n+ * values of child options that follow that instance of the parent option.\n+ * For instance, with a `capture` option specified as a child of a `camera`\n+ * array option, parsing the command line\n+ *\n+ * `--camera 1 --capture=10 --camera 2 --capture=20`\n+ *\n+ * will return an Options instance containing a single OptionValue instance of\n+ * array type, for the `camera` option. The OptionValue will contain two\n+ * entries, with the first entry containing the integer value 1 and the second\n+ * entry the integer value 2. Each of those entries will in turn store an\n+ * Options instance that contains the respective children. The first entry will\n+ * store in its children a `capture` option of value 10, and the second entry a\n+ * `capture` option of value 20.\n+ *\n+ * The command line\n+ *\n+ * `--capture=10 --camera 1`\n+ *\n+ * would result in a parsing error, as the `capture` option has no preceding\n+ * `camera` option on the command line.\n */\n \n /**\n@@ -748,13 +796,14 @@ OptionsParser::~OptionsParser() = default;\n * mandatory argument, or no argument at all\n * \\param[in] argumentName The argument name used in the help text\n * \\param[in] array Whether the option can appear once or multiple times\n+ * \\param[in] parent The identifier of the parent option (optional)\n *\n * \\return True if the option was added successfully, false if an error\n * occurred.\n */\n bool OptionsParser::addOption(int opt, OptionType type, const char *help,\n \t\t\t const char *name, OptionArgument argument,\n-\t\t\t const char *argumentName, bool array)\n+\t\t\t const char *argumentName, bool array, int parent)\n {\n \t/*\n \t * Options must have at least a short or long name, and a text message.\n@@ -771,9 +820,31 @@ bool OptionsParser::addOption(int opt, OptionType type, const char *help,\n \tif (optionsMap_.find(opt) != optionsMap_.end())\n \t\treturn false;\n \n-\toptions_.push_back(Option({ opt, type, name, argument, argumentName,\n-\t\t\t\t help, nullptr, array }));\n-\toptionsMap_[opt] = &options_.back();\n+\t/*\n+\t * If a parent is specified, create the option as a child of its parent.\n+\t * Otherwise, create it in the parser's options list.\n+\t */\n+\tOption *option;\n+\n+\tif (parent) {\n+\t\tauto iter = optionsMap_.find(parent);\n+\t\tif (iter == optionsMap_.end())\n+\t\t\treturn false;\n+\n+\t\tOption *parentOpt = iter->second;\n+\t\tparentOpt->children.push_back({\n+\t\t\topt, type, name, argument, argumentName, help, nullptr,\n+\t\t\tarray, parentOpt, {}\n+\t\t});\n+\t\toption = &parentOpt->children.back();\n+\t} else {\n+\t\toptions_.push_back({ opt, type, name, argument, argumentName,\n+\t\t\t\t help, nullptr, array, nullptr, {} });\n+\t\toption = &options_.back();\n+\t}\n+\n+\toptionsMap_[opt] = option;\n+\n \treturn true;\n }\n \n@@ -791,13 +862,13 @@ bool OptionsParser::addOption(int opt, OptionType type, const char *help,\n * occurred.\n */\n bool OptionsParser::addOption(int opt, KeyValueParser *parser, const char *help,\n-\t\t\t const char *name, bool array)\n+\t\t\t const char *name, bool array, int parent)\n {\n \tif (!addOption(opt, OptionKeyValue, help, name, ArgumentRequired,\n-\t\t \"key=value[,key=value,...]\", array))\n+\t\t \"key=value[,key=value,...]\", array, parent))\n \t\treturn false;\n \n-\toptions_.back().keyValueParser = parser;\n+\toptionsMap_[opt]->keyValueParser = parser;\n \treturn true;\n }\n \n@@ -822,26 +893,26 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n \t * Allocate short and long options arrays large enough to contain all\n \t * options.\n \t */\n-\tchar shortOptions[options_.size() * 3 + 2];\n-\tstruct option longOptions[options_.size() + 1];\n+\tchar shortOptions[optionsMap_.size() * 3 + 2];\n+\tstruct option longOptions[optionsMap_.size() + 1];\n \tunsigned int ids = 0;\n \tunsigned int idl = 0;\n \n \tshortOptions[ids++] = ':';\n \n-\tfor (const Option &option : options_) {\n-\t\tif (option.hasShortOption()) {\n-\t\t\tshortOptions[ids++] = option.opt;\n-\t\t\tif (option.argument != ArgumentNone)\n+\tfor (const auto [opt, option] : optionsMap_) {\n+\t\tif (option->hasShortOption()) {\n+\t\t\tshortOptions[ids++] = opt;\n+\t\t\tif (option->argument != ArgumentNone)\n \t\t\t\tshortOptions[ids++] = ':';\n-\t\t\tif (option.argument == ArgumentOptional)\n+\t\t\tif (option->argument == ArgumentOptional)\n \t\t\t\tshortOptions[ids++] = ':';\n \t\t}\n \n-\t\tif (option.hasLongOption()) {\n-\t\t\tlongOptions[idl].name = option.name;\n+\t\tif (option->hasLongOption()) {\n+\t\t\tlongOptions[idl].name = option->name;\n \n-\t\t\tswitch (option.argument) {\n+\t\t\tswitch (option->argument) {\n \t\t\tcase ArgumentNone:\n \t\t\t\tlongOptions[idl].has_arg = no_argument;\n \t\t\t\tbreak;\n@@ -854,7 +925,7 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n \t\t\t}\n \n \t\t\tlongOptions[idl].flag = 0;\n-\t\t\tlongOptions[idl].val = option.opt;\n+\t\t\tlongOptions[idl].val = option->opt;\n \t\t\tidl++;\n \t\t}\n \t}\n@@ -882,10 +953,7 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n \t\t}\n \n \t\tconst Option &option = *optionsMap_[c];\n-\t\tif (!options.parseValue(c, option, optarg)) {\n-\t\t\tstd::cerr << \"Can't parse \" << option.typeName()\n-\t\t\t\t << \" argument for option \" << option.optionName()\n-\t\t\t\t << std::endl;\n+\t\tif (!parseValue(option, optarg, &options)) {\n \t\t\tusage();\n \t\t\treturn options;\n \t\t}\n@@ -907,15 +975,16 @@ void OptionsParser::usage()\n {\n \tunsigned int indent = 0;\n \n-\tfor (const Option &option : options_) {\n+\tfor (const auto &opt : optionsMap_) {\n+\t\tconst Option *option = opt.second;\n \t\tunsigned int length = 14;\n-\t\tif (option.hasLongOption())\n-\t\t\tlength += 2 + strlen(option.name);\n-\t\tif (option.argument != ArgumentNone)\n-\t\t\tlength += 1 + strlen(option.argumentName);\n-\t\tif (option.argument == ArgumentOptional)\n+\t\tif (option->hasLongOption())\n+\t\t\tlength += 2 + strlen(option->name);\n+\t\tif (option->argument != ArgumentNone)\n+\t\t\tlength += 1 + strlen(option->argumentName);\n+\t\tif (option->argument == ArgumentOptional)\n \t\t\tlength += 2;\n-\t\tif (option.isArray)\n+\t\tif (option->isArray)\n \t\t\tlength += 4;\n \n \t\tif (length > indent)\n@@ -938,6 +1007,8 @@ void OptionsParser::usage()\n void OptionsParser::usageOptions(const std::list<Option> &options,\n \t\t\t\t unsigned int indent)\n {\n+\tstd::vector<const Option *> parentOptions;\n+\n \tfor (const Option &option : options) {\n \t\tstd::string argument;\n \t\tif (option.hasShortOption())\n@@ -982,5 +1053,91 @@ void OptionsParser::usageOptions(const std::list<Option> &options,\n \n \t\tif (option.keyValueParser)\n \t\t\toption.keyValueParser->usage(indent);\n+\n+\t\tif (!option.children.empty())\n+\t\t\tparentOptions.push_back(&option);\n \t}\n+\n+\tif (parentOptions.empty())\n+\t\treturn;\n+\n+\tfor (const Option *option : parentOptions) {\n+\t\tstd::cerr << std::endl << \"Options valid in the context of \"\n+\t\t\t << option->optionName() << \":\" << std::endl;\n+\t\tusageOptions(option->children, indent);\n+\t}\n+}\n+\n+std::tuple<OptionsParser::Options *, const Option *>\n+OptionsParser::childOption(const Option *parent, Options *options)\n+{\n+\t/*\n+\t * The parent argument points to the parent of the leaf node Option,\n+\t * and the options argument to the root node of the Options tree. Use\n+\t * recursive calls traverse the Option tree up to the root node while\n+\t * traversing the Options tree down to the leaf node:\n+\t */\n+\n+\t/*\n+\t * - If we have no parent, we're reached the root node of the Option\n+\t * tree, the options argument is what we need.\n+\t */\n+\tif (!parent)\n+\t\treturn { options, nullptr };\n+\n+\t/*\n+\t * - If the parent has a parent, use recursion to move one level up the\n+\t * Option tree. This returns the Options corresponding to parent, or\n+\t * nullptr if a suitable Options child isn't found.\n+\t */\n+\tif (parent->parent) {\n+\t\tconst Option *error;\n+\t\tstd::tie(options, error) = childOption(parent->parent, options);\n+\n+\t\t/* Propagate the error all the way up. */\n+\t\tif (!error)\n+\t\t\treturn { options, error };\n+\t}\n+\n+\t/*\n+\t * - The parent has no parent, we're now one level down the root.\n+\t * Return the Options child corresponding to the parent. The child may\n+\t * not exists if options are specified in an incorrect order.\n+\t */\n+\tif (!options->isSet(parent->opt))\n+\t\treturn { nullptr, parent };\n+\n+\t/*\n+\t * If the child value is of array type, children are not stored in the\n+\t * value .children() list, but in the .children() of the value's array\n+\t * elements. Use the last array element in that case, as a child option\n+\t * relates to the last instance of its parent option.\n+\t */\n+\tconst OptionValue *value = &(*options)[parent->opt];\n+\tif (value->type() == OptionValue::ValueArray)\n+\t\tvalue = &value->toArray().back();\n+\n+\treturn { const_cast<Options *>(&value->children()), nullptr };\n+}\n+\n+bool OptionsParser::parseValue(const Option &option, const char *arg,\n+\t\t\t Options *options)\n+{\n+\tconst Option *error;\n+\n+\tstd::tie(options, error) = childOption(option.parent, options);\n+\tif (error) {\n+\t\tstd::cerr << \"Option \" << option.optionName() << \" requires a \"\n+\t\t\t << error->optionName() << \" context\" << std::endl;\n+\t\treturn false;\n+\t}\n+\n+\tif (!options->parseValue(option.opt, option, arg)) {\n+\t\tstd::cerr << \"Can't parse \" << option.typeName()\n+\t\t\t << \" argument for option \" << option.optionName()\n+\t\t\t << std::endl;\n+\t\treturn false;\n+\t}\n+\n+\treturn true;\n }\ndiff --git a/src/cam/options.h b/src/cam/options.h\nindex 5c51a94c2f37..e894822c0061 100644\n--- a/src/cam/options.h\n+++ b/src/cam/options.h\n@@ -10,6 +10,7 @@\n #include <ctype.h>\n #include <list>\n #include <map>\n+#include <tuple>\n #include <vector>\n \n class KeyValueParser;\n@@ -91,9 +92,11 @@ public:\n \tbool addOption(int opt, OptionType type, const char *help,\n \t\t const char *name = nullptr,\n \t\t OptionArgument argument = ArgumentNone,\n-\t\t const char *argumentName = nullptr, bool array = false);\n+\t\t const char *argumentName = nullptr, bool array = false,\n+\t\t int parent = 0);\n \tbool addOption(int opt, KeyValueParser *parser, const char *help,\n-\t\t const char *name = nullptr, bool array = false);\n+\t\t const char *name = nullptr, bool array = false,\n+\t\t int parent = 0);\n \n \tOptions parse(int argc, char *argv[]);\n \tvoid usage();\n@@ -104,6 +107,10 @@ private:\n \n \tvoid usageOptions(const std::list<Option> &options, unsigned int indent);\n \n+\tstd::tuple<OptionsParser::Options *, const Option *>\n+\tchildOption(const Option *parent, Options *options);\n+\tbool parseValue(const Option &option, const char *arg, Options *options);\n+\n \tstd::list<Option> options_;\n \tstd::map<unsigned int, Option *> optionsMap_;\n };\n@@ -139,12 +146,15 @@ public:\n \tKeyValueParser::Options toKeyValues() const;\n \tstd::vector<OptionValue> toArray() const;\n \n+\tconst OptionsParser::Options &children() const;\n+\n private:\n \tValueType type_;\n \tint integer_;\n \tstd::string string_;\n \tKeyValueParser::Options keyValues_;\n \tstd::vector<OptionValue> array_;\n+\tOptionsParser::Options children_;\n };\n \n #endif /* __CAM_OPTIONS_H__ */\n", "prefixes": [ "libcamera-devel", "09/30" ] }