[{"id":1127,"web_url":"https://patchwork.libcamera.org/comment/1127/","msgid":"<20190326062337.GG4563@pendragon.ideasonboard.com>","date":"2019-03-26T06:23:37","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> To allow specifying the same argument option multiple times a new type\n> of OptionValue is needed. As parsing of options is an iterative process\n> there is a need to append options as they are parsed so instead of\n> setting values using the constructor a new add() method is used.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/options.cpp | 19 +++++++++++++++++++\n> src/cam/options.h | 7 +++++++\n> 2 files changed, 26 insertions(+)\n> \n> diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> index 497833397d894f82..0dec154815d3cad5 100644\n> --- a/src/cam/options.cpp\n> +++ b/src/cam/options.cpp\n> @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> {\n> }\n> \n> +void OptionValue::add(const OptionValue &value)\n\nI think add is a bit too generic as a name. I would name this addValue()\nor possibly push_back() if we want to stick with the STL semantics (I\nthink I have a preference for the former though).\n\n> +{\n> +\ttype_ = ValueArray;\n\nI wonder if this should error out if type_ is different than ValueArray\nor ValueNone. I suppose the caller wouldn't check for this though, and\nit would be a bug in the option parser, so I suppose we could keep it\nas-is (possibly with an std::assert ?).\n\nReviewed-by: Laurent Pinchart \n\n> +\tarray_.push_back(value);\n> +}\n> +\n> OptionValue::operator int() const\n> {\n> \treturn toInteger();\n> @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> \treturn toKeyValues();\n> }\n> \n> +OptionValue::operator std::vector() const\n> +{\n> +\treturn toArray();\n> +}\n> +\n> int OptionValue::toInteger() const\n> {\n> \tif (type_ != ValueInteger)\n> @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> \treturn keyValues_;\n> }\n> \n> +std::vector OptionValue::toArray() const\n> +{\n> +\tif (type_ != ValueArray)\n> +\t\treturn std::vector{};\n> +\n> +\treturn array_;\n> +}\n> +\n> /* -----------------------------------------------------------------------------\n> * OptionsParser\n> */\n> diff --git a/src/cam/options.h b/src/cam/options.h\n> index b33a90fc6058febf..6a887416c0070c41 100644\n> --- a/src/cam/options.h\n> +++ b/src/cam/options.h\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> class KeyValueParser;\n> class OptionValue;\n> @@ -84,6 +85,7 @@ public:\n> \t\tValueInteger,\n> \t\tValueString,\n> \t\tValueKeyValue,\n> +\t\tValueArray,\n> \t};\n> \n> \tOptionValue();\n> @@ -92,21 +94,26 @@ public:\n> \tOptionValue(const std::string &value);\n> \tOptionValue(const KeyValueParser::Options &value);\n> \n> +\tvoid add(const OptionValue &value);\n> +\n> \tValueType type() const { return type_; }\n> \n> \toperator int() const;\n> \toperator std::string() const;\n> \toperator KeyValueParser::Options() const;\n> +\toperator std::vector() const;\n> \n> \tint toInteger() const;\n> \tstd::string toString() const;\n> \tKeyValueParser::Options toKeyValues() const;\n> +\tstd::vector toArray() const;\n> \n> private:\n> \tValueType type_;\n> \tint integer_;\n> \tstd::string string_;\n> \tKeyValueParser::Options keyValues_;\n> +\tstd::vector array_;\n> };\n> \n> class OptionsParser","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2526610B3\n\tfor ;\n\tTue, 26 Mar 2019 07:23:55 +0100 (CET)","from pendragon.ideasonboard.com (85-76-50-100-nat.elisa-mobile.fi\n\t[85.76.50.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D89C2E1;\n\tTue, 26 Mar 2019 07:23:53 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1553581434;\n\tbh=k1J9VtCILOqZJtGUL7xSckwt3cGVzSPkx+8aCzDd4Wo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BT2+a/W93D2RkpooUSvaoDxwX9v8uATE+69urDnSWyBFMbi9hLDkM0bCqzbd6JENC\n\tFgWn2PwQHbtaUiCGaZMB6FJxRTZFUMtYM0XlzaOVWb85hbNqcbwsFyMVe312Q46vaf\n\tV9ItiqVtsseAQ8PZ8+ZdNia9sMXsyZp5bAo1FqSw=","Date":"Tue, 26 Mar 2019 08:23:37 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326062337.GG4563@pendragon.ideasonboard.com>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190325234736.12533-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 06:23:56 -0000"}},{"id":1130,"web_url":"https://patchwork.libcamera.org/comment/1130/","msgid":"<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>","date":"2019-03-26T10:30:13","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> To allow specifying the same argument option multiple times a new type\n> of OptionValue is needed. As parsing of options is an iterative process\n> there is a need to append options as they are parsed so instead of\n> setting values using the constructor a new add() method is used.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/options.cpp | 19 +++++++++++++++++++\n> src/cam/options.h | 7 +++++++\n> 2 files changed, 26 insertions(+)\n>\n> diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> index 497833397d894f82..0dec154815d3cad5 100644\n> --- a/src/cam/options.cpp\n> +++ b/src/cam/options.cpp\n> @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> {\n> }\n>\n> +void OptionValue::add(const OptionValue &value)\n> +{\n> +\ttype_ = ValueArray;\n> +\tarray_.push_back(value);\n> +}\n> +\n\nI wonder how that would look like if we separate OptionValue (which\nholds the actual multi-type option value) from the array.\n\nI'm not expert of this code, but OptionBase has a 'values_' map, which\nassociates the opt key with an OptionValue that holds the actually option\nvalue and this OptionValue, since this patch, could be an array too.\n\nI wonder how that would look like it the 'values_' map would use\nanother type, which maintains OptionValues into a a vector, so that\nall OptionValues could be stored as array without introducing the new\n'array_' field.\n\nSomething like:\n\nclass OptionBase\n{\n\n ...\n\tstd::map values_;\n\n};\n\nclass OptionList\n{\n ...\n std::vector array_;\n};\n\nclass OptionValue\n{\n ....\n Hold the basic types as it did already;\n};\n\nDoes this make any sense to you?\n\nThanks\n j\n\n> OptionValue::operator int() const\n> {\n> \treturn toInteger();\n> @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> \treturn toKeyValues();\n> }\n>\n> +OptionValue::operator std::vector() const\n> +{\n> +\treturn toArray();\n> +}\n> +\n> int OptionValue::toInteger() const\n> {\n> \tif (type_ != ValueInteger)\n> @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> \treturn keyValues_;\n> }\n>\n> +std::vector OptionValue::toArray() const\n> +{\n> +\tif (type_ != ValueArray)\n> +\t\treturn std::vector{};\n> +\n> +\treturn array_;\n> +}\n> +\n> /* -----------------------------------------------------------------------------\n> * OptionsParser\n> */\n> diff --git a/src/cam/options.h b/src/cam/options.h\n> index b33a90fc6058febf..6a887416c0070c41 100644\n> --- a/src/cam/options.h\n> +++ b/src/cam/options.h\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> class KeyValueParser;\n> class OptionValue;\n> @@ -84,6 +85,7 @@ public:\n> \t\tValueInteger,\n> \t\tValueString,\n> \t\tValueKeyValue,\n> +\t\tValueArray,\n> \t};\n>\n> \tOptionValue();\n> @@ -92,21 +94,26 @@ public:\n> \tOptionValue(const std::string &value);\n> \tOptionValue(const KeyValueParser::Options &value);\n>\n> +\tvoid add(const OptionValue &value);\n> +\n> \tValueType type() const { return type_; }\n>\n> \toperator int() const;\n> \toperator std::string() const;\n> \toperator KeyValueParser::Options() const;\n> +\toperator std::vector() const;\n>\n> \tint toInteger() const;\n> \tstd::string toString() const;\n> \tKeyValueParser::Options toKeyValues() const;\n> +\tstd::vector toArray() const;\n>\n> private:\n> \tValueType type_;\n> \tint integer_;\n> \tstd::string string_;\n> \tKeyValueParser::Options keyValues_;\n> +\tstd::vector array_;\n> };\n>\n> class OptionsParser\n> --\n> 2.21.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BC636110D\n\tfor ;\n\tTue, 26 Mar 2019 11:29:32 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id E65FF60010;\n\tTue, 26 Mar 2019 10:29:31 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 26 Mar 2019 11:30:13 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"ngbkwg7zk5aixstt\"","Content-Disposition":"inline","In-Reply-To":"<20190325234736.12533-3-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 10:29:32 -0000"}},{"id":1131,"web_url":"https://patchwork.libcamera.org/comment/1131/","msgid":"<20190326103525.GB9119@bigcity.dyn.berto.se>","date":"2019-03-26T10:35:25","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2019-03-26 11:30:13 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> > To allow specifying the same argument option multiple times a new type\n> > of OptionValue is needed. As parsing of options is an iterative process\n> > there is a need to append options as they are parsed so instead of\n> > setting values using the constructor a new add() method is used.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/cam/options.cpp | 19 +++++++++++++++++++\n> > src/cam/options.h | 7 +++++++\n> > 2 files changed, 26 insertions(+)\n> >\n> > diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> > index 497833397d894f82..0dec154815d3cad5 100644\n> > --- a/src/cam/options.cpp\n> > +++ b/src/cam/options.cpp\n> > @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> > {\n> > }\n> >\n> > +void OptionValue::add(const OptionValue &value)\n> > +{\n> > +\ttype_ = ValueArray;\n> > +\tarray_.push_back(value);\n> > +}\n> > +\n> \n> I wonder how that would look like if we separate OptionValue (which\n> holds the actual multi-type option value) from the array.\n> \n> I'm not expert of this code, but OptionBase has a 'values_' map, which\n> associates the opt key with an OptionValue that holds the actually option\n> value and this OptionValue, since this patch, could be an array too.\n> \n> I wonder how that would look like it the 'values_' map would use\n> another type, which maintains OptionValues into a a vector, so that\n> all OptionValues could be stored as array without introducing the new\n> 'array_' field.\n> \n> Something like:\n> \n> class OptionBase\n> {\n> \n> ...\n> \tstd::map values_;\n> \n> };\n> \n> class OptionList\n> {\n> ...\n> std::vector array_;\n> };\n> \n> class OptionValue\n> {\n> ....\n> Hold the basic types as it did already;\n> };\n> \n> Does this make any sense to you?\n\nThat would have been a nice idea, if all options where arrays. As array \noptions are the exception, the main use-case is non-array options. Using \nyour suggestion a user of the parser would have to jump thru hoops to \naccess the non-array options by accessing them in a vector with 1 \nmember, right?\n\n> \n> Thanks\n> j\n> \n> > OptionValue::operator int() const\n> > {\n> > \treturn toInteger();\n> > @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> > \treturn toKeyValues();\n> > }\n> >\n> > +OptionValue::operator std::vector() const\n> > +{\n> > +\treturn toArray();\n> > +}\n> > +\n> > int OptionValue::toInteger() const\n> > {\n> > \tif (type_ != ValueInteger)\n> > @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> > \treturn keyValues_;\n> > }\n> >\n> > +std::vector OptionValue::toArray() const\n> > +{\n> > +\tif (type_ != ValueArray)\n> > +\t\treturn std::vector{};\n> > +\n> > +\treturn array_;\n> > +}\n> > +\n> > /* -----------------------------------------------------------------------------\n> > * OptionsParser\n> > */\n> > diff --git a/src/cam/options.h b/src/cam/options.h\n> > index b33a90fc6058febf..6a887416c0070c41 100644\n> > --- a/src/cam/options.h\n> > +++ b/src/cam/options.h\n> > @@ -10,6 +10,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> >\n> > class KeyValueParser;\n> > class OptionValue;\n> > @@ -84,6 +85,7 @@ public:\n> > \t\tValueInteger,\n> > \t\tValueString,\n> > \t\tValueKeyValue,\n> > +\t\tValueArray,\n> > \t};\n> >\n> > \tOptionValue();\n> > @@ -92,21 +94,26 @@ public:\n> > \tOptionValue(const std::string &value);\n> > \tOptionValue(const KeyValueParser::Options &value);\n> >\n> > +\tvoid add(const OptionValue &value);\n> > +\n> > \tValueType type() const { return type_; }\n> >\n> > \toperator int() const;\n> > \toperator std::string() const;\n> > \toperator KeyValueParser::Options() const;\n> > +\toperator std::vector() const;\n> >\n> > \tint toInteger() const;\n> > \tstd::string toString() const;\n> > \tKeyValueParser::Options toKeyValues() const;\n> > +\tstd::vector toArray() const;\n> >\n> > private:\n> > \tValueType type_;\n> > \tint integer_;\n> > \tstd::string string_;\n> > \tKeyValueParser::Options keyValues_;\n> > +\tstd::vector array_;\n> > };\n> >\n> > class OptionsParser\n> > --\n> > 2.21.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com\n\t[IPv6:2a00:1450:4864:20::22d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1ABD86110D\n\tfor ;\n\tTue, 26 Mar 2019 11:35:28 +0100 (CET)","by mail-lj1-x22d.google.com with SMTP id y6so10599170ljd.12\n\tfor ;\n\tTue, 26 Mar 2019 03:35:28 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tu18sm4013708ljd.77.2019.03.26.03.35.26\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 26 Mar 2019 03:35:26 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=UXiW6/RojxPNdcrllNf+rKxCFqsurkIQ2KYsCsr/46Y=;\n\tb=ms5UaVkCQeGjFOec+kAGXezSqo/IXUthHL92SV7oEz+1Q8jqJqp9LTInQwJtoY7Bn/\n\tlQZok6SwWm1NauT0uUan0TvGsIwFp5dYcBDcloSYVw9n0x/BVh9Hej7dDB5cjCJJckxE\n\tTDE+5DQWQlr/7tZ34u0AbckRy9EjXpjoHBXysnOYqRWYkADXm1goCZjXObKWoF1kpLE2\n\tCsMKmOYFU3Y6ffF4MIEgtRU3m16Vzml0inqL2+ZAQwSjllb/7KxIpWBH03kankRvS3Pn\n\tIIPGsDgyERSwvk60EPHvTG+gtbQiUVJybVH5kbsyAAIdRJwCZkKCbW13lNYsvIr2RqAR\n\t0meQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=UXiW6/RojxPNdcrllNf+rKxCFqsurkIQ2KYsCsr/46Y=;\n\tb=MiNGemdiHE6ZC6bZ9ps9Zyi4UhErVLRce3Zaao0jfu2dha9vvB9ZJCW+0qpy1eMiY9\n\taV7M19SFJ2MH1exsA3hnqE2FjzB0Oii47CSWGTObEAKH7BzXnu412Jmb9rrMKyNz8+ER\n\tp5PKJ+/8l3S6IPzLQdEd4YnQNd4hXtm7StY7/KvJ4m34d1tGdTx5A37KH6I9/6g7+aJE\n\tKiJKiKuxtJj59WQRNHHrQU6gWIxZUu3NbQOLMca+NCFMWhZUKl76o6SLzKkh/omhkhzX\n\tQ85ukAgOKyrJOSW9CeFCEVW/amOR2yMmeaS/jQ5jI6d5KqFwoa534HKyU6ZetUYaf4TX\n\t8eyQ==","X-Gm-Message-State":"APjAAAWtt1zvxPH2TxUgeH0puYH0JXMt4OuQHFEvmhIAH3jPXdgdCji6\n\tmx45xaBoYTzVlLPktNVYWPrt5A==","X-Google-Smtp-Source":"APXvYqydpPAiHlrdfwj+5zHcB9yMh/8ej9YqQALrTaopRkKHbMjB53Arb7BiYmyapiCyhSBfPm97yw==","X-Received":"by 2002:a2e:8658:: with SMTP id\n\ti24mr15723594ljj.194.1553596527122; \n\tTue, 26 Mar 2019 03:35:27 -0700 (PDT)","Date":"Tue, 26 Mar 2019 11:35:25 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326103525.GB9119@bigcity.dyn.berto.se>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>\n\t<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 10:35:28 -0000"}},{"id":1132,"web_url":"https://patchwork.libcamera.org/comment/1132/","msgid":"<20190326104045.vnwlgi6t7aarclz6@uno.localdomain>","date":"2019-03-26T10:40:45","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Mar 26, 2019 at 11:35:25AM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback.\n>\n> On 2019-03-26 11:30:13 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> > > To allow specifying the same argument option multiple times a new type\n> > > of OptionValue is needed. As parsing of options is an iterative process\n> > > there is a need to append options as they are parsed so instead of\n> > > setting values using the constructor a new add() method is used.\n> > >\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > src/cam/options.cpp | 19 +++++++++++++++++++\n> > > src/cam/options.h | 7 +++++++\n> > > 2 files changed, 26 insertions(+)\n> > >\n> > > diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> > > index 497833397d894f82..0dec154815d3cad5 100644\n> > > --- a/src/cam/options.cpp\n> > > +++ b/src/cam/options.cpp\n> > > @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> > > {\n> > > }\n> > >\n> > > +void OptionValue::add(const OptionValue &value)\n> > > +{\n> > > +\ttype_ = ValueArray;\n> > > +\tarray_.push_back(value);\n> > > +}\n> > > +\n> >\n> > I wonder how that would look like if we separate OptionValue (which\n> > holds the actual multi-type option value) from the array.\n> >\n> > I'm not expert of this code, but OptionBase has a 'values_' map, which\n> > associates the opt key with an OptionValue that holds the actually option\n> > value and this OptionValue, since this patch, could be an array too.\n> >\n> > I wonder how that would look like it the 'values_' map would use\n> > another type, which maintains OptionValues into a a vector, so that\n> > all OptionValues could be stored as array without introducing the new\n> > 'array_' field.\n> >\n> > Something like:\n> >\n> > class OptionBase\n> > {\n> >\n> > ...\n> > \tstd::map values_;\n> >\n> > };\n> >\n> > class OptionList\n> > {\n> > ...\n> > std::vector array_;\n> > };\n> >\n> > class OptionValue\n> > {\n> > ....\n> > Hold the basic types as it did already;\n> > };\n> >\n> > Does this make any sense to you?\n>\n> That would have been a nice idea, if all options where arrays. As array\n> options are the exception, the main use-case is non-array options. Using\n> your suggestion a user of the parser would have to jump thru hoops to\n> access the non-array options by accessing them in a vector with 1\n> member, right?\n\nYes, they would be vectors with 1 member. Which part of handling a\nsingle entry vector concerns you? Insertion at parsing time or access\nto the option values?\n\nThanks\n j\n\n>\n> >\n> > Thanks\n> > j\n> >\n> > > OptionValue::operator int() const\n> > > {\n> > > \treturn toInteger();\n> > > @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> > > \treturn toKeyValues();\n> > > }\n> > >\n> > > +OptionValue::operator std::vector() const\n> > > +{\n> > > +\treturn toArray();\n> > > +}\n> > > +\n> > > int OptionValue::toInteger() const\n> > > {\n> > > \tif (type_ != ValueInteger)\n> > > @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> > > \treturn keyValues_;\n> > > }\n> > >\n> > > +std::vector OptionValue::toArray() const\n> > > +{\n> > > +\tif (type_ != ValueArray)\n> > > +\t\treturn std::vector{};\n> > > +\n> > > +\treturn array_;\n> > > +}\n> > > +\n> > > /* -----------------------------------------------------------------------------\n> > > * OptionsParser\n> > > */\n> > > diff --git a/src/cam/options.h b/src/cam/options.h\n> > > index b33a90fc6058febf..6a887416c0070c41 100644\n> > > --- a/src/cam/options.h\n> > > +++ b/src/cam/options.h\n> > > @@ -10,6 +10,7 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > class KeyValueParser;\n> > > class OptionValue;\n> > > @@ -84,6 +85,7 @@ public:\n> > > \t\tValueInteger,\n> > > \t\tValueString,\n> > > \t\tValueKeyValue,\n> > > +\t\tValueArray,\n> > > \t};\n> > >\n> > > \tOptionValue();\n> > > @@ -92,21 +94,26 @@ public:\n> > > \tOptionValue(const std::string &value);\n> > > \tOptionValue(const KeyValueParser::Options &value);\n> > >\n> > > +\tvoid add(const OptionValue &value);\n> > > +\n> > > \tValueType type() const { return type_; }\n> > >\n> > > \toperator int() const;\n> > > \toperator std::string() const;\n> > > \toperator KeyValueParser::Options() const;\n> > > +\toperator std::vector() const;\n> > >\n> > > \tint toInteger() const;\n> > > \tstd::string toString() const;\n> > > \tKeyValueParser::Options toKeyValues() const;\n> > > +\tstd::vector toArray() const;\n> > >\n> > > private:\n> > > \tValueType type_;\n> > > \tint integer_;\n> > > \tstd::string string_;\n> > > \tKeyValueParser::Options keyValues_;\n> > > +\tstd::vector array_;\n> > > };\n> > >\n> > > class OptionsParser\n> > > --\n> > > 2.21.0\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n>\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 499646110D\n\tfor ;\n\tTue, 26 Mar 2019 11:40:04 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id ABBA2240023;\n\tTue, 26 Mar 2019 10:40:03 +0000 (UTC)"],"Date":"Tue, 26 Mar 2019 11:40:45 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326104045.vnwlgi6t7aarclz6@uno.localdomain>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>\n\t<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>\n\t<20190326103525.GB9119@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"yuir2wr3fmg654fm\"","Content-Disposition":"inline","In-Reply-To":"<20190326103525.GB9119@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 10:40:04 -0000"}},{"id":1136,"web_url":"https://patchwork.libcamera.org/comment/1136/","msgid":"<20190326110018.GD9119@bigcity.dyn.berto.se>","date":"2019-03-26T11:00:19","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nOn 2019-03-26 11:40:45 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Mar 26, 2019 at 11:35:25AM +0100, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your feedback.\n> >\n> > On 2019-03-26 11:30:13 +0100, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> > > > To allow specifying the same argument option multiple times a new type\n> > > > of OptionValue is needed. As parsing of options is an iterative process\n> > > > there is a need to append options as they are parsed so instead of\n> > > > setting values using the constructor a new add() method is used.\n> > > >\n> > > > Signed-off-by: Niklas Söderlund \n> > > > ---\n> > > > src/cam/options.cpp | 19 +++++++++++++++++++\n> > > > src/cam/options.h | 7 +++++++\n> > > > 2 files changed, 26 insertions(+)\n> > > >\n> > > > diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> > > > index 497833397d894f82..0dec154815d3cad5 100644\n> > > > --- a/src/cam/options.cpp\n> > > > +++ b/src/cam/options.cpp\n> > > > @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> > > > {\n> > > > }\n> > > >\n> > > > +void OptionValue::add(const OptionValue &value)\n> > > > +{\n> > > > +\ttype_ = ValueArray;\n> > > > +\tarray_.push_back(value);\n> > > > +}\n> > > > +\n> > >\n> > > I wonder how that would look like if we separate OptionValue (which\n> > > holds the actual multi-type option value) from the array.\n> > >\n> > > I'm not expert of this code, but OptionBase has a 'values_' map, which\n> > > associates the opt key with an OptionValue that holds the actually option\n> > > value and this OptionValue, since this patch, could be an array too.\n> > >\n> > > I wonder how that would look like it the 'values_' map would use\n> > > another type, which maintains OptionValues into a a vector, so that\n> > > all OptionValues could be stored as array without introducing the new\n> > > 'array_' field.\n> > >\n> > > Something like:\n> > >\n> > > class OptionBase\n> > > {\n> > >\n> > > ...\n> > > \tstd::map values_;\n> > >\n> > > };\n> > >\n> > > class OptionList\n> > > {\n> > > ...\n> > > std::vector array_;\n> > > };\n> > >\n> > > class OptionValue\n> > > {\n> > > ....\n> > > Hold the basic types as it did already;\n> > > };\n> > >\n> > > Does this make any sense to you?\n> >\n> > That would have been a nice idea, if all options where arrays. As array\n> > options are the exception, the main use-case is non-array options. Using\n> > your suggestion a user of the parser would have to jump thru hoops to\n> > access the non-array options by accessing them in a vector with 1\n> > member, right?\n> \n> Yes, they would be vectors with 1 member. Which part of handling a\n> single entry vector concerns you? Insertion at parsing time or access\n> to the option values?\n\nNone of the above. What concerns me is how the usage of the parser, \nright now and with the array additions uses still access non-array \noptions as:\n\nstd::string name = options[OptCamera];\n\nIf everything was vectors it would need to be something like\n\nstd::string name = options[OptCamera].front();\n\nWhich is not a nice usage interface as the bulk of options would not be \nrepeatable.\n\n> \n> Thanks\n> j\n> \n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > OptionValue::operator int() const\n> > > > {\n> > > > \treturn toInteger();\n> > > > @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> > > > \treturn toKeyValues();\n> > > > }\n> > > >\n> > > > +OptionValue::operator std::vector() const\n> > > > +{\n> > > > +\treturn toArray();\n> > > > +}\n> > > > +\n> > > > int OptionValue::toInteger() const\n> > > > {\n> > > > \tif (type_ != ValueInteger)\n> > > > @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> > > > \treturn keyValues_;\n> > > > }\n> > > >\n> > > > +std::vector OptionValue::toArray() const\n> > > > +{\n> > > > +\tif (type_ != ValueArray)\n> > > > +\t\treturn std::vector{};\n> > > > +\n> > > > +\treturn array_;\n> > > > +}\n> > > > +\n> > > > /* -----------------------------------------------------------------------------\n> > > > * OptionsParser\n> > > > */\n> > > > diff --git a/src/cam/options.h b/src/cam/options.h\n> > > > index b33a90fc6058febf..6a887416c0070c41 100644\n> > > > --- a/src/cam/options.h\n> > > > +++ b/src/cam/options.h\n> > > > @@ -10,6 +10,7 @@\n> > > > #include \n> > > > #include \n> > > > #include \n> > > > +#include \n> > > >\n> > > > class KeyValueParser;\n> > > > class OptionValue;\n> > > > @@ -84,6 +85,7 @@ public:\n> > > > \t\tValueInteger,\n> > > > \t\tValueString,\n> > > > \t\tValueKeyValue,\n> > > > +\t\tValueArray,\n> > > > \t};\n> > > >\n> > > > \tOptionValue();\n> > > > @@ -92,21 +94,26 @@ public:\n> > > > \tOptionValue(const std::string &value);\n> > > > \tOptionValue(const KeyValueParser::Options &value);\n> > > >\n> > > > +\tvoid add(const OptionValue &value);\n> > > > +\n> > > > \tValueType type() const { return type_; }\n> > > >\n> > > > \toperator int() const;\n> > > > \toperator std::string() const;\n> > > > \toperator KeyValueParser::Options() const;\n> > > > +\toperator std::vector() const;\n> > > >\n> > > > \tint toInteger() const;\n> > > > \tstd::string toString() const;\n> > > > \tKeyValueParser::Options toKeyValues() const;\n> > > > +\tstd::vector toArray() const;\n> > > >\n> > > > private:\n> > > > \tValueType type_;\n> > > > \tint integer_;\n> > > > \tstd::string string_;\n> > > > \tKeyValueParser::Options keyValues_;\n> > > > +\tstd::vector array_;\n> > > > };\n> > > >\n> > > > class OptionsParser\n> > > > --\n> > > > 2.21.0\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> >\n> >\n> > --\n> > Regards,\n> > Niklas Söderlund","headers":{"Return-Path":"","Received":["from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 939056110A\n\tfor ;\n\tTue, 26 Mar 2019 12:00:20 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id k8so10673379lja.8\n\tfor ;\n\tTue, 26 Mar 2019 04:00:20 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tg6sm403563lja.64.2019.03.26.04.00.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 26 Mar 2019 04:00:19 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=hnJ3cZH/TshCm9vUl54KdAgH+J6JmDN4uXqzB8xyusI=;\n\tb=Jfh+Y18tD00vITS+XLF2XjmG5KBY8e0ZjUduyVmsPOlCgom+2L95p+z/1UsYLOvJja\n\tc3KAvMYbMq6R3Ayo5Db8M0mNgI2AfN4H/w5BgUn3pyF6L92RrFu387bx9hICKWZKR8ML\n\tzh4K8ic3Q2CMjhcQx7gwYpAI9N3C7NF/h6GQpYpMiKK/kJrRGxVhaoiexmPWa/Jn7G3Z\n\tSb+mSkeC0EMER7wVa6teQ59Rq8xDbzR4eauoDaDWDmFGC8JS3z5d9E2/Xn9OAtKOqikS\n\tluXxWKJK/CRu2RRT6RitNLxzEE91lomW+fTfvUuecD/6vHTwXLoIjuI0UToSYQaehKy1\n\twsBA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=hnJ3cZH/TshCm9vUl54KdAgH+J6JmDN4uXqzB8xyusI=;\n\tb=S8qUmvfxR8D6TcoPqNr4CDdv0U0SsenIb+5198nhcR/dCsYL2BpTc1cuq+x41J3qxi\n\tp0ctlyCS12xtOdiqTEgcy4GPc1lM/oa0vaBDI+W/dsX8BLDmCb6ny7VF003PChLobP2F\n\tXnHzfzqg7bU2sMHTlZiJ4Z8MJE6AIp8VJjxDi+CN8E0S47yciG7USEHgHV74iuRjs4ck\n\t7jk9aoCK/Q5pKcs6Iby0DB7LFYcN7PbndSs2qiAu7ybJm7Ao5LI8CWDEfN4n2xjjhmR8\n\tFNHCyUjhnLNFQFDJmSGSXpZG6HsJVON2HIRefByeMHL5mjVZcedvqr+5H0ZZxOZQGvVh\n\tp+iQ==","X-Gm-Message-State":"APjAAAX71NkYssg+TjjKYwh2LQoVpbFRZSv/QLCXuboMQ4bPDZnfujT6\n\t64+7HHDD97QkpqYNAP7kVmhKwVXJs1U=","X-Google-Smtp-Source":"APXvYqxHfIIjdp/2yKHaujEfNjdkzzKyCwQfNub1tGOsAkT0Cte+XyDXO2hpZ4UdlLlhL3Q/R21Giw==","X-Received":"by 2002:a2e:8059:: with SMTP id\n\tp25mr13909799ljg.178.1553598020036; \n\tTue, 26 Mar 2019 04:00:20 -0700 (PDT)","Date":"Tue, 26 Mar 2019 12:00:19 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326110018.GD9119@bigcity.dyn.berto.se>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>\n\t<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>\n\t<20190326103525.GB9119@bigcity.dyn.berto.se>\n\t<20190326104045.vnwlgi6t7aarclz6@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190326104045.vnwlgi6t7aarclz6@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 11:00:20 -0000"}},{"id":1138,"web_url":"https://patchwork.libcamera.org/comment/1138/","msgid":"<20190326112705.z65hdzrp2tvazltc@uno.localdomain>","date":"2019-03-26T11:27:05","subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Mar 26, 2019 at 12:00:19PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> On 2019-03-26 11:40:45 +0100, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Tue, Mar 26, 2019 at 11:35:25AM +0100, Niklas Söderlund wrote:\n> > > Hi Jacopo,\n> > >\n> > > Thanks for your feedback.\n> > >\n> > > On 2019-03-26 11:30:13 +0100, Jacopo Mondi wrote:\n> > > > Hi Niklas,\n> > > >\n> > > > On Tue, Mar 26, 2019 at 12:47:35AM +0100, Niklas Söderlund wrote:\n> > > > > To allow specifying the same argument option multiple times a new type\n> > > > > of OptionValue is needed. As parsing of options is an iterative process\n> > > > > there is a need to append options as they are parsed so instead of\n> > > > > setting values using the constructor a new add() method is used.\n> > > > >\n> > > > > Signed-off-by: Niklas Söderlund \n> > > > > ---\n> > > > > src/cam/options.cpp | 19 +++++++++++++++++++\n> > > > > src/cam/options.h | 7 +++++++\n> > > > > 2 files changed, 26 insertions(+)\n> > > > >\n> > > > > diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> > > > > index 497833397d894f82..0dec154815d3cad5 100644\n> > > > > --- a/src/cam/options.cpp\n> > > > > +++ b/src/cam/options.cpp\n> > > > > @@ -272,6 +272,12 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> > > > > {\n> > > > > }\n> > > > >\n> > > > > +void OptionValue::add(const OptionValue &value)\n> > > > > +{\n> > > > > +\ttype_ = ValueArray;\n> > > > > +\tarray_.push_back(value);\n> > > > > +}\n> > > > > +\n> > > >\n> > > > I wonder how that would look like if we separate OptionValue (which\n> > > > holds the actual multi-type option value) from the array.\n> > > >\n> > > > I'm not expert of this code, but OptionBase has a 'values_' map, which\n> > > > associates the opt key with an OptionValue that holds the actually option\n> > > > value and this OptionValue, since this patch, could be an array too.\n> > > >\n> > > > I wonder how that would look like it the 'values_' map would use\n> > > > another type, which maintains OptionValues into a a vector, so that\n> > > > all OptionValues could be stored as array without introducing the new\n> > > > 'array_' field.\n> > > >\n> > > > Something like:\n> > > >\n> > > > class OptionBase\n> > > > {\n> > > >\n> > > > ...\n> > > > \tstd::map values_;\n> > > >\n> > > > };\n> > > >\n> > > > class OptionList\n> > > > {\n> > > > ...\n> > > > std::vector array_;\n> > > > };\n> > > >\n> > > > class OptionValue\n> > > > {\n> > > > ....\n> > > > Hold the basic types as it did already;\n> > > > };\n> > > >\n> > > > Does this make any sense to you?\n> > >\n> > > That would have been a nice idea, if all options where arrays. As array\n> > > options are the exception, the main use-case is non-array options. Using\n> > > your suggestion a user of the parser would have to jump thru hoops to\n> > > access the non-array options by accessing them in a vector with 1\n> > > member, right?\n> >\n> > Yes, they would be vectors with 1 member. Which part of handling a\n> > single entry vector concerns you? Insertion at parsing time or access\n> > to the option values?\n>\n> None of the above. What concerns me is how the usage of the parser,\n> right now and with the array additions uses still access non-array\n> options as:\n>\n> std::string name = options[OptCamera];\n>\n> If everything was vectors it would need to be something like\n>\n> std::string name = options[OptCamera].front();\n>\n> Which is not a nice usage interface as the bulk of options would not be\n> repeatable.\n>\n\nI don't want to insist, as this was just a suggestion and I don't know\nthis code well enough, but I think with overloading of operator[] that\nyou already implement to access the OptionValues stored in\nOptionBase's 'values_' this could be hidden easily (ie looking at the\nsize of the options array in the below proposed OptionList or\nwhatever). If that OptionList would provide an overloaded operator[]\ntoo you could then access array type options with\n'options[OptSomething][SuboptSomething]'.\n\nJust suggestions anyway, surely not blocking this series :)\nThanks\n j\n\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > > > OptionValue::operator int() const\n> > > > > {\n> > > > > \treturn toInteger();\n> > > > > @@ -287,6 +293,11 @@ OptionValue::operator KeyValueParser::Options() const\n> > > > > \treturn toKeyValues();\n> > > > > }\n> > > > >\n> > > > > +OptionValue::operator std::vector() const\n> > > > > +{\n> > > > > +\treturn toArray();\n> > > > > +}\n> > > > > +\n> > > > > int OptionValue::toInteger() const\n> > > > > {\n> > > > > \tif (type_ != ValueInteger)\n> > > > > @@ -311,6 +322,14 @@ KeyValueParser::Options OptionValue::toKeyValues() const\n> > > > > \treturn keyValues_;\n> > > > > }\n> > > > >\n> > > > > +std::vector OptionValue::toArray() const\n> > > > > +{\n> > > > > +\tif (type_ != ValueArray)\n> > > > > +\t\treturn std::vector{};\n> > > > > +\n> > > > > +\treturn array_;\n> > > > > +}\n> > > > > +\n> > > > > /* -----------------------------------------------------------------------------\n> > > > > * OptionsParser\n> > > > > */\n> > > > > diff --git a/src/cam/options.h b/src/cam/options.h\n> > > > > index b33a90fc6058febf..6a887416c0070c41 100644\n> > > > > --- a/src/cam/options.h\n> > > > > +++ b/src/cam/options.h\n> > > > > @@ -10,6 +10,7 @@\n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > +#include \n> > > > >\n> > > > > class KeyValueParser;\n> > > > > class OptionValue;\n> > > > > @@ -84,6 +85,7 @@ public:\n> > > > > \t\tValueInteger,\n> > > > > \t\tValueString,\n> > > > > \t\tValueKeyValue,\n> > > > > +\t\tValueArray,\n> > > > > \t};\n> > > > >\n> > > > > \tOptionValue();\n> > > > > @@ -92,21 +94,26 @@ public:\n> > > > > \tOptionValue(const std::string &value);\n> > > > > \tOptionValue(const KeyValueParser::Options &value);\n> > > > >\n> > > > > +\tvoid add(const OptionValue &value);\n> > > > > +\n> > > > > \tValueType type() const { return type_; }\n> > > > >\n> > > > > \toperator int() const;\n> > > > > \toperator std::string() const;\n> > > > > \toperator KeyValueParser::Options() const;\n> > > > > +\toperator std::vector() const;\n> > > > >\n> > > > > \tint toInteger() const;\n> > > > > \tstd::string toString() const;\n> > > > > \tKeyValueParser::Options toKeyValues() const;\n> > > > > +\tstd::vector toArray() const;\n> > > > >\n> > > > > private:\n> > > > > \tValueType type_;\n> > > > > \tint integer_;\n> > > > > \tstd::string string_;\n> > > > > \tKeyValueParser::Options keyValues_;\n> > > > > +\tstd::vector array_;\n> > > > > };\n> > > > >\n> > > > > class OptionsParser\n> > > > > --\n> > > > > 2.21.0\n> > > > >\n> > > > > _______________________________________________\n> > > > > libcamera-devel mailing list\n> > > > > libcamera-devel@lists.libcamera.org\n> > > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> > >\n> > >\n> > >\n> > > --\n> > > Regards,\n> > > Niklas Söderlund\n>\n>\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31F7E6110A\n\tfor ;\n\tTue, 26 Mar 2019 12:26:24 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id B0DDF240015;\n\tTue, 26 Mar 2019 11:26:23 +0000 (UTC)"],"Date":"Tue, 26 Mar 2019 12:27:05 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190326112705.z65hdzrp2tvazltc@uno.localdomain>","References":"<20190325234736.12533-1-niklas.soderlund@ragnatech.se>\n\t<20190325234736.12533-3-niklas.soderlund@ragnatech.se>\n\t<20190326103013.77q4mdobe6ddgv3r@uno.localdomain>\n\t<20190326103525.GB9119@bigcity.dyn.berto.se>\n\t<20190326104045.vnwlgi6t7aarclz6@uno.localdomain>\n\t<20190326110018.GD9119@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"cibnqsgxce6hp3of\"","Content-Disposition":"inline","In-Reply-To":"<20190326110018.GD9119@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 2/3] cam: options: Add an array data\n\ttype to OptionValue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 26 Mar 2019 11:26:24 -0000"}}]