[{"id":23582,"web_url":"https://patchwork.libcamera.org/comment/23582/","msgid":"","date":"2022-06-24T17:47:11","subject":"Re: [libcamera-devel] [PATCH v2 1/2] cam: Add Rectangle type\n\tparsing in capture script","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Daniel,\n\nThank you for the patch.\n\nOn Fri, Jun 24, 2022 at 03:05:22PM +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 \n> ---\n> src/cam/capture_script.cpp | 109 +++++++++++++++++++++++++++++++++----\n> src/cam/capture_script.h | 6 +-\n> 2 files changed, 102 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp\n> index 9f22d5f7..5812b122 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,68 @@ std::string CaptureScript::parseScalar()\n> \treturn eventScalarValue(event);\n> }\n> \n> +ControlValue CaptureScript::parseRectangles()\n> +{\n> +\tEventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> +\tif (!event)\n> +\t\treturn {};\n> +\n> +\tstd::vector rectangles;\n\nHmmm... we have an issue here. The code will work fine for AfWindows, as\nit's an array of rectangles, but it will fail for controls are expressed\nas a single rectangle, unless the YAML file stores them as an array\ncontaining one rectangle.\n\nDo you have an idea on how we could fix that ?\n\n> +\n> +\twhile (1) {\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn {};\n> +\n> +\t\tswitch (event->type) {\n> +\t\tcase YAML_SEQUENCE_START_EVENT: {\n> +\t\t\tstd::vector values = parseArray();\n> +\t\t\tif (values.size() != 4) {\n> +\t\t\t\tstd::cerr << \"Error parsing Rectangle: expected \"\n> +\t\t\t\t\t \"array with 4 parameters\" << std::endl;\n> +\t\t\t\treturn {};\n> +\t\t\t}\n> +\n> +\t\t\tRectangle rect = unpackRectangle(values);\n> +\t\t\trectangles.push_back(rect);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase YAML_SEQUENCE_END_EVENT: {\n> +\t\t\tControlValue controlValue;\n> +\t\t\tcontrolValue.set(Span(rectangles));\n> +\t\t\treturn controlValue;\n> +\t\t}\n> +\t\tdefault:\n> +\t\t\treturn {};\n> +\t\t}\n> +\t}\n> +}\n> +\n> +std::vector CaptureScript::parseArray()\n> +{\n> +\tstd::vector 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 typeNames = {\n> @@ -277,9 +342,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\nNitpicking, s/types/types./\n\n> +\tswitch (id->type()) {\n> +\tcase ControlTypeRectangle:\n> +\t\treturn parseRectangles();\n> +\tcase ControlTypeSize:\n> +\t\t/* \\todo Parse Sizes. */\n> +\t\treturn {};\n\nFeel free to send another patch to address this too ;-)\n\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n> +\t/* Parse basic types represented by a single scalar */\n\nSame here, s/scalar/scalar./\n\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 +404,20 @@ ControlValue CaptureScript::unpackControl(const ControlId *id,\n> \t\tvalue.set(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 &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..130872b4 100644\n> --- a/src/cam/capture_script.h\n> +++ b/src/cam/capture_script.h\n> @@ -54,9 +54,11 @@ private:\n> \tint parseControl(EventPtr event, libcamera::ControlList &controls);\n> \n> \tstd::string parseScalar();\n> +\tlibcamera::ControlValue parseRectangles();\n> +\tstd::vector parseArray();\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 &strVec);\n> };","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 025B9BE173\n\tfor ;\n\tFri, 24 Jun 2022 17:47:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F2FCF65636;\n\tFri, 24 Jun 2022 19:47:29 +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 233B26559A\n\tfor ;\n\tFri, 24 Jun 2022 19:47:29 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5B4BA575;\n\tFri, 24 Jun 2022 19:47:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656092850;\n\tbh=G8jV2G+ZUCqe8rmDFj37jGWKldHjbJscbhAg4F1hNhA=;\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=G++CEDlWdBUCR4MIhujSMqdox1zOQbs3/tQnE/LSo1rsZjj0esNKQVC91i6AS3i3m\n\thNWjFxn13DzBS7ZK0fs7bvP6L5L13o8TAkCBkrjTZ2r0VF6CzGBl57R+cmwpQ11Aca\n\txndcyUB5V7+fz+mbb+2gu2oo42n5r+cHVpUSuUatqeXnD8ido7G87ZCAcs/4sWBZe/\n\tLlj9FUW+2s7t7BCw/lpNkHPcT8G8EapNivxXU3JOGIBOA6lFU2B7EqJZDehXtXHnV6\n\tE76H/V41rhkUsHzK1M2QV8PW90wsWRiPSVUv5i7nbHC+/YViiyPL0tvQsBtrBjj+DR\n\t9HPD4wtjOUWrA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656092848;\n\tbh=G8jV2G+ZUCqe8rmDFj37jGWKldHjbJscbhAg4F1hNhA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HM21J2Z0pGqBxphRuUEOeUlwpjq2vqjqnWntEjDH343ogKl/w7+jXbKN9XHqV8ygk\n\tR7NVlr/p8roJGaEIVbH75JWUJbFnt4gAa5lMtFMDclWKtRbf5XAQz5Ow3nY565uKd1\n\txVbZwhndP9ZZvIw/3RLgNNLyxUFYQipIlUwES45I="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HM21J2Z0\"; dkim-atps=neutral","Date":"Fri, 24 Jun 2022 20:47:11 +0300","To":"Daniel Semkowicz ","Message-ID":"","References":"<20220624130523.41535-1-dse@thaumatec.com>\n\t<20220624130523.41535-2-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220624130523.41535-2-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v2 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]