[{"id":27592,"web_url":"https://patchwork.libcamera.org/comment/27592/","msgid":"<CAHW6GYJ6bohzBXQ_hx1C8qfXdk6vmYb2t+zhLJqk85PmJe52bg@mail.gmail.com>","date":"2023-07-20T10:16:40","subject":"Re: [libcamera-devel] [PATCH v3 10/10] apps: cam: Add option to set\n\tstream orientation","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the patch.\n\nOn Tue, 18 Jul 2023 at 11:52, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Add a '--orientation|-o' option to the cam test application to set\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\nI guess it offends my mathematical sense of symmetry and completeness\never so slightly not to have an explicit parameter for the fourth and\nfinal supported orientation... namely \"identity\" (or \"rot0\" or\nsomething)!! But as a non-cam user, please feel free to ignore me.\n\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\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>                 return;\n>         }\n>\n> +       if (options_.isSet(OptOrientation)) {\n> +               std::string orient = options_[OptOrientation].toString();\n> +               if (orient == \"rot180\") {\n\nDo we care about case in option parameters? I guess not. In which case\nit all LGTM.\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n> +                       config->orientation =\n> +                               libcamera::Orientation::rotate180;\n> +               } else if (orient == \"mirror\") {\n> +                       config->orientation =\n> +                               libcamera::Orientation::rotate0Flip;\n> +               } else if (orient == \"flip\") {\n> +                       config->orientation =\n> +                               libcamera::Orientation::rotate180Flip;\n> +               } else {\n> +                       std::cerr << \"Invalid orientation \" << orient << std::endl;\n> +                       return;\n> +               }\n> +       }\n> +\n>         /* Apply configuration if explicitly requested. */\n>         if (StreamKeyValueParser::updateConfiguration(config.get(),\n>                                                       options_[OptStream])) {\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index 5d8a57bc14e5..8698d5eedcf8 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>                          \"Capture until interrupted by user or until <count> frames captured\",\n>                          \"capture\", ArgumentOptional, \"count\", false,\n>                          OptCamera);\n> +\n> +       parser.addOption(OptOrientation, OptionString,\n> +                        \"The camera stream image orientation (rot180, mirror, flip)\",\n> +                        \"orientation\", ArgumentRequired, \"orientation\", false,\n> +                        OptCamera);\n>  #ifdef HAVE_KMS\n>         parser.addOption(OptDisplay, OptionString,\n>                          \"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>         OptList = 'l',\n>         OptListProperties = 'p',\n>         OptMonitor = 'm',\n> +       OptOrientation = 'o',\n>         OptSDL = 'S',\n>         OptStream = 's',\n>         OptListControls = 256,\n> --\n> 2.40.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1847FBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 20 Jul 2023 10:16:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 767C2628C0;\n\tThu, 20 Jul 2023 12:16:53 +0200 (CEST)","from mail-qv1-xf2d.google.com (mail-qv1-xf2d.google.com\n\t[IPv6:2607:f8b0:4864:20::f2d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B855F61E2A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 20 Jul 2023 12:16:52 +0200 (CEST)","by mail-qv1-xf2d.google.com with SMTP id\n\t6a1803df08f44-634ddd17647so4856986d6.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 20 Jul 2023 03:16:52 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689848213;\n\tbh=OeCmcK3SCMiqDQzqyX5EKfZYwbrHEAKOhP4Pi/MRC2Y=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QzWEsA+967jCpLzm1saE25PWpkxN7Lyu/hPmek0b0nwAx4TTgJcu2dTpUC9A8vzT0\n\tj0UAabnRg4MK5ac6IhOxgFBAI4Qs46idmtd0m78UvxCtB5eocnrUglUKkmxqybmqXy\n\t3w8FQxh/EBV/AspZqQQrwRdp6DJ2K4lqTeSdNTgTnWwLKs9BuaIE5mo/kNjNKKauAf\n\tzFvTjjqGxfNzahOEHKcu2J+LmPec3nBOggOymoAkSTCY+FX350StR/ha3xwHto+w4I\n\t0wCZfg4uEP0YxsvzGEnhjbI6ObSrWpGN33i6MhB6OmywR206FJvSBiF/4uk3E/OmS/\n\tScDbrS6mQ70mg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689848211; x=1690453011;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=LAvXMw5XiFu5e1by3Ur/KkMnlbpSntpVm/7N/JRCRhE=;\n\tb=XuotnwXUU7whmCxBTLIrtNqkGn4tK+fSiX66rAn3/lrcCckm8YlZfF45EKSETWM+AA\n\tHBNyEm2XMxbzqVTTATh0Wq2MfdBiPBiyWxx4F0LccF/d/I1CDrUSjiKXubx8W7BBErWj\n\tCN8CzzgukiSY8jHwuvRYPOTFr7N/pxpZDP/+UvBVG6DmFpQxzG9cWS2VAHGy9EgubsEC\n\tnhmBK3Os8cTj9XCLcKXkryddvM1riDuz0Mlgr+uuT5o3mLEacIfd06Cnbxm90E9QxiZS\n\t+P1B3ltk6QxB4dzrPSqZdw7fcCElzXleRos+kDyTgmU15ySXrpi8VJMOHHXT7kZNdYiN\n\tg9ZQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"XuotnwXU\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689848211; x=1690453011;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=LAvXMw5XiFu5e1by3Ur/KkMnlbpSntpVm/7N/JRCRhE=;\n\tb=EcqwW+3reduWIHyQ5ivAaCWbex+VvPnUj3R0N+kegeRMsTqiUwyiHF22MGBRGqq7vg\n\t7jxue8PHeGjl1XMXuuCkrrFgFiUJG9u80cRh1nd6kk5Hysieg6KGQsybwHlQphZPuSed\n\t3a6d0EL3iVnh+ssSGU0y2as7YEnxpcRCZuQ68rkacd1MxUPje+qEMZkAD2YogE+ZRa1l\n\t0WQRY5MlkVL1b/VpQKPXyORWzhuwylCqAYExmFmb32nNl/1kwbdeLbdo/sdrLtVzzNxr\n\tSUg3ZwsmtptjAOgl1srlFpj2aokhGABq71SH30i1J5maokl2HQHNsu3dvMMJ7oxpSH8C\n\tTKgg==","X-Gm-Message-State":"ABy/qLbAuqnXrY/B5dMUK08UZC+z9F1D7zEIxftWMV0I9rGghchuP11R\n\tlYjeiZTph6IWzpdJVDVXfPCd6MwX7Aek2k33WEaOxg==","X-Google-Smtp-Source":"APBJJlHUV5TAbmNpRBRuoELjBUfJOCYP0ju69I7MfbBj5GiEOzsF+bP5yOkndLM05aUPOZ7FQuOcuautU2E1KoUpV68=","X-Received":"by 2002:a05:6214:2dc5:b0:635:fb19:2ebd with SMTP id\n\tnc5-20020a0562142dc500b00635fb192ebdmr1744738qvb.13.1689848211636;\n\tThu, 20 Jul 2023 03:16:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20230718105210.83558-1-jacopo.mondi@ideasonboard.com>\n\t<20230718105210.83558-11-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230718105210.83558-11-jacopo.mondi@ideasonboard.com>","Date":"Thu, 20 Jul 2023 11:16:40 +0100","Message-ID":"<CAHW6GYJ6bohzBXQ_hx1C8qfXdk6vmYb2t+zhLJqk85PmJe52bg@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3 10/10] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27597,"web_url":"https://patchwork.libcamera.org/comment/27597/","msgid":"<p7vhvvx6cpoaupaeks2epr5mhofg6gnaailgj3ytnqq6utaqvy@rleho4roooaf>","date":"2023-07-24T07:00:33","subject":"Re: [libcamera-devel] [PATCH v3 10/10] 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 David\n\nOn Thu, Jul 20, 2023 at 11:16:40AM +0100, David Plowman via libcamera-devel wrote:\n> Hi Jacopo\n>\n> Thanks for the patch.\n>\n> On Tue, 18 Jul 2023 at 11:52, Jacopo Mondi\n> <jacopo.mondi@ideasonboard.com> wrote:\n> >\n> > Add a '--orientation|-o' option to the cam test application to set\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> I guess it offends my mathematical sense of symmetry and completeness\n> ever so slightly not to have an explicit parameter for the fourth and\n> final supported orientation... namely \"identity\" (or \"rot0\" or\n> something)!! But as a non-cam user, please feel free to ignore me.\n>\n\nIsn't \"Identity\" the default ?\n\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\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> >                 return;\n> >         }\n> >\n> > +       if (options_.isSet(OptOrientation)) {\n> > +               std::string orient = options_[OptOrientation].toString();\n> > +               if (orient == \"rot180\") {\n>\n> Do we care about case in option parameters? I guess not. In which case\n\nI don't think we do for any of the other ?\n\n> it all LGTM.\n>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks\n  j\n>\n> Thanks!\n> David\n>\n> > +                       config->orientation =\n> > +                               libcamera::Orientation::rotate180;\n> > +               } else if (orient == \"mirror\") {\n> > +                       config->orientation =\n> > +                               libcamera::Orientation::rotate0Flip;\n> > +               } else if (orient == \"flip\") {\n> > +                       config->orientation =\n> > +                               libcamera::Orientation::rotate180Flip;\n> > +               } else {\n> > +                       std::cerr << \"Invalid orientation \" << orient << std::endl;\n> > +                       return;\n> > +               }\n> > +       }\n> > +\n> >         /* Apply configuration if explicitly requested. */\n> >         if (StreamKeyValueParser::updateConfiguration(config.get(),\n> >                                                       options_[OptStream])) {\n> > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> > index 5d8a57bc14e5..8698d5eedcf8 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> >                          \"Capture until interrupted by user or until <count> frames captured\",\n> >                          \"capture\", ArgumentOptional, \"count\", false,\n> >                          OptCamera);\n> > +\n> > +       parser.addOption(OptOrientation, OptionString,\n> > +                        \"The camera stream image orientation (rot180, mirror, flip)\",\n> > +                        \"orientation\", ArgumentRequired, \"orientation\", false,\n> > +                        OptCamera);\n> >  #ifdef HAVE_KMS\n> >         parser.addOption(OptDisplay, OptionString,\n> >                          \"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> >         OptList = 'l',\n> >         OptListProperties = 'p',\n> >         OptMonitor = 'm',\n> > +       OptOrientation = 'o',\n> >         OptSDL = 'S',\n> >         OptStream = 's',\n> >         OptListControls = 256,\n> > --\n> > 2.40.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 29801BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Jul 2023 07:00:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C482B628BF;\n\tMon, 24 Jul 2023 09:00:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 568B861E25\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Jul 2023 09:00:36 +0200 (CEST)","from ideasonboard.com (mob-5-91-20-233.net.vodafone.it\n\t[5.91.20.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D8EB2C67;\n\tMon, 24 Jul 2023 08:59:38 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690182038;\n\tbh=r7TNqJwYpJgRXWgkIK75s/DlA4Qg4FE++XymV+axDz4=;\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=aVS5WriHVZ17GuCImDd9qqgsfgQBLE2ZtfO6lkyCmyPk5b0cSw+FsmX/TC+89sbkK\n\tLugIpY8B08PL2hrpQXtVOxIgjeVSU7cO/5pGCPjUQ4UWAcVjrrPXGSZu368u92CM6L\n\tuEbf93fNkoPrk8kozuXW0e6U1orq/MmfpJfPOzv868TWeGagm0o/CiAMPZd6F6IO2I\n\tgoZp8Tfss9hnrZhIA1cmRy76gzQuOAD8zjtvcs3Gak3gvt/eiiqdN6D0chD9+4KAdY\n\toFo4oTDXXNcfg2pcpvAzeE7tMXX6gfjnrYIWZbp1pl2AkbAgkkyh/Wz1gb3Eif4HlP\n\thSvNTvCiQjRQA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1690181979;\n\tbh=r7TNqJwYpJgRXWgkIK75s/DlA4Qg4FE++XymV+axDz4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uQN1OPtAo1EC520PZeCbb5YuROYEoZjihhAj/lQWbwNz4Gur+iYvW7hN2dfg1Bx/p\n\taBigYqlc1BqxOk2AtLQN+/F70x9DYlbuc+39VIDigaOUeJyFjlEEL7mb76P6ne0Avm\n\tQ6fz5eudFksPTAy5Y9JDslxhxjvksVaZuouqlURw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uQN1OPtA\"; dkim-atps=neutral","Date":"Mon, 24 Jul 2023 09:00:33 +0200","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<p7vhvvx6cpoaupaeks2epr5mhofg6gnaailgj3ytnqq6utaqvy@rleho4roooaf>","References":"<20230718105210.83558-1-jacopo.mondi@ideasonboard.com>\n\t<20230718105210.83558-11-jacopo.mondi@ideasonboard.com>\n\t<CAHW6GYJ6bohzBXQ_hx1C8qfXdk6vmYb2t+zhLJqk85PmJe52bg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAHW6GYJ6bohzBXQ_hx1C8qfXdk6vmYb2t+zhLJqk85PmJe52bg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 10/10] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]