[libcamera-devel] [PATCH 3/3] qcam: Make log less verbose by default

[Jacopo Mondi]

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 at 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 at 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 at 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 at jmondi.org>

Thanks
  j

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