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

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

Commit Message

Niklas Söderlund June 16, 2019, 1:34 p.m. UTC 
Add a new option to the cam tool that prints information about the
configuration supplied by the user. If the option is specified,
information about the configuration is printed after the configuration
has been verified possibly adjusted by the camera.

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

Comments

Laurent Pinchart June 18, 2019, 11:58 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Sun, Jun 16, 2019 at 03:34:01PM +0200, Niklas Söderlund wrote:
> Add a new option to the cam tool that prints information about the
> configuration supplied by the user. If the option is specified,
> information about the configuration is printed after the configuration
> has been verified possibly adjusted by the camera.

s/verified/verified and/

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/cam/info.cpp    | 37 +++++++++++++++++++++++++++++++++++++
>  src/cam/info.h      | 14 ++++++++++++++
>  src/cam/main.cpp    | 13 +++++++++++++
>  src/cam/main.h      |  1 +
>  src/cam/meson.build |  1 +
>  5 files changed, 66 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..fe32ceb614e85794
> --- /dev/null
> +++ b/src/cam/info.cpp
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * info.cpp - Display stream information
> + */
> +
> +#include <iomanip>
> +#include <iostream>
> +
> +#include "info.h"
> +
> +using namespace libcamera;
> +
> +int infoConfiguration(const libcamera::CameraConfiguration &config)

Do you need the libcamera namespace qualifier as you use the namespace ?

> +{
> +	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;

The function never returns an error so you can make it void.

> +}
> diff --git a/src/cam/info.h b/src/cam/info.h
> new file mode 100644
> index 0000000000000000..65e31497e5cac358
> --- /dev/null
> +++ b/src/cam/info.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * info.h - Display stream information
> + */
> +#ifndef __CAM_INFO_H__
> +#define __CAM_INFO_H__
> +
> +#include <libcamera/camera.h>
> +
> +int infoConfiguration(const libcamera::CameraConfiguration &config);
> +
> +#endif /* __CAM_INFO_H__ */
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 191fef3a3c8a2b64..7c68cb290d597430 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"

I would just move the infoConfiguration() function to this file
(possibly as a member of the CamApp class) and drop info.h and info.cpp.
It will only be called from a single place.

>  #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,16 @@ int CamApp::run()
>  			std::cout << "- " << cam->name() << std::endl;
>  	}
>  
> +	if (options_.isSet(OptInfo)) {
> +		if (!config_.get()) {

		if (!config) {

should do fine.

> +			std::cout << "Can't print information without a camera"

"Cannot print stream information without a camera"

> +				  << std::endl;
> +			return -ENODEV;

Maybe -EINVAL ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		}
> +
> +		infoConfiguration(*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..0997476bb335e446 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',
>  ])

Patch
diff mbox series 

diff --git a/src/cam/info.cpp b/src/cam/info.cpp
new file mode 100644
index 0000000000000000..fe32ceb614e85794
--- /dev/null
+++ b/src/cam/info.cpp
@@ -0,0 +1,37 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * info.cpp - Display stream information
+ */
+
+#include <iomanip>
+#include <iostream>
+
+#include "info.h"
+
+using namespace libcamera;
+
+int infoConfiguration(const libcamera::CameraConfiguration &config)
+{
+	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..65e31497e5cac358
--- /dev/null
+++ b/src/cam/info.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * info.h - Display stream information
+ */
+#ifndef __CAM_INFO_H__
+#define __CAM_INFO_H__
+
+#include <libcamera/camera.h>
+
+int infoConfiguration(const libcamera::CameraConfiguration &config);
+
+#endif /* __CAM_INFO_H__ */
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 191fef3a3c8a2b64..7c68cb290d597430 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,16 @@  int CamApp::run()
 			std::cout << "- " << cam->name() << std::endl;
 	}
 
+	if (options_.isSet(OptInfo)) {
+		if (!config_.get()) {
+			std::cout << "Can't print information without a camera"
+				  << std::endl;
+			return -ENODEV;
+		}
+
+		infoConfiguration(*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..0997476bb335e446 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',
 ])