[libcamera-devel,3/3] qcam: Make log less verbose by default
diff mbox series

Message ID 20201124174342.19587-4-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • qcam: Miscellaneous patches
Related show

Commit Message

Laurent Pinchart Nov. 24, 2020, 5:43 p.m. UTC 
The qcam log prints one message per frame, which is pretty verbose. This
feature is useful for debugging, but not necessarily as a default
option. Silence it by default, and add a -v/--verbose command line
parameter to make the log verbose.

While this could have been handled manually by checking a verbose flag
when printing the message, the feature is instead integrated with the Qt
log infrastructure to make it more flexible. Messages printed by
qDebug() are now silenced by default and controlled by the -v/--verbose
argument.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main.cpp            |  5 +++++
 src/qcam/main_window.cpp     |  2 +-
 src/qcam/main_window.h       |  2 ++
 src/qcam/meson.build         |  1 +
 src/qcam/message_handler.cpp | 27 +++++++++++++++++++++++++++
 src/qcam/message_handler.h   | 26 ++++++++++++++++++++++++++
 6 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 src/qcam/message_handler.cpp
 create mode 100644 src/qcam/message_handler.h

Comments

Kieran Bingham Nov. 24, 2020, 7:09 p.m. UTC | #1 
On 24/11/2020 17:43, Laurent Pinchart wrote:
> The qcam log prints one message per frame, which is pretty verbose. This
> feature is useful for debugging, but not necessarily as a default
> option. Silence it by default, and add a -v/--verbose command line
> parameter to make the log verbose.
> 
> While this could have been handled manually by checking a verbose flag
> when printing the message, the feature is instead integrated with the Qt
> log infrastructure to make it more flexible. Messages printed by
> qDebug() are now silenced by default and controlled by the -v/--verbose
> argument.

Seems potentially overkill, but if it works, and gives flexibility later on:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main.cpp            |  5 +++++
>  src/qcam/main_window.cpp     |  2 +-
>  src/qcam/main_window.h       |  2 ++
>  src/qcam/meson.build         |  1 +
>  src/qcam/message_handler.cpp | 27 +++++++++++++++++++++++++++
>  src/qcam/message_handler.h   | 26 ++++++++++++++++++++++++++
>  6 files changed, 62 insertions(+), 1 deletion(-)
>  create mode 100644 src/qcam/message_handler.cpp
>  create mode 100644 src/qcam/message_handler.h
> 
> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> index 5505a5d88ef6..5eff90a3fa66 100644
> --- a/src/qcam/main.cpp
> +++ b/src/qcam/main.cpp
> @@ -16,6 +16,7 @@
>  #include "../cam/options.h"
>  #include "../cam/stream_options.h"
>  #include "main_window.h"
> +#include "message_handler.h"
>  
>  void signalHandler([[maybe_unused]] int signal)
>  {
> @@ -38,6 +39,8 @@ OptionsParser::Options parseOptions(int argc, char *argv[])
>  			 "renderer", ArgumentRequired, "renderer");
>  	parser.addOption(OptStream, &streamKeyValue,
>  			 "Set configuration of a camera stream", "stream", true);
> +	parser.addOption(OptVerbose, OptionNone,
> +			 "Print verbose log messages", "verbose");
>  
>  	OptionsParser::Options options = parser.parse(argc, argv);
>  	if (options.isSet(OptHelp))
> @@ -57,6 +60,8 @@ int main(int argc, char **argv)
>  	if (options.isSet(OptHelp))
>  		return 0;
>  
> +	MessageHandler msgHandler(options.isSet(OptVerbose));
> +
>  	struct sigaction sa = {};
>  	sa.sa_handler = &signalHandler;
>  	sigaction(SIGINT, &sa, nullptr);
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 2502ecd40abe..39d034de6bb2 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -746,7 +746,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  	fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
>  	lastBufferTime_ = metadata.timestamp;
>  
> -	qInfo().noquote()
> +	qDebug().noquote()
>  		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
>  		<< "bytesused:" << metadata.planes[0].bytesused
>  		<< "timestamp:" << metadata.timestamp
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 64bcfebc7dbd..a9aa8ffcd8fd 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -41,6 +41,7 @@ enum {
>  	OptHelp = 'h',
>  	OptRenderer = 'r',
>  	OptStream = 's',
> +	OptVerbose = 'v',
>  };
>  
>  class MainWindow : public QMainWindow
> @@ -98,6 +99,7 @@ private:
>  
>  	/* Options */
>  	const OptionsParser::Options &options_;
> +	bool verbose_;
>  
>  	/* Camera manager, camera, configuration and buffers */
>  	CameraManager *cm_;
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 9bb48c0d06c5..ebcd5ca010cf 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -6,6 +6,7 @@ qcam_sources = files([
>      'format_converter.cpp',
>      'main.cpp',
>      'main_window.cpp',
> +    'message_handler.cpp',
>      'viewfinder_qt.cpp',
>  ])
>  
> diff --git a/src/qcam/message_handler.cpp b/src/qcam/message_handler.cpp
> new file mode 100644
> index 000000000000..261623e19ca9
> --- /dev/null
> +++ b/src/qcam/message_handler.cpp
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * message_handler.cpp - qcam - Log message handling
> + */
> +
> +#include "message_handler.h"
> +
> +QtMessageHandler MessageHandler::handler_ = nullptr;
> +bool MessageHandler::verbose_ = false;
> +
> +MessageHandler::MessageHandler(bool verbose)
> +{
> +	verbose_ = verbose;
> +	handler_ = qInstallMessageHandler(&MessageHandler::handleMessage);
> +}
> +
> +void MessageHandler::handleMessage(QtMsgType type,
> +				   const QMessageLogContext &context,
> +				   const QString &msg)
> +{
> +	if (type == QtDebugMsg && !verbose_)
> +		return;
> +
> +	handler_(type, context, msg);
> +}
> diff --git a/src/qcam/message_handler.h b/src/qcam/message_handler.h
> new file mode 100644
> index 000000000000..4534db9d93f7
> --- /dev/null
> +++ b/src/qcam/message_handler.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * message_handler.cpp - qcam - Log message handling
> + */
> +#ifndef __QCAM_MESSAGE_HANDLER_H__
> +#define __QCAM_MESSAGE_HANDLER_H__
> +
> +#include <QtGlobal>
> +
> +class MessageHandler
> +{
> +public:
> +	MessageHandler(bool verbose);
> +
> +private:
> +	static void handleMessage(QtMsgType type,
> +				  const QMessageLogContext &context,
> +				  const QString &msg);
> +
> +	static QtMessageHandler handler_;
> +	static bool verbose_;
> +};
> +
> +#endif /* __QCAM_MESSAGE_HANDLER_H__ */
>
Jacopo Mondi Nov. 24, 2020, 8:28 p.m. UTC | #2 
Hi Laurent,

On Tue, Nov 24, 2020 at 07:43:42PM +0200, Laurent Pinchart wrote:
> The qcam log prints one message per frame, which is pretty verbose. This
> feature is useful for debugging, but not necessarily as a default
> option. Silence it by default, and add a -v/--verbose command line
> parameter to make the log verbose.
>
> While this could have been handled manually by checking a verbose flag
> when printing the message, the feature is instead integrated with the Qt
> log infrastructure to make it more flexible. Messages printed by
> qDebug() are now silenced by default and controlled by the -v/--verbose
> argument.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main.cpp            |  5 +++++
>  src/qcam/main_window.cpp     |  2 +-
>  src/qcam/main_window.h       |  2 ++
>  src/qcam/meson.build         |  1 +
>  src/qcam/message_handler.cpp | 27 +++++++++++++++++++++++++++
>  src/qcam/message_handler.h   | 26 ++++++++++++++++++++++++++
>  6 files changed, 62 insertions(+), 1 deletion(-)
>  create mode 100644 src/qcam/message_handler.cpp
>  create mode 100644 src/qcam/message_handler.h
>
> diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> index 5505a5d88ef6..5eff90a3fa66 100644
> --- a/src/qcam/main.cpp
> +++ b/src/qcam/main.cpp
> @@ -16,6 +16,7 @@
>  #include "../cam/options.h"
>  #include "../cam/stream_options.h"
>  #include "main_window.h"
> +#include "message_handler.h"
>
>  void signalHandler([[maybe_unused]] int signal)
>  {
> @@ -38,6 +39,8 @@ OptionsParser::Options parseOptions(int argc, char *argv[])
>  			 "renderer", ArgumentRequired, "renderer");
>  	parser.addOption(OptStream, &streamKeyValue,
>  			 "Set configuration of a camera stream", "stream", true);
> +	parser.addOption(OptVerbose, OptionNone,
> +			 "Print verbose log messages", "verbose");
>
>  	OptionsParser::Options options = parser.parse(argc, argv);
>  	if (options.isSet(OptHelp))
> @@ -57,6 +60,8 @@ int main(int argc, char **argv)
>  	if (options.isSet(OptHelp))
>  		return 0;
>
> +	MessageHandler msgHandler(options.isSet(OptVerbose));
> +
>  	struct sigaction sa = {};
>  	sa.sa_handler = &signalHandler;
>  	sigaction(SIGINT, &sa, nullptr);
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 2502ecd40abe..39d034de6bb2 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -746,7 +746,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  	fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
>  	lastBufferTime_ = metadata.timestamp;
>
> -	qInfo().noquote()
> +	qDebug().noquote()
>  		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
>  		<< "bytesused:" << metadata.planes[0].bytesused
>  		<< "timestamp:" << metadata.timestamp
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 64bcfebc7dbd..a9aa8ffcd8fd 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -41,6 +41,7 @@ enum {
>  	OptHelp = 'h',
>  	OptRenderer = 'r',
>  	OptStream = 's',
> +	OptVerbose = 'v',
>  };
>
>  class MainWindow : public QMainWindow
> @@ -98,6 +99,7 @@ private:
>
>  	/* Options */
>  	const OptionsParser::Options &options_;
> +	bool verbose_;

Is this used ?
It seems you pass the result of options.isSet() directly.

>
>  	/* Camera manager, camera, configuration and buffers */
>  	CameraManager *cm_;
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 9bb48c0d06c5..ebcd5ca010cf 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -6,6 +6,7 @@ qcam_sources = files([
>      'format_converter.cpp',
>      'main.cpp',
>      'main_window.cpp',
> +    'message_handler.cpp',
>      'viewfinder_qt.cpp',
>  ])
>
> diff --git a/src/qcam/message_handler.cpp b/src/qcam/message_handler.cpp
> new file mode 100644
> index 000000000000..261623e19ca9
> --- /dev/null
> +++ b/src/qcam/message_handler.cpp
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * message_handler.cpp - qcam - Log message handling
> + */
> +
> +#include "message_handler.h"
> +
> +QtMessageHandler MessageHandler::handler_ = nullptr;
> +bool MessageHandler::verbose_ = false;
> +
> +MessageHandler::MessageHandler(bool verbose)
> +{
> +	verbose_ = verbose;
> +	handler_ = qInstallMessageHandler(&MessageHandler::handleMessage);
> +}
> +
> +void MessageHandler::handleMessage(QtMsgType type,
> +				   const QMessageLogContext &context,
> +				   const QString &msg)
> +{
> +	if (type == QtDebugMsg && !verbose_)
> +		return;
> +
> +	handler_(type, context, msg);
> +}
> diff --git a/src/qcam/message_handler.h b/src/qcam/message_handler.h
> new file mode 100644
> index 000000000000..4534db9d93f7
> --- /dev/null
> +++ b/src/qcam/message_handler.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + *
> + * message_handler.cpp - qcam - Log message handling

s/.cpp/.h

> + */
> +#ifndef __QCAM_MESSAGE_HANDLER_H__
> +#define __QCAM_MESSAGE_HANDLER_H__
> +
> +#include <QtGlobal>
> +
> +class MessageHandler
> +{
> +public:
> +	MessageHandler(bool verbose);
> +
> +private:
> +	static void handleMessage(QtMsgType type,
> +				  const QMessageLogContext &context,
> +				  const QString &msg);
> +
> +	static QtMessageHandler handler_;
> +	static bool verbose_;
> +};
> +
> +#endif /* __QCAM_MESSAGE_HANDLER_H__ */

As Kieran said, potentially an overkill, but if we want more control
we can filter on the QtMsgType with this

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
index 5505a5d88ef6..5eff90a3fa66 100644
--- a/src/qcam/main.cpp
+++ b/src/qcam/main.cpp
@@ -16,6 +16,7 @@ 
 #include "../cam/options.h"
 #include "../cam/stream_options.h"
 #include "main_window.h"
+#include "message_handler.h"
 
 void signalHandler([[maybe_unused]] int signal)
 {
@@ -38,6 +39,8 @@  OptionsParser::Options parseOptions(int argc, char *argv[])
 			 "renderer", ArgumentRequired, "renderer");
 	parser.addOption(OptStream, &streamKeyValue,
 			 "Set configuration of a camera stream", "stream", true);
+	parser.addOption(OptVerbose, OptionNone,
+			 "Print verbose log messages", "verbose");
 
 	OptionsParser::Options options = parser.parse(argc, argv);
 	if (options.isSet(OptHelp))
@@ -57,6 +60,8 @@  int main(int argc, char **argv)
 	if (options.isSet(OptHelp))
 		return 0;
 
+	MessageHandler msgHandler(options.isSet(OptVerbose));
+
 	struct sigaction sa = {};
 	sa.sa_handler = &signalHandler;
 	sigaction(SIGINT, &sa, nullptr);
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 2502ecd40abe..39d034de6bb2 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -746,7 +746,7 @@  void MainWindow::processViewfinder(FrameBuffer *buffer)
 	fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0;
 	lastBufferTime_ = metadata.timestamp;
 
-	qInfo().noquote()
+	qDebug().noquote()
 		<< QString("seq: %1").arg(metadata.sequence, 6, 10, QLatin1Char('0'))
 		<< "bytesused:" << metadata.planes[0].bytesused
 		<< "timestamp:" << metadata.timestamp
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 64bcfebc7dbd..a9aa8ffcd8fd 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -41,6 +41,7 @@  enum {
 	OptHelp = 'h',
 	OptRenderer = 'r',
 	OptStream = 's',
+	OptVerbose = 'v',
 };
 
 class MainWindow : public QMainWindow
@@ -98,6 +99,7 @@  private:
 
 	/* Options */
 	const OptionsParser::Options &options_;
+	bool verbose_;
 
 	/* Camera manager, camera, configuration and buffers */
 	CameraManager *cm_;
diff --git a/src/qcam/meson.build b/src/qcam/meson.build
index 9bb48c0d06c5..ebcd5ca010cf 100644
--- a/src/qcam/meson.build
+++ b/src/qcam/meson.build
@@ -6,6 +6,7 @@  qcam_sources = files([
     'format_converter.cpp',
     'main.cpp',
     'main_window.cpp',
+    'message_handler.cpp',
     'viewfinder_qt.cpp',
 ])
 
diff --git a/src/qcam/message_handler.cpp b/src/qcam/message_handler.cpp
new file mode 100644
index 000000000000..261623e19ca9
--- /dev/null
+++ b/src/qcam/message_handler.cpp
@@ -0,0 +1,27 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * message_handler.cpp - qcam - Log message handling
+ */
+
+#include "message_handler.h"
+
+QtMessageHandler MessageHandler::handler_ = nullptr;
+bool MessageHandler::verbose_ = false;
+
+MessageHandler::MessageHandler(bool verbose)
+{
+	verbose_ = verbose;
+	handler_ = qInstallMessageHandler(&MessageHandler::handleMessage);
+}
+
+void MessageHandler::handleMessage(QtMsgType type,
+				   const QMessageLogContext &context,
+				   const QString &msg)
+{
+	if (type == QtDebugMsg && !verbose_)
+		return;
+
+	handler_(type, context, msg);
+}
diff --git a/src/qcam/message_handler.h b/src/qcam/message_handler.h
new file mode 100644
index 000000000000..4534db9d93f7
--- /dev/null
+++ b/src/qcam/message_handler.h
@@ -0,0 +1,26 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020, Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+ *
+ * message_handler.cpp - qcam - Log message handling
+ */
+#ifndef __QCAM_MESSAGE_HANDLER_H__
+#define __QCAM_MESSAGE_HANDLER_H__
+
+#include <QtGlobal>
+
+class MessageHandler
+{
+public:
+	MessageHandler(bool verbose);
+
+private:
+	static void handleMessage(QtMsgType type,
+				  const QMessageLogContext &context,
+				  const QString &msg);
+
+	static QtMessageHandler handler_;
+	static bool verbose_;
+};
+
+#endif /* __QCAM_MESSAGE_HANDLER_H__ */