{"id":12980,"url":"https://patchwork.libcamera.org/api/patches/12980/?format=json","web_url":"https://patchwork.libcamera.org/patch/12980/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210715211459.19373-10-laurent.pinchart@ideasonboard.com>","date":"2021-07-15T21:14:35","name":"[libcamera-devel,v3,09/33] cam: options: Support parent-child relationship between options","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"9d38d7acdaadaf90539d11cab79c212871f958cb","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/12980/mbox/","series":[{"id":2242,"url":"https://patchwork.libcamera.org/api/series/2242/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2242","date":"2021-07-15T21:14:26","name":"Multi-camera support in the cam application","version":3,"mbox":"https://patchwork.libcamera.org/series/2242/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12980/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12980/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 B7DB1C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jul 2021 21:15:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE02F68570;\n\tThu, 15 Jul 2021 23:15:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E33968546\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jul 2021 23:15:07 +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 2AA20A24\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jul 2021 23:15:07 +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=\"oOApXN9Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626383707;\n\tbh=3CbVBqlvR4y2KFGEeYoAcZ0MxdCSJT+hk2B2NKx6XR4=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=oOApXN9Y1ntIx2bgN16xXcpuqq1nQ6fWTb9GEj24skxrkbMDEtXGY+/tQJYXeM6O5\n\tjsfX6eJL7p+V8sI7+ujihoNlyIlsBswvJ7Bp/qxaAkyE/b878zE0bE6/MxbD7tpwUW\n\tHjOa+RbEVQBDyxNxNBYUvCcJZwrrZOktJkRZBDIw=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 16 Jul 2021 00:14:35 +0300","Message-Id":"<20210715211459.19373-10-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20210715211459.19373-1-laurent.pinchart@ideasonboard.com>","References":"<20210715211459.19373-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 09/33] cam: options: Support\n\tparent-child relationship 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 hieararchy 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\nThe usage text automatically documents the options in their\ncorresponding context:\n\nOptions:\n  -c, --camera camera ...                               Specify which camera to operate on, by id or by index\n  -h, --help                                            Display this help message\n  -I, --info                                            Display information about stream(s)\n  -l, --list                                            List all cameras\n      --list-controls                                   List cameras controls\n  -p, --list-properties                                 List cameras properties\n  -m, --monitor                                         Monitor for hotplug and unplug camera events\n\nOptions valid in the context of --camera:\n  -C, --capture[=count]                                 Capture until interrupted by user or until <count> frames captured\n  -F, --file[=filename]                                 Write captured frames to disk\n                                                        If the file name ends with a '/', it sets the directory in which\n                                                        to write files, using the default file name. Otherwise it sets the\n                                                        full file path and name. The first '#' character in the file name\n                                                        is expanded to the camera index, stream name and frame sequence number.\n                                                        The default file name is 'frame-#.bin'.\n  -s, --stream key=value[,key=value,...] ...            Set configuration of a camera stream\n          height=integer                                Height in pixels\n          pixelformat=string                            Pixel format name\n          role=string                                   Role for the stream (viewfinder, video, still, raw)\n          width=integer                                 Width in pixels\n      --strict-formats                                  Do not allow requested stream format(s) to be adjusted\n      --metadata                                        Print the metadata for completed requests\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\nChanges since v1:\n\n- Add a sample help text to the commit message\n- Clarify comments\n- Fix typos\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 379b68c3be60..59b26be4deff 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@@ -335,7 +343,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@@ -472,6 +480,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 retrieved with the children() function, and are stored as an\n+ * OptionsBase<int>.\n  */\n \n /**\n@@ -662,6 +675,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@@ -724,6 +746,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@@ -747,13 +795,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@@ -770,9 +819,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@@ -790,13 +861,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@@ -821,26 +892,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@@ -853,7 +924,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@@ -881,10 +952,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@@ -906,15 +974,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@@ -937,6 +1006,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@@ -981,5 +1052,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 to 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've 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 back up the call stack. */\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 exist 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","v3","09/33"]}