[{"id":27987,"web_url":"https://patchwork.libcamera.org/comment/27987/","msgid":"<20231018213110.GM1512@pendragon.ideasonboard.com>","date":"2023-10-18T21:31:10","subject":"Re: [libcamera-devel] [PATCH v5 11/12] apps: 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\nThank you for the patch.\n\nOn Fri, Sep 01, 2023 at 05:02:14PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Add a '--orientation|-o' option to the cam test application to set\n\nHmmmm... '-o' is usually used as a short form of '--output'. Do we want\nto keep it free for that purpose in case we add an output argument later\n? I don't mind either way, I suppose we'll still b e able to change it.\n\n> an orientation to the image stream.\n> \n> Supported values are the ones obtained by applying flips to the camera\n> sensor:\n> - rot180: Rotate 180 degrees\n> - flip: vertical flip\n> - mirror: horizontal flip\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: David Plowman \n> ---\n> src/apps/cam/camera_session.cpp | 17 +++++++++++++++++\n> src/apps/cam/main.cpp | 5 +++++\n> src/apps/cam/main.h | 1 +\n> 3 files changed, 23 insertions(+)\n> \n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 066e397b5f47..1cf7a245aaa7 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -65,6 +65,23 @@ CameraSession::CameraSession(CameraManager *cm,\n> \t\treturn;\n> \t}\n> \n> +\tif (options_.isSet(OptOrientation)) {\n> +\t\tstd::string orient = options_[OptOrientation].toString();\n> +\t\tif (orient == \"rot180\") {\n> +\t\t\tconfig->orientation =\n> +\t\t\t\tlibcamera::Orientation::rotate180;\n> +\t\t} else if (orient == \"mirror\") {\n> +\t\t\tconfig->orientation =\n> +\t\t\t\tlibcamera::Orientation::rotate0Flip;\n\nThis makes me wonder if we shouldn't call the orientation rotate0Mirror.\nWhat do you think ?\n\n> +\t\t} else if (orient == \"flip\") {\n> +\t\t\tconfig->orientation =\n> +\t\t\t\tlibcamera::Orientation::rotate180Flip;\n> +\t\t} else {\n> +\t\t\tstd::cerr << \"Invalid orientation \" << orient << std::endl;\n> +\t\t\treturn;\n> +\t\t}\n\nThis would be more C++, and easier to extend:\n\n\t\tstatic const std::map orientations{\n\t\t\t{ \"rot180\", libcamera::Orientation::rotate180 },\n\t\t\t{ \"mirror\", libcamera::Orientation::rotate0Flip },\n\t\t\t{ \"flip\", libcamera::Orientation::rotate180Flip },\n\t\t};\n\n\t\tauto orientation = orientations.find(options_[OptOrientation].toString());\n\t\tif (orientation == orientations.end()) {\n\t\t\tstd::cerr << \"Invalid orientation \" << orient << std::endl;\n\t\t\treturn;\n\t\t}\n\n\t\tconfig->orientation = orientation.second;\n\nEntirely up to you.\n\n> +\t}\n> +\n> \t/* Apply configuration if explicitly requested. */\n> \tif (StreamKeyValueParser::updateConfiguration(config.get(),\n> \t\t\t\t\t\t options_[OptStream])) {\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index 5d8a57bc14e5..d68243edac3f 100644\n> --- a/src/apps/cam/main.cpp\n> +++ b/src/apps/cam/main.cpp\n> @@ -133,6 +133,11 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \t\t\t \"Capture until interrupted by user or until frames captured\",\n> \t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> \t\t\t OptCamera);\n> +\n> +\tparser.addOption(OptOrientation, OptionString,\n> +\t\t\t \"Desired image orientation (rot180, mirror, flip)\",\n> +\t\t\t \"orientation\", ArgumentRequired, \"orientation\", false,\n> +\t\t\t OptCamera);\n> #ifdef HAVE_KMS\n> \tparser.addOption(OptDisplay, OptionString,\n> \t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> diff --git a/src/apps/cam/main.h b/src/apps/cam/main.h\n> index 526aecece3f3..4aa959b32e13 100644\n> --- a/src/apps/cam/main.h\n> +++ b/src/apps/cam/main.h\n> @@ -17,6 +17,7 @@ enum {\n> \tOptList = 'l',\n> \tOptListProperties = 'p',\n> \tOptMonitor = 'm',\n> +\tOptOrientation = 'o',\n> \tOptSDL = 'S',\n> \tOptStream = 's',\n> \tOptListControls = 256,","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 ED5CAC3272\n\tfor ;\n\tWed, 18 Oct 2023 21:31:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C9746296F;\n\tWed, 18 Oct 2023 23:31:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BD7861DD1\n\tfor ;\n\tWed, 18 Oct 2023 23:31:03 +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 C5AD6666;\n\tWed, 18 Oct 2023 23:30:55 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697664665;\n\tbh=VVif5835IOCTloX+IHZy8/O2KibNEMOP0S98iVGUxqw=;\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=u9M4qGw2iS8QaaSB7A9a+9qDZ8r6MHLun96RmOpJObAOyB2ir5o2U99wybAzag2Wb\n\tLaj1fbWaZ0f6QNvWtoMbTca82x9sPe8qORjiGTX4h2+owPgDXH7PNyRz5tZeImblHV\n\tbk6kKOKR2w+PDQNexjNz/ryIqrf1EC9pPWxoaqjWS5tIpNWk1DbK4W6BVQAU/1ieyf\n\t3Gi+9rmsqUqh8rZ/vvwL8SgAN60fBbUwESldSvw9CO198Sjb8MfqWQcW1xJDRnn+uA\n\tZsgAanM6rYtALyCSKRLu14Xj0iiy56xCPAVKPvCdbQXT4sOydpa+dU23idYXsldguU\n\tTKkLfjPW3+v7w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697664656;\n\tbh=VVif5835IOCTloX+IHZy8/O2KibNEMOP0S98iVGUxqw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bKP4fPPKUUT9MaXCtL3grSHWoYQfnPVhs3IH623RmJaufYgQn3M6hYH1EdFLru2qt\n\t1cXjvd+X63I/gArlPeA5jFRATzTgFDBEUpfOBC0bXDjKtMjiUD3jpvIWuPJJn+Wlhd\n\tUb82B+Q9MgxQKUtLbAPCTMxMHVpfKXGA936AbpSE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bKP4fPPK\"; dkim-atps=neutral","Date":"Thu, 19 Oct 2023 00:31:10 +0300","To":"Jacopo Mondi ","Message-ID":"<20231018213110.GM1512@pendragon.ideasonboard.com>","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-12-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230901150215.11585-12-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 11/12] apps: 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":27999,"web_url":"https://patchwork.libcamera.org/comment/27999/","msgid":"","date":"2023-10-19T11:55:59","subject":"Re: [libcamera-devel] [PATCH v5 11/12] apps: 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 Laurent\n\nOn Thu, Oct 19, 2023 at 12:31:10AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Fri, Sep 01, 2023 at 05:02:14PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > Add a '--orientation|-o' option to the cam test application to set\n>\n> Hmmmm... '-o' is usually used as a short form of '--output'. Do we want\n> to keep it free for that purpose in case we add an output argument later\n> ? I don't mind either way, I suppose we'll still b e able to change it.\n>\n\nI presume so, and since we already have different options to select\nfile output, SDL output or DRM/KMS output, it feels like a -o option\nto 'cam' is probably not urgent ?\n\n> > an orientation to the image stream.\n> >\n> > Supported values are the ones obtained by applying flips to the camera\n> > sensor:\n> > - rot180: Rotate 180 degrees\n> > - flip: vertical flip\n> > - mirror: horizontal flip\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: David Plowman \n> > ---\n> > src/apps/cam/camera_session.cpp | 17 +++++++++++++++++\n> > src/apps/cam/main.cpp | 5 +++++\n> > src/apps/cam/main.h | 1 +\n> > 3 files changed, 23 insertions(+)\n> >\n> > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > index 066e397b5f47..1cf7a245aaa7 100644\n> > --- a/src/apps/cam/camera_session.cpp\n> > +++ b/src/apps/cam/camera_session.cpp\n> > @@ -65,6 +65,23 @@ CameraSession::CameraSession(CameraManager *cm,\n> > \t\treturn;\n> > \t}\n> >\n> > +\tif (options_.isSet(OptOrientation)) {\n> > +\t\tstd::string orient = options_[OptOrientation].toString();\n> > +\t\tif (orient == \"rot180\") {\n> > +\t\t\tconfig->orientation =\n> > +\t\t\t\tlibcamera::Orientation::rotate180;\n> > +\t\t} else if (orient == \"mirror\") {\n> > +\t\t\tconfig->orientation =\n> > +\t\t\t\tlibcamera::Orientation::rotate0Flip;\n>\n> This makes me wonder if we shouldn't call the orientation rotate0Mirror.\n> What do you think ?\n>\n\nYes, sounds much better\n\n> > +\t\t} else if (orient == \"flip\") {\n> > +\t\t\tconfig->orientation =\n> > +\t\t\t\tlibcamera::Orientation::rotate180Flip;\n> > +\t\t} else {\n> > +\t\t\tstd::cerr << \"Invalid orientation \" << orient << std::endl;\n> > +\t\t\treturn;\n> > +\t\t}\n>\n> This would be more C++, and easier to extend:\n>\n> \t\tstatic const std::map orientations{\n> \t\t\t{ \"rot180\", libcamera::Orientation::rotate180 },\n> \t\t\t{ \"mirror\", libcamera::Orientation::rotate0Flip },\n> \t\t\t{ \"flip\", libcamera::Orientation::rotate180Flip },\n> \t\t};\n>\n> \t\tauto orientation = orientations.find(options_[OptOrientation].toString());\n> \t\tif (orientation == orientations.end()) {\n> \t\t\tstd::cerr << \"Invalid orientation \" << orient << std::endl;\n> \t\t\treturn;\n> \t\t}\n>\n> \t\tconfig->orientation = orientation.second;\n>\n> Entirely up to you.\n\nNicer, I'll take it in\n\nThanks\n j\n\n>\n> > +\t}\n> > +\n> > \t/* Apply configuration if explicitly requested. */\n> > \tif (StreamKeyValueParser::updateConfiguration(config.get(),\n> > \t\t\t\t\t\t options_[OptStream])) {\n> > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> > index 5d8a57bc14e5..d68243edac3f 100644\n> > --- a/src/apps/cam/main.cpp\n> > +++ b/src/apps/cam/main.cpp\n> > @@ -133,6 +133,11 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > \t\t\t \"Capture until interrupted by user or until frames captured\",\n> > \t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> > \t\t\t OptCamera);\n> > +\n> > +\tparser.addOption(OptOrientation, OptionString,\n> > +\t\t\t \"Desired image orientation (rot180, mirror, flip)\",\n> > +\t\t\t \"orientation\", ArgumentRequired, \"orientation\", false,\n> > +\t\t\t OptCamera);\n> > #ifdef HAVE_KMS\n> > \tparser.addOption(OptDisplay, OptionString,\n> > \t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> > diff --git a/src/apps/cam/main.h b/src/apps/cam/main.h\n> > index 526aecece3f3..4aa959b32e13 100644\n> > --- a/src/apps/cam/main.h\n> > +++ b/src/apps/cam/main.h\n> > @@ -17,6 +17,7 @@ enum {\n> > \tOptList = 'l',\n> > \tOptListProperties = 'p',\n> > \tOptMonitor = 'm',\n> > +\tOptOrientation = 'o',\n> > \tOptSDL = 'S',\n> > \tOptStream = 's',\n> > \tOptListControls = 256,\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 24446C3272\n\tfor ;\n\tThu, 19 Oct 2023 11:56:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 684B761DD2;\n\tThu, 19 Oct 2023 13:56:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBCD96055B\n\tfor ;\n\tThu, 19 Oct 2023 13:56:02 +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 BF23425A;\n\tThu, 19 Oct 2023 13:55:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697716564;\n\tbh=n5GIoqSCaBqfbFzLXLbQPzzvC/nUvIBxA7yk1ihF1x8=;\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=O0jVrWHTn+u1Kip9B5rgR4jpDcevhqMnbPEMSBC6+UKtN1PGQkiojHK1Np8VkJdjT\n\tZBPiW3w1kMnWCUbNC+uyfDyh7jmqmS5rxJQeshKLH4iEJmxdCaLJRWdXaMGtHhsERd\n\tJE07p8w0lg2Q7C81SyR0DIhEqlDj2YnAbrR57h6g2KHEEDOOXjlNgqoIqEbNrwREId\n\tO84f9g6xsiYYNiS3P0M9XflVBuLVhQQkDTMyNqLFfQA7nGMGEGCicTyeZ/kFCHAYzO\n\tlD3JZukueilsp0fR2CQjgwR/N2M6UFtcumXS3l2bqk8iVnuqNFzNa8I1o8P6wEBlAB\n\tg/FTUz+Q5a2aA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697716554;\n\tbh=n5GIoqSCaBqfbFzLXLbQPzzvC/nUvIBxA7yk1ihF1x8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=D2U+zL4Z5pFJ1TbFtmnbDjrohpiemFQLVuQR8rHNxTuk5MtIO13FEHYfyZXZ9IxDm\n\t0EG8c+doJc0YSRfbzWbqitfX8qM+Kcs8WVGEDwh2gGpWprmYCGoN4jqyoB1680oHhP\n\tCrblJMP6uGeb0A45ZeXM09DklOoV32kqAPLF70YI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"D2U+zL4Z\"; dkim-atps=neutral","Date":"Thu, 19 Oct 2023 13:55:59 +0200","To":"Laurent Pinchart ","Message-ID":"","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-12-jacopo.mondi@ideasonboard.com>\n\t<20231018213110.GM1512@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231018213110.GM1512@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 11/12] apps: 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\" "}}]