[{"id":27732,"web_url":"https://patchwork.libcamera.org/comment/27732/","msgid":"<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>","date":"2023-09-04T17:18:11","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> Add a '--orientation|-o' option to the Python version of the cam test\n> application to set an orientation to the image stream.\n> \n> Supported values are:\n> - Rot180: Rotate 180 degrees\n> - Flip: vertical flip\n> - Mirror: horizontal flip\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/py/cam/cam.py | 20 ++++++++++++++++++++\n> 1 file changed, 20 insertions(+)\n> \n> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> index a2a115c164b4..84d8ca1716fd 100755\n> --- a/src/py/cam/cam.py\n> +++ b/src/py/cam/cam.py\n> @@ -23,6 +23,7 @@ class CameraContext:\n> opt_metadata: bool\n> opt_save_frames: bool\n> opt_capture: int\n> + opt_orientation: str\n> \n> stream_names: dict[libcam.Stream, str]\n> streams: list[libcam.Stream]\n> @@ -146,6 +147,20 @@ class CameraContext:\n> if 'pixelformat' in stream_opts:\n> stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> \n> + if self.opt_orientation is not None:\n> + orientation_map = {\n> + 'Rot180': libcam.Orientation.Rotate180,\n> + 'Mirror': libcam.Orientation.Rotate0Flip,\n> + 'Flip': libcam.Orientation.Rotate180Flip,\n> + }\n\nAny reason to not support all orientations? In python you can get the \nname of the enum as a string, and you could just compare that directly \nto the string from the user. Also, you could lower-case both before \ncomparison, so that the user could say \"-o rotate270\".\n\n> +\n> + orient = orientation_map.get(self.opt_orientation, None)\n> + if orient is None:\n> + print('Bad orientation: ', self.opt_orientation)\n> + sys.exit(-1)\n> +\n> + camconfig.orientation = orient\n> +\n> stat = camconfig.validate()\n> \n> if stat == libcam.CameraConfiguration.Status.Invalid:\n> @@ -385,6 +400,7 @@ def main():\n> parser.add_argument('--metadata', nargs=0, type=bool, action=CustomAction, help='Print the metadata for completed requests')\n> parser.add_argument('--strict-formats', type=bool, nargs=0, action=CustomAction, help='Do not allow requested stream format(s) to be adjusted')\n> parser.add_argument('-s', '--stream', nargs='+', action=CustomAction)\n> + parser.add_argument('-o', '--orientation', help='Desired image orientation (Rot180, Mirror, Flip)')\n\nHmm, you need to use the action=CustomAction to get the option to apply \nto a camera context.\n\nAnd then I think you can do this below:\n\nctx.opt_orientation = args.orientation.get(cam_idx, None)\n\n> args = parser.parse_args()\n> \n> cm = libcam.CameraManager.singleton()\n> @@ -408,6 +424,10 @@ def main():\n> ctx.opt_metadata = args.metadata.get(cam_idx, False)\n> ctx.opt_strict_formats = args.strict_formats.get(cam_idx, False)\n> ctx.opt_stream = args.stream.get(cam_idx, ['role=viewfinder'])\n> + if args.orientation is not None:\n> + ctx.opt_orientation = args.orientation\n> + else:\n> + ctx.opt_orientation = None\n> contexts.append(ctx)\n> \n> for ctx in contexts:","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 5CFF8C0F1B\n\tfor ;\n\tMon, 4 Sep 2023 17:18:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81B03628E9;\n\tMon, 4 Sep 2023 19:18:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81EDA628D7\n\tfor ;\n\tMon, 4 Sep 2023 19:18:15 +0200 (CEST)","from [192.168.88.20] (91-154-35-171.elisa-laajakaista.fi\n\t[91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4AD1B31B9;\n\tMon, 4 Sep 2023 19:16:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693847896;\n\tbh=SZRTBSveKaq4fwNbYdM0ExvmdMtmsxlpevEEd4uhUEo=;\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:\n\tFrom;\n\tb=cMSSfjFyFiYoSFlSOoKii570V8xlf3luvDBeW1XItrjGC6LLLhbj1KGkgiVvabE/m\n\t/FJ1bkqx8shwIcivky1hIAcTD9vMxW6lN3eQNr2OaYTujMUjgajG2hUpS9AWOq/kmz\n\tBHuYGH0wYXZD8LBhFXGrQQTQrcfNTIJ2v2U34g/dQbXlmpGafxF89/53KtDMQYo/O4\n\t3Qe4e93Yw/nO+SXrd57QBlhhKgl5cPyVSZCrv4VuJltMICvTxUoTd96++zjKcAURsD\n\tw7xDadE0VO9vJXzrf105JPuZx1EnY79bV5QRm3ZDyI12Bxm3QjQu00A0fReGUMeSC7\n\ttgDV6CXm+Rm9Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693847809;\n\tbh=SZRTBSveKaq4fwNbYdM0ExvmdMtmsxlpevEEd4uhUEo=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=SCmIkyawjmRN5zncHJGUJJsU23Xwty3X0JTs0fMWKiKW60VnG3VSteXNAHKyCzpmi\n\tSflB0qyRuToxy5mJVz7Gv7OcXCSGtTmagSEA2T3oXhS0Sy/LUEZzu8tD83f22L513V\n\t1hhOibuPUaLFg8g+NA4F6ibG4TxFW613whVa8RbA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SCmIkyaw\"; dkim-atps=neutral","Message-ID":"<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>","Date":"Mon, 4 Sep 2023 20:18:11 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.13.0","Content-Language":"en-US","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27733,"web_url":"https://patchwork.libcamera.org/comment/27733/","msgid":"","date":"2023-09-04T19:07:22","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Tomi\n\nOn Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n> On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> > Add a '--orientation|-o' option to the Python version of the cam test\n> > application to set an orientation to the image stream.\n> >\n> > Supported values are:\n> > - Rot180: Rotate 180 degrees\n> > - Flip: vertical flip\n> > - Mirror: horizontal flip\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/py/cam/cam.py | 20 ++++++++++++++++++++\n> > 1 file changed, 20 insertions(+)\n> >\n> > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> > index a2a115c164b4..84d8ca1716fd 100755\n> > --- a/src/py/cam/cam.py\n> > +++ b/src/py/cam/cam.py\n> > @@ -23,6 +23,7 @@ class CameraContext:\n> > opt_metadata: bool\n> > opt_save_frames: bool\n> > opt_capture: int\n> > + opt_orientation: str\n> > stream_names: dict[libcam.Stream, str]\n> > streams: list[libcam.Stream]\n> > @@ -146,6 +147,20 @@ class CameraContext:\n> > if 'pixelformat' in stream_opts:\n> > stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> > + if self.opt_orientation is not None:\n> > + orientation_map = {\n> > + 'Rot180': libcam.Orientation.Rotate180,\n> > + 'Mirror': libcam.Orientation.Rotate0Flip,\n> > + 'Flip': libcam.Orientation.Rotate180Flip,\n> > + }\n>\n> Any reason to not support all orientations? In python you can get the name\n> of the enum as a string, and you could just compare that directly to the\n> string from the user. Also, you could lower-case both before comparison, so\n> that the user could say \"-o rotate270\".\n>\n\nThe three supported options are the ones supporte by the C++ version\nof cam, and I like the two to behave the same. The reason why the C++\nversion of cam only supports the three above options is because\nthey're usually the most common features as user expects. The list\ncould indeed be expanded, but for now I would like the two versions to\nbehave the same. Is this ok with you ?\n\n> > +\n> > + orient = orientation_map.get(self.opt_orientation, None)\n> > + if orient is None:\n> > + print('Bad orientation: ', self.opt_orientation)\n> > + sys.exit(-1)\n> > +\n> > + camconfig.orientation = orient\n> > +\n> > stat = camconfig.validate()\n> > if stat == libcam.CameraConfiguration.Status.Invalid:\n> > @@ -385,6 +400,7 @@ def main():\n> > parser.add_argument('--metadata', nargs=0, type=bool, action=CustomAction, help='Print the metadata for completed requests')\n> > parser.add_argument('--strict-formats', type=bool, nargs=0, action=CustomAction, help='Do not allow requested stream format(s) to be adjusted')\n> > parser.add_argument('-s', '--stream', nargs='+', action=CustomAction)\n> > + parser.add_argument('-o', '--orientation', help='Desired image orientation (Rot180, Mirror, Flip)')\n>\n> Hmm, you need to use the action=CustomAction to get the option to apply to a\n> camera context.\n\nUh right, orientation is indeed per camera\n>\n> And then I think you can do this below:\n>\n> ctx.opt_orientation = args.orientation.get(cam_idx, None)\n>\n\nThanks\n j\n\n> > args = parser.parse_args()\n> > cm = libcam.CameraManager.singleton()\n> > @@ -408,6 +424,10 @@ def main():\n> > ctx.opt_metadata = args.metadata.get(cam_idx, False)\n> > ctx.opt_strict_formats = args.strict_formats.get(cam_idx, False)\n> > ctx.opt_stream = args.stream.get(cam_idx, ['role=viewfinder'])\n> > + if args.orientation is not None:\n> > + ctx.opt_orientation = args.orientation\n> > + else:\n> > + ctx.opt_orientation = None\n> > contexts.append(ctx)\n> > for ctx in contexts:\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 98EB2BE080\n\tfor ;\n\tMon, 4 Sep 2023 19:07:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9123628DA;\n\tMon, 4 Sep 2023 21:07:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5592A628D7\n\tfor ;\n\tMon, 4 Sep 2023 21:07:28 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30952AF2;\n\tMon, 4 Sep 2023 21:06:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693854449;\n\tbh=+cy8njwBs4NIs/kUTgj1C0uEIddmI2rGyO0wEocm/VQ=;\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=ZBybqRM9j710yxR4+2obX6okfiI5rJZjVFT80B2wyXf/CM8ZqVi0VzC2sJ7LMXeP9\n\tpUexJBb/BuuWJQlMVzgFor4T0TLRrVjm2IwwsDYhTEmkAxfBkJCx7q6FpfbiT6+AwS\n\tTNlvVikCXFGUr6Bi8iCqDc+YXUKaDsBabERtcFvX2+xtDF0Y3QdHbAyD3G7qgobZ9w\n\tfIh+WzEDBN+B10dhd7WpH5Fovrup7NK5a4hsZybegGSUq7CJg3vcJe1OP/NYn6KfFv\n\tYU5gNYiPiLO+QDL0h7DZUhi8pBJi+lJe2H+yWiV1O4XW5/YAHGcnCuuUkFZ1iTQw2+\n\t9fo4mDoQGpmMg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693854362;\n\tbh=+cy8njwBs4NIs/kUTgj1C0uEIddmI2rGyO0wEocm/VQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ustWa68Kj6Oi/nWegflLcBshGeuAQvBNfJbCS5cXZe2/+zThoczsN/NV0AxV6bZHp\n\tApkKNVuZSXsx9lMwLRbsLkQTMgw4bXE0hk2XYVD69xWJDNRed4MWB2qEr2xE45KFIv\n\tSewk3F4sQBXerELhWdZ4Uw3wVuuCvL6rsX48Ii3Y="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ustWa68K\"; dkim-atps=neutral","Date":"Mon, 4 Sep 2023 21:07:22 +0200","To":"Tomi Valkeinen ","Message-ID":"","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27736,"web_url":"https://patchwork.libcamera.org/comment/27736/","msgid":"","date":"2023-09-05T07:51:51","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 04/09/2023 22:07, Jacopo Mondi wrote:\n> Hi Tomi\n> \n> On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n>> On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n>>> Add a '--orientation|-o' option to the Python version of the cam test\n>>> application to set an orientation to the image stream.\n>>>\n>>> Supported values are:\n>>> - Rot180: Rotate 180 degrees\n>>> - Flip: vertical flip\n>>> - Mirror: horizontal flip\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/py/cam/cam.py | 20 ++++++++++++++++++++\n>>> 1 file changed, 20 insertions(+)\n>>>\n>>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n>>> index a2a115c164b4..84d8ca1716fd 100755\n>>> --- a/src/py/cam/cam.py\n>>> +++ b/src/py/cam/cam.py\n>>> @@ -23,6 +23,7 @@ class CameraContext:\n>>> opt_metadata: bool\n>>> opt_save_frames: bool\n>>> opt_capture: int\n>>> + opt_orientation: str\n>>> stream_names: dict[libcam.Stream, str]\n>>> streams: list[libcam.Stream]\n>>> @@ -146,6 +147,20 @@ class CameraContext:\n>>> if 'pixelformat' in stream_opts:\n>>> stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n>>> + if self.opt_orientation is not None:\n>>> + orientation_map = {\n>>> + 'Rot180': libcam.Orientation.Rotate180,\n>>> + 'Mirror': libcam.Orientation.Rotate0Flip,\n>>> + 'Flip': libcam.Orientation.Rotate180Flip,\n>>> + }\n>>\n>> Any reason to not support all orientations? In python you can get the name\n>> of the enum as a string, and you could just compare that directly to the\n>> string from the user. Also, you could lower-case both before comparison, so\n>> that the user could say \"-o rotate270\".\n>>\n> \n> The three supported options are the ones supporte by the C++ version\n> of cam, and I like the two to behave the same. The reason why the C++\n> version of cam only supports the three above options is because\n> they're usually the most common features as user expects. The list\n> could indeed be expanded, but for now I would like the two versions to\n> behave the same. Is this ok with you ?\n\nWell... cam.py resembles the C++ version, but it's not identical. I \ninitially thought I'd write a Python clone of the C++ cam, but then I \nrealized that perhaps it doesn't make sense, and instead it's better to \nwrite a cam.py that is more Pythonic, does things differently than the \nC++ version when it makes sense, and, when possible, does extra things \nthat are easy to do in Python.\n\nGenerally speaking, I'd rather support all features, as otherwise will \nthey ever be tested?\n\nIf you use the enum names directly, you can just do:\n\nself.opt_orientation.lower() in [x.lower() for x in \nlibcam.Orientation.__members__]\n\nHowever, I now realized you used different naming in the option, \ncompared to the enum (e.g. mirror vs rotate0flip). For that you, of \ncourse, need a mapping there. And if that's the target, then I think the \nabove code is fine.\n\nHowever, and this goes a bit outside the topic here, why do you expose \nthe enums via different names? Or, if those names are better, then why \nnot use those \"better\" names for the enum? And if you specifically want \nto use EXIF spec names for the enum, then wouldn't it be better to \nexpose the orientation in cam/cam.py also with the EXIF spec names?\n\n Tomi","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 97B46C0F1B\n\tfor ;\n\tTue, 5 Sep 2023 07:51:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 07A27628E9;\n\tTue, 5 Sep 2023 09:51: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 B212B627DF\n\tfor ;\n\tTue, 5 Sep 2023 09:51:54 +0200 (CEST)","from [192.168.88.20] (91-154-35-171.elisa-laajakaista.fi\n\t[91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C05D75A;\n\tTue, 5 Sep 2023 09:50:28 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693900316;\n\tbh=jGcp0G5DewSxQb4gPb5mo2Hm74SsBX95KrhM8EnYcjI=;\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=JkkPiF3RGxuKWEULlGNh6DtY/gds9s2tQERzF7n3EEEnKEfTUZHHAkGMD5Tu9Bbhg\n\tFoCMRrrgxbH/gFRdPypSQRp2yOXkOrRy3GSf3WCS4kxlzz9d/gDudxDHPWOelfvLrE\n\tSAnX0/6iOG6Gul82k8f/vLNzVpnJabRT7Io+cSS1RIXINbEMYi7rwJ+mHnGQ6IgbIm\n\twy/COMp4BhkSVm0pPq2ax5Zj0JVGQpifKu39zASHpytN4cZhVABJgsciKuvvECGup5\n\tZBR7RdjWxZCvnud70y8fyTMfk6vexM72fC1vFHDAkpjJcK+Excwu99bq1eJ6ph9XxH\n\tV6QydUyOxf6hg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693900228;\n\tbh=jGcp0G5DewSxQb4gPb5mo2Hm74SsBX95KrhM8EnYcjI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=SX1hdUvo2CEH2WtRM+exAUCWIxJsdJhV1tjep1GpYOv4lIDctSHK1ZSLzT9UBsNTu\n\t9jSzbfqDfh2rnzNtbg5hiLeNtdbUZnjNhwO4vIdE9oOFzN9rf6kK9MymwMm107GH7E\n\tW4D8UbFuCb6xShO8u0V51HXoLcghABk6/ekWB89Y="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SX1hdUvo\"; dkim-atps=neutral","Message-ID":"","Date":"Tue, 5 Sep 2023 10:51:51 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.13.0","Content-Language":"en-US","To":"Jacopo Mondi ","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27740,"web_url":"https://patchwork.libcamera.org/comment/27740/","msgid":"","date":"2023-09-06T07:10:10","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Tomi\n\nOn Tue, Sep 05, 2023 at 10:51:51AM +0300, Tomi Valkeinen via libcamera-devel wrote:\n> On 04/09/2023 22:07, Jacopo Mondi wrote:\n> > Hi Tomi\n> >\n> > On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n> > > On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> > > > Add a '--orientation|-o' option to the Python version of the cam test\n> > > > application to set an orientation to the image stream.\n> > > >\n> > > > Supported values are:\n> > > > - Rot180: Rotate 180 degrees\n> > > > - Flip: vertical flip\n> > > > - Mirror: horizontal flip\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/py/cam/cam.py | 20 ++++++++++++++++++++\n> > > > 1 file changed, 20 insertions(+)\n> > > >\n> > > > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> > > > index a2a115c164b4..84d8ca1716fd 100755\n> > > > --- a/src/py/cam/cam.py\n> > > > +++ b/src/py/cam/cam.py\n> > > > @@ -23,6 +23,7 @@ class CameraContext:\n> > > > opt_metadata: bool\n> > > > opt_save_frames: bool\n> > > > opt_capture: int\n> > > > + opt_orientation: str\n> > > > stream_names: dict[libcam.Stream, str]\n> > > > streams: list[libcam.Stream]\n> > > > @@ -146,6 +147,20 @@ class CameraContext:\n> > > > if 'pixelformat' in stream_opts:\n> > > > stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> > > > + if self.opt_orientation is not None:\n> > > > + orientation_map = {\n> > > > + 'Rot180': libcam.Orientation.Rotate180,\n> > > > + 'Mirror': libcam.Orientation.Rotate0Flip,\n> > > > + 'Flip': libcam.Orientation.Rotate180Flip,\n> > > > + }\n> > >\n> > > Any reason to not support all orientations? In python you can get the name\n> > > of the enum as a string, and you could just compare that directly to the\n> > > string from the user. Also, you could lower-case both before comparison, so\n> > > that the user could say \"-o rotate270\".\n> > >\n> >\n> > The three supported options are the ones supporte by the C++ version\n> > of cam, and I like the two to behave the same. The reason why the C++\n> > version of cam only supports the three above options is because\n> > they're usually the most common features as user expects. The list\n> > could indeed be expanded, but for now I would like the two versions to\n> > behave the same. Is this ok with you ?\n>\n> Well... cam.py resembles the C++ version, but it's not identical. I\n> initially thought I'd write a Python clone of the C++ cam, but then I\n> realized that perhaps it doesn't make sense, and instead it's better to\n> write a cam.py that is more Pythonic, does things differently than the C++\n> version when it makes sense, and, when possible, does extra things that are\n> easy to do in Python.\n>\n> Generally speaking, I'd rather support all features, as otherwise will they\n> ever be tested?\n\nI understand this point, but the only other Orientation that does not\ninclude a transpose is: rotate0 (Identity) (which is equivalent to not\nspecifying a --orientation). All the other ones require a Transpose,\nwhich none of our pipelines can do.\n\n>\n> If you use the enum names directly, you can just do:\n>\n> self.opt_orientation.lower() in [x.lower() for x in\n> libcam.Orientation.__members__]\n>\n> However, I now realized you used different naming in the option, compared to\n> the enum (e.g. mirror vs rotate0flip). For that you, of course, need a\n> mapping there. And if that's the target, then I think the above code is\n> fine.\n>\n> However, and this goes a bit outside the topic here, why do you expose the\n> enums via different names? Or, if those names are better, then why not use\n> those \"better\" names for the enum? And if you specifically want to use EXIF\n> spec names for the enum, then wouldn't it be better to expose the\n> orientation in cam/cam.py also with the EXIF spec names?\n\nEXIF specs do not define names but only the numerical identifiers, and\nI considered 'mirror' more intuitive than 'rotate0Flip' for users.\n\n>\n> Tomi\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 B6F7BC0F1B\n\tfor ;\n\tWed, 6 Sep 2023 07:10:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2AC6E628E7;\n\tWed, 6 Sep 2023 09:10:15 +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 6887E61DF8\n\tfor ;\n\tWed, 6 Sep 2023 09:10:13 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 355803F1;\n\tWed, 6 Sep 2023 09:08:46 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693984215;\n\tbh=vW0oaOAXEmQMDjIqtLugIYaNnTc36zA7A3kA3sh708o=;\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=OEqJvKTojAl2wMvz7D6c2MHjY2mSSTmLfc6JIXvHxyiUjmxht7FqHa2HgtcsopzMu\n\tcNWxGYHgNzltDBfEEZ/D2vIpR7F3JPxLFSzqm5Xc7/dtQspEKM2HYsy3NFzw7sm9HV\n\tV79gt21q+/04RmAVqF7vsSGDm1boBazdC+r2rp8xNm9bUJ6fPbEYMxd5JG3TvW6pkF\n\t1g77UooE0iX6tmFvxrXUdd1bHWycTEeWIc2DFRF5KECNkIENcMvMtcTdiE9JtITddT\n\tSJ5Xvbx78tU9UloCwFjnnvEYXWRqSjVFlSO5iJLGYpfhF3FP8pCqLa/sq0I33Vaqeo\n\tv1HbT9iDiD2AA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693984126;\n\tbh=vW0oaOAXEmQMDjIqtLugIYaNnTc36zA7A3kA3sh708o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pMpgbgiokZa4mg1Zld33IjxRtZRy9v43mciX3QmDri/LoXh60bqrLzs9lcsldM3Jw\n\tZmx76+ra3BIWpQYhQCq+Z1RoFDZ3zMrTzTfuQDscpd15draUKPby9s1UGbh3kgyO4C\n\ttp/95xBYIuFUHvG0rovIFKeDYRHl9wA2joV2m7nM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pMpgbgio\"; dkim-atps=neutral","Date":"Wed, 6 Sep 2023 09:10:10 +0200","To":"Tomi Valkeinen ","Message-ID":"","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27988,"web_url":"https://patchwork.libcamera.org/comment/27988/","msgid":"<20231018214034.GO1512@pendragon.ideasonboard.com>","date":"2023-10-18T21:40:34","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Sep 06, 2023 at 09:10:10AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> On Tue, Sep 05, 2023 at 10:51:51AM +0300, Tomi Valkeinen via libcamera-devel wrote:\n> > On 04/09/2023 22:07, Jacopo Mondi wrote:\n> > > On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n> > > > On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> > > > > Add a '--orientation|-o' option to the Python version of the cam test\n> > > > > application to set an orientation to the image stream.\n> > > > >\n> > > > > Supported values are:\n> > > > > - Rot180: Rotate 180 degrees\n> > > > > - Flip: vertical flip\n> > > > > - Mirror: horizontal flip\n> > > > >\n> > > > > Signed-off-by: Jacopo Mondi \n> > > > > ---\n> > > > > src/py/cam/cam.py | 20 ++++++++++++++++++++\n> > > > > 1 file changed, 20 insertions(+)\n> > > > >\n> > > > > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> > > > > index a2a115c164b4..84d8ca1716fd 100755\n> > > > > --- a/src/py/cam/cam.py\n> > > > > +++ b/src/py/cam/cam.py\n> > > > > @@ -23,6 +23,7 @@ class CameraContext:\n> > > > > opt_metadata: bool\n> > > > > opt_save_frames: bool\n> > > > > opt_capture: int\n> > > > > + opt_orientation: str\n> > > > > stream_names: dict[libcam.Stream, str]\n> > > > > streams: list[libcam.Stream]\n> > > > > @@ -146,6 +147,20 @@ class CameraContext:\n> > > > > if 'pixelformat' in stream_opts:\n> > > > > stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> > > > > + if self.opt_orientation is not None:\n> > > > > + orientation_map = {\n> > > > > + 'Rot180': libcam.Orientation.Rotate180,\n> > > > > + 'Mirror': libcam.Orientation.Rotate0Flip,\n> > > > > + 'Flip': libcam.Orientation.Rotate180Flip,\n> > > > > + }\n> > > >\n> > > > Any reason to not support all orientations? In python you can get the name\n> > > > of the enum as a string, and you could just compare that directly to the\n> > > > string from the user. Also, you could lower-case both before comparison, so\n> > > > that the user could say \"-o rotate270\".\n> > >\n> > > The three supported options are the ones supporte by the C++ version\n> > > of cam, and I like the two to behave the same. The reason why the C++\n> > > version of cam only supports the three above options is because\n> > > they're usually the most common features as user expects. The list\n> > > could indeed be expanded, but for now I would like the two versions to\n> > > behave the same. Is this ok with you ?\n\nCould we at least have the same case for the C++ and Python applications\n? Having to write '-o mirror' for one and '-o Mirror' for the other\nisn't nice.\n\nAnd I also wonder if we should also support a rot0 value. It would allow\nusers of the application to unconditionally pass the -o option, which\nmay make usage of cam/cam.py easier from scripts.\n\n> > Well... cam.py resembles the C++ version, but it's not identical. I\n> > initially thought I'd write a Python clone of the C++ cam, but then I\n> > realized that perhaps it doesn't make sense, and instead it's better to\n> > write a cam.py that is more Pythonic, does things differently than the C++\n> > version when it makes sense, and, when possible, does extra things that are\n> > easy to do in Python.\n> >\n> > Generally speaking, I'd rather support all features, as otherwise will they\n> > ever be tested?\n> \n> I understand this point, but the only other Orientation that does not\n> include a transpose is: rotate0 (Identity) (which is equivalent to not\n> specifying a --orientation). All the other ones require a Transpose,\n> which none of our pipelines can do.\n> \n> > If you use the enum names directly, you can just do:\n> >\n> > self.opt_orientation.lower() in [x.lower() for x in\n> > libcam.Orientation.__members__]\n> >\n> > However, I now realized you used different naming in the option, compared to\n> > the enum (e.g. mirror vs rotate0flip). For that you, of course, need a\n> > mapping there. And if that's the target, then I think the above code is\n> > fine.\n> >\n> > However, and this goes a bit outside the topic here, why do you expose the\n> > enums via different names? Or, if those names are better, then why not use\n> > those \"better\" names for the enum? And if you specifically want to use EXIF\n> > spec names for the enum, then wouldn't it be better to expose the\n> > orientation in cam/cam.py also with the EXIF spec names?\n> \n> EXIF specs do not define names but only the numerical identifiers, and\n\nI wish EXIF had names. As far as I can tell, there's no standardized\nnames for orientations, which leads to everybody using their own naming\nscheme :-(\n\n> I considered 'mirror' more intuitive than 'rotate0Flip' for users.\n\nAre users of cam and cam.py that different from applications developers\n? If not, maybe libcamera::Orientation::Mirror would also be a more\nintuitive name than rotate0Flip ?","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 1F393BDCBD\n\tfor ;\n\tWed, 18 Oct 2023 21:40:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71C526297F;\n\tWed, 18 Oct 2023 23:40:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6913561DD1\n\tfor ;\n\tWed, 18 Oct 2023 23:40:27 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93C40666;\n\tWed, 18 Oct 2023 23:40:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697665228;\n\tbh=/q8NJ214WsUUzmwvHhbv+Gik6mifwN+NiJoBA6ci2MY=;\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=ezyu0AmFflpzJ/aEWvYpHrQ4LtlXOBbbXjSYyd86DINCzwC08W3kUtpKuEpG8dlmE\n\tCjFFgQU6UXC6jSZJWMv0JesYeQt9ehCRK9zV4nHiyAXe8QQgxYtBDJpPJ8yIjQz7PA\n\tx7Pvpdi8EJSPXW5tedxW5lpOVd7ZfUJdY7oRdZhxxhAgyTGQF4cxj7QXwd94VxbqWW\n\tn6Ab0KVN3dFWCyhDyjKKdNX8QNfIUQsaayDFwPS5+GZfYldUlFnfkr6tHHZFWAg19v\n\tmzZJ+ltK7TAjNEkBzry+sW25syzZpxogtQLMg+McwYQXX4VJj2aHOTMBVrC1YO8Hhu\n\tAYHl07WiYTOTA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697665219;\n\tbh=/q8NJ214WsUUzmwvHhbv+Gik6mifwN+NiJoBA6ci2MY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Hty7hbCV5Ef4tDntxX8LRmf6kh6d3JsnLYNWoYj9JwOJBAH/m+9Kr7tXdqEhTbHew\n\tONbDh/7/9Njzlh2HwvmjNhlnQIhuCRCjh7tpcmtcg6heh+xIm2rpsqrzLrecKraMPF\n\tLx3VoVFqAAr8k6hefsWMKOJ+udrW6hGztvQqayrA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Hty7hbCV\"; dkim-atps=neutral","Date":"Thu, 19 Oct 2023 00:40:34 +0300","To":"Jacopo Mondi ","Message-ID":"<20231018214034.GO1512@pendragon.ideasonboard.com>","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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\" "}},{"id":28000,"web_url":"https://patchwork.libcamera.org/comment/28000/","msgid":"","date":"2023-10-19T11:58:43","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Thu, Oct 19, 2023 at 12:40:34AM +0300, Laurent Pinchart wrote:\n> On Wed, Sep 06, 2023 at 09:10:10AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > On Tue, Sep 05, 2023 at 10:51:51AM +0300, Tomi Valkeinen via libcamera-devel wrote:\n> > > On 04/09/2023 22:07, Jacopo Mondi wrote:\n> > > > On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n> > > > > On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> > > > > > Add a '--orientation|-o' option to the Python version of the cam test\n> > > > > > application to set an orientation to the image stream.\n> > > > > >\n> > > > > > Supported values are:\n> > > > > > - Rot180: Rotate 180 degrees\n> > > > > > - Flip: vertical flip\n> > > > > > - Mirror: horizontal flip\n> > > > > >\n> > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > ---\n> > > > > > src/py/cam/cam.py | 20 ++++++++++++++++++++\n> > > > > > 1 file changed, 20 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> > > > > > index a2a115c164b4..84d8ca1716fd 100755\n> > > > > > --- a/src/py/cam/cam.py\n> > > > > > +++ b/src/py/cam/cam.py\n> > > > > > @@ -23,6 +23,7 @@ class CameraContext:\n> > > > > > opt_metadata: bool\n> > > > > > opt_save_frames: bool\n> > > > > > opt_capture: int\n> > > > > > + opt_orientation: str\n> > > > > > stream_names: dict[libcam.Stream, str]\n> > > > > > streams: list[libcam.Stream]\n> > > > > > @@ -146,6 +147,20 @@ class CameraContext:\n> > > > > > if 'pixelformat' in stream_opts:\n> > > > > > stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> > > > > > + if self.opt_orientation is not None:\n> > > > > > + orientation_map = {\n> > > > > > + 'Rot180': libcam.Orientation.Rotate180,\n> > > > > > + 'Mirror': libcam.Orientation.Rotate0Flip,\n> > > > > > + 'Flip': libcam.Orientation.Rotate180Flip,\n> > > > > > + }\n> > > > >\n> > > > > Any reason to not support all orientations? In python you can get the name\n> > > > > of the enum as a string, and you could just compare that directly to the\n> > > > > string from the user. Also, you could lower-case both before comparison, so\n> > > > > that the user could say \"-o rotate270\".\n> > > >\n> > > > The three supported options are the ones supporte by the C++ version\n> > > > of cam, and I like the two to behave the same. The reason why the C++\n> > > > version of cam only supports the three above options is because\n> > > > they're usually the most common features as user expects. The list\n> > > > could indeed be expanded, but for now I would like the two versions to\n> > > > behave the same. Is this ok with you ?\n>\n> Could we at least have the same case for the C++ and Python applications\n> ? Having to write '-o mirror' for one and '-o Mirror' for the other\n> isn't nice.\n>\n\nYes, I don't remeber why I went for capital letter for the py\nimplementation..\n\n> And I also wonder if we should also support a rot0 value. It would allow\n> users of the application to unconditionally pass the -o option, which\n> may make usage of cam/cam.py easier from scripts.\n>\n\nTrue, I'll add it!\n\n> > > Well... cam.py resembles the C++ version, but it's not identical. I\n> > > initially thought I'd write a Python clone of the C++ cam, but then I\n> > > realized that perhaps it doesn't make sense, and instead it's better to\n> > > write a cam.py that is more Pythonic, does things differently than the C++\n> > > version when it makes sense, and, when possible, does extra things that are\n> > > easy to do in Python.\n> > >\n> > > Generally speaking, I'd rather support all features, as otherwise will they\n> > > ever be tested?\n> >\n> > I understand this point, but the only other Orientation that does not\n> > include a transpose is: rotate0 (Identity) (which is equivalent to not\n> > specifying a --orientation). All the other ones require a Transpose,\n> > which none of our pipelines can do.\n> >\n> > > If you use the enum names directly, you can just do:\n> > >\n> > > self.opt_orientation.lower() in [x.lower() for x in\n> > > libcam.Orientation.__members__]\n> > >\n> > > However, I now realized you used different naming in the option, compared to\n> > > the enum (e.g. mirror vs rotate0flip). For that you, of course, need a\n> > > mapping there. And if that's the target, then I think the above code is\n> > > fine.\n> > >\n> > > However, and this goes a bit outside the topic here, why do you expose the\n> > > enums via different names? Or, if those names are better, then why not use\n> > > those \"better\" names for the enum? And if you specifically want to use EXIF\n> > > spec names for the enum, then wouldn't it be better to expose the\n> > > orientation in cam/cam.py also with the EXIF spec names?\n> >\n> > EXIF specs do not define names but only the numerical identifiers, and\n>\n> I wish EXIF had names. As far as I can tell, there's no standardized\n> names for orientations, which leads to everybody using their own naming\n> scheme :-(\n>\n> > I considered 'mirror' more intuitive than 'rotate0Flip' for users.\n>\n> Are users of cam and cam.py that different from applications developers\n> ? If not, maybe libcamera::Orientation::Mirror would also be a more\n> intuitive name than rotate0Flip ?\n>\n\nTrue, but that would break the enumeration naming rule.. true, if\nrotation is 0 it could be omitted, but then how would you name\nOrientation::Rotate0 ?\n\n> --\n> Regards,\n>\n> Laurent Pinchart","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 D386DBDCBD\n\tfor ;\n\tThu, 19 Oct 2023 11:58:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D8A362980;\n\tThu, 19 Oct 2023 13:58:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DDA6861DD2\n\tfor ;\n\tThu, 19 Oct 2023 13:58:46 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C67A625A;\n\tThu, 19 Oct 2023 13:58:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697716728;\n\tbh=2ttsKg4KG3GF0S5m3nNjhi+a2tePOtpI5n9gc+kmLio=;\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=yZF2bQcNadwy6uPIj2avinPfmbayaB0im3oNxYvS3rrAe2xDsns9zOsm175yMP+/g\n\t4q1mNFC5Na8bTOlUEMwIzsZNYmdCDp55lUKazHw36eh6yrupDaOzF0o8p/GB4L9sSW\n\tPXFpETL6tdW11oIWEC00h0V3miMTqdIzfByfHkRA3nDIW2U1mY37dGlXtCZQvUKQPd\n\tKdoiWGG0vz3DxDC56fQ6qIEwcJ8UbkILd1+c2QPT0M7ZGyhRvdirNuoq1wfe6M726n\n\t+nBfzeggE65kAKhUPwPMtfF0P+zUx/LY7fqDnlgnUkVXyznYjrJ6kZl5grU7+MQhdP\n\t2BURN7JA8jMBw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697716718;\n\tbh=2ttsKg4KG3GF0S5m3nNjhi+a2tePOtpI5n9gc+kmLio=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jyCZZYHubE+SHlqmCqRydK6eY4x4rqHfzXhykpn/yQq5a06O2UlcYRI/CV/UudaR8\n\tEvkpoYBguhSOcS40JBj7q6Si9HAV6dkWNiP32vPRHr4znd4t389Y4NecMGSZinunYk\n\tyxmtxMKiP/0UTw8IAEb51RAG+FeYGXwCJ7L4KVJ4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jyCZZYHu\"; dkim-atps=neutral","Date":"Thu, 19 Oct 2023 13:58:43 +0200","To":"Laurent Pinchart ","Message-ID":"","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>\n\t\n\t\n\t\n\t<20231018214034.GO1512@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231018214034.GO1512@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28005,"web_url":"https://patchwork.libcamera.org/comment/28005/","msgid":"<20231019135202.GJ14832@pendragon.ideasonboard.com>","date":"2023-10-19T13:52:02","subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Oct 19, 2023 at 01:58:43PM +0200, Jacopo Mondi wrote:\n> On Thu, Oct 19, 2023 at 12:40:34AM +0300, Laurent Pinchart wrote:\n> > On Wed, Sep 06, 2023 at 09:10:10AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > > On Tue, Sep 05, 2023 at 10:51:51AM +0300, Tomi Valkeinen via libcamera-devel wrote:\n> > > > On 04/09/2023 22:07, Jacopo Mondi wrote:\n> > > > > On Mon, Sep 04, 2023 at 08:18:11PM +0300, Tomi Valkeinen wrote:\n> > > > > > On 01/09/2023 18:02, Jacopo Mondi via libcamera-devel wrote:\n> > > > > > > Add a '--orientation|-o' option to the Python version of the cam test\n> > > > > > > application to set an orientation to the image stream.\n> > > > > > >\n> > > > > > > Supported values are:\n> > > > > > > - Rot180: Rotate 180 degrees\n> > > > > > > - Flip: vertical flip\n> > > > > > > - Mirror: horizontal flip\n> > > > > > >\n> > > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > > ---\n> > > > > > > src/py/cam/cam.py | 20 ++++++++++++++++++++\n> > > > > > > 1 file changed, 20 insertions(+)\n> > > > > > >\n> > > > > > > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> > > > > > > index a2a115c164b4..84d8ca1716fd 100755\n> > > > > > > --- a/src/py/cam/cam.py\n> > > > > > > +++ b/src/py/cam/cam.py\n> > > > > > > @@ -23,6 +23,7 @@ class CameraContext:\n> > > > > > > opt_metadata: bool\n> > > > > > > opt_save_frames: bool\n> > > > > > > opt_capture: int\n> > > > > > > + opt_orientation: str\n> > > > > > > stream_names: dict[libcam.Stream, str]\n> > > > > > > streams: list[libcam.Stream]\n> > > > > > > @@ -146,6 +147,20 @@ class CameraContext:\n> > > > > > > if 'pixelformat' in stream_opts:\n> > > > > > > stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> > > > > > > + if self.opt_orientation is not None:\n> > > > > > > + orientation_map = {\n> > > > > > > + 'Rot180': libcam.Orientation.Rotate180,\n> > > > > > > + 'Mirror': libcam.Orientation.Rotate0Flip,\n> > > > > > > + 'Flip': libcam.Orientation.Rotate180Flip,\n> > > > > > > + }\n> > > > > >\n> > > > > > Any reason to not support all orientations? In python you can get the name\n> > > > > > of the enum as a string, and you could just compare that directly to the\n> > > > > > string from the user. Also, you could lower-case both before comparison, so\n> > > > > > that the user could say \"-o rotate270\".\n> > > > >\n> > > > > The three supported options are the ones supporte by the C++ version\n> > > > > of cam, and I like the two to behave the same. The reason why the C++\n> > > > > version of cam only supports the three above options is because\n> > > > > they're usually the most common features as user expects. The list\n> > > > > could indeed be expanded, but for now I would like the two versions to\n> > > > > behave the same. Is this ok with you ?\n> >\n> > Could we at least have the same case for the C++ and Python applications\n> > ? Having to write '-o mirror' for one and '-o Mirror' for the other\n> > isn't nice.\n> \n> Yes, I don't remeber why I went for capital letter for the py\n> implementation..\n> \n> > And I also wonder if we should also support a rot0 value. It would allow\n> > users of the application to unconditionally pass the -o option, which\n> > may make usage of cam/cam.py easier from scripts.\n> \n> True, I'll add it!\n> \n> > > > Well... cam.py resembles the C++ version, but it's not identical. I\n> > > > initially thought I'd write a Python clone of the C++ cam, but then I\n> > > > realized that perhaps it doesn't make sense, and instead it's better to\n> > > > write a cam.py that is more Pythonic, does things differently than the C++\n> > > > version when it makes sense, and, when possible, does extra things that are\n> > > > easy to do in Python.\n> > > >\n> > > > Generally speaking, I'd rather support all features, as otherwise will they\n> > > > ever be tested?\n> > >\n> > > I understand this point, but the only other Orientation that does not\n> > > include a transpose is: rotate0 (Identity) (which is equivalent to not\n> > > specifying a --orientation). All the other ones require a Transpose,\n> > > which none of our pipelines can do.\n> > >\n> > > > If you use the enum names directly, you can just do:\n> > > >\n> > > > self.opt_orientation.lower() in [x.lower() for x in\n> > > > libcam.Orientation.__members__]\n> > > >\n> > > > However, I now realized you used different naming in the option, compared to\n> > > > the enum (e.g. mirror vs rotate0flip). For that you, of course, need a\n> > > > mapping there. And if that's the target, then I think the above code is\n> > > > fine.\n> > > >\n> > > > However, and this goes a bit outside the topic here, why do you expose the\n> > > > enums via different names? Or, if those names are better, then why not use\n> > > > those \"better\" names for the enum? And if you specifically want to use EXIF\n> > > > spec names for the enum, then wouldn't it be better to expose the\n> > > > orientation in cam/cam.py also with the EXIF spec names?\n> > >\n> > > EXIF specs do not define names but only the numerical identifiers, and\n> >\n> > I wish EXIF had names. As far as I can tell, there's no standardized\n> > names for orientations, which leads to everybody using their own naming\n> > scheme :-(\n> >\n> > > I considered 'mirror' more intuitive than 'rotate0Flip' for users.\n> >\n> > Are users of cam and cam.py that different from applications developers\n> > ? If not, maybe libcamera::Orientation::Mirror would also be a more\n> > intuitive name than rotate0Flip ?\n> \n> True, but that would break the enumeration naming rule.. true, if\n> rotation is 0 it could be omitted, but then how would you name\n> Orientation::Rotate0 ?\n\nYou're right. I don't have a better name, so let's keep it as-is.","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 5154DC3272\n\tfor ;\n\tThu, 19 Oct 2023 13:51:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B4F3161DD2;\n\tThu, 19 Oct 2023 15:51:57 +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 51D946055B\n\tfor ;\n\tThu, 19 Oct 2023 15:51:56 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F11525A;\n\tThu, 19 Oct 2023 15:51:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697723517;\n\tbh=hO8VRyleVllHABfI3A5XVhBn9QAbzo5PSs6dzEFvOJw=;\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=cQJ1MDRoer5f9/OXscVJChp9vr3paSM5sRuo87g1AWsyzQlxdPo4qMv8m8BdCv7L2\n\tHzyEM9p7Pg7QJ1A99iGAKk9yDKnxRATPgmkPmGyyoItu3UUJOQTwRBxrKPcyw7JGnS\n\t54L66S861s76sgltUCXA3WLgK9SoJ6Qp5ubG+51PhKK+2X93xj7xtlITdkBKVepcz/\n\tYH6E2gU+/FB3lVK2uhh3p2QtfoTeMvGqj/6VjjoW445Z/mUVtGsoP1HL3YQM330qly\n\tw9jZ7HYFHI2IqxQcxkwUQJOw0xXRG+X7NGJN51vjnt8hSfxEhY+wTXfWqTqpRvIa1A\n\tmlLQubWjjtgzQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697723508;\n\tbh=hO8VRyleVllHABfI3A5XVhBn9QAbzo5PSs6dzEFvOJw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cUH3cs+N6xkIfCfyKTlIAWCdTrz3x3j5bEPQNdZVD+MJzaENQ7Vc3BgMciRPPy6db\n\tgDItTned6lMWRx+iBlcKQf7uE2pEtDV14RClBWU43/NS+ka9KRtxb1RoDMyOTehxEi\n\t9x5t9k0DlwXarnJO9YpLu3K59+QYp6DWb4eUDJs4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cUH3cs+N\"; dkim-atps=neutral","Date":"Thu, 19 Oct 2023 16:52:02 +0300","To":"Jacopo Mondi ","Message-ID":"<20231019135202.GJ14832@pendragon.ideasonboard.com>","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-13-jacopo.mondi@ideasonboard.com>\n\t<1d4d5bf5-5cbf-6d9d-69ab-2e3ffe18d424@ideasonboard.com>\n\t\n\t\n\t\n\t<20231018214034.GO1512@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 12/12] py: cam: Add option to set\n\tstream orientation","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\" "}}]