[libcamera-devel,16/17] cam: Add --info option to print information about stream(s)
diff mbox series

Message ID 20190527001543.13593-17-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Add support for format information and validation
Related show

Commit Message

Niklas Söderlund May 27, 2019, 12:15 a.m. UTC 
Add a new option to cam tool which prints information about the
configuration supplied by the user. If specified information is printed
about the configuration after its been verified and possibly adjusted by
the camera and information about each stream in the camera
configuration.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/cam/info.cpp    | 44 ++++++++++++++++++++++++++++++++++++++++++++
 src/cam/info.h      | 18 ++++++++++++++++++
 src/cam/main.cpp    |  8 ++++++++
 src/cam/main.h      |  1 +
 src/cam/meson.build |  1 +
 5 files changed, 72 insertions(+)
 create mode 100644 src/cam/info.cpp
 create mode 100644 src/cam/info.h

Comments

Kieran Bingham May 29, 2019, 10:50 p.m. UTC | #1 
Hi Niklas,

On 27/05/2019 01:15, Niklas Söderlund wrote:
> Add a new option to cam tool which prints information about the
> configuration supplied by the user.



> If specified information is printed
> about the configuration after its been verified and possibly adjusted by
> the camera and information about each stream in the camera
> configuration.

That paragraph is hard to interpret. But it's late so perhaps I'm just
tired.

Do you mean something like:

If provided by the pipeline handler, information is printed about the
configuration after it has been verified (and potentially adjusted) by
the camera and information about each stream in the camera configuration
is presented.

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/cam/info.cpp    | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  src/cam/info.h      | 18 ++++++++++++++++++
>  src/cam/main.cpp    |  8 ++++++++
>  src/cam/main.h      |  1 +
>  src/cam/meson.build |  1 +
>  5 files changed, 72 insertions(+)
>  create mode 100644 src/cam/info.cpp
>  create mode 100644 src/cam/info.h
> 
> diff --git a/src/cam/info.cpp b/src/cam/info.cpp
> new file mode 100644
> index 0000000000000000..35271942600494c7
> --- /dev/null
> +++ b/src/cam/info.cpp
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * info.cpp - Displat stream information
> + */
> +
> +#include <iomanip>
> +#include <iostream>
> +
> +#include "info.h"
> +
> +using namespace libcamera;
> +
> +int Info::run(const CameraConfiguration *config)
> +{
> +	if (!config) {
> +		std::cout
> +			<< "Can't display information, no configuration"
> +			<< std::endl;
> +		return -ENODEV;
> +	}
> +
> +	unsigned int index = 0;
> +	for (const StreamConfiguration cfg : *config) {
> +		std::cout << index << ": " << cfg.toString() << std::endl;
> +
> +		const StreamFormats &formats = cfg.formats();
> +		for (unsigned int pixelformat : formats.pixelformats()) {
> +			std::cout << " * Pixelformat: 0x" << std::hex
> +				  << std::setw(8) << pixelformat << " "
> +				  << formats.range(pixelformat).toString()
> +				  << std::endl;
> +
> +			for (const Size &size : formats.sizes(pixelformat))
> +				std::cout << "  - " << size.toString()
> +					  << std::endl;
> +		}

This looks quite nice:

./build/src/cam/cam -c "HP Wide Vision FHD Camera: HP W" -i
Using camera HP Wide Vision FHD Camera: HP W
0: 1920x1080-0x47504a4d
 * Pixelformat: 0x47504a4d Width: 176-1920 Height: 144-1080 hStep: 1
vStep: 1
  - 176x144
  - 320x240
  - 352x288
  - 640x360
  - 640x480
  - 1280x720
  - 1920x1080

Except for this bit:
 * Pixelformat: 0x47504a4d Width: 176-1920 Height: 144-1080 hStep: 1
vStep: 1

 - 176-1920 ?

Perhaps should we format this as
 * Pixelformat 0x47504a4d 176x144 -> 1920x1080 ?

Infact, that's a print option in the formats.range() ...


> +
> +		index++;
> +	}
> +
> +	return 0;
> +}
> diff --git a/src/cam/info.h b/src/cam/info.h
> new file mode 100644
> index 0000000000000000..72fd446491f95033
> --- /dev/null
> +++ b/src/cam/info.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * info.h - Displat stream information
> + */
> +#ifndef __CAM_INFO_H__
> +#define __CAM_INFO_H__
> +
> +#include <libcamera/camera.h>
> +
> +class Info
> +{
> +public:
> +	int run(const libcamera::CameraConfiguration *config);
> +};
> +
> +#endif /* __CAM_INFO_H__ */
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 25538f5ba95552d6..bf7a92b51d27c890 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -13,6 +13,7 @@
>  
>  #include "capture.h"
>  #include "event_loop.h"
> +#include "info.h"
>  #include "main.h"
>  #include "options.h"
>  
> @@ -162,6 +163,8 @@ int CamApp::parseOptions(int argc, char *argv[])
>  			 "Set configuration of a camera stream", "stream", true);
>  	parser.addOption(OptHelp, OptionNone, "Display this help message",
>  			 "help");
> +	parser.addOption(OptInfo, OptionNone,
> +			 "Display information about stream(s)", "info");
>  	parser.addOption(OptList, OptionNone, "List all cameras", "list");
>  
>  	options_ = parser.parse(argc, argv);
> @@ -259,6 +262,11 @@ int CamApp::run()
>  			std::cout << "- " << cam->name() << std::endl;
>  	}
>  
> +	if (options_.isSet(OptInfo)) {
> +		Info info;
> +		info.run(config_.get());
> +	}
> +
>  	if (options_.isSet(OptCapture)) {
>  		Capture capture(camera_.get(), config_.get());
>  		return capture.run(loop_, options_);
> diff --git a/src/cam/main.h b/src/cam/main.h
> index fff81b1f6c860b57..6324c042f89e7396 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -12,6 +12,7 @@ enum {
>  	OptCapture = 'C',
>  	OptFile = 'F',
>  	OptHelp = 'h',
> +	OptInfo = 'i',
>  	OptList = 'l',
>  	OptStream = 's',
>  };
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 478346c59590631d..ee5b28421e4c1235 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -2,6 +2,7 @@ cam_sources = files([
>      'buffer_writer.cpp',
>      'capture.cpp',
>      'event_loop.cpp',
> +    'info.cpp',
>      'main.cpp',
>      'options.cpp',
>  ])
>
Laurent Pinchart June 10, 2019, 7:10 a.m. UTC | #2 
Hi Niklas,

On Wed, May 29, 2019 at 11:50:47PM +0100, Kieran Bingham wrote:
> On 27/05/2019 01:15, Niklas Söderlund wrote:
> > Add a new option to cam tool which prints information about the

s/to cam tool/to the cam tool/
s/which/that/

> > configuration supplied by the user.
> 
> 
> 
> > If specified information is printed
> > about the configuration after its been verified and possibly adjusted by
> > the camera and information about each stream in the camera
> > configuration.
> 
> That paragraph is hard to interpret. But it's late so perhaps I'm just
> tired.
> 
> Do you mean something like:
> 
> If provided by the pipeline handler, information is printed about the
> configuration after it has been verified (and potentially adjusted) by
> the camera and information about each stream in the camera configuration
> is presented.

It was hard to parse for me indeed. I think Niklas meant "If the option
is specified, information about the configuration is printed after the
configuration has been verified ..."

> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/cam/info.cpp    | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  src/cam/info.h      | 18 ++++++++++++++++++
> >  src/cam/main.cpp    |  8 ++++++++
> >  src/cam/main.h      |  1 +
> >  src/cam/meson.build |  1 +
> >  5 files changed, 72 insertions(+)
> >  create mode 100644 src/cam/info.cpp
> >  create mode 100644 src/cam/info.h
> > 
> > diff --git a/src/cam/info.cpp b/src/cam/info.cpp
> > new file mode 100644
> > index 0000000000000000..35271942600494c7
> > --- /dev/null
> > +++ b/src/cam/info.cpp
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * info.cpp - Displat stream information

s/Displat/Display/

> > + */
> > +
> > +#include <iomanip>
> > +#include <iostream>
> > +
> > +#include "info.h"
> > +
> > +using namespace libcamera;
> > +
> > +int Info::run(const CameraConfiguration *config)
> > +{
> > +	if (!config) {
> > +		std::cout
> > +			<< "Can't display information, no configuration"
> > +			<< std::endl;
> > +		return -ENODEV;
> > +	}

Is this possible ? I would remove this check and pass the configuration
by const reference.

> > +
> > +	unsigned int index = 0;
> > +	for (const StreamConfiguration cfg : *config) {

	     const StreamConfiguration &cfg

> > +		std::cout << index << ": " << cfg.toString() << std::endl;
> > +
> > +		const StreamFormats &formats = cfg.formats();
> > +		for (unsigned int pixelformat : formats.pixelformats()) {
> > +			std::cout << " * Pixelformat: 0x" << std::hex
> > +				  << std::setw(8) << pixelformat << " "
> > +				  << formats.range(pixelformat).toString()
> > +				  << std::endl;
> > +
> > +			for (const Size &size : formats.sizes(pixelformat))
> > +				std::cout << "  - " << size.toString()
> > +					  << std::endl;
> > +		}
> 
> This looks quite nice:
> 
> ./build/src/cam/cam -c "HP Wide Vision FHD Camera: HP W" -i
> Using camera HP Wide Vision FHD Camera: HP W
> 0: 1920x1080-0x47504a4d
>  * Pixelformat: 0x47504a4d Width: 176-1920 Height: 144-1080 hStep: 1
> vStep: 1
>   - 176x144
>   - 320x240
>   - 352x288
>   - 640x360
>   - 640x480
>   - 1280x720
>   - 1920x1080
> 
> Except for this bit:
>  * Pixelformat: 0x47504a4d Width: 176-1920 Height: 144-1080 hStep: 1
> vStep: 1
> 
>  - 176-1920 ?
> 
> Perhaps should we format this as
>  * Pixelformat 0x47504a4d 176x144 -> 1920x1080 ?
> 
> Infact, that's a print option in the formats.range() ...
> 
> > +
> > +		index++;
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/src/cam/info.h b/src/cam/info.h
> > new file mode 100644
> > index 0000000000000000..72fd446491f95033
> > --- /dev/null
> > +++ b/src/cam/info.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * info.h - Displat stream information
> > + */
> > +#ifndef __CAM_INFO_H__
> > +#define __CAM_INFO_H__
> > +
> > +#include <libcamera/camera.h>
> > +
> > +class Info
> > +{
> > +public:
> > +	int run(const libcamera::CameraConfiguration *config);
> > +};

You don't need a class for this, you can create a global function.

> > +
> > +#endif /* __CAM_INFO_H__ */
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 25538f5ba95552d6..bf7a92b51d27c890 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -13,6 +13,7 @@
> >  
> >  #include "capture.h"
> >  #include "event_loop.h"
> > +#include "info.h"
> >  #include "main.h"
> >  #include "options.h"
> >  
> > @@ -162,6 +163,8 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  			 "Set configuration of a camera stream", "stream", true);
> >  	parser.addOption(OptHelp, OptionNone, "Display this help message",
> >  			 "help");
> > +	parser.addOption(OptInfo, OptionNone,
> > +			 "Display information about stream(s)", "info");
> >  	parser.addOption(OptList, OptionNone, "List all cameras", "list");
> >  
> >  	options_ = parser.parse(argc, argv);
> > @@ -259,6 +262,11 @@ int CamApp::run()
> >  			std::cout << "- " << cam->name() << std::endl;
> >  	}
> >  
> > +	if (options_.isSet(OptInfo)) {
> > +		Info info;
> > +		info.run(config_.get());
> > +	}
> > +
> >  	if (options_.isSet(OptCapture)) {
> >  		Capture capture(camera_.get(), config_.get());
> >  		return capture.run(loop_, options_);
> > diff --git a/src/cam/main.h b/src/cam/main.h
> > index fff81b1f6c860b57..6324c042f89e7396 100644
> > --- a/src/cam/main.h
> > +++ b/src/cam/main.h
> > @@ -12,6 +12,7 @@ enum {
> >  	OptCapture = 'C',
> >  	OptFile = 'F',
> >  	OptHelp = 'h',
> > +	OptInfo = 'i',

-i is usually used for input files, I wonder if it could make sense to
reserve it for that purpose. One option could be to add a verbose
argument instead, but one may argue that controlling the display of
supported formats explicitly could also be useful.

> >  	OptList = 'l',
> >  	OptStream = 's',
> >  };
> > diff --git a/src/cam/meson.build b/src/cam/meson.build
> > index 478346c59590631d..ee5b28421e4c1235 100644
> > --- a/src/cam/meson.build
> > +++ b/src/cam/meson.build
> > @@ -2,6 +2,7 @@ cam_sources = files([
> >      'buffer_writer.cpp',
> >      'capture.cpp',
> >      'event_loop.cpp',
> > +    'info.cpp',
> >      'main.cpp',
> >      'options.cpp',
> >  ])

Patch
diff mbox series 

diff --git a/src/cam/info.cpp b/src/cam/info.cpp
new file mode 100644
index 0000000000000000..35271942600494c7
--- /dev/null
+++ b/src/cam/info.cpp
@@ -0,0 +1,44 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * info.cpp - Displat stream information
+ */
+
+#include <iomanip>
+#include <iostream>
+
+#include "info.h"
+
+using namespace libcamera;
+
+int Info::run(const CameraConfiguration *config)
+{
+	if (!config) {
+		std::cout
+			<< "Can't display information, no configuration"
+			<< std::endl;
+		return -ENODEV;
+	}
+
+	unsigned int index = 0;
+	for (const StreamConfiguration cfg : *config) {
+		std::cout << index << ": " << cfg.toString() << std::endl;
+
+		const StreamFormats &formats = cfg.formats();
+		for (unsigned int pixelformat : formats.pixelformats()) {
+			std::cout << " * Pixelformat: 0x" << std::hex
+				  << std::setw(8) << pixelformat << " "
+				  << formats.range(pixelformat).toString()
+				  << std::endl;
+
+			for (const Size &size : formats.sizes(pixelformat))
+				std::cout << "  - " << size.toString()
+					  << std::endl;
+		}
+
+		index++;
+	}
+
+	return 0;
+}
diff --git a/src/cam/info.h b/src/cam/info.h
new file mode 100644
index 0000000000000000..72fd446491f95033
--- /dev/null
+++ b/src/cam/info.h
@@ -0,0 +1,18 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * info.h - Displat stream information
+ */
+#ifndef __CAM_INFO_H__
+#define __CAM_INFO_H__
+
+#include <libcamera/camera.h>
+
+class Info
+{
+public:
+	int run(const libcamera::CameraConfiguration *config);
+};
+
+#endif /* __CAM_INFO_H__ */
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 25538f5ba95552d6..bf7a92b51d27c890 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -13,6 +13,7 @@ 
 
 #include "capture.h"
 #include "event_loop.h"
+#include "info.h"
 #include "main.h"
 #include "options.h"
 
@@ -162,6 +163,8 @@  int CamApp::parseOptions(int argc, char *argv[])
 			 "Set configuration of a camera stream", "stream", true);
 	parser.addOption(OptHelp, OptionNone, "Display this help message",
 			 "help");
+	parser.addOption(OptInfo, OptionNone,
+			 "Display information about stream(s)", "info");
 	parser.addOption(OptList, OptionNone, "List all cameras", "list");
 
 	options_ = parser.parse(argc, argv);
@@ -259,6 +262,11 @@  int CamApp::run()
 			std::cout << "- " << cam->name() << std::endl;
 	}
 
+	if (options_.isSet(OptInfo)) {
+		Info info;
+		info.run(config_.get());
+	}
+
 	if (options_.isSet(OptCapture)) {
 		Capture capture(camera_.get(), config_.get());
 		return capture.run(loop_, options_);
diff --git a/src/cam/main.h b/src/cam/main.h
index fff81b1f6c860b57..6324c042f89e7396 100644
--- a/src/cam/main.h
+++ b/src/cam/main.h
@@ -12,6 +12,7 @@  enum {
 	OptCapture = 'C',
 	OptFile = 'F',
 	OptHelp = 'h',
+	OptInfo = 'i',
 	OptList = 'l',
 	OptStream = 's',
 };
diff --git a/src/cam/meson.build b/src/cam/meson.build
index 478346c59590631d..ee5b28421e4c1235 100644
--- a/src/cam/meson.build
+++ b/src/cam/meson.build
@@ -2,6 +2,7 @@  cam_sources = files([
     'buffer_writer.cpp',
     'capture.cpp',
     'event_loop.cpp',
+    'info.cpp',
     'main.cpp',
     'options.cpp',
 ])