[{"id":23612,"web_url":"https://patchwork.libcamera.org/comment/23612/","msgid":"<20220627160633.emtfplwafkpdkqyg@uno.localdomain>","date":"2022-06-27T16:06:33","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Daniel\n\nOn Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> This change is required for AfWindows control from capture script.\n> Parser expects array of arrays of parameters, so it is possible to\n> specify multiple rectangles.\n>\n> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> ---\n>  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n>  src/cam/capture_script.h   |   7 +-\n>  2 files changed, 139 insertions(+), 13 deletions(-)\n>\n> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> index 9f22d5f7..6278a152 100644\n> --- a/src/cam/capture_script.cpp\n> +++ b/src/cam/capture_script.cpp\n> @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n>  \t\treturn -EINVAL;\n>  \t}\n>\n> -\tstd::string value = parseScalar();\n> -\tif (value.empty())\n> +\tconst ControlId *controlId = it->second;\n> +\n> +\tControlValue val = unpackControl(controlId);\n> +\tif (val.isNone()) {\n> +\t\tstd::cerr << \"Error unpacking control '\" << name << \"'\"\n> +\t\t\t  << std::endl;\n>  \t\treturn -EINVAL;\n> +\t}\n>\n> -\tconst ControlId *controlId = it->second;\n> -\tControlValue val = unpackControl(controlId, value);\n>  \tcontrols.set(controlId->id(), val);\n>\n>  \treturn 0;\n> @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n>  \treturn eventScalarValue(event);\n>  }\n>\n> +ControlValue CaptureScript::parseRectangles()\n> +{\n> +\tstd::vector<libcamera::Rectangle> rectangles;\n> +\n> +\tstd::vector<std::vector<std::string>> arrays = parseArrays();\n> +\tif (arrays.empty())\n> +\t\treturn {};\n> +\n> +\tfor (const std::vector<std::string> &values : arrays) {\n> +\t\tif (values.size() != 4) {\n> +\t\t\tstd::cerr << \"Error parsing Rectangle: expected \"\n> +\t\t\t\t     \"array with 4 parameters\" << std::endl;\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tRectangle rect = unpackRectangle(values);\n> +\t\trectangles.push_back(rect);\n> +\t}\n> +\n> +\tControlValue controlValue;\n> +\tcontrolValue.set(Span<const Rectangle>(rectangles));\n> +\n> +\treturn controlValue;\n> +}\n> +\n> +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> +{\n> +\tEventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> +\tif (!event)\n> +\t\treturn {};\n> +\n> +\tevent = nextEvent();\n> +\tif (!event)\n> +\t\treturn {};\n> +\n> +\tstd::vector<std::vector<std::string>> valueArrays;\n> +\n> +\t/* Parse single array. */\n> +\tif (event->type == YAML_SCALAR_EVENT) {\n> +\t\tstd::string firstValue = eventScalarValue(event);\n> +\t\tif (firstValue.empty())\n> +\t\t\treturn {};\n> +\n> +\t\tstd::vector<std::string> remaining = parseSingleArray();\n\nnit: remaining can potentilly be {}\n\n> +\n> +\t\tstd::vector<std::string> values = { firstValue };\n> +\t\tvalues.insert(std::end(values),\n> +\t\t\t      std::begin(remaining), std::end(remaining));\n> +\t\tvalueArrays.push_back(values);\n> +\n> +\t\treturn valueArrays;\n> +\t}\n\nThis should address Laurent's comment and I assume it now works with\nboth\n        AfWindows:\n          - [x, y, w, h]\n\nand\n        AfWindows:\n          - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n\n> +\n> +\t/* Parse array of arrays. */\n> +\twhile (1) {\n> +\t\tswitch (event->type) {\n> +\t\tcase YAML_SEQUENCE_START_EVENT: {\n> +\t\t\tstd::vector<std::string> values = parseSingleArray();\n> +\t\t\tvalueArrays.push_back(values);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase YAML_SEQUENCE_END_EVENT:\n> +\t\t\treturn valueArrays;\n> +\t\tdefault:\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn {};\n> +\t}\n> +}\n> +\n> +std::vector<std::string> CaptureScript::parseSingleArray()\n> +{\n> +\tstd::vector<std::string> values;\n> +\n> +\twhile (1) {\n> +\t\tEventPtr event = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn {};\n> +\n> +\t\tswitch (event->type) {\n> +\t\tcase YAML_SCALAR_EVENT: {\n> +\t\t\tstd::string value = eventScalarValue(event);\n> +\t\t\tif (value.empty())\n> +\t\t\t\treturn {};\n> +\t\t\tvalues.push_back(value);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase YAML_SEQUENCE_END_EVENT:\n> +\t\t\treturn values;\n> +\t\tdefault:\n> +\t\t\treturn {};\n> +\t\t}\n> +\t}\n> +}\n> +\n>  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n>  {\n>  \tstatic const std::map<unsigned int, const char *> typeNames = {\n> @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n>  \t\t  << typeName << \" control \" << id->name() << std::endl;\n>  }\n>\n> -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> -\t\t\t\t\t  const std::string &repr)\n> +ControlValue CaptureScript::unpackControl(const ControlId *id)\n>  {\n> +\t/* Parse complex types. */\n> +\tswitch (id->type()) {\n> +\tcase ControlTypeRectangle:\n> +\t\treturn parseRectangles();\n> +\tcase ControlTypeSize:\n> +\t\t/* \\todo Parse Sizes. */\n> +\t\treturn {};\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n> +\t/* Parse basic types represented by a single scalar. */\n> +\tconst std::string repr = parseScalar();\n> +\tif (repr.empty())\n> +\t\treturn {};\n> +\n\nThanks, this address my comment on v1.\n\nNits apart it looks good to me\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>  \tControlValue value{};\n>\n>  \tswitch (id->type()) {\n> @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n>  \t\tvalue.set<std::string>(repr);\n>  \t\tbreak;\n>  \t}\n> -\tcase ControlTypeRectangle:\n> -\t\t/* \\todo Parse rectangles. */\n> -\t\tbreak;\n> -\tcase ControlTypeSize:\n> -\t\t/* \\todo Parse Sizes. */\n> +\tdefault:\n> +\t\tstd::cerr << \"Unsupported control type\" << std::endl;\n>  \t\tbreak;\n>  \t}\n>\n>  \treturn value;\n>  }\n> +\n> +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> +{\n> +\tint x = strtol(strVec[0].c_str(), NULL, 10);\n> +\tint y = strtol(strVec[1].c_str(), NULL, 10);\n> +\tunsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> +\tunsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> +\n> +\treturn Rectangle(x, y, width, height);\n> +}\n> diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> index 8b4f8f62..fffe67e5 100644\n> --- a/src/cam/capture_script.h\n> +++ b/src/cam/capture_script.h\n> @@ -54,9 +54,12 @@ private:\n>  \tint parseControl(EventPtr event, libcamera::ControlList &controls);\n>\n>  \tstd::string parseScalar();\n> +\tlibcamera::ControlValue parseRectangles();\n> +\tstd::vector<std::vector<std::string>> parseArrays();\n> +\tstd::vector<std::string> parseSingleArray();\n>\n>  \tvoid unpackFailure(const libcamera::ControlId *id,\n>  \t\t\t   const std::string &repr);\n> -\tlibcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> -\t\t\t\t\t      const std::string &repr);\n> +\tlibcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> +\tlibcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n>  };\n> --\n> 2.34.1\n>","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 07DB6BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jun 2022 16:06:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18E3565635;\n\tMon, 27 Jun 2022 18:06:38 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E18E86059B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jun 2022 18:06:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 4312A1C0005;\n\tMon, 27 Jun 2022 16:06:35 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656345998;\n\tbh=RBJ8vI/PoAVsBEAckg/1IZcnUDp/gDh+UhmgG0bHjcs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=M1zAPDdIJaCyMygRW2lx22ksDJ4je5cfhaphlWsbKjHz1kgSxI+pdDlH9eFaefp6i\n\txIeGNTQRqO5R+KOKJE6B0KTkVy6QQMTlMMMAA2ppPgbiSSaSgwq846CbFaqlIXmh5o\n\tEsGV6FaezUe05bH4+nKawOShiyGHWdcrOcAJw26Qs8k9u4HtuLecPqS+uIiwJmC0ze\n\tz5uEJ7cas8DZZdqRxcLOWApaGQkq7x3NJu600EKk3/UsXOdcNmyCeRSmKEKOmtez6/\n\t0+5uyFhzFLYQj++Oen+Sh2hzwPfMf8XT6ywJHB+1XEnFcoxnffOnD206BgMgb5DcZV\n\tnnjvYOdVgAhTQ==","Date":"Mon, 27 Jun 2022 18:06:33 +0200","To":"Daniel Semkowicz <dse@thaumatec.com>","Message-ID":"<20220627160633.emtfplwafkpdkqyg@uno.localdomain>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220627122806.29586-2-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23629,"web_url":"https://patchwork.libcamera.org/comment/23629/","msgid":"<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>","date":"2022-06-28T06:35:03","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nOn Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi Daniel\n>\n> On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > This change is required for AfWindows control from capture script.\n> > Parser expects array of arrays of parameters, so it is possible to\n> > specify multiple rectangles.\n> >\n> > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > ---\n> >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> >  src/cam/capture_script.h   |   7 +-\n> >  2 files changed, 139 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > index 9f22d5f7..6278a152 100644\n> > --- a/src/cam/capture_script.cpp\n> > +++ b/src/cam/capture_script.cpp\n> > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> >               return -EINVAL;\n> >       }\n> >\n> > -     std::string value = parseScalar();\n> > -     if (value.empty())\n> > +     const ControlId *controlId = it->second;\n> > +\n> > +     ControlValue val = unpackControl(controlId);\n> > +     if (val.isNone()) {\n> > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > +                       << std::endl;\n> >               return -EINVAL;\n> > +     }\n> >\n> > -     const ControlId *controlId = it->second;\n> > -     ControlValue val = unpackControl(controlId, value);\n> >       controls.set(controlId->id(), val);\n> >\n> >       return 0;\n> > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> >       return eventScalarValue(event);\n> >  }\n> >\n> > +ControlValue CaptureScript::parseRectangles()\n> > +{\n> > +     std::vector<libcamera::Rectangle> rectangles;\n> > +\n> > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > +     if (arrays.empty())\n> > +             return {};\n> > +\n> > +     for (const std::vector<std::string> &values : arrays) {\n> > +             if (values.size() != 4) {\n> > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > +                                  \"array with 4 parameters\" << std::endl;\n> > +                     return {};\n> > +             }\n> > +\n> > +             Rectangle rect = unpackRectangle(values);\n> > +             rectangles.push_back(rect);\n> > +     }\n> > +\n> > +     ControlValue controlValue;\n> > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > +\n> > +     return controlValue;\n> > +}\n> > +\n> > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > +{\n> > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > +     if (!event)\n> > +             return {};\n> > +\n> > +     event = nextEvent();\n> > +     if (!event)\n> > +             return {};\n> > +\n> > +     std::vector<std::vector<std::string>> valueArrays;\n> > +\n> > +     /* Parse single array. */\n> > +     if (event->type == YAML_SCALAR_EVENT) {\n> > +             std::string firstValue = eventScalarValue(event);\n> > +             if (firstValue.empty())\n> > +                     return {};\n> > +\n> > +             std::vector<std::string> remaining = parseSingleArray();\n>\n> nit: remaining can potentilly be {}\n\nYes, but the vector can be empty not only because of error, but also\nbecause there was a valid array but with only one element (already read\nin firstValue).\nWe would need to add additional parameter returned from method to\ndistinguish between these two situations.\nThe higher level that calls parseArrays() should already know the\nexpected number of elements in the single array, so the error will be\ncaught anyway. I wouldn't spend too much time tweaking this solution as\nit is not critical.\nWhat do you think about leaving it as it is?\n\n>\n> > +\n> > +             std::vector<std::string> values = { firstValue };\n> > +             values.insert(std::end(values),\n> > +                           std::begin(remaining), std::end(remaining));\n> > +             valueArrays.push_back(values);\n> > +\n> > +             return valueArrays;\n> > +     }\n>\n> This should address Laurent's comment and I assume it now works with\n> both\n>         AfWindows:\n>           - [x, y, w, h]\n>\n> and\n>         AfWindows:\n>           - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n>\n\nExactly, I tested it with both versions :)\n\nBest regards\nDaniel\n\n> > +\n> > +     /* Parse array of arrays. */\n> > +     while (1) {\n> > +             switch (event->type) {\n> > +             case YAML_SEQUENCE_START_EVENT: {\n> > +                     std::vector<std::string> values = parseSingleArray();\n> > +                     valueArrays.push_back(values);\n> > +                     break;\n> > +             }\n> > +             case YAML_SEQUENCE_END_EVENT:\n> > +                     return valueArrays;\n> > +             default:\n> > +                     return {};\n> > +             }\n> > +\n> > +             event = nextEvent();\n> > +             if (!event)\n> > +                     return {};\n> > +     }\n> > +}\n> > +\n> > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > +{\n> > +     std::vector<std::string> values;\n> > +\n> > +     while (1) {\n> > +             EventPtr event = nextEvent();\n> > +             if (!event)\n> > +                     return {};\n> > +\n> > +             switch (event->type) {\n> > +             case YAML_SCALAR_EVENT: {\n> > +                     std::string value = eventScalarValue(event);\n> > +                     if (value.empty())\n> > +                             return {};\n> > +                     values.push_back(value);\n> > +                     break;\n> > +             }\n> > +             case YAML_SEQUENCE_END_EVENT:\n> > +                     return values;\n> > +             default:\n> > +                     return {};\n> > +             }\n> > +     }\n> > +}\n> > +\n> >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> >  {\n> >       static const std::map<unsigned int, const char *> typeNames = {\n> > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> >                 << typeName << \" control \" << id->name() << std::endl;\n> >  }\n> >\n> > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > -                                       const std::string &repr)\n> > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> >  {\n> > +     /* Parse complex types. */\n> > +     switch (id->type()) {\n> > +     case ControlTypeRectangle:\n> > +             return parseRectangles();\n> > +     case ControlTypeSize:\n> > +             /* \\todo Parse Sizes. */\n> > +             return {};\n> > +     default:\n> > +             break;\n> > +     }\n> > +\n> > +     /* Parse basic types represented by a single scalar. */\n> > +     const std::string repr = parseScalar();\n> > +     if (repr.empty())\n> > +             return {};\n> > +\n>\n> Thanks, this address my comment on v1.\n>\n> Nits apart it looks good to me\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks\n>    j\n>\n> >       ControlValue value{};\n> >\n> >       switch (id->type()) {\n> > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> >               value.set<std::string>(repr);\n> >               break;\n> >       }\n> > -     case ControlTypeRectangle:\n> > -             /* \\todo Parse rectangles. */\n> > -             break;\n> > -     case ControlTypeSize:\n> > -             /* \\todo Parse Sizes. */\n> > +     default:\n> > +             std::cerr << \"Unsupported control type\" << std::endl;\n> >               break;\n> >       }\n> >\n> >       return value;\n> >  }\n> > +\n> > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > +{\n> > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > +\n> > +     return Rectangle(x, y, width, height);\n> > +}\n> > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > index 8b4f8f62..fffe67e5 100644\n> > --- a/src/cam/capture_script.h\n> > +++ b/src/cam/capture_script.h\n> > @@ -54,9 +54,12 @@ private:\n> >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> >\n> >       std::string parseScalar();\n> > +     libcamera::ControlValue parseRectangles();\n> > +     std::vector<std::vector<std::string>> parseArrays();\n> > +     std::vector<std::string> parseSingleArray();\n> >\n> >       void unpackFailure(const libcamera::ControlId *id,\n> >                          const std::string &repr);\n> > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > -                                           const std::string &repr);\n> > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> >  };\n> > --\n> > 2.34.1\n> >","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 2B24BBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 28 Jun 2022 06:35:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B3B265637;\n\tTue, 28 Jun 2022 08:35:17 +0200 (CEST)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7EA5E61FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 Jun 2022 08:35:15 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id bx13so5053935ljb.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jun 2022 23:35:15 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656398117;\n\tbh=CafQk12elV2rON1q9zElDOIyFHSJZBDpGO/eWfJsUlA=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=slt8/GL66Z+uSrpfNIo/SNEAdTd+6gAyweCeYRivLevaIc7uHDrKwqA1xBlJzFtUX\n\tsbnWUv+3FOa6ffIvBnyztkrbvcUI/9sqghcsVIo2/lxt4T3KVSpeWcjGMpt/lR6zPK\n\tVUD9YRcRByrz06/PQjUrk5duIlfNFMUoXsQlU6RhfTKKoDZWzH6w9HPaxbS/GNScR0\n\tuw7NnDjuDpWuKfH94M21Dk0QKMUm+vzLvTQVPM5kFNYWAzHKZ8pMrLB3BHYt/xSb4+\n\tJRvSGnj3g8hrH5+3oYD91wwzREKs9fbCI215vOPChiHdt7LVjS/Muj3R60cl78VW/Z\n\t0YGYLjUEdSMuw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=WckF2l+diE7kAMfx4zRUrOjHVnaLXM0L7IznJ9FtWuk=;\n\tb=MTCx2NPZdw814KxQtsCQmFZUCq3JzBp47bvaxusmW+dqpiTIF//qRobaQbzYBEuH4r\n\tyMgXcSA1PfTqvCifxqV9e20bTztjVJ2qs7L7w4sm3OwuYOTgxbOZGeFqE/lv2X9uebrk\n\tDOlUUkLvVbSARAPbfyNpQVO/eY7tBfvSfaivQ0qnC6sKWAEk6uzOOI0xop2Xnce7+CEz\n\ti5UFmPzbTatRSUf/qZDMjnAjR7z11+HuOUE4QDSUxQf1iPCY3stl4eqzAVnujKeSMhJF\n\tKDcenkDXNd+1EAoUWBCzXvRsCwbheteZutwflI0gYF+Eof5j4nvQMj8L+uBgE79XJSK6\n\tR5JQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"MTCx2NPZ\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=WckF2l+diE7kAMfx4zRUrOjHVnaLXM0L7IznJ9FtWuk=;\n\tb=vY0pUUMpWbECNuByErsmEFzrLaeZb+JMFB1uVjg710la73sYyeURzZwGHXbXyZjJAH\n\ts4+M/sNOlOWjaKka+pXUiBTVOlRuzd/1QmoHFOQ5gOxumb9T5Zyw49CHv6pERVzZv29a\n\t7KE1A6hdBM/hZd4yAHEv+rO7/oxwB6an6BSKxvPnDcGJSRWdZBmD4LrwzBFC/ONXXCpj\n\tUVapjU+HhwpyHnheNYlK3tGG5TlPkezGkhHYLmO57Nre5MYSmEMvxhShGi7nPHWmZbV8\n\txnDgutgHVBRli0oWU3JsnpZMOrzuiDKvyRKa94gkv2JY/CFwxhhxe60U+KE1YZOAfdSe\n\tUeCA==","X-Gm-Message-State":"AJIora/XS9fM9dHxTWg+YtolxlBIo6IPDZpVEiNuf0bG6cI6GMKGQALu\n\tzQrxZhe0EQA5WMKlTN8LXMR+ZYyj1U3io1UUHHmJsjYnXWxR6zE/","X-Google-Smtp-Source":"AGRyM1vlKKsmUxJi193XbEyjNGiH9g5sFM/A0PpVC3hh2oIiGVU+pI2vtaiXVBMwW6oZ+/U7aKNuD4fhStiLIChX4pE=","X-Received":"by 2002:a05:651c:178b:b0:258:ed21:efbd with SMTP id\n\tbn11-20020a05651c178b00b00258ed21efbdmr8393298ljb.464.1656398114561;\n\tMon, 27 Jun 2022 23:35:14 -0700 (PDT)","MIME-Version":"1.0","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220627160633.emtfplwafkpdkqyg@uno.localdomain>","In-Reply-To":"<20220627160633.emtfplwafkpdkqyg@uno.localdomain>","Date":"Tue, 28 Jun 2022 08:35:03 +0200","Message-ID":"<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Daniel Semkowicz via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Daniel Semkowicz <dse@thaumatec.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24382,"web_url":"https://patchwork.libcamera.org/comment/24382/","msgid":"<20220804194746.GP311202@pyrite.rasen.tech>","date":"2022-08-04T19:47:46","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Daniel,\n\nThanks for the patches.\n\nOn Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> Hi Jacopo,\n> \n> On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > Hi Daniel\n> >\n> > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > This change is required for AfWindows control from capture script.\n> > > Parser expects array of arrays of parameters, so it is possible to\n> > > specify multiple rectangles.\n> > >\n> > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > > ---\n> > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > >  src/cam/capture_script.h   |   7 +-\n> > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > index 9f22d5f7..6278a152 100644\n> > > --- a/src/cam/capture_script.cpp\n> > > +++ b/src/cam/capture_script.cpp\n> > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > >               return -EINVAL;\n> > >       }\n> > >\n> > > -     std::string value = parseScalar();\n> > > -     if (value.empty())\n> > > +     const ControlId *controlId = it->second;\n> > > +\n> > > +     ControlValue val = unpackControl(controlId);\n> > > +     if (val.isNone()) {\n> > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > +                       << std::endl;\n> > >               return -EINVAL;\n> > > +     }\n> > >\n> > > -     const ControlId *controlId = it->second;\n> > > -     ControlValue val = unpackControl(controlId, value);\n> > >       controls.set(controlId->id(), val);\n> > >\n> > >       return 0;\n> > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > >       return eventScalarValue(event);\n> > >  }\n> > >\n> > > +ControlValue CaptureScript::parseRectangles()\n> > > +{\n> > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > +\n> > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > +     if (arrays.empty())\n> > > +             return {};\n> > > +\n> > > +     for (const std::vector<std::string> &values : arrays) {\n> > > +             if (values.size() != 4) {\n> > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > +                                  \"array with 4 parameters\" << std::endl;\n> > > +                     return {};\n> > > +             }\n> > > +\n> > > +             Rectangle rect = unpackRectangle(values);\n> > > +             rectangles.push_back(rect);\n> > > +     }\n> > > +\n> > > +     ControlValue controlValue;\n> > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > +\n> > > +     return controlValue;\n> > > +}\n> > > +\n> > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > +{\n> > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > +     if (!event)\n> > > +             return {};\n> > > +\n> > > +     event = nextEvent();\n> > > +     if (!event)\n> > > +             return {};\n> > > +\n> > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > +\n> > > +     /* Parse single array. */\n> > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > +             std::string firstValue = eventScalarValue(event);\n> > > +             if (firstValue.empty())\n> > > +                     return {};\n> > > +\n> > > +             std::vector<std::string> remaining = parseSingleArray();\n> >\n> > nit: remaining can potentilly be {}\n> \n> Yes, but the vector can be empty not only because of error, but also\n> because there was a valid array but with only one element (already read\n> in firstValue).\n> We would need to add additional parameter returned from method to\n> distinguish between these two situations.\n> The higher level that calls parseArrays() should already know the\n> expected number of elements in the single array, so the error will be\n> caught anyway. I wouldn't spend too much time tweaking this solution as\n> it is not critical.\n> What do you think about leaving it as it is?\n> \n> >\n> > > +\n> > > +             std::vector<std::string> values = { firstValue };\n> > > +             values.insert(std::end(values),\n> > > +                           std::begin(remaining), std::end(remaining));\n> > > +             valueArrays.push_back(values);\n> > > +\n> > > +             return valueArrays;\n> > > +     }\n> >\n> > This should address Laurent's comment and I assume it now works with\n> > both\n> >         AfWindows:\n> >           - [x, y, w, h]\n\nGotta be honest, I'm not a big fan of representing Rectangles this way;\nit's a bit... flat (if that's the right word).\nI'd prefer it to be like (x,y)/wxh\n\n\nThanks,\n\nPaul\n\n> >\n> > and\n> >         AfWindows:\n> >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n> >\n> \n> Exactly, I tested it with both versions :)\n> \n> Best regards\n> Daniel\n> \n> > > +\n> > > +     /* Parse array of arrays. */\n> > > +     while (1) {\n> > > +             switch (event->type) {\n> > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > +                     std::vector<std::string> values = parseSingleArray();\n> > > +                     valueArrays.push_back(values);\n> > > +                     break;\n> > > +             }\n> > > +             case YAML_SEQUENCE_END_EVENT:\n> > > +                     return valueArrays;\n> > > +             default:\n> > > +                     return {};\n> > > +             }\n> > > +\n> > > +             event = nextEvent();\n> > > +             if (!event)\n> > > +                     return {};\n> > > +     }\n> > > +}\n> > > +\n> > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > +{\n> > > +     std::vector<std::string> values;\n> > > +\n> > > +     while (1) {\n> > > +             EventPtr event = nextEvent();\n> > > +             if (!event)\n> > > +                     return {};\n> > > +\n> > > +             switch (event->type) {\n> > > +             case YAML_SCALAR_EVENT: {\n> > > +                     std::string value = eventScalarValue(event);\n> > > +                     if (value.empty())\n> > > +                             return {};\n> > > +                     values.push_back(value);\n> > > +                     break;\n> > > +             }\n> > > +             case YAML_SEQUENCE_END_EVENT:\n> > > +                     return values;\n> > > +             default:\n> > > +                     return {};\n> > > +             }\n> > > +     }\n> > > +}\n> > > +\n> > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > >  {\n> > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > >                 << typeName << \" control \" << id->name() << std::endl;\n> > >  }\n> > >\n> > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > -                                       const std::string &repr)\n> > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > >  {\n> > > +     /* Parse complex types. */\n> > > +     switch (id->type()) {\n> > > +     case ControlTypeRectangle:\n> > > +             return parseRectangles();\n> > > +     case ControlTypeSize:\n> > > +             /* \\todo Parse Sizes. */\n> > > +             return {};\n> > > +     default:\n> > > +             break;\n> > > +     }\n> > > +\n> > > +     /* Parse basic types represented by a single scalar. */\n> > > +     const std::string repr = parseScalar();\n> > > +     if (repr.empty())\n> > > +             return {};\n> > > +\n> >\n> > Thanks, this address my comment on v1.\n> >\n> > Nits apart it looks good to me\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > Thanks\n> >    j\n> >\n> > >       ControlValue value{};\n> > >\n> > >       switch (id->type()) {\n> > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > >               value.set<std::string>(repr);\n> > >               break;\n> > >       }\n> > > -     case ControlTypeRectangle:\n> > > -             /* \\todo Parse rectangles. */\n> > > -             break;\n> > > -     case ControlTypeSize:\n> > > -             /* \\todo Parse Sizes. */\n> > > +     default:\n> > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > >               break;\n> > >       }\n> > >\n> > >       return value;\n> > >  }\n> > > +\n> > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > +{\n> > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > +\n> > > +     return Rectangle(x, y, width, height);\n> > > +}\n> > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > index 8b4f8f62..fffe67e5 100644\n> > > --- a/src/cam/capture_script.h\n> > > +++ b/src/cam/capture_script.h\n> > > @@ -54,9 +54,12 @@ private:\n> > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > >\n> > >       std::string parseScalar();\n> > > +     libcamera::ControlValue parseRectangles();\n> > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > +     std::vector<std::string> parseSingleArray();\n> > >\n> > >       void unpackFailure(const libcamera::ControlId *id,\n> > >                          const std::string &repr);\n> > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > -                                           const std::string &repr);\n> > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > >  };\n> > > --\n> > > 2.34.1\n> > >","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 77C4ABE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Aug 2022 19:47:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C49C263311;\n\tThu,  4 Aug 2022 21:47:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1EB816330D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Aug 2022 21:47:55 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76DA9481;\n\tThu,  4 Aug 2022 21:47:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659642476;\n\tbh=40b+4lPSbOTR7KYXWMD0KWMmadthUybOI2h2h7BLDGQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hvp0eT8+SxVILofgddL8niKgrIqa7BUJLE3CQELzCn8tSNyMSGdhy2Md1Ot6XER+1\n\t1wjyhtgpygaV10QTCz9zXUzzNGLeiBGPsIv5TUe+YLeJvL5Ky4AIgXfT5+X68GA9Es\n\ttWULoGhn6K3VGHpkxfcC5tHl7bOykR+IS3U3VMLcQzosV/46MZCfK5BFaF0rfKvC7e\n\tJP0PGS6JzCJL7v4VoVYR9ySMtR+J8har3Cmy8uGLmqHu5iWO7M6pQrofS8IV9xULow\n\tEEC8+azu7zwcyd+WhDk6b/kOOwu23PwXqUuucohBg5qJzEvX3ZifGNnbL8BwxL0lPO\n\twtUWLkcUvholg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659642474;\n\tbh=40b+4lPSbOTR7KYXWMD0KWMmadthUybOI2h2h7BLDGQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QWKt+Pzhx0JTqL4WjmbNXPV5FSb4ul14kJo9/G88Djzj3HCrQRj6irRaO+4LAAQoH\n\tGFfrRzI46YJcVTWcJmrGpzkgjDN5GJJ4DBjBoWuDxMHKP8Y8iCONURBhcVrYwau811\n\tGRz1Nx/0Z5cUgy3e+X48yDmnrEJzR5pGjiZseEm8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QWKt+Pzh\"; dkim-atps=neutral","Date":"Fri, 5 Aug 2022 04:47:46 +0900","To":"Daniel Semkowicz <dse@thaumatec.com>","Message-ID":"<20220804194746.GP311202@pyrite.rasen.tech>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220627160633.emtfplwafkpdkqyg@uno.localdomain>\n\t<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24388,"web_url":"https://patchwork.libcamera.org/comment/24388/","msgid":"<20220805072026.zuwuf5yj3t5o4whj@uno.localdomain>","date":"2022-08-05T07:20:26","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul\n\nOn Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Daniel,\n>\n> Thanks for the patches.\n>\n> On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > Hi Jacopo,\n> >\n> > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > >\n> > > Hi Daniel\n> > >\n> > > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > This change is required for AfWindows control from capture script.\n> > > > Parser expects array of arrays of parameters, so it is possible to\n> > > > specify multiple rectangles.\n> > > >\n> > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > > > ---\n> > > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > > >  src/cam/capture_script.h   |   7 +-\n> > > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > > >\n> > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > > index 9f22d5f7..6278a152 100644\n> > > > --- a/src/cam/capture_script.cpp\n> > > > +++ b/src/cam/capture_script.cpp\n> > > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > > >               return -EINVAL;\n> > > >       }\n> > > >\n> > > > -     std::string value = parseScalar();\n> > > > -     if (value.empty())\n> > > > +     const ControlId *controlId = it->second;\n> > > > +\n> > > > +     ControlValue val = unpackControl(controlId);\n> > > > +     if (val.isNone()) {\n> > > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > > +                       << std::endl;\n> > > >               return -EINVAL;\n> > > > +     }\n> > > >\n> > > > -     const ControlId *controlId = it->second;\n> > > > -     ControlValue val = unpackControl(controlId, value);\n> > > >       controls.set(controlId->id(), val);\n> > > >\n> > > >       return 0;\n> > > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > > >       return eventScalarValue(event);\n> > > >  }\n> > > >\n> > > > +ControlValue CaptureScript::parseRectangles()\n> > > > +{\n> > > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > > +\n> > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > > +     if (arrays.empty())\n> > > > +             return {};\n> > > > +\n> > > > +     for (const std::vector<std::string> &values : arrays) {\n> > > > +             if (values.size() != 4) {\n> > > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > > +                                  \"array with 4 parameters\" << std::endl;\n> > > > +                     return {};\n> > > > +             }\n> > > > +\n> > > > +             Rectangle rect = unpackRectangle(values);\n> > > > +             rectangles.push_back(rect);\n> > > > +     }\n> > > > +\n> > > > +     ControlValue controlValue;\n> > > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > > +\n> > > > +     return controlValue;\n> > > > +}\n> > > > +\n> > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > > +{\n> > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > > +     if (!event)\n> > > > +             return {};\n> > > > +\n> > > > +     event = nextEvent();\n> > > > +     if (!event)\n> > > > +             return {};\n> > > > +\n> > > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > > +\n> > > > +     /* Parse single array. */\n> > > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > > +             std::string firstValue = eventScalarValue(event);\n> > > > +             if (firstValue.empty())\n> > > > +                     return {};\n> > > > +\n> > > > +             std::vector<std::string> remaining = parseSingleArray();\n> > >\n> > > nit: remaining can potentilly be {}\n> >\n> > Yes, but the vector can be empty not only because of error, but also\n> > because there was a valid array but with only one element (already read\n> > in firstValue).\n> > We would need to add additional parameter returned from method to\n> > distinguish between these two situations.\n> > The higher level that calls parseArrays() should already know the\n> > expected number of elements in the single array, so the error will be\n> > caught anyway. I wouldn't spend too much time tweaking this solution as\n> > it is not critical.\n> > What do you think about leaving it as it is?\n> >\n> > >\n> > > > +\n> > > > +             std::vector<std::string> values = { firstValue };\n> > > > +             values.insert(std::end(values),\n> > > > +                           std::begin(remaining), std::end(remaining));\n> > > > +             valueArrays.push_back(values);\n> > > > +\n> > > > +             return valueArrays;\n> > > > +     }\n> > >\n> > > This should address Laurent's comment and I assume it now works with\n> > > both\n> > >         AfWindows:\n> > >           - [x, y, w, h]\n>\n> Gotta be honest, I'm not a big fan of representing Rectangles this way;\n> it's a bit... flat (if that's the right word).\n> I'd prefer it to be like (x,y)/wxh\n\nThis would require specifying the rectangle as a string. How will it\nlook like in the .yaml file ? Do you have examples you have tested\nyour version to share ?\n\n>\n>\n> Thanks,\n>\n> Paul\n>\n> > >\n> > > and\n> > >         AfWindows:\n> > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n> > >\n> >\n> > Exactly, I tested it with both versions :)\n> >\n> > Best regards\n> > Daniel\n> >\n> > > > +\n> > > > +     /* Parse array of arrays. */\n> > > > +     while (1) {\n> > > > +             switch (event->type) {\n> > > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > > +                     std::vector<std::string> values = parseSingleArray();\n> > > > +                     valueArrays.push_back(values);\n> > > > +                     break;\n> > > > +             }\n> > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > +                     return valueArrays;\n> > > > +             default:\n> > > > +                     return {};\n> > > > +             }\n> > > > +\n> > > > +             event = nextEvent();\n> > > > +             if (!event)\n> > > > +                     return {};\n> > > > +     }\n> > > > +}\n> > > > +\n> > > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > > +{\n> > > > +     std::vector<std::string> values;\n> > > > +\n> > > > +     while (1) {\n> > > > +             EventPtr event = nextEvent();\n> > > > +             if (!event)\n> > > > +                     return {};\n> > > > +\n> > > > +             switch (event->type) {\n> > > > +             case YAML_SCALAR_EVENT: {\n> > > > +                     std::string value = eventScalarValue(event);\n> > > > +                     if (value.empty())\n> > > > +                             return {};\n> > > > +                     values.push_back(value);\n> > > > +                     break;\n> > > > +             }\n> > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > +                     return values;\n> > > > +             default:\n> > > > +                     return {};\n> > > > +             }\n> > > > +     }\n> > > > +}\n> > > > +\n> > > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > >  {\n> > > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > >                 << typeName << \" control \" << id->name() << std::endl;\n> > > >  }\n> > > >\n> > > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > -                                       const std::string &repr)\n> > > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > > >  {\n> > > > +     /* Parse complex types. */\n> > > > +     switch (id->type()) {\n> > > > +     case ControlTypeRectangle:\n> > > > +             return parseRectangles();\n> > > > +     case ControlTypeSize:\n> > > > +             /* \\todo Parse Sizes. */\n> > > > +             return {};\n> > > > +     default:\n> > > > +             break;\n> > > > +     }\n> > > > +\n> > > > +     /* Parse basic types represented by a single scalar. */\n> > > > +     const std::string repr = parseScalar();\n> > > > +     if (repr.empty())\n> > > > +             return {};\n> > > > +\n> > >\n> > > Thanks, this address my comment on v1.\n> > >\n> > > Nits apart it looks good to me\n> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >\n> > > Thanks\n> > >    j\n> > >\n> > > >       ControlValue value{};\n> > > >\n> > > >       switch (id->type()) {\n> > > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > >               value.set<std::string>(repr);\n> > > >               break;\n> > > >       }\n> > > > -     case ControlTypeRectangle:\n> > > > -             /* \\todo Parse rectangles. */\n> > > > -             break;\n> > > > -     case ControlTypeSize:\n> > > > -             /* \\todo Parse Sizes. */\n> > > > +     default:\n> > > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > > >               break;\n> > > >       }\n> > > >\n> > > >       return value;\n> > > >  }\n> > > > +\n> > > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > > +{\n> > > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > > +\n> > > > +     return Rectangle(x, y, width, height);\n> > > > +}\n> > > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > > index 8b4f8f62..fffe67e5 100644\n> > > > --- a/src/cam/capture_script.h\n> > > > +++ b/src/cam/capture_script.h\n> > > > @@ -54,9 +54,12 @@ private:\n> > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > > >\n> > > >       std::string parseScalar();\n> > > > +     libcamera::ControlValue parseRectangles();\n> > > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > > +     std::vector<std::string> parseSingleArray();\n> > > >\n> > > >       void unpackFailure(const libcamera::ControlId *id,\n> > > >                          const std::string &repr);\n> > > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > > -                                           const std::string &repr);\n> > > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > > >  };\n> > > > --\n> > > > 2.34.1\n> > > >","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 8AF07C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Aug 2022 07:20:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB4C96332B;\n\tFri,  5 Aug 2022 09:20:30 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B49E863312\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Aug 2022 09:20:29 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 787D920009;\n\tFri,  5 Aug 2022 07:20:28 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659684030;\n\tbh=IHR2ZJXoQLxj/C8DwDBNxh8VaWeaTUYMt7fpcqZyCsM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Uux/Tr50z3vGdVbuLFq+Nt0QKgtKQPCSCO8NBMAz+aKpMZTuul0K+turjRVJV3BhJ\n\tP9vCb7wEh5IggBlDvDPn8AenTGZ3wSqRttxn36wjHTigj4xns1jaKwF825GbstJLFo\n\tg+QeWLPnbNABmNNJiJOTFs6LZdArWygGOeJix//DsU61vFRWFGG0FH5b8tAKEuHOIh\n\tsw+pE19Bspd4j21fZpuunAzVrisrBCbxZlIB/9kPXZBLAdS0xFuAh1rV/Ruzr9233K\n\tkVqnizm0R9nqNbuhTn1PY+YpgpH9EIteWSOACOD2nTpK/Uf6OcyQGHWE8hAHtF0nEK\n\tOvfy0FR5cWWMQ==","Date":"Fri, 5 Aug 2022 09:20:26 +0200","To":"paul.elder@ideasonboard.com","Message-ID":"<20220805072026.zuwuf5yj3t5o4whj@uno.localdomain>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220627160633.emtfplwafkpdkqyg@uno.localdomain>\n\t<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>\n\t<20220804194746.GP311202@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220804194746.GP311202@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24405,"web_url":"https://patchwork.libcamera.org/comment/24405/","msgid":"<20220808065605.GQ311202@pyrite.rasen.tech>","date":"2022-08-08T06:56:05","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Jacopo,\n\nOn Fri, Aug 05, 2022 at 09:20:26AM +0200, Jacopo Mondi wrote:\n> Hi Paul\n> \n> On Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:\n> > Hi Daniel,\n> >\n> > Thanks for the patches.\n> >\n> > On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > > >\n> > > > Hi Daniel\n> > > >\n> > > > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > This change is required for AfWindows control from capture script.\n> > > > > Parser expects array of arrays of parameters, so it is possible to\n> > > > > specify multiple rectangles.\n> > > > >\n> > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > > > > ---\n> > > > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > > > >  src/cam/capture_script.h   |   7 +-\n> > > > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > > > >\n> > > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > > > index 9f22d5f7..6278a152 100644\n> > > > > --- a/src/cam/capture_script.cpp\n> > > > > +++ b/src/cam/capture_script.cpp\n> > > > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > > > >               return -EINVAL;\n> > > > >       }\n> > > > >\n> > > > > -     std::string value = parseScalar();\n> > > > > -     if (value.empty())\n> > > > > +     const ControlId *controlId = it->second;\n> > > > > +\n> > > > > +     ControlValue val = unpackControl(controlId);\n> > > > > +     if (val.isNone()) {\n> > > > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > > > +                       << std::endl;\n> > > > >               return -EINVAL;\n> > > > > +     }\n> > > > >\n> > > > > -     const ControlId *controlId = it->second;\n> > > > > -     ControlValue val = unpackControl(controlId, value);\n> > > > >       controls.set(controlId->id(), val);\n> > > > >\n> > > > >       return 0;\n> > > > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > > > >       return eventScalarValue(event);\n> > > > >  }\n> > > > >\n> > > > > +ControlValue CaptureScript::parseRectangles()\n> > > > > +{\n> > > > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > > > +\n> > > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > > > +     if (arrays.empty())\n> > > > > +             return {};\n> > > > > +\n> > > > > +     for (const std::vector<std::string> &values : arrays) {\n> > > > > +             if (values.size() != 4) {\n> > > > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > > > +                                  \"array with 4 parameters\" << std::endl;\n> > > > > +                     return {};\n> > > > > +             }\n> > > > > +\n> > > > > +             Rectangle rect = unpackRectangle(values);\n> > > > > +             rectangles.push_back(rect);\n> > > > > +     }\n> > > > > +\n> > > > > +     ControlValue controlValue;\n> > > > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > > > +\n> > > > > +     return controlValue;\n> > > > > +}\n> > > > > +\n> > > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > > > +{\n> > > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > > > +     if (!event)\n> > > > > +             return {};\n> > > > > +\n> > > > > +     event = nextEvent();\n> > > > > +     if (!event)\n> > > > > +             return {};\n> > > > > +\n> > > > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > > > +\n> > > > > +     /* Parse single array. */\n> > > > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > > > +             std::string firstValue = eventScalarValue(event);\n> > > > > +             if (firstValue.empty())\n> > > > > +                     return {};\n> > > > > +\n> > > > > +             std::vector<std::string> remaining = parseSingleArray();\n> > > >\n> > > > nit: remaining can potentilly be {}\n> > >\n> > > Yes, but the vector can be empty not only because of error, but also\n> > > because there was a valid array but with only one element (already read\n> > > in firstValue).\n> > > We would need to add additional parameter returned from method to\n> > > distinguish between these two situations.\n> > > The higher level that calls parseArrays() should already know the\n> > > expected number of elements in the single array, so the error will be\n> > > caught anyway. I wouldn't spend too much time tweaking this solution as\n> > > it is not critical.\n> > > What do you think about leaving it as it is?\n> > >\n> > > >\n> > > > > +\n> > > > > +             std::vector<std::string> values = { firstValue };\n> > > > > +             values.insert(std::end(values),\n> > > > > +                           std::begin(remaining), std::end(remaining));\n> > > > > +             valueArrays.push_back(values);\n> > > > > +\n> > > > > +             return valueArrays;\n> > > > > +     }\n> > > >\n> > > > This should address Laurent's comment and I assume it now works with\n> > > > both\n> > > >         AfWindows:\n> > > >           - [x, y, w, h]\n> >\n> > Gotta be honest, I'm not a big fan of representing Rectangles this way;\n> > it's a bit... flat (if that's the right word).\n> > I'd prefer it to be like (x,y)/wxh\n> \n> This would require specifying the rectangle as a string. How will it\n> look like in the .yaml file ? Do you have examples you have tested\n> your version to share ?\n\nOh right I forgot we've got a yaml spec to conform to :S\n\nI just have it in there as-is:\n\nframes:\n  - 1:\n      ScalerCrop: (0,0)/1920x1080\n\nI suppose yaml doesn't have a \"rectangle\" type.\n\nSo if it's turning that into a string vs into an array... yeah I guess\nan array of four ints is better.\n\nAlright, nvm my objection then :)\n\n\nPaul\n\n> \n> >\n> >\n> > > >\n> > > > and\n> > > >         AfWindows:\n> > > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n> > > >\n> > >\n> > > Exactly, I tested it with both versions :)\n> > >\n> > > Best regards\n> > > Daniel\n> > >\n> > > > > +\n> > > > > +     /* Parse array of arrays. */\n> > > > > +     while (1) {\n> > > > > +             switch (event->type) {\n> > > > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > > > +                     std::vector<std::string> values = parseSingleArray();\n> > > > > +                     valueArrays.push_back(values);\n> > > > > +                     break;\n> > > > > +             }\n> > > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > > +                     return valueArrays;\n> > > > > +             default:\n> > > > > +                     return {};\n> > > > > +             }\n> > > > > +\n> > > > > +             event = nextEvent();\n> > > > > +             if (!event)\n> > > > > +                     return {};\n> > > > > +     }\n> > > > > +}\n> > > > > +\n> > > > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > > > +{\n> > > > > +     std::vector<std::string> values;\n> > > > > +\n> > > > > +     while (1) {\n> > > > > +             EventPtr event = nextEvent();\n> > > > > +             if (!event)\n> > > > > +                     return {};\n> > > > > +\n> > > > > +             switch (event->type) {\n> > > > > +             case YAML_SCALAR_EVENT: {\n> > > > > +                     std::string value = eventScalarValue(event);\n> > > > > +                     if (value.empty())\n> > > > > +                             return {};\n> > > > > +                     values.push_back(value);\n> > > > > +                     break;\n> > > > > +             }\n> > > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > > +                     return values;\n> > > > > +             default:\n> > > > > +                     return {};\n> > > > > +             }\n> > > > > +     }\n> > > > > +}\n> > > > > +\n> > > > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > > >  {\n> > > > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > > >                 << typeName << \" control \" << id->name() << std::endl;\n> > > > >  }\n> > > > >\n> > > > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > > -                                       const std::string &repr)\n> > > > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > > > >  {\n> > > > > +     /* Parse complex types. */\n> > > > > +     switch (id->type()) {\n> > > > > +     case ControlTypeRectangle:\n> > > > > +             return parseRectangles();\n> > > > > +     case ControlTypeSize:\n> > > > > +             /* \\todo Parse Sizes. */\n> > > > > +             return {};\n> > > > > +     default:\n> > > > > +             break;\n> > > > > +     }\n> > > > > +\n> > > > > +     /* Parse basic types represented by a single scalar. */\n> > > > > +     const std::string repr = parseScalar();\n> > > > > +     if (repr.empty())\n> > > > > +             return {};\n> > > > > +\n> > > >\n> > > > Thanks, this address my comment on v1.\n> > > >\n> > > > Nits apart it looks good to me\n> > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > >\n> > > > Thanks\n> > > >    j\n> > > >\n> > > > >       ControlValue value{};\n> > > > >\n> > > > >       switch (id->type()) {\n> > > > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > >               value.set<std::string>(repr);\n> > > > >               break;\n> > > > >       }\n> > > > > -     case ControlTypeRectangle:\n> > > > > -             /* \\todo Parse rectangles. */\n> > > > > -             break;\n> > > > > -     case ControlTypeSize:\n> > > > > -             /* \\todo Parse Sizes. */\n> > > > > +     default:\n> > > > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > > > >               break;\n> > > > >       }\n> > > > >\n> > > > >       return value;\n> > > > >  }\n> > > > > +\n> > > > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > > > +{\n> > > > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > > > +\n> > > > > +     return Rectangle(x, y, width, height);\n> > > > > +}\n> > > > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > > > index 8b4f8f62..fffe67e5 100644\n> > > > > --- a/src/cam/capture_script.h\n> > > > > +++ b/src/cam/capture_script.h\n> > > > > @@ -54,9 +54,12 @@ private:\n> > > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > > > >\n> > > > >       std::string parseScalar();\n> > > > > +     libcamera::ControlValue parseRectangles();\n> > > > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > > > +     std::vector<std::string> parseSingleArray();\n> > > > >\n> > > > >       void unpackFailure(const libcamera::ControlId *id,\n> > > > >                          const std::string &repr);\n> > > > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > > > -                                           const std::string &repr);\n> > > > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > > > >  };\n> > > > > --\n> > > > > 2.34.1\n> > > > >","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 07A92C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Aug 2022 06:56:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 267DF6332B;\n\tMon,  8 Aug 2022 08:56:16 +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 6B769603EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Aug 2022 08:56:14 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D55B749C;\n\tMon,  8 Aug 2022 08:56:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659941776;\n\tbh=CvRDI5xX+xreXgtWd9BVbjcsyZhxWqk7ibNhf0PI2kA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=yVYZqq4ZKl11Mm9Cx4kanZbObqkiuJTBLSXdhbqDM33xrEyImDICe3M9KJKr9hS1m\n\tICY4GcG6iCC3xu9ZTTuBOCENekRK2Mw2DAV7K0oajp/KFuwkRmS3uV5bWcuX5IrCLp\n\t5PsVUO7LkJFddSaMF5VhTmuEPRNCQW4yxDTJkXWzMneWfv8FSBDPOJ3fwDHcdaVLDm\n\tKyF0sbjqHpt2zM00jgHHhi11rxNDpKQYLJ9Jko4rR68fvWlKOQzmnreJX4Cso0lUPv\n\trPLtYZ1rHqh5QbMUzRFPFkDfe0QIaX7MItepaGj/+cqxp0Zq6X0RujI6keBYU1Yup5\n\t+NfJIUWM9c8NQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659941774;\n\tbh=CvRDI5xX+xreXgtWd9BVbjcsyZhxWqk7ibNhf0PI2kA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=C3Yqo/qtE7qhHHwz7BgYVRjGMB8LHKKAWZle7RUHXz2cvfdWe8Cw6604W21lNT5H0\n\tC1XSae6lc/Jm9g+Y/iqUVFhASik7ERwvlzcL6sj5Y2/eKQcvqwHThd0RPx8+LD8SVN\n\tfkBo9UpY9y6w7h1RCNI5+wI6JCQAGsh3mGdLlHH0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"C3Yqo/qt\"; dkim-atps=neutral","Date":"Mon, 8 Aug 2022 15:56:05 +0900","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20220808065605.GQ311202@pyrite.rasen.tech>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220627160633.emtfplwafkpdkqyg@uno.localdomain>\n\t<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>\n\t<20220804194746.GP311202@pyrite.rasen.tech>\n\t<20220805072026.zuwuf5yj3t5o4whj@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220805072026.zuwuf5yj3t5o4whj@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24415,"web_url":"https://patchwork.libcamera.org/comment/24415/","msgid":"<20220808084958.aesfz4u6zimr54d2@uno.localdomain>","date":"2022-08-08T08:49:58","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Mon, Aug 08, 2022 at 03:56:05PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Jacopo,\n>\n> On Fri, Aug 05, 2022 at 09:20:26AM +0200, Jacopo Mondi wrote:\n> > Hi Paul\n> >\n> > On Fri, Aug 05, 2022 at 04:47:46AM +0900, paul.elder@ideasonboard.com wrote:\n> > > Hi Daniel,\n> > >\n> > > Thanks for the patches.\n> > >\n> > > On Tue, Jun 28, 2022 at 08:35:03AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > On Mon, Jun 27, 2022 at 6:06 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > > > >\n> > > > > Hi Daniel\n> > > > >\n> > > > > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > > This change is required for AfWindows control from capture script.\n> > > > > > Parser expects array of arrays of parameters, so it is possible to\n> > > > > > specify multiple rectangles.\n> > > > > >\n> > > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > > > > > ---\n> > > > > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > > > > >  src/cam/capture_script.h   |   7 +-\n> > > > > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > > > > >\n> > > > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > > > > index 9f22d5f7..6278a152 100644\n> > > > > > --- a/src/cam/capture_script.cpp\n> > > > > > +++ b/src/cam/capture_script.cpp\n> > > > > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > > > > >               return -EINVAL;\n> > > > > >       }\n> > > > > >\n> > > > > > -     std::string value = parseScalar();\n> > > > > > -     if (value.empty())\n> > > > > > +     const ControlId *controlId = it->second;\n> > > > > > +\n> > > > > > +     ControlValue val = unpackControl(controlId);\n> > > > > > +     if (val.isNone()) {\n> > > > > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > > > > +                       << std::endl;\n> > > > > >               return -EINVAL;\n> > > > > > +     }\n> > > > > >\n> > > > > > -     const ControlId *controlId = it->second;\n> > > > > > -     ControlValue val = unpackControl(controlId, value);\n> > > > > >       controls.set(controlId->id(), val);\n> > > > > >\n> > > > > >       return 0;\n> > > > > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > > > > >       return eventScalarValue(event);\n> > > > > >  }\n> > > > > >\n> > > > > > +ControlValue CaptureScript::parseRectangles()\n> > > > > > +{\n> > > > > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > > > > +\n> > > > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > > > > +     if (arrays.empty())\n> > > > > > +             return {};\n> > > > > > +\n> > > > > > +     for (const std::vector<std::string> &values : arrays) {\n> > > > > > +             if (values.size() != 4) {\n> > > > > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > > > > +                                  \"array with 4 parameters\" << std::endl;\n> > > > > > +                     return {};\n> > > > > > +             }\n> > > > > > +\n> > > > > > +             Rectangle rect = unpackRectangle(values);\n> > > > > > +             rectangles.push_back(rect);\n> > > > > > +     }\n> > > > > > +\n> > > > > > +     ControlValue controlValue;\n> > > > > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > > > > +\n> > > > > > +     return controlValue;\n> > > > > > +}\n> > > > > > +\n> > > > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > > > > +{\n> > > > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > > > > +     if (!event)\n> > > > > > +             return {};\n> > > > > > +\n> > > > > > +     event = nextEvent();\n> > > > > > +     if (!event)\n> > > > > > +             return {};\n> > > > > > +\n> > > > > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > > > > +\n> > > > > > +     /* Parse single array. */\n> > > > > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > > > > +             std::string firstValue = eventScalarValue(event);\n> > > > > > +             if (firstValue.empty())\n> > > > > > +                     return {};\n> > > > > > +\n> > > > > > +             std::vector<std::string> remaining = parseSingleArray();\n> > > > >\n> > > > > nit: remaining can potentilly be {}\n> > > >\n> > > > Yes, but the vector can be empty not only because of error, but also\n> > > > because there was a valid array but with only one element (already read\n> > > > in firstValue).\n> > > > We would need to add additional parameter returned from method to\n> > > > distinguish between these two situations.\n> > > > The higher level that calls parseArrays() should already know the\n> > > > expected number of elements in the single array, so the error will be\n> > > > caught anyway. I wouldn't spend too much time tweaking this solution as\n> > > > it is not critical.\n> > > > What do you think about leaving it as it is?\n> > > >\n> > > > >\n> > > > > > +\n> > > > > > +             std::vector<std::string> values = { firstValue };\n> > > > > > +             values.insert(std::end(values),\n> > > > > > +                           std::begin(remaining), std::end(remaining));\n> > > > > > +             valueArrays.push_back(values);\n> > > > > > +\n> > > > > > +             return valueArrays;\n> > > > > > +     }\n> > > > >\n> > > > > This should address Laurent's comment and I assume it now works with\n> > > > > both\n> > > > >         AfWindows:\n> > > > >           - [x, y, w, h]\n> > >\n> > > Gotta be honest, I'm not a big fan of representing Rectangles this way;\n> > > it's a bit... flat (if that's the right word).\n> > > I'd prefer it to be like (x,y)/wxh\n> >\n> > This would require specifying the rectangle as a string. How will it\n> > look like in the .yaml file ? Do you have examples you have tested\n> > your version to share ?\n>\n> Oh right I forgot we've got a yaml spec to conform to :S\n>\n> I just have it in there as-is:\n>\n> frames:\n>   - 1:\n>       ScalerCrop: (0,0)/1920x1080\n>\n> I suppose yaml doesn't have a \"rectangle\" type.\n>\n> So if it's turning that into a string vs into an array... yeah I guess\n> an array of four ints is better.\n\nI would indeed prefer to avoid strings. From a correctness point of\nview we won't lose much as your code parses the string and if we ever\nwould have a schema we can impose a format, but I feel it would be\nharder to express multiple rectangles. We could consider more complex\nmappings like:\n\n         Rectangles:\n           - x: 0\n             y: 0\n             w: 100\n             h: 100\n           - x: 120\n             y: 120\n             w: 80\n             h: 80\n\nwhich are maybe easier to validate, but as far as I can tell won't\ngive us anything more than a list of integers\n\n>\n> Alright, nvm my objection then :)\n\nCare to send a tag ? :)\nI guess we can collect that patch if it's fine with you!\n\nThanks\n  j\n\n>\n>\n> Paul\n>\n> >\n> > >\n> > >\n> > > > >\n> > > > > and\n> > > > >         AfWindows:\n> > > > >           - [[x, y, w, h], [x1, y1, w1, h1] .. ]\n> > > > >\n> > > >\n> > > > Exactly, I tested it with both versions :)\n> > > >\n> > > > Best regards\n> > > > Daniel\n> > > >\n> > > > > > +\n> > > > > > +     /* Parse array of arrays. */\n> > > > > > +     while (1) {\n> > > > > > +             switch (event->type) {\n> > > > > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > > > > +                     std::vector<std::string> values = parseSingleArray();\n> > > > > > +                     valueArrays.push_back(values);\n> > > > > > +                     break;\n> > > > > > +             }\n> > > > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > > > +                     return valueArrays;\n> > > > > > +             default:\n> > > > > > +                     return {};\n> > > > > > +             }\n> > > > > > +\n> > > > > > +             event = nextEvent();\n> > > > > > +             if (!event)\n> > > > > > +                     return {};\n> > > > > > +     }\n> > > > > > +}\n> > > > > > +\n> > > > > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > > > > +{\n> > > > > > +     std::vector<std::string> values;\n> > > > > > +\n> > > > > > +     while (1) {\n> > > > > > +             EventPtr event = nextEvent();\n> > > > > > +             if (!event)\n> > > > > > +                     return {};\n> > > > > > +\n> > > > > > +             switch (event->type) {\n> > > > > > +             case YAML_SCALAR_EVENT: {\n> > > > > > +                     std::string value = eventScalarValue(event);\n> > > > > > +                     if (value.empty())\n> > > > > > +                             return {};\n> > > > > > +                     values.push_back(value);\n> > > > > > +                     break;\n> > > > > > +             }\n> > > > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > > > +                     return values;\n> > > > > > +             default:\n> > > > > > +                     return {};\n> > > > > > +             }\n> > > > > > +     }\n> > > > > > +}\n> > > > > > +\n> > > > > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > > > >  {\n> > > > > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > > > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > > > >                 << typeName << \" control \" << id->name() << std::endl;\n> > > > > >  }\n> > > > > >\n> > > > > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > > > -                                       const std::string &repr)\n> > > > > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > > > > >  {\n> > > > > > +     /* Parse complex types. */\n> > > > > > +     switch (id->type()) {\n> > > > > > +     case ControlTypeRectangle:\n> > > > > > +             return parseRectangles();\n> > > > > > +     case ControlTypeSize:\n> > > > > > +             /* \\todo Parse Sizes. */\n> > > > > > +             return {};\n> > > > > > +     default:\n> > > > > > +             break;\n> > > > > > +     }\n> > > > > > +\n> > > > > > +     /* Parse basic types represented by a single scalar. */\n> > > > > > +     const std::string repr = parseScalar();\n> > > > > > +     if (repr.empty())\n> > > > > > +             return {};\n> > > > > > +\n> > > > >\n> > > > > Thanks, this address my comment on v1.\n> > > > >\n> > > > > Nits apart it looks good to me\n> > > > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > > >\n> > > > > Thanks\n> > > > >    j\n> > > > >\n> > > > > >       ControlValue value{};\n> > > > > >\n> > > > > >       switch (id->type()) {\n> > > > > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > > >               value.set<std::string>(repr);\n> > > > > >               break;\n> > > > > >       }\n> > > > > > -     case ControlTypeRectangle:\n> > > > > > -             /* \\todo Parse rectangles. */\n> > > > > > -             break;\n> > > > > > -     case ControlTypeSize:\n> > > > > > -             /* \\todo Parse Sizes. */\n> > > > > > +     default:\n> > > > > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > > > > >               break;\n> > > > > >       }\n> > > > > >\n> > > > > >       return value;\n> > > > > >  }\n> > > > > > +\n> > > > > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > > > > +{\n> > > > > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > > > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > > > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > > > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > > > > +\n> > > > > > +     return Rectangle(x, y, width, height);\n> > > > > > +}\n> > > > > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > > > > index 8b4f8f62..fffe67e5 100644\n> > > > > > --- a/src/cam/capture_script.h\n> > > > > > +++ b/src/cam/capture_script.h\n> > > > > > @@ -54,9 +54,12 @@ private:\n> > > > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > > > > >\n> > > > > >       std::string parseScalar();\n> > > > > > +     libcamera::ControlValue parseRectangles();\n> > > > > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > > > > +     std::vector<std::string> parseSingleArray();\n> > > > > >\n> > > > > >       void unpackFailure(const libcamera::ControlId *id,\n> > > > > >                          const std::string &repr);\n> > > > > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > > > > -                                           const std::string &repr);\n> > > > > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > > > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > > > > >  };\n> > > > > > --\n> > > > > > 2.34.1\n> > > > > >","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 1A0E2C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Aug 2022 08:50:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F5B16332B;\n\tMon,  8 Aug 2022 10:50:02 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C7A463326\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Aug 2022 10:50:01 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id DDABFFF806;\n\tMon,  8 Aug 2022 08:49:59 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659948602;\n\tbh=DD9fQRp23xTp+BQoXtLRmW/CGmVHY3ca1DZfay014j0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=n6IL6ArbBx5fVXV/YfBS8Hsy0cyST+sBgi9/6v5xCwMe444FwW5S6SV3BhsQprDGm\n\tYxVbRyKXYs6z3/6Rv92cJ1X2SZx34FoRuOQicPEPuEJgD0R/Eeg8sW7QEeZ7jK1eUb\n\tNwGvDfLEO90tVILAX/XuxhOgGHz9H0fksWbYSD7AXpK1dogSiBmQELyX7qCvxY8Akf\n\ttrd0q/xpgHOOpHaMfbpZ3tI+4A3055skCs6LVhLtngsOwxH2Ac+J4XwTHbA4guw5EX\n\tv4PpK0FDyv9+J9gANX/bVvmWUs4ZxNlArQGI3nWyXgsvbbNGUxeNBCqtIsOxQ+udrt\n\tpGcYgbihbg7Eg==","Date":"Mon, 8 Aug 2022 10:49:58 +0200","To":"paul.elder@ideasonboard.com","Message-ID":"<20220808084958.aesfz4u6zimr54d2@uno.localdomain>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220627160633.emtfplwafkpdkqyg@uno.localdomain>\n\t<CAHgnY3=ZqgOc+KHm9uJ4zJ81m=5B2RPh0F-tnPYumCm1sUnkYg@mail.gmail.com>\n\t<20220804194746.GP311202@pyrite.rasen.tech>\n\t<20220805072026.zuwuf5yj3t5o4whj@uno.localdomain>\n\t<20220808065605.GQ311202@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220808065605.GQ311202@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24424,"web_url":"https://patchwork.libcamera.org/comment/24424/","msgid":"<20220808102638.GR311202@pyrite.rasen.tech>","date":"2022-08-08T10:26:38","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Daniel,\n\nOn Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> This change is required for AfWindows control from capture script.\n> Parser expects array of arrays of parameters, so it is possible to\n> specify multiple rectangles.\n> \n> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n>  src/cam/capture_script.h   |   7 +-\n>  2 files changed, 139 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> index 9f22d5f7..6278a152 100644\n> --- a/src/cam/capture_script.cpp\n> +++ b/src/cam/capture_script.cpp\n> @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> -\tstd::string value = parseScalar();\n> -\tif (value.empty())\n> +\tconst ControlId *controlId = it->second;\n> +\n> +\tControlValue val = unpackControl(controlId);\n> +\tif (val.isNone()) {\n> +\t\tstd::cerr << \"Error unpacking control '\" << name << \"'\"\n> +\t\t\t  << std::endl;\n>  \t\treturn -EINVAL;\n> +\t}\n>  \n> -\tconst ControlId *controlId = it->second;\n> -\tControlValue val = unpackControl(controlId, value);\n>  \tcontrols.set(controlId->id(), val);\n>  \n>  \treturn 0;\n> @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n>  \treturn eventScalarValue(event);\n>  }\n>  \n> +ControlValue CaptureScript::parseRectangles()\n> +{\n> +\tstd::vector<libcamera::Rectangle> rectangles;\n> +\n> +\tstd::vector<std::vector<std::string>> arrays = parseArrays();\n> +\tif (arrays.empty())\n> +\t\treturn {};\n> +\n> +\tfor (const std::vector<std::string> &values : arrays) {\n> +\t\tif (values.size() != 4) {\n> +\t\t\tstd::cerr << \"Error parsing Rectangle: expected \"\n> +\t\t\t\t     \"array with 4 parameters\" << std::endl;\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tRectangle rect = unpackRectangle(values);\n> +\t\trectangles.push_back(rect);\n> +\t}\n> +\n> +\tControlValue controlValue;\n> +\tcontrolValue.set(Span<const Rectangle>(rectangles));\n> +\n> +\treturn controlValue;\n> +}\n> +\n> +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> +{\n> +\tEventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> +\tif (!event)\n> +\t\treturn {};\n> +\n> +\tevent = nextEvent();\n> +\tif (!event)\n> +\t\treturn {};\n> +\n> +\tstd::vector<std::vector<std::string>> valueArrays;\n> +\n> +\t/* Parse single array. */\n> +\tif (event->type == YAML_SCALAR_EVENT) {\n> +\t\tstd::string firstValue = eventScalarValue(event);\n> +\t\tif (firstValue.empty())\n> +\t\t\treturn {};\n> +\n> +\t\tstd::vector<std::string> remaining = parseSingleArray();\n> +\n> +\t\tstd::vector<std::string> values = { firstValue };\n> +\t\tvalues.insert(std::end(values),\n> +\t\t\t      std::begin(remaining), std::end(remaining));\n> +\t\tvalueArrays.push_back(values);\n> +\n> +\t\treturn valueArrays;\n> +\t}\n> +\n> +\t/* Parse array of arrays. */\n> +\twhile (1) {\n> +\t\tswitch (event->type) {\n> +\t\tcase YAML_SEQUENCE_START_EVENT: {\n> +\t\t\tstd::vector<std::string> values = parseSingleArray();\n> +\t\t\tvalueArrays.push_back(values);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase YAML_SEQUENCE_END_EVENT:\n> +\t\t\treturn valueArrays;\n> +\t\tdefault:\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn {};\n> +\t}\n> +}\n> +\n> +std::vector<std::string> CaptureScript::parseSingleArray()\n> +{\n> +\tstd::vector<std::string> values;\n> +\n> +\twhile (1) {\n> +\t\tEventPtr event = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn {};\n> +\n> +\t\tswitch (event->type) {\n> +\t\tcase YAML_SCALAR_EVENT: {\n> +\t\t\tstd::string value = eventScalarValue(event);\n> +\t\t\tif (value.empty())\n> +\t\t\t\treturn {};\n> +\t\t\tvalues.push_back(value);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase YAML_SEQUENCE_END_EVENT:\n> +\t\t\treturn values;\n> +\t\tdefault:\n> +\t\t\treturn {};\n> +\t\t}\n> +\t}\n> +}\n> +\n>  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n>  {\n>  \tstatic const std::map<unsigned int, const char *> typeNames = {\n> @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n>  \t\t  << typeName << \" control \" << id->name() << std::endl;\n>  }\n>  \n> -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> -\t\t\t\t\t  const std::string &repr)\n> +ControlValue CaptureScript::unpackControl(const ControlId *id)\n>  {\n> +\t/* Parse complex types. */\n> +\tswitch (id->type()) {\n> +\tcase ControlTypeRectangle:\n> +\t\treturn parseRectangles();\n> +\tcase ControlTypeSize:\n> +\t\t/* \\todo Parse Sizes. */\n> +\t\treturn {};\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n> +\t/* Parse basic types represented by a single scalar. */\n> +\tconst std::string repr = parseScalar();\n> +\tif (repr.empty())\n> +\t\treturn {};\n> +\n>  \tControlValue value{};\n>  \n>  \tswitch (id->type()) {\n> @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n>  \t\tvalue.set<std::string>(repr);\n>  \t\tbreak;\n>  \t}\n> -\tcase ControlTypeRectangle:\n> -\t\t/* \\todo Parse rectangles. */\n> -\t\tbreak;\n> -\tcase ControlTypeSize:\n> -\t\t/* \\todo Parse Sizes. */\n> +\tdefault:\n> +\t\tstd::cerr << \"Unsupported control type\" << std::endl;\n>  \t\tbreak;\n>  \t}\n>  \n>  \treturn value;\n>  }\n> +\n> +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> +{\n> +\tint x = strtol(strVec[0].c_str(), NULL, 10);\n> +\tint y = strtol(strVec[1].c_str(), NULL, 10);\n> +\tunsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> +\tunsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> +\n> +\treturn Rectangle(x, y, width, height);\n> +}\n> diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> index 8b4f8f62..fffe67e5 100644\n> --- a/src/cam/capture_script.h\n> +++ b/src/cam/capture_script.h\n> @@ -54,9 +54,12 @@ private:\n>  \tint parseControl(EventPtr event, libcamera::ControlList &controls);\n>  \n>  \tstd::string parseScalar();\n> +\tlibcamera::ControlValue parseRectangles();\n> +\tstd::vector<std::vector<std::string>> parseArrays();\n> +\tstd::vector<std::string> parseSingleArray();\n>  \n>  \tvoid unpackFailure(const libcamera::ControlId *id,\n>  \t\t\t   const std::string &repr);\n> -\tlibcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> -\t\t\t\t\t      const std::string &repr);\n> +\tlibcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> +\tlibcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n>  };\n> -- \n> 2.34.1\n>","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 CB700C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Aug 2022 10:27:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F19C46332B;\n\tMon,  8 Aug 2022 12:27:23 +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 EA9D163315\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Aug 2022 12:27:22 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2548A481;\n\tMon,  8 Aug 2022 12:27:20 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659954444;\n\tbh=uBcBrY3DKvEugzT59VjFqZ4QCSL0XVqWWxWS4cS10Es=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JQZ+04ewxcBoL6GttANTqctXDmu3jGijAXPyoBDtjjzsAdVnEFP39WHZHdnaR3RnO\n\tnahT0SOWvS+hDz4AGHnKx/6ZIQPIdqegllcG8PfWWbQehxLxIDOTqIHJRbO9tFwTsV\n\tJ+DbZBKGkP///exPgjS/HXRaDlEFZ9+0J3PCZ8AGvHgArO4O1E6Wag9cgeb/0/ix6J\n\tQEHfsNOaW1vWMCdvoNnIaEVQerhJb6zc6EaJ51Zk6OVrMuw5AXZpqlHNPX+ioN5ZZm\n\tTK9gcdWItxCpSJWY6DUJYOlYxaqtJERy430i/wE03b7co5k0nC/RD5wD1wWok+ceF7\n\tgrBjXI6MORW5g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659954442;\n\tbh=uBcBrY3DKvEugzT59VjFqZ4QCSL0XVqWWxWS4cS10Es=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GLQIDZEjbSFKpNTG9PF14kkr7LE9mmlDAT9JQ2Ujh7bsi4WAksBaBo9bxtQVoVOL4\n\tliSdhU8qx0jORsIT+bEfvZAn7ENws03GDN5m26V9xbsScEFJ6l8siLweMlo5nwDvUN\n\tzFY3miSLXEWvanRi+dB0eFA8a3op4Yw6c95HFahA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GLQIDZEj\"; dkim-atps=neutral","Date":"Mon, 8 Aug 2022 19:26:38 +0900","To":"Daniel Semkowicz <dse@thaumatec.com>","Message-ID":"<20220808102638.GR311202@pyrite.rasen.tech>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220627122806.29586-2-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24891,"web_url":"https://patchwork.libcamera.org/comment/24891/","msgid":"<CAHgnY3k2K4atZRFXymWa_ZxtaBQVS2KbFHvXhZ+RXc-H0KPUVQ@mail.gmail.com>","date":"2022-09-02T08:02:40","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi,\n\nAre there plans to merge it to master or additional changes are required?\n\nBest regards\nDaniel\n\nOn Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:\n>\n> Hi Daniel,\n>\n> On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > This change is required for AfWindows control from capture script.\n> > Parser expects array of arrays of parameters, so it is possible to\n> > specify multiple rectangles.\n> >\n> > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n> > ---\n> >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> >  src/cam/capture_script.h   |   7 +-\n> >  2 files changed, 139 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > index 9f22d5f7..6278a152 100644\n> > --- a/src/cam/capture_script.cpp\n> > +++ b/src/cam/capture_script.cpp\n> > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> >               return -EINVAL;\n> >       }\n> >\n> > -     std::string value = parseScalar();\n> > -     if (value.empty())\n> > +     const ControlId *controlId = it->second;\n> > +\n> > +     ControlValue val = unpackControl(controlId);\n> > +     if (val.isNone()) {\n> > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > +                       << std::endl;\n> >               return -EINVAL;\n> > +     }\n> >\n> > -     const ControlId *controlId = it->second;\n> > -     ControlValue val = unpackControl(controlId, value);\n> >       controls.set(controlId->id(), val);\n> >\n> >       return 0;\n> > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> >       return eventScalarValue(event);\n> >  }\n> >\n> > +ControlValue CaptureScript::parseRectangles()\n> > +{\n> > +     std::vector<libcamera::Rectangle> rectangles;\n> > +\n> > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > +     if (arrays.empty())\n> > +             return {};\n> > +\n> > +     for (const std::vector<std::string> &values : arrays) {\n> > +             if (values.size() != 4) {\n> > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > +                                  \"array with 4 parameters\" << std::endl;\n> > +                     return {};\n> > +             }\n> > +\n> > +             Rectangle rect = unpackRectangle(values);\n> > +             rectangles.push_back(rect);\n> > +     }\n> > +\n> > +     ControlValue controlValue;\n> > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > +\n> > +     return controlValue;\n> > +}\n> > +\n> > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > +{\n> > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > +     if (!event)\n> > +             return {};\n> > +\n> > +     event = nextEvent();\n> > +     if (!event)\n> > +             return {};\n> > +\n> > +     std::vector<std::vector<std::string>> valueArrays;\n> > +\n> > +     /* Parse single array. */\n> > +     if (event->type == YAML_SCALAR_EVENT) {\n> > +             std::string firstValue = eventScalarValue(event);\n> > +             if (firstValue.empty())\n> > +                     return {};\n> > +\n> > +             std::vector<std::string> remaining = parseSingleArray();\n> > +\n> > +             std::vector<std::string> values = { firstValue };\n> > +             values.insert(std::end(values),\n> > +                           std::begin(remaining), std::end(remaining));\n> > +             valueArrays.push_back(values);\n> > +\n> > +             return valueArrays;\n> > +     }\n> > +\n> > +     /* Parse array of arrays. */\n> > +     while (1) {\n> > +             switch (event->type) {\n> > +             case YAML_SEQUENCE_START_EVENT: {\n> > +                     std::vector<std::string> values = parseSingleArray();\n> > +                     valueArrays.push_back(values);\n> > +                     break;\n> > +             }\n> > +             case YAML_SEQUENCE_END_EVENT:\n> > +                     return valueArrays;\n> > +             default:\n> > +                     return {};\n> > +             }\n> > +\n> > +             event = nextEvent();\n> > +             if (!event)\n> > +                     return {};\n> > +     }\n> > +}\n> > +\n> > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > +{\n> > +     std::vector<std::string> values;\n> > +\n> > +     while (1) {\n> > +             EventPtr event = nextEvent();\n> > +             if (!event)\n> > +                     return {};\n> > +\n> > +             switch (event->type) {\n> > +             case YAML_SCALAR_EVENT: {\n> > +                     std::string value = eventScalarValue(event);\n> > +                     if (value.empty())\n> > +                             return {};\n> > +                     values.push_back(value);\n> > +                     break;\n> > +             }\n> > +             case YAML_SEQUENCE_END_EVENT:\n> > +                     return values;\n> > +             default:\n> > +                     return {};\n> > +             }\n> > +     }\n> > +}\n> > +\n> >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> >  {\n> >       static const std::map<unsigned int, const char *> typeNames = {\n> > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> >                 << typeName << \" control \" << id->name() << std::endl;\n> >  }\n> >\n> > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > -                                       const std::string &repr)\n> > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> >  {\n> > +     /* Parse complex types. */\n> > +     switch (id->type()) {\n> > +     case ControlTypeRectangle:\n> > +             return parseRectangles();\n> > +     case ControlTypeSize:\n> > +             /* \\todo Parse Sizes. */\n> > +             return {};\n> > +     default:\n> > +             break;\n> > +     }\n> > +\n> > +     /* Parse basic types represented by a single scalar. */\n> > +     const std::string repr = parseScalar();\n> > +     if (repr.empty())\n> > +             return {};\n> > +\n> >       ControlValue value{};\n> >\n> >       switch (id->type()) {\n> > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> >               value.set<std::string>(repr);\n> >               break;\n> >       }\n> > -     case ControlTypeRectangle:\n> > -             /* \\todo Parse rectangles. */\n> > -             break;\n> > -     case ControlTypeSize:\n> > -             /* \\todo Parse Sizes. */\n> > +     default:\n> > +             std::cerr << \"Unsupported control type\" << std::endl;\n> >               break;\n> >       }\n> >\n> >       return value;\n> >  }\n> > +\n> > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > +{\n> > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > +\n> > +     return Rectangle(x, y, width, height);\n> > +}\n> > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > index 8b4f8f62..fffe67e5 100644\n> > --- a/src/cam/capture_script.h\n> > +++ b/src/cam/capture_script.h\n> > @@ -54,9 +54,12 @@ private:\n> >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> >\n> >       std::string parseScalar();\n> > +     libcamera::ControlValue parseRectangles();\n> > +     std::vector<std::vector<std::string>> parseArrays();\n> > +     std::vector<std::string> parseSingleArray();\n> >\n> >       void unpackFailure(const libcamera::ControlId *id,\n> >                          const std::string &repr);\n> > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > -                                           const std::string &repr);\n> > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> >  };\n> > --\n> > 2.34.1\n> >","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 EF325C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Sep 2022 08:02:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A67561FDB;\n\tFri,  2 Sep 2022 10:02:54 +0200 (CEST)","from mail-io1-xd32.google.com (mail-io1-xd32.google.com\n\t[IPv6:2607:f8b0:4864:20::d32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A738161FBB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Sep 2022 10:02:52 +0200 (CEST)","by mail-io1-xd32.google.com with SMTP id y187so1067017iof.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 02 Sep 2022 01:02:52 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662105774;\n\tbh=ZRfzcYlNQE5yStu+unVUTdWhTuc8paRHXdNzNSxALzk=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1QHi+SwhgoTwVJYVmg8sqJhzcHsMOSmrb9cZT51xuBNTCIgAPQ2B4YTL5HdjDWE6r\n\tpn03TT0nDvCuc1IIEUJa/qZUf0qkS+I6dOt4siwWPiOcmheQADDcPTqrvJK4yHcYEv\n\tGwhccZQlJqtNWP/KpRHPmoGtEeZIHBeCNoxfiSsFaPV2GM6aP9FK864w6q4WYpf4x9\n\tnYwmDMwOrKh9Peicgr3mCnz5covx1k916FwgnTKTmhVZ2UoUXFKjqiFIhVjON+E8Ci\n\tcuLP0aPZrs880F+xYQl8ZVdcYw8WR/oja5EGfUPlysT2pmezf5MaCAAZ5DevTkDDp2\n\tg+h1dARY0eApw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date;\n\tbh=DpKAkK+AWmYWo/xPEE0pGkIsMpT3UgN61Lg3KlL6Kx4=;\n\tb=VVj/RPvJY0qUys/+xlureCPQYrvjCuKdWL1POsdgI8+Z6tdonl+bJ8HDMZw38gUjLY\n\tsGlDFhSYi6MQapPZjl9n+F8DiGN6CXPS7KlULPUghYqxQOVqT/NK1mG31XBaZLVNKuTK\n\t0Na24p0FcQCWYy3ut2BhDdZn25ty9obfkub4VxwSg66y7Iv1wnrcGl09R2OHFTiG1kRr\n\t0U2apAalp77i2H39gGg/Zs2nZICJRywJoULpyrOmsw2HhDMzjGt4c8p50Ifuf1VJ2KSh\n\tPxiypIskdA8dPDenxiCSp3LwnCybQ0OIqPKSF9RvxHCevUItUz9UMZK+uAiZrt7Y3jG5\n\tIg/A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"VVj/RPvJ\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date;\n\tbh=DpKAkK+AWmYWo/xPEE0pGkIsMpT3UgN61Lg3KlL6Kx4=;\n\tb=tg7z6eEARFDPg33am6pnIniqqicY+bnPnMLOJu4gVsvmNbqNRzeX5ivQpqg0fek6jR\n\t//5cJ1wugrvYiNh00T1L9DlkVd0iRwLyggOBINZns6c/FeN31jWoBJE/Uqr5TsCMa1Ci\n\takUnXTIxC0DGaUCvLvnGdI7RQeArvEpd8atU40DTokdm/t75uqZg8EH0bLJCL1j04zy+\n\tLv9tDyewFp4T5WJRJoYss070q19O/TM4TCqKjL7n1EVLO3pVmvHihtb8IIQDe/FlsPLb\n\tDl48+XZA1i4JMcQLeFHQ66AXARRsm5zIaWVrPK6fASRT2PSyBHgxsIbdyEPkDQU1ymkk\n\t0rYg==","X-Gm-Message-State":"ACgBeo2ws3yedEa0twCafC3zW/rQXLDFATOOl59f5ODD7JvAvxe4VOYX\n\t+XR3QsZxe+2KFsNhd+2TnD+ljjaJfiuxBxFjDd9cMg==","X-Google-Smtp-Source":"AA6agR7K+/wfEkiU9gyIymC6+uXLg1PpaYMQSgWydm0xW59ezlZASwu96+pp2uP7BO3m7Ue2KQB1Twim7u5IpX9I+m4=","X-Received":"by 2002:a05:6638:480a:b0:346:a98b:1764 with SMTP id\n\tcp10-20020a056638480a00b00346a98b1764mr19201605jab.108.1662105771071;\n\tFri, 02 Sep 2022 01:02:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220808102638.GR311202@pyrite.rasen.tech>","In-Reply-To":"<20220808102638.GR311202@pyrite.rasen.tech>","Date":"Fri, 2 Sep 2022 10:02:40 +0200","Message-ID":"<CAHgnY3k2K4atZRFXymWa_ZxtaBQVS2KbFHvXhZ+RXc-H0KPUVQ@mail.gmail.com>","To":"paul.elder@ideasonboard.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Daniel Semkowicz via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Daniel Semkowicz <dse@thaumatec.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24892,"web_url":"https://patchwork.libcamera.org/comment/24892/","msgid":"<20220902103219.loqlyobhvg3hke2n@uno.localdomain>","date":"2022-09-02T10:32:19","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"You're right, the series is tagged and can be merged.\n\nI'll take care of that and sorry for the delay :)\n\nOn Fri, Sep 02, 2022 at 10:02:40AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> Hi,\n>\n> Are there plans to merge it to master or additional changes are required?\n>\n> Best regards\n> Daniel\n>\n> On Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:\n> >\n> > Hi Daniel,\n> >\n> > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > This change is required for AfWindows control from capture script.\n> > > Parser expects array of arrays of parameters, so it is possible to\n> > > specify multiple rectangles.\n> > >\n> > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> >\n> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> >\n> > > ---\n> > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > >  src/cam/capture_script.h   |   7 +-\n> > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > >\n> > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > index 9f22d5f7..6278a152 100644\n> > > --- a/src/cam/capture_script.cpp\n> > > +++ b/src/cam/capture_script.cpp\n> > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > >               return -EINVAL;\n> > >       }\n> > >\n> > > -     std::string value = parseScalar();\n> > > -     if (value.empty())\n> > > +     const ControlId *controlId = it->second;\n> > > +\n> > > +     ControlValue val = unpackControl(controlId);\n> > > +     if (val.isNone()) {\n> > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > +                       << std::endl;\n> > >               return -EINVAL;\n> > > +     }\n> > >\n> > > -     const ControlId *controlId = it->second;\n> > > -     ControlValue val = unpackControl(controlId, value);\n> > >       controls.set(controlId->id(), val);\n> > >\n> > >       return 0;\n> > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > >       return eventScalarValue(event);\n> > >  }\n> > >\n> > > +ControlValue CaptureScript::parseRectangles()\n> > > +{\n> > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > +\n> > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > +     if (arrays.empty())\n> > > +             return {};\n> > > +\n> > > +     for (const std::vector<std::string> &values : arrays) {\n> > > +             if (values.size() != 4) {\n> > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > +                                  \"array with 4 parameters\" << std::endl;\n> > > +                     return {};\n> > > +             }\n> > > +\n> > > +             Rectangle rect = unpackRectangle(values);\n> > > +             rectangles.push_back(rect);\n> > > +     }\n> > > +\n> > > +     ControlValue controlValue;\n> > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > +\n> > > +     return controlValue;\n> > > +}\n> > > +\n> > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > +{\n> > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > +     if (!event)\n> > > +             return {};\n> > > +\n> > > +     event = nextEvent();\n> > > +     if (!event)\n> > > +             return {};\n> > > +\n> > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > +\n> > > +     /* Parse single array. */\n> > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > +             std::string firstValue = eventScalarValue(event);\n> > > +             if (firstValue.empty())\n> > > +                     return {};\n> > > +\n> > > +             std::vector<std::string> remaining = parseSingleArray();\n> > > +\n> > > +             std::vector<std::string> values = { firstValue };\n> > > +             values.insert(std::end(values),\n> > > +                           std::begin(remaining), std::end(remaining));\n> > > +             valueArrays.push_back(values);\n> > > +\n> > > +             return valueArrays;\n> > > +     }\n> > > +\n> > > +     /* Parse array of arrays. */\n> > > +     while (1) {\n> > > +             switch (event->type) {\n> > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > +                     std::vector<std::string> values = parseSingleArray();\n> > > +                     valueArrays.push_back(values);\n> > > +                     break;\n> > > +             }\n> > > +             case YAML_SEQUENCE_END_EVENT:\n> > > +                     return valueArrays;\n> > > +             default:\n> > > +                     return {};\n> > > +             }\n> > > +\n> > > +             event = nextEvent();\n> > > +             if (!event)\n> > > +                     return {};\n> > > +     }\n> > > +}\n> > > +\n> > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > +{\n> > > +     std::vector<std::string> values;\n> > > +\n> > > +     while (1) {\n> > > +             EventPtr event = nextEvent();\n> > > +             if (!event)\n> > > +                     return {};\n> > > +\n> > > +             switch (event->type) {\n> > > +             case YAML_SCALAR_EVENT: {\n> > > +                     std::string value = eventScalarValue(event);\n> > > +                     if (value.empty())\n> > > +                             return {};\n> > > +                     values.push_back(value);\n> > > +                     break;\n> > > +             }\n> > > +             case YAML_SEQUENCE_END_EVENT:\n> > > +                     return values;\n> > > +             default:\n> > > +                     return {};\n> > > +             }\n> > > +     }\n> > > +}\n> > > +\n> > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > >  {\n> > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > >                 << typeName << \" control \" << id->name() << std::endl;\n> > >  }\n> > >\n> > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > -                                       const std::string &repr)\n> > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > >  {\n> > > +     /* Parse complex types. */\n> > > +     switch (id->type()) {\n> > > +     case ControlTypeRectangle:\n> > > +             return parseRectangles();\n> > > +     case ControlTypeSize:\n> > > +             /* \\todo Parse Sizes. */\n> > > +             return {};\n> > > +     default:\n> > > +             break;\n> > > +     }\n> > > +\n> > > +     /* Parse basic types represented by a single scalar. */\n> > > +     const std::string repr = parseScalar();\n> > > +     if (repr.empty())\n> > > +             return {};\n> > > +\n> > >       ControlValue value{};\n> > >\n> > >       switch (id->type()) {\n> > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > >               value.set<std::string>(repr);\n> > >               break;\n> > >       }\n> > > -     case ControlTypeRectangle:\n> > > -             /* \\todo Parse rectangles. */\n> > > -             break;\n> > > -     case ControlTypeSize:\n> > > -             /* \\todo Parse Sizes. */\n> > > +     default:\n> > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > >               break;\n> > >       }\n> > >\n> > >       return value;\n> > >  }\n> > > +\n> > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > +{\n> > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > +\n> > > +     return Rectangle(x, y, width, height);\n> > > +}\n> > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > index 8b4f8f62..fffe67e5 100644\n> > > --- a/src/cam/capture_script.h\n> > > +++ b/src/cam/capture_script.h\n> > > @@ -54,9 +54,12 @@ private:\n> > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > >\n> > >       std::string parseScalar();\n> > > +     libcamera::ControlValue parseRectangles();\n> > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > +     std::vector<std::string> parseSingleArray();\n> > >\n> > >       void unpackFailure(const libcamera::ControlId *id,\n> > >                          const std::string &repr);\n> > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > -                                           const std::string &repr);\n> > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > >  };\n> > > --\n> > > 2.34.1\n> > >","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 CA694C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Sep 2022 10:32:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1ED6661FDD;\n\tFri,  2 Sep 2022 12:32:23 +0200 (CEST)","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 B6915603E1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Sep 2022 12:32:21 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id F16B46000A;\n\tFri,  2 Sep 2022 10:32:20 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662114743;\n\tbh=G3e8mwb/2KAZkRT1iom6Vjfcj+z13vFmwbRPJsZYRmQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=l4cBoO3zjGmzbjl0ea36EniuUhWMNQ/AGgNuO7bgBO8kBAUjH+0CbShDGlsnq8BJl\n\tItlL3qUAmMjzONnH9aySL6vU+yL3JFoehl1h94u6EuItAmglEr1s2sPwb6znMLLMTx\n\tRrG1/1L1ZU0/lYzX/RdNiiKiypM0ETzaxolDmQ9a3YLWK5kWUon2IF5aomPwStS6sN\n\tj4ZsibL/hptDli7sLNdYWfXiaeaOoIUSrDIQ10zAx9mbrxvtlmUD1t9qPJGnQ4Incu\n\tY93gAMYOWSSPYXNVhjgXeSSkOfF1+jHkxg8CQW/8HU0EgxeV4vdzAGAvq03NaJZEH9\n\tTpPtKi3WJz0GA==","Date":"Fri, 2 Sep 2022 12:32:19 +0200","To":"Daniel Semkowicz <dse@thaumatec.com>","Message-ID":"<20220902103219.loqlyobhvg3hke2n@uno.localdomain>","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220808102638.GR311202@pyrite.rasen.tech>\n\t<CAHgnY3k2K4atZRFXymWa_ZxtaBQVS2KbFHvXhZ+RXc-H0KPUVQ@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAHgnY3k2K4atZRFXymWa_ZxtaBQVS2KbFHvXhZ+RXc-H0KPUVQ@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24894,"web_url":"https://patchwork.libcamera.org/comment/24894/","msgid":"<CAHgnY3k8gMouRBbwR1NEdkbcff1L=eTNn8gi88ARqzfj514h3Q@mail.gmail.com>","date":"2022-09-02T12:47:40","subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Thank you :)\n\nOn Fri, Sep 2, 2022 at 12:32 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> You're right, the series is tagged and can be merged.\n>\n> I'll take care of that and sorry for the delay :)\n>\n> On Fri, Sep 02, 2022 at 10:02:40AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > Hi,\n> >\n> > Are there plans to merge it to master or additional changes are required?\n> >\n> > Best regards\n> > Daniel\n> >\n> > On Mon, Aug 8, 2022 at 12:27 PM <paul.elder@ideasonboard.com> wrote:\n> > >\n> > > Hi Daniel,\n> > >\n> > > On Mon, Jun 27, 2022 at 02:28:05PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > This change is required for AfWindows control from capture script.\n> > > > Parser expects array of arrays of parameters, so it is possible to\n> > > > specify multiple rectangles.\n> > > >\n> > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>\n> > >\n> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> > >\n> > > > ---\n> > > >  src/cam/capture_script.cpp | 145 ++++++++++++++++++++++++++++++++++---\n> > > >  src/cam/capture_script.h   |   7 +-\n> > > >  2 files changed, 139 insertions(+), 13 deletions(-)\n> > > >\n> > > > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> > > > index 9f22d5f7..6278a152 100644\n> > > > --- a/src/cam/capture_script.cpp\n> > > > +++ b/src/cam/capture_script.cpp\n> > > > @@ -232,12 +232,15 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n> > > >               return -EINVAL;\n> > > >       }\n> > > >\n> > > > -     std::string value = parseScalar();\n> > > > -     if (value.empty())\n> > > > +     const ControlId *controlId = it->second;\n> > > > +\n> > > > +     ControlValue val = unpackControl(controlId);\n> > > > +     if (val.isNone()) {\n> > > > +             std::cerr << \"Error unpacking control '\" << name << \"'\"\n> > > > +                       << std::endl;\n> > > >               return -EINVAL;\n> > > > +     }\n> > > >\n> > > > -     const ControlId *controlId = it->second;\n> > > > -     ControlValue val = unpackControl(controlId, value);\n> > > >       controls.set(controlId->id(), val);\n> > > >\n> > > >       return 0;\n> > > > @@ -252,6 +255,104 @@ std::string CaptureScript::parseScalar()\n> > > >       return eventScalarValue(event);\n> > > >  }\n> > > >\n> > > > +ControlValue CaptureScript::parseRectangles()\n> > > > +{\n> > > > +     std::vector<libcamera::Rectangle> rectangles;\n> > > > +\n> > > > +     std::vector<std::vector<std::string>> arrays = parseArrays();\n> > > > +     if (arrays.empty())\n> > > > +             return {};\n> > > > +\n> > > > +     for (const std::vector<std::string> &values : arrays) {\n> > > > +             if (values.size() != 4) {\n> > > > +                     std::cerr << \"Error parsing Rectangle: expected \"\n> > > > +                                  \"array with 4 parameters\" << std::endl;\n> > > > +                     return {};\n> > > > +             }\n> > > > +\n> > > > +             Rectangle rect = unpackRectangle(values);\n> > > > +             rectangles.push_back(rect);\n> > > > +     }\n> > > > +\n> > > > +     ControlValue controlValue;\n> > > > +     controlValue.set(Span<const Rectangle>(rectangles));\n> > > > +\n> > > > +     return controlValue;\n> > > > +}\n> > > > +\n> > > > +std::vector<std::vector<std::string>> CaptureScript::parseArrays()\n> > > > +{\n> > > > +     EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> > > > +     if (!event)\n> > > > +             return {};\n> > > > +\n> > > > +     event = nextEvent();\n> > > > +     if (!event)\n> > > > +             return {};\n> > > > +\n> > > > +     std::vector<std::vector<std::string>> valueArrays;\n> > > > +\n> > > > +     /* Parse single array. */\n> > > > +     if (event->type == YAML_SCALAR_EVENT) {\n> > > > +             std::string firstValue = eventScalarValue(event);\n> > > > +             if (firstValue.empty())\n> > > > +                     return {};\n> > > > +\n> > > > +             std::vector<std::string> remaining = parseSingleArray();\n> > > > +\n> > > > +             std::vector<std::string> values = { firstValue };\n> > > > +             values.insert(std::end(values),\n> > > > +                           std::begin(remaining), std::end(remaining));\n> > > > +             valueArrays.push_back(values);\n> > > > +\n> > > > +             return valueArrays;\n> > > > +     }\n> > > > +\n> > > > +     /* Parse array of arrays. */\n> > > > +     while (1) {\n> > > > +             switch (event->type) {\n> > > > +             case YAML_SEQUENCE_START_EVENT: {\n> > > > +                     std::vector<std::string> values = parseSingleArray();\n> > > > +                     valueArrays.push_back(values);\n> > > > +                     break;\n> > > > +             }\n> > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > +                     return valueArrays;\n> > > > +             default:\n> > > > +                     return {};\n> > > > +             }\n> > > > +\n> > > > +             event = nextEvent();\n> > > > +             if (!event)\n> > > > +                     return {};\n> > > > +     }\n> > > > +}\n> > > > +\n> > > > +std::vector<std::string> CaptureScript::parseSingleArray()\n> > > > +{\n> > > > +     std::vector<std::string> values;\n> > > > +\n> > > > +     while (1) {\n> > > > +             EventPtr event = nextEvent();\n> > > > +             if (!event)\n> > > > +                     return {};\n> > > > +\n> > > > +             switch (event->type) {\n> > > > +             case YAML_SCALAR_EVENT: {\n> > > > +                     std::string value = eventScalarValue(event);\n> > > > +                     if (value.empty())\n> > > > +                             return {};\n> > > > +                     values.push_back(value);\n> > > > +                     break;\n> > > > +             }\n> > > > +             case YAML_SEQUENCE_END_EVENT:\n> > > > +                     return values;\n> > > > +             default:\n> > > > +                     return {};\n> > > > +             }\n> > > > +     }\n> > > > +}\n> > > > +\n> > > >  void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > >  {\n> > > >       static const std::map<unsigned int, const char *> typeNames = {\n> > > > @@ -277,9 +378,24 @@ void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)\n> > > >                 << typeName << \" control \" << id->name() << std::endl;\n> > > >  }\n> > > >\n> > > > -ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > > -                                       const std::string &repr)\n> > > > +ControlValue CaptureScript::unpackControl(const ControlId *id)\n> > > >  {\n> > > > +     /* Parse complex types. */\n> > > > +     switch (id->type()) {\n> > > > +     case ControlTypeRectangle:\n> > > > +             return parseRectangles();\n> > > > +     case ControlTypeSize:\n> > > > +             /* \\todo Parse Sizes. */\n> > > > +             return {};\n> > > > +     default:\n> > > > +             break;\n> > > > +     }\n> > > > +\n> > > > +     /* Parse basic types represented by a single scalar. */\n> > > > +     const std::string repr = parseScalar();\n> > > > +     if (repr.empty())\n> > > > +             return {};\n> > > > +\n> > > >       ControlValue value{};\n> > > >\n> > > >       switch (id->type()) {\n> > > > @@ -324,13 +440,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> > > >               value.set<std::string>(repr);\n> > > >               break;\n> > > >       }\n> > > > -     case ControlTypeRectangle:\n> > > > -             /* \\todo Parse rectangles. */\n> > > > -             break;\n> > > > -     case ControlTypeSize:\n> > > > -             /* \\todo Parse Sizes. */\n> > > > +     default:\n> > > > +             std::cerr << \"Unsupported control type\" << std::endl;\n> > > >               break;\n> > > >       }\n> > > >\n> > > >       return value;\n> > > >  }\n> > > > +\n> > > > +libcamera::Rectangle CaptureScript::unpackRectangle(const std::vector<std::string> &strVec)\n> > > > +{\n> > > > +     int x = strtol(strVec[0].c_str(), NULL, 10);\n> > > > +     int y = strtol(strVec[1].c_str(), NULL, 10);\n> > > > +     unsigned int width = strtoul(strVec[2].c_str(), NULL, 10);\n> > > > +     unsigned int height = strtoul(strVec[3].c_str(), NULL, 10);\n> > > > +\n> > > > +     return Rectangle(x, y, width, height);\n> > > > +}\n> > > > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h\n> > > > index 8b4f8f62..fffe67e5 100644\n> > > > --- a/src/cam/capture_script.h\n> > > > +++ b/src/cam/capture_script.h\n> > > > @@ -54,9 +54,12 @@ private:\n> > > >       int parseControl(EventPtr event, libcamera::ControlList &controls);\n> > > >\n> > > >       std::string parseScalar();\n> > > > +     libcamera::ControlValue parseRectangles();\n> > > > +     std::vector<std::vector<std::string>> parseArrays();\n> > > > +     std::vector<std::string> parseSingleArray();\n> > > >\n> > > >       void unpackFailure(const libcamera::ControlId *id,\n> > > >                          const std::string &repr);\n> > > > -     libcamera::ControlValue unpackControl(const libcamera::ControlId *id,\n> > > > -                                           const std::string &repr);\n> > > > +     libcamera::ControlValue unpackControl(const libcamera::ControlId *id);\n> > > > +     libcamera::Rectangle unpackRectangle(const std::vector<std::string> &strVec);\n> > > >  };\n> > > > --\n> > > > 2.34.1\n> > > >","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 BA566C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  2 Sep 2022 12:47:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D85C61FE1;\n\tFri,  2 Sep 2022 14:47:55 +0200 (CEST)","from mail-il1-x130.google.com (mail-il1-x130.google.com\n\t[IPv6:2607:f8b0:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3196E61FDD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  2 Sep 2022 14:47:53 +0200 (CEST)","by mail-il1-x130.google.com with SMTP id v15so1066537iln.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 02 Sep 2022 05:47:53 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1662122875;\n\tbh=32Z7Nrp56+K2KEDWNMRJ7nL342jGcs05CqhRnGeKWdU=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=HjuNL8k2CBpqmzL/TzRVMC0HbWNZcEV7XtTX3QdFrMFWQYcDUwhDnqDW8+rVHZbrW\n\tkiq/NxHPMt+zYAe1p+rf+E8Jk7j6gDfrNRUauO3xOiQefpmi0SzR59Qr4b/LCMYeS/\n\txBLgCVSST47e1svjBRjoLrsR8GlC3l3PW4CGeatk+YKZCOiTDDF5o2363VN7UVr9IU\n\ttrCHDw/7s/j64EqAPd5W4Igs8XTWgafyDXUlCZSOmtMyOs6pvFAJo4ko7aJv8i2yfU\n\t6X1o/+mIl3JaBI/0MA/FcAzKthGOO3ekh4u9CSnVCdokkux639cJM0geZcNVx1GrX7\n\tNjqqQVVSlMYCg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date;\n\tbh=qFmpDs0CXycPH9VX+igoe50rWj1j7KPAZjX1MppfRgs=;\n\tb=mz/xi3s+69gFw6FgXGLnM1F6XlQlwCBWGM304EuoaIofn7ypWokgGdCfunLSpDAnBh\n\t8TyEx075uBJgC5lUtPervENNXf2Hok1ZAyJdAR0s6NEtPWvkh/HYpenavrep7J3qY4bT\n\tBmRB19cj6GU9jccn5O3gHXqvyp0oy5DOaWtmVq6CEnnDo2vC768xufI2zIPwaTc255Ur\n\tVTBYqojMOiDZCrlo2v5IOl/5y3tgkIxbyFEjTFvicIJ0LCT1Fjm18wFIfAZuD2hg8ynW\n\trIjiy1Tq8xFMo+bIK0nFjR1achbeBdz4nfdQ8bCRk1BnH87Qtn3BiIKUpH+gpIj5j826\n\tMqUQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"mz/xi3s+\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date;\n\tbh=qFmpDs0CXycPH9VX+igoe50rWj1j7KPAZjX1MppfRgs=;\n\tb=FLfqQwX8dP0hgf1L8eRFjwBNGQo/5HunWY5EbfEEGpU6GbSs/BETmIvg5k6B9HnHoj\n\tnSDsTiDYxgcFnGzPE9tdfVyEluWMTCEmovYtthqKzoasotex+RlLRIt9Ch4kYWGf45vR\n\tnqZ01j3IJXUmDs08Laat9Kh1yBccDRTN94OB2qJcDWmKlohFUWmQPm7jtWVmEx9bns84\n\tI0GutssdPUwDOazNuJFRmWhn6IFuSPAUtTZRV8TF/5FXr8ba/Erf/7hyxlLjCD2ndtl8\n\tZkWwAcIOazigjT9XbnxNPg9jWR1fMbD098Qu1gIKfHKeCuOMGcf1A7U4ZB1k2TcrMELc\n\t9QXw==","X-Gm-Message-State":"ACgBeo0XrHPcKFPhwRT1ttmueaaa2GGz7V9ruXNDZksaOBOvX1aO/VY8\n\t5WORXhxOStekyvuzjIPqKef/TFmUyeJpKteWYy9oG67bqR8=","X-Google-Smtp-Source":"AA6agR6XLdUZvpWij2rri5aim9gz1P2uSVtbvZFpPNAr/9eGR0AC1cLGRzo/pdV4TncyC9axS+0K7uE9Qr748Zzx6PA=","X-Received":"by 2002:a92:c20f:0:b0:2ea:e8b5:ba16 with SMTP id\n\tj15-20020a92c20f000000b002eae8b5ba16mr12399920ilo.280.1662122871831;\n\tFri, 02 Sep 2022 05:47:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20220627122806.29586-1-dse@thaumatec.com>\n\t<20220627122806.29586-2-dse@thaumatec.com>\n\t<20220808102638.GR311202@pyrite.rasen.tech>\n\t<CAHgnY3k2K4atZRFXymWa_ZxtaBQVS2KbFHvXhZ+RXc-H0KPUVQ@mail.gmail.com>\n\t<20220902103219.loqlyobhvg3hke2n@uno.localdomain>","In-Reply-To":"<20220902103219.loqlyobhvg3hke2n@uno.localdomain>","Date":"Fri, 2 Sep 2022 14:47:40 +0200","Message-ID":"<CAHgnY3k8gMouRBbwR1NEdkbcff1L=eTNn8gi88ARqzfj514h3Q@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] cam: Add Rectangle type\n\tparsing in capture script","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>","From":"Daniel Semkowicz via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Daniel Semkowicz <dse@thaumatec.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]