[{"id":24514,"web_url":"https://patchwork.libcamera.org/comment/24514/","msgid":"<20220810082829.ehcoemvklqjgkuys@uno.localdomain>","date":"2022-08-10T08:28:29","subject":"Re: [libcamera-devel] [PATCH 2/4] utils: gen-controls: Improve YAML\n\tnotation for variable-size array controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Wed, Aug 10, 2022 at 03:29:04AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Array controls specify the array size through the YAML 'size' element,\n> which stores a list of values, one per dimension. Variable-size arrays\n> currently use an empty 'size' list, which prevents describing the number\n> of dimensions of the array.\n>\n> Improve this by using the same notation for fixed-size and variable-size\n> array controls. Dimensions that are not fixed are described as a string\n> instead of an integer, such as [n], [n,3] or [w,h]. The strings have\n> currently no special meaning, this may change in the future.\n\nI can't tell what's best between [] and [n]\n\nFor what I can see [n,3] doesn't give much in terms of parse-time\ncorrectness but it's just a notion for the reader of the\ndocumentation, so yeah, nice but I would like to understand first why\nChristian proposed initially to go from [n] to [].\n\nThanks\n j\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/control_ids.yaml | 2 +-\n> src/libcamera/property_ids.yaml | 4 ++--\n> utils/gen-controls.py | 31 +++++++++++++++++++++++++------\n> 3 files changed, 28 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index c93f362463dd..a456e6c0358f 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -525,7 +525,7 @@ controls:\n> the window where the focal distance for the objects shown in that part\n> of the image are closest to the camera.\n>\n> - size: []\n> + size: [n]\n>\n> - AfTrigger:\n> type: int32_t\n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index 960e254495b7..cb55e0ed2283 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -497,7 +497,7 @@ controls:\n>\n> - PixelArrayOpticalBlackRectangles:\n> type: Rectangle\n> - size: []\n> + size: [n]\n> description: |\n> The pixel array region(s) which contain optical black pixels\n> considered valid for calibration purposes.\n> @@ -592,7 +592,7 @@ controls:\n>\n> - PixelArrayActiveAreas:\n> type: Rectangle\n> - size: []\n> + size: [n]\n> description: |\n> The PixelArrayActiveAreas property defines the (possibly multiple and\n> overlapping) portions of the camera sensor readable pixel matrix\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index f6a3ae39cc84..bcfbeb7f9f17 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -39,11 +39,33 @@ class Control(object):\n> self.__name = name\n> self.__data = data\n> self.__enum = None\n> + self.__size = None\n>\n> enum_values = data.get('enum')\n> if enum_values is not None:\n> self.__enum_values = [ControlEnum(enum) for enum in enum_values]\n>\n> + size = self.__data.get('size')\n> + if size is not None:\n> + if len(size) == 0:\n> + raise RuntimeError(f'Control `{self.__name}` size must have at least one dimension')\n> +\n> + # Compute the total number of elemens in the array. If any of the\n> + # array dimension is a string, the array is variable-sized.\n> + num_elems = 1\n> + for dim in size:\n> + if type(dim) is str:\n> + num_elems = 0\n> + break\n> +\n> + dim = int(dim)\n> + if dim <= 0:\n> + raise RuntimeError(f'Control `{self.__name}` size must have positive values only')\n> +\n> + num_elems *= dim\n> +\n> + self.__size = num_elems\n> +\n> @property\n> def description(self):\n> \"\"\"The control description\"\"\"\n> @@ -86,15 +108,12 @@ class Control(object):\n> if typ == 'string':\n> return 'std::string'\n>\n> - if size is None:\n> + if self.__size is None:\n> return typ\n>\n> - if len(size) > 0:\n> - # fixed-sized Span\n> - span_size = reduce(operator.mul, size)\n> - return f\"Span\"\n> + if self.__size:\n> + return f\"Span\"\n> else:\n> - # variable-sized Span\n> return f\"Span\"\n>\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 D2DFFC3272\n\tfor ;\n\tWed, 10 Aug 2022 08:28:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D2D96332B;\n\tWed, 10 Aug 2022 10:28:32 +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 A817C61FAB\n\tfor ;\n\tWed, 10 Aug 2022 10:28:31 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id DEBC51C0003;\n\tWed, 10 Aug 2022 08:28:30 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660120112;\n\tbh=90buiYAoNMHWz6tqCd0JvOjqfKlHnkfQv5QQdhc7SLc=;\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=w62tVHxUc2opKi2vawQ0O1f6bprOjwRkS7yXSAM8PIKJHJyGfUGT28e1kB+KEGbj5\n\tuG/09bR+cXLMCdsYF9zmnq8jaWm+4W434pkvRmmgBBHIPxZkHhAQ5hRF4KhwIKlm11\n\tpoHYZLkxSIrZiMJoI8e6L7chCTR2g6n15bIcrK1BANVrjcBfYcDezCVnEr3BBhjrne\n\trwZdkPQRDKOrVRLU5Th1SZz07j32GDi5pRqxBC+FkJfIRSirssGbMup6jttgXB5ubf\n\tP3GlwBP8iAweGVhz37muvqdglXdCp4zRwCsE3TNg5eJrouiCw0JmqytVzaAZOb9pOE\n\ti5y8DBErKYJ3g==","Date":"Wed, 10 Aug 2022 10:28:29 +0200","To":"Laurent Pinchart ","Message-ID":"<20220810082829.ehcoemvklqjgkuys@uno.localdomain>","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220810002906.5406-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/4] utils: gen-controls: Improve YAML\n\tnotation for variable-size array controls","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24529,"web_url":"https://patchwork.libcamera.org/comment/24529/","msgid":"","date":"2022-08-10T22:30:41","subject":"Re: [libcamera-devel] [PATCH 2/4] utils: gen-controls: Improve YAML\n\tnotation for variable-size array controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 10, 2022 at 10:28:29AM +0200, Jacopo Mondi wrote:\n> On Wed, Aug 10, 2022 at 03:29:04AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Array controls specify the array size through the YAML 'size' element,\n> > which stores a list of values, one per dimension. Variable-size arrays\n> > currently use an empty 'size' list, which prevents describing the number\n> > of dimensions of the array.\n> >\n> > Improve this by using the same notation for fixed-size and variable-size\n> > array controls. Dimensions that are not fixed are described as a string\n> > instead of an integer, such as [n], [n,3] or [w,h]. The strings have\n> > currently no special meaning, this may change in the future.\n> \n> I can't tell what's best between [] and [n]\n> \n> For what I can see [n,3] doesn't give much in terms of parse-time\n> correctness but it's just a notion for the reader of the\n> documentation, so yeah, nice but I would like to understand first why\n> Christian proposed initially to go from [n] to [].\n\nAt the moment it's for the reader only, I can imagine using it in the\nControl class too in the future, but I'm not sure if there would be any\nuse case for that beside showing that C++ templates can do crazy stuff.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/control_ids.yaml | 2 +-\n> > src/libcamera/property_ids.yaml | 4 ++--\n> > utils/gen-controls.py | 31 +++++++++++++++++++++++++------\n> > 3 files changed, 28 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index c93f362463dd..a456e6c0358f 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -525,7 +525,7 @@ controls:\n> > the window where the focal distance for the objects shown in that part\n> > of the image are closest to the camera.\n> >\n> > - size: []\n> > + size: [n]\n> >\n> > - AfTrigger:\n> > type: int32_t\n> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> > index 960e254495b7..cb55e0ed2283 100644\n> > --- a/src/libcamera/property_ids.yaml\n> > +++ b/src/libcamera/property_ids.yaml\n> > @@ -497,7 +497,7 @@ controls:\n> >\n> > - PixelArrayOpticalBlackRectangles:\n> > type: Rectangle\n> > - size: []\n> > + size: [n]\n> > description: |\n> > The pixel array region(s) which contain optical black pixels\n> > considered valid for calibration purposes.\n> > @@ -592,7 +592,7 @@ controls:\n> >\n> > - PixelArrayActiveAreas:\n> > type: Rectangle\n> > - size: []\n> > + size: [n]\n> > description: |\n> > The PixelArrayActiveAreas property defines the (possibly multiple and\n> > overlapping) portions of the camera sensor readable pixel matrix\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index f6a3ae39cc84..bcfbeb7f9f17 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -39,11 +39,33 @@ class Control(object):\n> > self.__name = name\n> > self.__data = data\n> > self.__enum = None\n> > + self.__size = None\n> >\n> > enum_values = data.get('enum')\n> > if enum_values is not None:\n> > self.__enum_values = [ControlEnum(enum) for enum in enum_values]\n> >\n> > + size = self.__data.get('size')\n> > + if size is not None:\n> > + if len(size) == 0:\n> > + raise RuntimeError(f'Control `{self.__name}` size must have at least one dimension')\n> > +\n> > + # Compute the total number of elemens in the array. If any of the\n> > + # array dimension is a string, the array is variable-sized.\n> > + num_elems = 1\n> > + for dim in size:\n> > + if type(dim) is str:\n> > + num_elems = 0\n> > + break\n> > +\n> > + dim = int(dim)\n> > + if dim <= 0:\n> > + raise RuntimeError(f'Control `{self.__name}` size must have positive values only')\n> > +\n> > + num_elems *= dim\n> > +\n> > + self.__size = num_elems\n> > +\n> > @property\n> > def description(self):\n> > \"\"\"The control description\"\"\"\n> > @@ -86,15 +108,12 @@ class Control(object):\n> > if typ == 'string':\n> > return 'std::string'\n> >\n> > - if size is None:\n> > + if self.__size is None:\n> > return typ\n> >\n> > - if len(size) > 0:\n> > - # fixed-sized Span\n> > - span_size = reduce(operator.mul, size)\n> > - return f\"Span\"\n> > + if self.__size:\n> > + return f\"Span\"\n> > else:\n> > - # variable-sized Span\n> > return f\"Span\"\n> >\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 A87E8BE173\n\tfor ;\n\tWed, 10 Aug 2022 22:30:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18A5F6332B;\n\tThu, 11 Aug 2022 00:30:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 059EE61FAA\n\tfor ;\n\tThu, 11 Aug 2022 00:30:54 +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 64D923F1;\n\tThu, 11 Aug 2022 00:30:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660170655;\n\tbh=NcQAvQpzehl8DPnWK4AG6SDJe6LQ8M5herlKk3EGUh4=;\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=nMRMape9cwZMblVAy7Ryz63YorAG8lgyeJ6jUKcSMh7jShCuqIOriz2IfUZrscH47\n\tNJdd6RQSnlQebIYQPuiYuop6fY/84dOFVuIWHY+W9g1UrjQXyrtIvh8D0k3A7/a410\n\tMllOgfKdKawUjfDlJHbD23O6OemnERXBynLQRGe6JyUcwbwEFJzPuTwl0tySjWvHoV\n\tIzChBicFdiVN4negaQG08ltQUeRZqSaU4phZdreJcs6hrs+ZLp6Kq0lgz82nEZeI57\n\tPvkuV+1GiKKghXNEfIKir/1zZnEiFH2U5q7cnMYL+8NGJF0lTABd4tYB5ft4AWfeQz\n\tV93smoitPlHNA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660170653;\n\tbh=NcQAvQpzehl8DPnWK4AG6SDJe6LQ8M5herlKk3EGUh4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lJo+VGEn67RgLSk5iAfRRuU310CGuyFSAoIKZCQ5M+mm1abfBgnIkMm3km2oAuwip\n\teHLHaaD4eRoe/4zpg1TBSCdXVPpZtAFuf5QNDybyKKMz5x+tHo16daPSbfE96Knxzb\n\tNMTRKKbgMMmfA4cSS0QWj91GJ/H1FgR2/SsMtF58="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lJo+VGEn\"; dkim-atps=neutral","Date":"Thu, 11 Aug 2022 01:30:41 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-3-laurent.pinchart@ideasonboard.com>\n\t<20220810082829.ehcoemvklqjgkuys@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220810082829.ehcoemvklqjgkuys@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/4] utils: gen-controls: Improve YAML\n\tnotation for variable-size array controls","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\" "}}]