[{"id":5376,"web_url":"https://patchwork.libcamera.org/comment/5376/","msgid":"<20200624192318.GF1595450@oden.dyn.berto.se>","date":"2020-06-24T19:23:18","subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: stream_option: use\n\tformat name to set cam/qcam format","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kaaira,\n\nThanks for your work.\n\nOn 2020-06-23 19:10:16 +0530, Kaaira Gupta wrote:\n> Replace hex input for pixelformats with their format names, for input in\n> cam and qcam.\n> \n> Signed-off-by: Kaaira Gupta \n> ---\n> src/cam/stream_options.cpp | 15 ++++++++++++---\n> 1 file changed, 12 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp\n> index bd12c8f..b163177 100644\n> --- a/src/cam/stream_options.cpp\n> +++ b/src/cam/stream_options.cpp\n> @@ -7,6 +7,7 @@\n> #include \"stream_options.h\"\n> \n> #include \n> +#include \n> \n> using namespace libcamera;\n> \n> @@ -19,7 +20,7 @@ StreamKeyValueParser::StreamKeyValueParser()\n> \t\t ArgumentRequired);\n> \taddOption(\"height\", OptionInteger, \"Height in pixels\",\n> \t\t ArgumentRequired);\n> -\taddOption(\"pixelformat\", OptionInteger, \"Pixel format\",\n> +\taddOption(\"pixelformat\", OptionString, \"Pixel format name\",\n> \t\t ArgumentRequired);\n> }\n> \n> @@ -96,8 +97,16 @@ int StreamKeyValueParser::updateConfiguration(CameraConfiguration *config,\n> \t\t}\n> \n> \t\t/* \\todo Translate 4CC string to pixelformat with modifier. */\n> -\t\tif (opts.isSet(\"pixelformat\"))\n> -\t\t\tcfg.pixelFormat = PixelFormat(opts[\"pixelformat\"]);\n> +\t\tif (opts.isSet(\"pixelformat\")) {\n> +\t\t\tconst StreamFormats &formats = cfg.formats();\n> +\t\t\tstd::vector pixelFormats = formats.pixelformats();\n> +\t\t\tstd::vector::iterator ptr;\n> +\t\t\tfor (ptr = pixelFormats.begin(); ptr < pixelFormats.end(); ptr++) {\n\nThis will only iterate over the formats exposed by the \nCamera::generateConfiguration(). In itself that is not so bad as the \nformats returned form it are the only possible ones. But it will \nprevent the user from detecting that the configuration asked for have \nbeen Adjusted. I think you should iterate over all formats in the \nformats:: namespace here.\n\nSmall nit: You could have simplified the loop to something like this \n(not testd):\n\n for (const PixelFormat &format : formats.pixelformats())\n if (opts[\"pixelformat\"].toString() == format.toString())\n cfg.pixelFormat = format;\n\n> +\t\t\t\tif (opts[\"pixelformat\"].toString() == ptr->toString()) {\n> +\t\t\t\t\tcfg.pixelFormat = PixelFormat(*ptr);\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n> \t}\n> \n> \treturn 0;\n> -- \n> 2.17.1\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 BBF58C0101\n\tfor ;\n\tWed, 24 Jun 2020 19:23:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 412CA609B3;\n\tWed, 24 Jun 2020 21:23:22 +0200 (CEST)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27C15603BA\n\tfor ;\n\tWed, 24 Jun 2020 21:23:21 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id q19so3855041lji.2\n\tfor ;\n\tWed, 24 Jun 2020 12:23:21 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tn21sm4379678ljj.97.2020.06.24.12.23.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 24 Jun 2020 12:23:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"tdgYPKHK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=TsZ7cVLi2lwqRTqCzy1WlC4PPqViufQ3x2qbHkI+WyA=;\n\tb=tdgYPKHKfo9qnZcLtS5TNhAL5RxL/IQsXRknJdk9OQ10VA2lRQphkkzBOuWpLHWdgR\n\tqmlYlb7uWy36I8TUeKg0i+N8BK4XfX6G5Ohzyomht5LTIguosQHA//gkA91cEalYzMlU\n\tDcK50tp9Z/FiENhbFjrALyF89jYfqqwnNpMbq7o3EO1gYqPii2yXXiGtA16KmZwX2v6C\n\tdWoaUFFfJFTU/FnMGgxmiF0A3dq61OVWcZAEl6YC8AeDGOhPuKYWJ99yaK2YZ9HERMaH\n\tmrwN0RGh1lgbXMub2DecVRWSUpNjilshyD3AxThifEggbyQVJcvKt6hgszVHKGD0Bs4F\n\tADLA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=TsZ7cVLi2lwqRTqCzy1WlC4PPqViufQ3x2qbHkI+WyA=;\n\tb=YBYgEV0ZAG316yJfpA2kAmY2wevT68Ak7axspWt5pnAsU4JZKetHLS0AGNjK7lKB+s\n\t/Cg2DNZDairmKaXleZXjtYcacvy+xE6wIpQ64IJPNR3NHJ9KK06PWh8GfEAWqWv1l1ff\n\tT8mntv3XpmNBB5DAJnB1WADwJ8tQzxwlCf6JW8m64IZcfYlQ8ZawujCJn11NvSr3Fy2i\n\tGtz0yBd6A7oSV7COac+hSGXbgyJRZB0C6smQQ957Pf329g5F6xraVrY1bTwq2qXceQBD\n\tChiYpgQRSfL12AHbHlnV12Fp9CCkSuQ5NyU/s0AiSqOn44/jbp5rLqojkopHDGoXdnZ7\n\tpuyQ==","X-Gm-Message-State":"AOAM530T16gY9AfxpDMpqPZ56OZbltOVsefpglB10lex491BNdI31m2h\n\tOP/y7eHWAxvDbadEFYeg9jrGwQ==","X-Google-Smtp-Source":"ABdhPJy1bjZsjcFfNgpWuVekqsa4Y7NMXJxLvedA6ygrxi1ZLKYLHORCCLXoNzDARWoAe9Ak112nAQ==","X-Received":"by 2002:a2e:9810:: with SMTP id\n\ta16mr13557699ljj.157.1593026600336; \n\tWed, 24 Jun 2020 12:23:20 -0700 (PDT)","Date":"Wed, 24 Jun 2020 21:23:18 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kaaira Gupta ","Message-ID":"<20200624192318.GF1595450@oden.dyn.berto.se>","References":"<20200623134016.GA17981@kaaira-HP-Pavilion-Notebook>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200623134016.GA17981@kaaira-HP-Pavilion-Notebook>","Subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: stream_option: use\n\tformat name to set cam/qcam format","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":5377,"web_url":"https://patchwork.libcamera.org/comment/5377/","msgid":"<20200624202155.GA26322@pendragon.ideasonboard.com>","date":"2020-06-24T20:21:55","subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: stream_option: use\n\tformat name to set cam/qcam format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jun 24, 2020 at 09:23:18PM +0200, Niklas Söderlund wrote:\n> Hi Kaaira,\n> \n> Thanks for your work.\n> \n> On 2020-06-23 19:10:16 +0530, Kaaira Gupta wrote:\n> > Replace hex input for pixelformats with their format names, for input in\n> > cam and qcam.\n> > \n> > Signed-off-by: Kaaira Gupta \n> > ---\n> > src/cam/stream_options.cpp | 15 ++++++++++++---\n> > 1 file changed, 12 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp\n> > index bd12c8f..b163177 100644\n> > --- a/src/cam/stream_options.cpp\n> > +++ b/src/cam/stream_options.cpp\n> > @@ -7,6 +7,7 @@\n> > #include \"stream_options.h\"\n> > \n> > #include \n> > +#include \n> > \n> > using namespace libcamera;\n> > \n> > @@ -19,7 +20,7 @@ StreamKeyValueParser::StreamKeyValueParser()\n> > \t\t ArgumentRequired);\n> > \taddOption(\"height\", OptionInteger, \"Height in pixels\",\n> > \t\t ArgumentRequired);\n> > -\taddOption(\"pixelformat\", OptionInteger, \"Pixel format\",\n> > +\taddOption(\"pixelformat\", OptionString, \"Pixel format name\",\n> > \t\t ArgumentRequired);\n> > }\n> > \n> > @@ -96,8 +97,16 @@ int StreamKeyValueParser::updateConfiguration(CameraConfiguration *config,\n> > \t\t}\n> > \n> > \t\t/* \\todo Translate 4CC string to pixelformat with modifier. */\n> > -\t\tif (opts.isSet(\"pixelformat\"))\n> > -\t\t\tcfg.pixelFormat = PixelFormat(opts[\"pixelformat\"]);\n> > +\t\tif (opts.isSet(\"pixelformat\")) {\n> > +\t\t\tconst StreamFormats &formats = cfg.formats();\n> > +\t\t\tstd::vector pixelFormats = formats.pixelformats();\n> > +\t\t\tstd::vector::iterator ptr;\n> > +\t\t\tfor (ptr = pixelFormats.begin(); ptr < pixelFormats.end(); ptr++) {\n> \n> This will only iterate over the formats exposed by the \n> Camera::generateConfiguration(). In itself that is not so bad as the \n> formats returned form it are the only possible ones. But it will \n> prevent the user from detecting that the configuration asked for have \n> been Adjusted. I think you should iterate over all formats in the \n> formats:: namespace here.\n\nI think that's why a PixelFormat constructor taking a string has been\nproposed, it would internally use a bew PixelFormatInfo::info(const char\n*).\n\n> Small nit: You could have simplified the loop to something like this \n> (not testd):\n> \n> for (const PixelFormat &format : formats.pixelformats())\n> if (opts[\"pixelformat\"].toString() == format.toString())\n> cfg.pixelFormat = format;\n> \n> > +\t\t\t\tif (opts[\"pixelformat\"].toString() == ptr->toString()) {\n> > +\t\t\t\t\tcfg.pixelFormat = PixelFormat(*ptr);\n> > +\t\t\t\t}\n> > +\t\t\t}\n> > +\t\t}\n> > \t}\n> > \n> > \treturn 0;","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 76118C0100\n\tfor ;\n\tWed, 24 Jun 2020 20:22:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E0D8B603BB;\n\tWed, 24 Jun 2020 22:22:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87CDC603BA\n\tfor ;\n\tWed, 24 Jun 2020 22:22:21 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EBD032A8;\n\tWed, 24 Jun 2020 22:22:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"U2JABs9z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593030141;\n\tbh=za2eYzeTx2BGNe04GhT2I0dg0aA9usf1GxV2+0MEonA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=U2JABs9zwSYj0gCUzwDbmOC51c2BrLqkRUzFlQuefeD5wzsBLQlD/YV38/+78BZTe\n\t8FZFRQ2NBf6Ozgp2ynU/9W6QQeMEUIx3IXWN+Z1N0LawEj7L0hDzTQgGXbcdw/m3EQ\n\t5bbWxyL/tYX+K5bkWtbsvxG8NasNDSm4Tm6hhECk=","Date":"Wed, 24 Jun 2020 23:21:55 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200624202155.GA26322@pendragon.ideasonboard.com>","References":"<20200623134016.GA17981@kaaira-HP-Pavilion-Notebook>\n\t<20200624192318.GF1595450@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200624192318.GF1595450@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: stream_option: use\n\tformat name to set cam/qcam format","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","Cc":"Kaaira Gupta , libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]