[{"id":1922,"web_url":"https://patchwork.libcamera.org/comment/1922/","msgid":"<20190618235805.GT23556@pendragon.ideasonboard.com>","date":"2019-06-18T23:58:05","subject":"Re: [libcamera-devel] [PATCH v3 15/16] cam: Add --info option to\n\tprint information about stream(s)","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Sun, Jun 16, 2019 at 03:34:01PM +0200, Niklas Söderlund wrote:\n> Add a new option to the cam tool that prints information about the\n> configuration supplied by the user. If the option is specified,\n> information about the configuration is printed after the configuration\n> has been verified possibly adjusted by the camera.\n\ns/verified/verified and/\n\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/info.cpp | 37 +++++++++++++++++++++++++++++++++++++\n> src/cam/info.h | 14 ++++++++++++++\n> src/cam/main.cpp | 13 +++++++++++++\n> src/cam/main.h | 1 +\n> src/cam/meson.build | 1 +\n> 5 files changed, 66 insertions(+)\n> create mode 100644 src/cam/info.cpp\n> create mode 100644 src/cam/info.h\n> \n> diff --git a/src/cam/info.cpp b/src/cam/info.cpp\n> new file mode 100644\n> index 0000000000000000..fe32ceb614e85794\n> --- /dev/null\n> +++ b/src/cam/info.cpp\n> @@ -0,0 +1,37 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * info.cpp - Display stream information\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \"info.h\"\n> +\n> +using namespace libcamera;\n> +\n> +int infoConfiguration(const libcamera::CameraConfiguration &config)\n\nDo you need the libcamera namespace qualifier as you use the namespace ?\n\n> +{\n> +\tunsigned int index = 0;\n> +\tfor (const StreamConfiguration &cfg : config) {\n> +\t\tstd::cout << index << \": \" << cfg.toString() << std::endl;\n> +\n> +\t\tconst StreamFormats &formats = cfg.formats();\n> +\t\tfor (unsigned int pixelformat : formats.pixelformats()) {\n> +\t\t\tstd::cout << \" * Pixelformat: 0x\" << std::hex\n> +\t\t\t\t << std::setw(8) << pixelformat << \" \"\n> +\t\t\t\t << formats.range(pixelformat).toString()\n> +\t\t\t\t << std::endl;\n> +\n> +\t\t\tfor (const Size &size : formats.sizes(pixelformat))\n> +\t\t\t\tstd::cout << \" - \" << size.toString()\n> +\t\t\t\t\t << std::endl;\n> +\t\t}\n> +\n> +\t\tindex++;\n> +\t}\n> +\n> +\treturn 0;\n\nThe function never returns an error so you can make it void.\n\n> +}\n> diff --git a/src/cam/info.h b/src/cam/info.h\n> new file mode 100644\n> index 0000000000000000..65e31497e5cac358\n> --- /dev/null\n> +++ b/src/cam/info.h\n> @@ -0,0 +1,14 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * info.h - Display stream information\n> + */\n> +#ifndef __CAM_INFO_H__\n> +#define __CAM_INFO_H__\n> +\n> +#include \n> +\n> +int infoConfiguration(const libcamera::CameraConfiguration &config);\n> +\n> +#endif /* __CAM_INFO_H__ */\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 191fef3a3c8a2b64..7c68cb290d597430 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -13,6 +13,7 @@\n> \n> #include \"capture.h\"\n> #include \"event_loop.h\"\n> +#include \"info.h\"\n\nI would just move the infoConfiguration() function to this file\n(possibly as a member of the CamApp class) and drop info.h and info.cpp.\nIt will only be called from a single place.\n\n> #include \"main.h\"\n> #include \"options.h\"\n> \n> @@ -162,6 +163,8 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \t\t\t \"Set configuration of a camera stream\", \"stream\", true);\n> \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> \t\t\t \"help\");\n> +\tparser.addOption(OptInfo, OptionNone,\n> +\t\t\t \"Display information about stream(s)\", \"info\");\n> \tparser.addOption(OptList, OptionNone, \"List all cameras\", \"list\");\n> \n> \toptions_ = parser.parse(argc, argv);\n> @@ -259,6 +262,16 @@ int CamApp::run()\n> \t\t\tstd::cout << \"- \" << cam->name() << std::endl;\n> \t}\n> \n> +\tif (options_.isSet(OptInfo)) {\n> +\t\tif (!config_.get()) {\n\n\t\tif (!config) {\n\nshould do fine.\n\n> +\t\t\tstd::cout << \"Can't print information without a camera\"\n\n\"Cannot print stream information without a camera\"\n\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn -ENODEV;\n\nMaybe -EINVAL ?\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t}\n> +\n> +\t\tinfoConfiguration(*config_.get());\n> +\t}\n> +\n> \tif (options_.isSet(OptCapture)) {\n> \t\tCapture capture(camera_.get(), config_.get());\n> \t\treturn capture.run(loop_, options_);\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index fff81b1f6c860b57..0997476bb335e446 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -12,6 +12,7 @@ enum {\n> \tOptCapture = 'C',\n> \tOptFile = 'F',\n> \tOptHelp = 'h',\n> +\tOptInfo = 'I',\n> \tOptList = 'l',\n> \tOptStream = 's',\n> };\n> diff --git a/src/cam/meson.build b/src/cam/meson.build\n> index 478346c59590631d..ee5b28421e4c1235 100644\n> --- a/src/cam/meson.build\n> +++ b/src/cam/meson.build\n> @@ -2,6 +2,7 @@ cam_sources = files([\n> 'buffer_writer.cpp',\n> 'capture.cpp',\n> 'event_loop.cpp',\n> + 'info.cpp',\n> 'main.cpp',\n> 'options.cpp',\n> ])","headers":{"Return-Path":"","Received":["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 D6E7561A27\n\tfor ;\n\tWed, 19 Jun 2019 01:58:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 62C6DD5;\n\tWed, 19 Jun 2019 01:58:22 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560902302;\n\tbh=Ipa5pZTSh8CQ3SBJGQHLGRSlDxgkS1KnoOIuJ8dG+BM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=prswZtd67xfw8mRuxkm2wPm70JqKJtfjRDgVRX1rPfV+ZxxxYVYYHmYru8NhG6TeT\n\tRI81NsokcPfIOMIEuOEJfgdUvWHXsPrtoLlD9KHApsukAhvr51lF9pebEhFqzkKH4C\n\t2PM/eHEh+MjjD5/RTa/EUlbgU79e6pL76FkL7ol8=","Date":"Wed, 19 Jun 2019 02:58:05 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190618235805.GT23556@pendragon.ideasonboard.com>","References":"<20190616133402.21934-1-niklas.soderlund@ragnatech.se>\n\t<20190616133402.21934-16-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190616133402.21934-16-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 15/16] cam: Add --info option to\n\tprint information about stream(s)","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 18 Jun 2019 23:58:23 -0000"}}]