[{"id":23155,"web_url":"https://patchwork.libcamera.org/comment/23155/","msgid":"","date":"2022-05-26T09:09:17","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Extend the logger to support coloring messages. The log level is\n> colorized with per-level colors, and the category with a fixed color.\n> This makes the log output more readable.\n> \n> Coloring is enabled by default when logging to std::cerr, and can be\n> disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> can be enabled for file logging using the logSetFile() function.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/environment_variables.rst | 21 +++--\n> include/libcamera/logging.h | 4 +-\n> src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> src/libcamera/camera_manager.cpp | 2 +-\n> 4 files changed, 109 insertions(+), 32 deletions(-)\n> \n> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> index fa703a726845..7028c024b14e 100644\n> --- a/Documentation/environment_variables.rst\n> +++ b/Documentation/environment_variables.rst\n> @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> \n> Example value: ``*:DEBUG``\n> \n> +LIBCAMERA_LOG_NO_COLOR\n> + Disable coloring of log messages (`more `__).\n> +\n> LIBCAMERA_IPA_CONFIG_PATH\n> Define custom search locations for IPA configurations (`more `__).\n> \n> @@ -40,12 +43,20 @@ Further details\n> Notes about debugging\n> ~~~~~~~~~~~~~~~~~~~~~\n> \n> -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> -are used to modify the destination and verbosity of messages provided by\n> -libcamera.\n> +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> +libcamera logger.\n> \n> -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> -'category:level' pairs.\n> +By default, libcamera logs all messages to the standard error (std::cerr).\n> +Messages are colored by default depending on the log level. Coloring can be\n> +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> +\n> +The default log destination can also be directed to a file by setting the\n> +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> +disables coloring.\n> +\n> +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> +accepts a comma-separated list of 'category:level' pairs.\n> \n> The `level `__ part is mandatory and can either be specified by\n> name or by numerical index associated with each level.\n> diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> index c36882b91974..cd842f67d553 100644\n> --- a/include/libcamera/logging.h\n> +++ b/include/libcamera/logging.h\n> @@ -16,8 +16,8 @@ enum LoggingTarget {\n> \tLoggingTargetStream,\n> };\n> \n> -int logSetFile(const char *path);\n> -int logSetStream(std::ostream *stream);\n> +int logSetFile(const char *path, bool color = false);\n> +int logSetStream(std::ostream *stream, bool color = false);\n> int logSetTarget(LoggingTarget target);\n> void logSetLevel(const char *category, const char *level);\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 26f1420703b9..a9f5bbbd36f7 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> class LogOutput\n> {\n> public:\n> -\tLogOutput(const char *path);\n> -\tLogOutput(std::ostream *stream);\n> +\tLogOutput(const char *path, bool color);\n> +\tLogOutput(std::ostream *stream, bool color);\n> \tLogOutput();\n> \t~LogOutput();\n> \n> @@ -119,14 +119,16 @@ private:\n> \n> \tstd::ostream *stream_;\n> \tLoggingTarget target_;\n> +\tbool color_;\n> };\n> \n> /**\n> * \\brief Construct a log output based on a file\n> * \\param[in] path Full path to log file\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(const char *path)\n> -\t: target_(LoggingTargetFile)\n> +LogOutput::LogOutput(const char *path, bool color)\n> +\t: target_(LoggingTargetFile), color_(color)\n> {\n> \tstream_ = new std::ofstream(path);\n> }\n> @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> /**\n> * \\brief Construct a log output based on a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(std::ostream *stream)\n> -\t: stream_(stream), target_(LoggingTargetStream)\n> +LogOutput::LogOutput(std::ostream *stream, bool color)\n> +\t: stream_(stream), target_(LoggingTargetStream), color_(color)\n> {\n> }\n> \n> @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> * \\brief Construct a log output to syslog\n> */\n> LogOutput::LogOutput()\n> -\t: stream_(nullptr), target_(LoggingTargetSyslog)\n> +\t: stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> {\n> \topenlog(\"libcamera\", LOG_PID, 0);\n> }\n> @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> \t}\n> }\n> \n> +namespace {\n> +\n> +constexpr const char *kColorReset = \"\\033[0m\";\n> +constexpr const char *kColorRed = \"\\033[31m\";\n\nclang complains about unused variables, so I'll have to drop a few, or\nadd [[maybe_unused]]. I think I'll go for the former in this case.\n\n> +constexpr const char *kColorGreen = \"\\033[32m\";\n> +constexpr const char *kColorYellow = \"\\033[33m\";\n> +constexpr const char *kColorBlue = \"\\033[34m\";\n> +constexpr const char *kColorMagenta = \"\\033[35m\";\n> +constexpr const char *kColorCyan = \"\\033[36m\";\n> +constexpr const char *kColorWhite = \"\\033[37m\";\n> +constexpr const char *kColorGrey = \"\\033[90m\";\n> +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> +\n> +} /* namespace */\n> +\n> /**\n> * \\brief Write message to log output\n> * \\param[in] msg Message to write\n> */\n> void LogOutput::write(const LogMessage &msg)\n> {\n> +\tstatic const char *const severityColors[] = {\n> +\t\tkColorBrightCyan,\n> +\t\tkColorBrightGreen,\n> +\t\tkColorBrightYellow,\n> +\t\tkColorBrightRed,\n> +\t\tkColorBrightMagenta,\n> +\t};\n> +\n> +\tconst char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> +\tconst char *fileColor = color_ ? kColorBrightBlue : \"\";\n> +\tconst char *msgColor = color_ ? kColorReset : \"\";\n> +\tconst char *severityColor = \"\";\n> +\tLogSeverity severity = msg.severity();\n> \tstd::string str;\n> \n> +\tif (color_) {\n> +\t\tif (static_cast(severity) < std::size(severityColors))\n> +\t\t\tseverityColor = severityColors[severity];\n> +\t\telse\n> +\t\t\tseverityColor = kColorBrightWhite;\n> +\t}\n> +\n> \tswitch (target_) {\n> \tcase LoggingTargetSyslog:\n> -\t\tstr = std::string(log_severity_name(msg.severity())) + \" \"\n> +\t\tstr = std::string(log_severity_name(severity)) + \" \"\n> \t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> \t\t + msg.msg();\n> -\t\twriteSyslog(msg.severity(), str);\n> +\t\twriteSyslog(severity, str);\n> \t\tbreak;\n> \tcase LoggingTargetStream:\n> \tcase LoggingTargetFile:\n> \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> \t\t + std::to_string(Thread::currentId()) + \"] \"\n> -\t\t + log_severity_name(msg.severity()) + \" \"\n> -\t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> -\t\t + msg.msg();\n> +\t\t + severityColor + log_severity_name(severity) + \" \"\n> +\t\t + categoryColor + msg.category().name() + \" \"\n> +\t\t + fileColor + msg.fileInfo() + \" \"\n> +\t\t + msgColor + msg.msg();\n> \t\twriteStream(str);\n> \t\tbreak;\n> \tdefault:\n> @@ -253,8 +298,8 @@ public:\n> \tvoid write(const LogMessage &msg);\n> \tvoid backtrace();\n> \n> -\tint logSetFile(const char *path);\n> -\tint logSetStream(std::ostream *stream);\n> +\tint logSetFile(const char *path, bool color);\n> +\tint logSetStream(std::ostream *stream, bool color);\n> \tint logSetTarget(LoggingTarget target);\n> \tvoid logSetLevel(const char *category, const char *level);\n> \n> @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> /**\n> * \\brief Direct logging to a file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to the file identified by \\a path. The\n> * previous log target, if any, is closed, and all new log messages will be\n> * written to the new log file.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log target is not changed.\n> *\n> * \\return Zero on success, or a negative error code otherwise\n> */\n> -int logSetFile(const char *path)\n> +int logSetFile(const char *path, bool color)\n> {\n> -\treturn Logger::instance()->logSetFile(path);\n> +\treturn Logger::instance()->logSetFile(path, color);\n> }\n> \n> /**\n> * \\brief Direct logging to a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to \\a stream. The previous log target,\n> * if any, is closed, and all new log messages will be written to the new log\n> * stream.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log file is not changed\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int logSetStream(std::ostream *stream)\n> +int logSetStream(std::ostream *stream, bool color)\n> {\n> -\treturn Logger::instance()->logSetStream(stream);\n> +\treturn Logger::instance()->logSetStream(stream, color);\n> }\n> \n> /**\n> @@ -437,14 +494,16 @@ void Logger::backtrace()\n> /**\n> * \\brief Set the log file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetFile()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetFile(const char *path)\n> +int Logger::logSetFile(const char *path, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(path);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(path, color);\n> \tif (!output->isValid())\n> \t\treturn -EINVAL;\n> \n> @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> /**\n> * \\brief Set the log stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetStream()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetStream(std::ostream *stream)\n> +int Logger::logSetStream(std::ostream *stream, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(stream);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(stream, color);\n> \tstd::atomic_store(&output_, output);\n> \treturn 0;\n> }\n> @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> \n> /**\n> * \\brief Construct a logger\n> + *\n> + * If the environment variable is not set, log to std::cerr. The log messages\n> + * are then colored by default. This can be overridden by setting the\n> + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> */\n> Logger::Logger()\n> {\n> -\tlogSetStream(&std::cerr);\n> +\tbool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> +\tlogSetStream(&std::cerr, color);\n> \n> \tparseLogFile();\n> \tparseLogLevels();\n> @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> \t\treturn;\n> \t}\n> \n> -\tlogSetFile(file);\n> +\tlogSetFile(file, false);\n> }\n> \n> /**\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 70d73822193b..d934596e4145 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> */\n> int CameraManager::start()\n> {\n> -\tLOG(Camera, Info) << \"libcamera \" << version_;\n> +\tLOG(Camera, Debug) << \"libcamera \" << version_;\n> \n> \tint ret = _d()->start();","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 44AF5BD161\n\tfor ;\n\tThu, 26 May 2022 09:09:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DE6765661;\n\tThu, 26 May 2022 11:09:25 +0200 (CEST)","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 4D5B86565D\n\tfor ;\n\tThu, 26 May 2022 11:09:24 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [37.120.212.14])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0E276D4\n\tfor ;\n\tThu, 26 May 2022 11:09:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653556165;\n\tbh=ZcAvJ8lroNBcGkVG1jMxXffnMF0lGi1RWLfXDwqLZHE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=w/zlBErbo3tdUZReaG0TjjeCnUA4DoUxW2Kx832GaZ3FpjWOw6hiNlPAJ7SzyCYXP\n\tjsEyadoxRC38gsZBMiNvgRWp8Dq+X2E4E5CZjn75OCLJ0BMsPkiscXz+QLJQQMuewL\n\tvh/ZMyclvLDvgwhtiHBJ7uHZc3CIczPtVIgb4FUZSzSJnMVKRB/sGlQ4ewwV3A+AIw\n\tJaOCKfeoBkSfd2RdyfIEOUiNP9csU73bjDGyRrKvABDBoWSyrKiJ42O1bL8CTz9C5y\n\tnjy67S86DBCCrnIyjLEZqz/2BxgUkloYhTi9ZyB1kgzh76ekmajbUJudMTj0Q0OO+F\n\tbUoHnVTb2yzQw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653556163;\n\tbh=ZcAvJ8lroNBcGkVG1jMxXffnMF0lGi1RWLfXDwqLZHE=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=QIbIYOD9TWoC0700MIkqy+8h8rocK3LIdOxXkoVyYmz9Vig1AJy6A6ONzujeav1WW\n\tA8CB7UM1nkWF7Wu0QKi5iNN2YKL8B9AKn1j+yFaX+PhBfWxRvwvWDnQgicX9RYLY4v\n\trma0Rsjm8pufnkvznLyTLjNAW/lQz91+4N6VSs8g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QIbIYOD9\"; dkim-atps=neutral","Date":"Thu, 26 May 2022 12:09:17 +0300","To":"libcamera-devel@lists.libcamera.org","Message-ID":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23205,"web_url":"https://patchwork.libcamera.org/comment/23205/","msgid":"<165377708944.2093890.15129535625982732642@Monstersaurus>","date":"2022-05-28T22:31:29","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-05-26 10:09:17)\n> On Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Extend the logger to support coloring messages. The log level is\n> > colorized with per-level colors, and the category with a fixed color.\n> > This makes the log output more readable.\n> > \n> > Coloring is enabled by default when logging to std::cerr, and can be\n> > disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> > When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> > can be enabled for file logging using the logSetFile() function.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/environment_variables.rst | 21 +++--\n> > include/libcamera/logging.h | 4 +-\n> > src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> > src/libcamera/camera_manager.cpp | 2 +-\n> > 4 files changed, 109 insertions(+), 32 deletions(-)\n> > \n> > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > index fa703a726845..7028c024b14e 100644\n> > --- a/Documentation/environment_variables.rst\n> > +++ b/Documentation/environment_variables.rst\n> > @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> > \n> > Example value: ``*:DEBUG``\n> > \n> > +LIBCAMERA_LOG_NO_COLOR\n> > + Disable coloring of log messages (`more `__).\n> > +\n> > LIBCAMERA_IPA_CONFIG_PATH\n> > Define custom search locations for IPA configurations (`more `__).\n> > \n> > @@ -40,12 +43,20 @@ Further details\n> > Notes about debugging\n> > ~~~~~~~~~~~~~~~~~~~~~\n> > \n> > -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> > -are used to modify the destination and verbosity of messages provided by\n> > -libcamera.\n> > +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> > +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> > +libcamera logger.\n> > \n> > -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> > -'category:level' pairs.\n> > +By default, libcamera logs all messages to the standard error (std::cerr).\n> > +Messages are colored by default depending on the log level. Coloring can be\n> > +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> > +\n> > +The default log destination can also be directed to a file by setting the\n> > +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> > +disables coloring.\n> > +\n> > +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> > +accepts a comma-separated list of 'category:level' pairs.\n> > \n> > The `level `__ part is mandatory and can either be specified by\n> > name or by numerical index associated with each level.\n> > diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> > index c36882b91974..cd842f67d553 100644\n> > --- a/include/libcamera/logging.h\n> > +++ b/include/libcamera/logging.h\n> > @@ -16,8 +16,8 @@ enum LoggingTarget {\n> > LoggingTargetStream,\n> > };\n> > \n> > -int logSetFile(const char *path);\n> > -int logSetStream(std::ostream *stream);\n> > +int logSetFile(const char *path, bool color = false);\n> > +int logSetStream(std::ostream *stream, bool color = false);\n> > int logSetTarget(LoggingTarget target);\n> > void logSetLevel(const char *category, const char *level);\n> > \n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 26f1420703b9..a9f5bbbd36f7 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> > class LogOutput\n> > {\n> > public:\n> > - LogOutput(const char *path);\n> > - LogOutput(std::ostream *stream);\n> > + LogOutput(const char *path, bool color);\n> > + LogOutput(std::ostream *stream, bool color);\n> > LogOutput();\n> > ~LogOutput();\n> > \n> > @@ -119,14 +119,16 @@ private:\n> > \n> > std::ostream *stream_;\n> > LoggingTarget target_;\n> > + bool color_;\n> > };\n> > \n> > /**\n> > * \\brief Construct a log output based on a file\n> > * \\param[in] path Full path to log file\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(const char *path)\n> > - : target_(LoggingTargetFile)\n> > +LogOutput::LogOutput(const char *path, bool color)\n> > + : target_(LoggingTargetFile), color_(color)\n> > {\n> > stream_ = new std::ofstream(path);\n> > }\n> > @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> > /**\n> > * \\brief Construct a log output based on a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(std::ostream *stream)\n> > - : stream_(stream), target_(LoggingTargetStream)\n> > +LogOutput::LogOutput(std::ostream *stream, bool color)\n> > + : stream_(stream), target_(LoggingTargetStream), color_(color)\n> > {\n> > }\n> > \n> > @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> > * \\brief Construct a log output to syslog\n> > */\n> > LogOutput::LogOutput()\n> > - : stream_(nullptr), target_(LoggingTargetSyslog)\n> > + : stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> > {\n> > openlog(\"libcamera\", LOG_PID, 0);\n> > }\n> > @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> > }\n> > }\n> > \n> > +namespace {\n> > +\n> > +constexpr const char *kColorReset = \"\\033[0m\";\n> > +constexpr const char *kColorRed = \"\\033[31m\";\n> \n> clang complains about unused variables, so I'll have to drop a few, or\n> add [[maybe_unused]]. I think I'll go for the former in this case.\n> \n\nHaving these as handy to reference would help for anyone who wants to\ntweak the colour schemes.\n\nI'd almost go as far to say maybe they should be #define'd to keep the\ncompiler ignorant and still available ...\n\nOr pragma out the unused variable warning just for this table...\n\n--\nKieran\n\n\n\n> > +constexpr const char *kColorGreen = \"\\033[32m\";\n> > +constexpr const char *kColorYellow = \"\\033[33m\";\n> > +constexpr const char *kColorBlue = \"\\033[34m\";\n> > +constexpr const char *kColorMagenta = \"\\033[35m\";\n> > +constexpr const char *kColorCyan = \"\\033[36m\";\n> > +constexpr const char *kColorWhite = \"\\033[37m\";\n> > +constexpr const char *kColorGrey = \"\\033[90m\";\n> > +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> > +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> > +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> > +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> > +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> > +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> > +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> > +\n> > +} /* namespace */\n> > +\n> > /**\n> > * \\brief Write message to log output\n> > * \\param[in] msg Message to write\n> > */\n> > void LogOutput::write(const LogMessage &msg)\n> > {\n> > + static const char *const severityColors[] = {\n> > + kColorBrightCyan,\n> > + kColorBrightGreen,\n> > + kColorBrightYellow,\n> > + kColorBrightRed,\n> > + kColorBrightMagenta,\n> > + };\n> > +\n> > + const char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> > + const char *fileColor = color_ ? kColorBrightBlue : \"\";\n> > + const char *msgColor = color_ ? kColorReset : \"\";\n> > + const char *severityColor = \"\";\n> > + LogSeverity severity = msg.severity();\n> > std::string str;\n> > \n> > + if (color_) {\n> > + if (static_cast(severity) < std::size(severityColors))\n> > + severityColor = severityColors[severity];\n> > + else\n> > + severityColor = kColorBrightWhite;\n> > + }\n> > +\n> > switch (target_) {\n> > case LoggingTargetSyslog:\n> > - str = std::string(log_severity_name(msg.severity())) + \" \"\n> > + str = std::string(log_severity_name(severity)) + \" \"\n> > + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > + msg.msg();\n> > - writeSyslog(msg.severity(), str);\n> > + writeSyslog(severity, str);\n> > break;\n> > case LoggingTargetStream:\n> > case LoggingTargetFile:\n> > str = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > + std::to_string(Thread::currentId()) + \"] \"\n> > - + log_severity_name(msg.severity()) + \" \"\n> > - + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > - + msg.msg();\n> > + + severityColor + log_severity_name(severity) + \" \"\n> > + + categoryColor + msg.category().name() + \" \"\n> > + + fileColor + msg.fileInfo() + \" \"\n> > + + msgColor + msg.msg();\n> > writeStream(str);\n> > break;\n> > default:\n> > @@ -253,8 +298,8 @@ public:\n> > void write(const LogMessage &msg);\n> > void backtrace();\n> > \n> > - int logSetFile(const char *path);\n> > - int logSetStream(std::ostream *stream);\n> > + int logSetFile(const char *path, bool color);\n> > + int logSetStream(std::ostream *stream, bool color);\n> > int logSetTarget(LoggingTarget target);\n> > void logSetLevel(const char *category, const char *level);\n> > \n> > @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> > /**\n> > * \\brief Direct logging to a file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to the file identified by \\a path. The\n> > * previous log target, if any, is closed, and all new log messages will be\n> > * written to the new log file.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log target is not changed.\n> > *\n> > * \\return Zero on success, or a negative error code otherwise\n> > */\n> > -int logSetFile(const char *path)\n> > +int logSetFile(const char *path, bool color)\n> > {\n> > - return Logger::instance()->logSetFile(path);\n> > + return Logger::instance()->logSetFile(path, color);\n> > }\n> > \n> > /**\n> > * \\brief Direct logging to a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to \\a stream. The previous log target,\n> > * if any, is closed, and all new log messages will be written to the new log\n> > * stream.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log file is not changed\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int logSetStream(std::ostream *stream)\n> > +int logSetStream(std::ostream *stream, bool color)\n> > {\n> > - return Logger::instance()->logSetStream(stream);\n> > + return Logger::instance()->logSetStream(stream, color);\n> > }\n> > \n> > /**\n> > @@ -437,14 +494,16 @@ void Logger::backtrace()\n> > /**\n> > * \\brief Set the log file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetFile()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetFile(const char *path)\n> > +int Logger::logSetFile(const char *path, bool color)\n> > {\n> > - std::shared_ptr output = std::make_shared(path);\n> > + std::shared_ptr output =\n> > + std::make_shared(path, color);\n> > if (!output->isValid())\n> > return -EINVAL;\n> > \n> > @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> > /**\n> > * \\brief Set the log stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetStream()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetStream(std::ostream *stream)\n> > +int Logger::logSetStream(std::ostream *stream, bool color)\n> > {\n> > - std::shared_ptr output = std::make_shared(stream);\n> > + std::shared_ptr output =\n> > + std::make_shared(stream, color);\n> > std::atomic_store(&output_, output);\n> > return 0;\n> > }\n> > @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > \n> > /**\n> > * \\brief Construct a logger\n> > + *\n> > + * If the environment variable is not set, log to std::cerr. The log messages\n> > + * are then colored by default. This can be overridden by setting the\n> > + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> > */\n> > Logger::Logger()\n> > {\n> > - logSetStream(&std::cerr);\n> > + bool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> > + logSetStream(&std::cerr, color);\n> > \n> > parseLogFile();\n> > parseLogLevels();\n> > @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> > return;\n> > }\n> > \n> > - logSetFile(file);\n> > + logSetFile(file, false);\n> > }\n> > \n> > /**\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 70d73822193b..d934596e4145 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> > */\n> > int CameraManager::start()\n> > {\n> > - LOG(Camera, Info) << \"libcamera \" << version_;\n> > + LOG(Camera, Debug) << \"libcamera \" << version_;\n> > \n> > int ret = _d()->start();\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A798EBD160\n\tfor ;\n\tSat, 28 May 2022 22:31:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E210165633;\n\tSun, 29 May 2022 00:31:34 +0200 (CEST)","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 C7B9B633A1\n\tfor ;\n\tSun, 29 May 2022 00:31:32 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30E4087A;\n\tSun, 29 May 2022 00:31:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653777094;\n\tbh=w+QZ3SU0sMVOo1Ptt1nI8ZOXOsCZps62Y8xS+ANfJ0M=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=TxdwwxzABWFrE0zVAxG5MFo2VETUhSOphFrOubbyEXo0c+5Z3fpHQLhmbhPfNuK3K\n\tDGbLXwH9K1ocJ1a5hYO0v4SfzcR0JcL209kUTUiwIgFIFc72/lD7F/T6/oUXI2QOgc\n\tJQ1QaH99cm4SKEgxfW317BjWleIIDSIiLqPDpRf2lWPU3piRKnX7/pc8BEPmnSMzsb\n\tCoVu+CBPwd0LMcgEpQJOkRKGSjOn97OFNqznsAFj/nMuVMHjA05taY7BMR1On6S/0I\n\t6M2ZIYAtI7cyPE8wMNYx+vaduKgkqTpSXHA3nBhl13aNW/o+e+sA1hkUTt9x5D6oNH\n\txgQDLkpISNyJA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653777092;\n\tbh=w+QZ3SU0sMVOo1Ptt1nI8ZOXOsCZps62Y8xS+ANfJ0M=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=iEB7g/c4d2n7vcFjfjIxVNaEGcG/LiphU49pJhtlR3YO4MqXyndmHNJvNky0QLYsq\n\twFOQg+I3L7DaxuNEfJoYRKNuTzPu/4BJYcwdejzOsozoFcv8ELy8DKayiDFAdAdQlJ\n\tQI/vkxpWWaVMJpy3liSVzz/4tm6uF20ByeDp2b3c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iEB7g/c4\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>\n\t","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Sat, 28 May 2022 23:31:29 +0100","Message-ID":"<165377708944.2093890.15129535625982732642@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23207,"web_url":"https://patchwork.libcamera.org/comment/23207/","msgid":"","date":"2022-05-29T08:59:09","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Sat, May 28, 2022 at 11:31:29PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-05-26 10:09:17)\n> > On Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > Extend the logger to support coloring messages. The log level is\n> > > colorized with per-level colors, and the category with a fixed color.\n> > > This makes the log output more readable.\n> > > \n> > > Coloring is enabled by default when logging to std::cerr, and can be\n> > > disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> > > When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> > > can be enabled for file logging using the logSetFile() function.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > Documentation/environment_variables.rst | 21 +++--\n> > > include/libcamera/logging.h | 4 +-\n> > > src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> > > src/libcamera/camera_manager.cpp | 2 +-\n> > > 4 files changed, 109 insertions(+), 32 deletions(-)\n> > > \n> > > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > > index fa703a726845..7028c024b14e 100644\n> > > --- a/Documentation/environment_variables.rst\n> > > +++ b/Documentation/environment_variables.rst\n> > > @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> > > \n> > > Example value: ``*:DEBUG``\n> > > \n> > > +LIBCAMERA_LOG_NO_COLOR\n> > > + Disable coloring of log messages (`more `__).\n> > > +\n> > > LIBCAMERA_IPA_CONFIG_PATH\n> > > Define custom search locations for IPA configurations (`more `__).\n> > > \n> > > @@ -40,12 +43,20 @@ Further details\n> > > Notes about debugging\n> > > ~~~~~~~~~~~~~~~~~~~~~\n> > > \n> > > -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> > > -are used to modify the destination and verbosity of messages provided by\n> > > -libcamera.\n> > > +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> > > +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> > > +libcamera logger.\n> > > \n> > > -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> > > -'category:level' pairs.\n> > > +By default, libcamera logs all messages to the standard error (std::cerr).\n> > > +Messages are colored by default depending on the log level. Coloring can be\n> > > +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> > > +\n> > > +The default log destination can also be directed to a file by setting the\n> > > +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> > > +disables coloring.\n> > > +\n> > > +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> > > +accepts a comma-separated list of 'category:level' pairs.\n> > > \n> > > The `level `__ part is mandatory and can either be specified by\n> > > name or by numerical index associated with each level.\n> > > diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> > > index c36882b91974..cd842f67d553 100644\n> > > --- a/include/libcamera/logging.h\n> > > +++ b/include/libcamera/logging.h\n> > > @@ -16,8 +16,8 @@ enum LoggingTarget {\n> > > LoggingTargetStream,\n> > > };\n> > > \n> > > -int logSetFile(const char *path);\n> > > -int logSetStream(std::ostream *stream);\n> > > +int logSetFile(const char *path, bool color = false);\n> > > +int logSetStream(std::ostream *stream, bool color = false);\n> > > int logSetTarget(LoggingTarget target);\n> > > void logSetLevel(const char *category, const char *level);\n> > > \n> > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > > index 26f1420703b9..a9f5bbbd36f7 100644\n> > > --- a/src/libcamera/base/log.cpp\n> > > +++ b/src/libcamera/base/log.cpp\n> > > @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> > > class LogOutput\n> > > {\n> > > public:\n> > > - LogOutput(const char *path);\n> > > - LogOutput(std::ostream *stream);\n> > > + LogOutput(const char *path, bool color);\n> > > + LogOutput(std::ostream *stream, bool color);\n> > > LogOutput();\n> > > ~LogOutput();\n> > > \n> > > @@ -119,14 +119,16 @@ private:\n> > > \n> > > std::ostream *stream_;\n> > > LoggingTarget target_;\n> > > + bool color_;\n> > > };\n> > > \n> > > /**\n> > > * \\brief Construct a log output based on a file\n> > > * \\param[in] path Full path to log file\n> > > + * \\param[in] color True to output colored messages\n> > > */\n> > > -LogOutput::LogOutput(const char *path)\n> > > - : target_(LoggingTargetFile)\n> > > +LogOutput::LogOutput(const char *path, bool color)\n> > > + : target_(LoggingTargetFile), color_(color)\n> > > {\n> > > stream_ = new std::ofstream(path);\n> > > }\n> > > @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> > > /**\n> > > * \\brief Construct a log output based on a stream\n> > > * \\param[in] stream Stream to send log output to\n> > > + * \\param[in] color True to output colored messages\n> > > */\n> > > -LogOutput::LogOutput(std::ostream *stream)\n> > > - : stream_(stream), target_(LoggingTargetStream)\n> > > +LogOutput::LogOutput(std::ostream *stream, bool color)\n> > > + : stream_(stream), target_(LoggingTargetStream), color_(color)\n> > > {\n> > > }\n> > > \n> > > @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> > > * \\brief Construct a log output to syslog\n> > > */\n> > > LogOutput::LogOutput()\n> > > - : stream_(nullptr), target_(LoggingTargetSyslog)\n> > > + : stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> > > {\n> > > openlog(\"libcamera\", LOG_PID, 0);\n> > > }\n> > > @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> > > }\n> > > }\n> > > \n> > > +namespace {\n> > > +\n> > > +constexpr const char *kColorReset = \"\\033[0m\";\n> > > +constexpr const char *kColorRed = \"\\033[31m\";\n> > \n> > clang complains about unused variables, so I'll have to drop a few, or\n> > add [[maybe_unused]]. I think I'll go for the former in this case.\n> \n> Having these as handy to reference would help for anyone who wants to\n> tweak the colour schemes.\n> \n> I'd almost go as far to say maybe they should be #define'd to keep the\n> compiler ignorant and still available ...\n> \n> Or pragma out the unused variable warning just for this table...\n\nI've thought about all that, but I'm not sure if it's worth it given how\neasy the information is to find:\nhttps://en.wikipedia.org/wiki/ANSI_escape_code#Colors.\n\n> > > +constexpr const char *kColorGreen = \"\\033[32m\";\n> > > +constexpr const char *kColorYellow = \"\\033[33m\";\n> > > +constexpr const char *kColorBlue = \"\\033[34m\";\n> > > +constexpr const char *kColorMagenta = \"\\033[35m\";\n> > > +constexpr const char *kColorCyan = \"\\033[36m\";\n> > > +constexpr const char *kColorWhite = \"\\033[37m\";\n> > > +constexpr const char *kColorGrey = \"\\033[90m\";\n> > > +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> > > +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> > > +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> > > +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> > > +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> > > +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> > > +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> > > +\n> > > +} /* namespace */\n> > > +\n> > > /**\n> > > * \\brief Write message to log output\n> > > * \\param[in] msg Message to write\n> > > */\n> > > void LogOutput::write(const LogMessage &msg)\n> > > {\n> > > + static const char *const severityColors[] = {\n> > > + kColorBrightCyan,\n> > > + kColorBrightGreen,\n> > > + kColorBrightYellow,\n> > > + kColorBrightRed,\n> > > + kColorBrightMagenta,\n> > > + };\n> > > +\n> > > + const char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> > > + const char *fileColor = color_ ? kColorBrightBlue : \"\";\n> > > + const char *msgColor = color_ ? kColorReset : \"\";\n> > > + const char *severityColor = \"\";\n> > > + LogSeverity severity = msg.severity();\n> > > std::string str;\n> > > \n> > > + if (color_) {\n> > > + if (static_cast(severity) < std::size(severityColors))\n> > > + severityColor = severityColors[severity];\n> > > + else\n> > > + severityColor = kColorBrightWhite;\n> > > + }\n> > > +\n> > > switch (target_) {\n> > > case LoggingTargetSyslog:\n> > > - str = std::string(log_severity_name(msg.severity())) + \" \"\n> > > + str = std::string(log_severity_name(severity)) + \" \"\n> > > + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > > + msg.msg();\n> > > - writeSyslog(msg.severity(), str);\n> > > + writeSyslog(severity, str);\n> > > break;\n> > > case LoggingTargetStream:\n> > > case LoggingTargetFile:\n> > > str = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > > + std::to_string(Thread::currentId()) + \"] \"\n> > > - + log_severity_name(msg.severity()) + \" \"\n> > > - + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > > - + msg.msg();\n> > > + + severityColor + log_severity_name(severity) + \" \"\n> > > + + categoryColor + msg.category().name() + \" \"\n> > > + + fileColor + msg.fileInfo() + \" \"\n> > > + + msgColor + msg.msg();\n> > > writeStream(str);\n> > > break;\n> > > default:\n> > > @@ -253,8 +298,8 @@ public:\n> > > void write(const LogMessage &msg);\n> > > void backtrace();\n> > > \n> > > - int logSetFile(const char *path);\n> > > - int logSetStream(std::ostream *stream);\n> > > + int logSetFile(const char *path, bool color);\n> > > + int logSetStream(std::ostream *stream, bool color);\n> > > int logSetTarget(LoggingTarget target);\n> > > void logSetLevel(const char *category, const char *level);\n> > > \n> > > @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> > > /**\n> > > * \\brief Direct logging to a file\n> > > * \\param[in] path Full path to the log file\n> > > + * \\param[in] color True to output colored messages\n> > > *\n> > > * This function directs the log output to the file identified by \\a path. The\n> > > * previous log target, if any, is closed, and all new log messages will be\n> > > * written to the new log file.\n> > > *\n> > > + * \\a color controls whether or not the messages will be colored with standard\n> > > + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> > > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > > + * not suitable for the log target.\n> > > + *\n> > > * If the function returns an error, the log target is not changed.\n> > > *\n> > > * \\return Zero on success, or a negative error code otherwise\n> > > */\n> > > -int logSetFile(const char *path)\n> > > +int logSetFile(const char *path, bool color)\n> > > {\n> > > - return Logger::instance()->logSetFile(path);\n> > > + return Logger::instance()->logSetFile(path, color);\n> > > }\n> > > \n> > > /**\n> > > * \\brief Direct logging to a stream\n> > > * \\param[in] stream Stream to send log output to\n> > > + * \\param[in] color True to output colored messages\n> > > *\n> > > * This function directs the log output to \\a stream. The previous log target,\n> > > * if any, is closed, and all new log messages will be written to the new log\n> > > * stream.\n> > > *\n> > > + * \\a color controls whether or not the messages will be colored with standard\n> > > + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> > > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > > + * not suitable for the log target.\n> > > + *\n> > > * If the function returns an error, the log file is not changed\n> > > *\n> > > * \\return Zero on success, or a negative error code otherwise.\n> > > */\n> > > -int logSetStream(std::ostream *stream)\n> > > +int logSetStream(std::ostream *stream, bool color)\n> > > {\n> > > - return Logger::instance()->logSetStream(stream);\n> > > + return Logger::instance()->logSetStream(stream, color);\n> > > }\n> > > \n> > > /**\n> > > @@ -437,14 +494,16 @@ void Logger::backtrace()\n> > > /**\n> > > * \\brief Set the log file\n> > > * \\param[in] path Full path to the log file\n> > > + * \\param[in] color True to output colored messages\n> > > *\n> > > * \\sa libcamera::logSetFile()\n> > > *\n> > > * \\return Zero on success, or a negative error code otherwise.\n> > > */\n> > > -int Logger::logSetFile(const char *path)\n> > > +int Logger::logSetFile(const char *path, bool color)\n> > > {\n> > > - std::shared_ptr output = std::make_shared(path);\n> > > + std::shared_ptr output =\n> > > + std::make_shared(path, color);\n> > > if (!output->isValid())\n> > > return -EINVAL;\n> > > \n> > > @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> > > /**\n> > > * \\brief Set the log stream\n> > > * \\param[in] stream Stream to send log output to\n> > > + * \\param[in] color True to output colored messages\n> > > *\n> > > * \\sa libcamera::logSetStream()\n> > > *\n> > > * \\return Zero on success, or a negative error code otherwise.\n> > > */\n> > > -int Logger::logSetStream(std::ostream *stream)\n> > > +int Logger::logSetStream(std::ostream *stream, bool color)\n> > > {\n> > > - std::shared_ptr output = std::make_shared(stream);\n> > > + std::shared_ptr output =\n> > > + std::make_shared(stream, color);\n> > > std::atomic_store(&output_, output);\n> > > return 0;\n> > > }\n> > > @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > > \n> > > /**\n> > > * \\brief Construct a logger\n> > > + *\n> > > + * If the environment variable is not set, log to std::cerr. The log messages\n> > > + * are then colored by default. This can be overridden by setting the\n> > > + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> > > */\n> > > Logger::Logger()\n> > > {\n> > > - logSetStream(&std::cerr);\n> > > + bool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> > > + logSetStream(&std::cerr, color);\n> > > \n> > > parseLogFile();\n> > > parseLogLevels();\n> > > @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> > > return;\n> > > }\n> > > \n> > > - logSetFile(file);\n> > > + logSetFile(file, false);\n> > > }\n> > > \n> > > /**\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 70d73822193b..d934596e4145 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> > > */\n> > > int CameraManager::start()\n> > > {\n> > > - LOG(Camera, Info) << \"libcamera \" << version_;\n> > > + LOG(Camera, Debug) << \"libcamera \" << version_;\n> > > \n> > > int ret = _d()->start();","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 492CCBD160\n\tfor ;\n\tSun, 29 May 2022 08:59:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6AEB565635;\n\tSun, 29 May 2022 10:59:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7A9B60413\n\tfor ;\n\tSun, 29 May 2022 10:59:12 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(static-5-51-54-220.ftth.abo.bbox.fr [5.51.54.220])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6348B825;\n\tSun, 29 May 2022 10:59:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653814754;\n\tbh=zDG4ZVeqoF7AwKAJh+d2mAVCNVT8BvK4J+TUGA+N6ys=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1QQX8DBoa26uaSPfV0poHwFnc1NnjR1uRiiYwzrY51D5hZ2RJFSLylxzHtyok2UC3\n\tVNWr13ka4+PZJG3k/K80H0s/CS6aVqB98eRXNhT9ULPtTa68WX2nVZZdeGnSuIhkNc\n\tFSxm6LuHNfXQHbH2JykYCPFZ2cLs+7yvBWDrxAP5nMU943gB4IlbvmHpDp48LMxzEI\n\teuuHitNY2i7ck9Qu8CL+DhgxP/UgnFuk1ntIJwzZjxC3/xVnaBeCbx4w+BPAefmOfM\n\trPKOxpAhEW9zyADn32tUuuz+lsVdzfqVhcY9B+osiKTqk4F4sX7Go18yfsVFuvGH3s\n\tATxmNc40s0UrQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653814752;\n\tbh=zDG4ZVeqoF7AwKAJh+d2mAVCNVT8BvK4J+TUGA+N6ys=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=k9d2h4dq8fFgp7QHDK367bO52hFhZ8qOCF5bINk696VSQXdCBImGYanl/jACNK0Oh\n\t8BjD0csHt98PNZp61D7xPw0CtEUYHYh7KrYxvUX+vITbnnWwEiKDhC11ylOq1K9ez7\n\tah+Q7MuQPtFlnaQxuzKlsEdRWAQbh5jzRzq2Svn4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"k9d2h4dq\"; dkim-atps=neutral","Date":"Sun, 29 May 2022 11:59:09 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<165377708944.2093890.15129535625982732642@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<165377708944.2093890.15129535625982732642@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23215,"web_url":"https://patchwork.libcamera.org/comment/23215/","msgid":"","date":"2022-05-29T12:25:22","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch\n\nOn 5/26/22 00:25, Laurent Pinchart via libcamera-devel wrote:\n> Extend the logger to support coloring messages. The log level is\n> colorized with per-level colors, and the category with a fixed color.\n> This makes the log output more readable.\n>\n> Coloring is enabled by default when logging to std::cerr, and can be\n> disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> can be enabled for file logging using the logSetFile() function.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/environment_variables.rst | 21 +++--\n> include/libcamera/logging.h | 4 +-\n> src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> src/libcamera/camera_manager.cpp | 2 +-\n> 4 files changed, 109 insertions(+), 32 deletions(-)\n>\n> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> index fa703a726845..7028c024b14e 100644\n> --- a/Documentation/environment_variables.rst\n> +++ b/Documentation/environment_variables.rst\n> @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> \n> Example value: ``*:DEBUG``\n> \n> +LIBCAMERA_LOG_NO_COLOR\n> + Disable coloring of log messages (`more `__).\n> +\n> LIBCAMERA_IPA_CONFIG_PATH\n> Define custom search locations for IPA configurations (`more `__).\n> \n> @@ -40,12 +43,20 @@ Further details\n> Notes about debugging\n> ~~~~~~~~~~~~~~~~~~~~~\n> \n> -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> -are used to modify the destination and verbosity of messages provided by\n> -libcamera.\n> +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n\n\ns/degault/default\n\n> +libcamera logger.\n> \n> -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> -'category:level' pairs.\n> +By default, libcamera logs all messages to the standard error (std::cerr).\n> +Messages are colored by default depending on the log level. Coloring can be\n> +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> +\n> +The default log destination can also be directed to a file by setting the\n> +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> +disables coloring.\n> +\n> +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> +accepts a comma-separated list of 'category:level' pairs.\n> \n> The `level `__ part is mandatory and can either be specified by\n> name or by numerical index associated with each level.\n> diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> index c36882b91974..cd842f67d553 100644\n> --- a/include/libcamera/logging.h\n> +++ b/include/libcamera/logging.h\n> @@ -16,8 +16,8 @@ enum LoggingTarget {\n> \tLoggingTargetStream,\n> };\n> \n> -int logSetFile(const char *path);\n> -int logSetStream(std::ostream *stream);\n> +int logSetFile(const char *path, bool color = false);\n> +int logSetStream(std::ostream *stream, bool color = false);\n> int logSetTarget(LoggingTarget target);\n> void logSetLevel(const char *category, const char *level);\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 26f1420703b9..a9f5bbbd36f7 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> class LogOutput\n> {\n> public:\n> -\tLogOutput(const char *path);\n> -\tLogOutput(std::ostream *stream);\n> +\tLogOutput(const char *path, bool color);\n> +\tLogOutput(std::ostream *stream, bool color);\n> \tLogOutput();\n> \t~LogOutput();\n> \n> @@ -119,14 +119,16 @@ private:\n> \n> \tstd::ostream *stream_;\n> \tLoggingTarget target_;\n> +\tbool color_;\n> };\n> \n> /**\n> * \\brief Construct a log output based on a file\n> * \\param[in] path Full path to log file\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(const char *path)\n> -\t: target_(LoggingTargetFile)\n> +LogOutput::LogOutput(const char *path, bool color)\n> +\t: target_(LoggingTargetFile), color_(color)\n> {\n> \tstream_ = new std::ofstream(path);\n> }\n> @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> /**\n> * \\brief Construct a log output based on a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(std::ostream *stream)\n> -\t: stream_(stream), target_(LoggingTargetStream)\n> +LogOutput::LogOutput(std::ostream *stream, bool color)\n> +\t: stream_(stream), target_(LoggingTargetStream), color_(color)\n> {\n> }\n> \n> @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> * \\brief Construct a log output to syslog\n> */\n> LogOutput::LogOutput()\n> -\t: stream_(nullptr), target_(LoggingTargetSyslog)\n> +\t: stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> {\n> \topenlog(\"libcamera\", LOG_PID, 0);\n> }\n> @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> \t}\n> }\n> \n> +namespace {\n> +\n> +constexpr const char *kColorReset = \"\\033[0m\";\n> +constexpr const char *kColorRed = \"\\033[31m\";\n> +constexpr const char *kColorGreen = \"\\033[32m\";\n> +constexpr const char *kColorYellow = \"\\033[33m\";\n> +constexpr const char *kColorBlue = \"\\033[34m\";\n> +constexpr const char *kColorMagenta = \"\\033[35m\";\n> +constexpr const char *kColorCyan = \"\\033[36m\";\n> +constexpr const char *kColorWhite = \"\\033[37m\";\n> +constexpr const char *kColorGrey = \"\\033[90m\";\n> +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> +\n> +} /* namespace */\n> +\n> /**\n> * \\brief Write message to log output\n> * \\param[in] msg Message to write\n> */\n> void LogOutput::write(const LogMessage &msg)\n> {\n> +\tstatic const char *const severityColors[] = {\n> +\t\tkColorBrightCyan,\n> +\t\tkColorBrightGreen,\n> +\t\tkColorBrightYellow,\n> +\t\tkColorBrightRed,\n> +\t\tkColorBrightMagenta,\n> +\t};\n> +\n> +\tconst char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> +\tconst char *fileColor = color_ ? kColorBrightBlue : \"\";\n> +\tconst char *msgColor = color_ ? kColorReset : \"\";\n> +\tconst char *severityColor = \"\";\n> +\tLogSeverity severity = msg.severity();\n> \tstd::string str;\n> \n> +\tif (color_) {\n> +\t\tif (static_cast(severity) < std::size(severityColors))\n> +\t\t\tseverityColor = severityColors[severity];\n> +\t\telse\n> +\t\t\tseverityColor = kColorBrightWhite;\n> +\t}\n> +\n> \tswitch (target_) {\n> \tcase LoggingTargetSyslog:\n> -\t\tstr = std::string(log_severity_name(msg.severity())) + \" \"\n> +\t\tstr = std::string(log_severity_name(severity)) + \" \"\n> \t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> \t\t + msg.msg();\n> -\t\twriteSyslog(msg.severity(), str);\n> +\t\twriteSyslog(severity, str);\n> \t\tbreak;\n> \tcase LoggingTargetStream:\n> \tcase LoggingTargetFile:\n> \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> \t\t + std::to_string(Thread::currentId()) + \"] \"\n> -\t\t + log_severity_name(msg.severity()) + \" \"\n> -\t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> -\t\t + msg.msg();\n> +\t\t + severityColor + log_severity_name(severity) + \" \"\n> +\t\t + categoryColor + msg.category().name() + \" \"\n> +\t\t + fileColor + msg.fileInfo() + \" \"\n> +\t\t + msgColor + msg.msg();\n> \t\twriteStream(str);\n> \t\tbreak;\n> \tdefault:\n> @@ -253,8 +298,8 @@ public:\n> \tvoid write(const LogMessage &msg);\n> \tvoid backtrace();\n> \n> -\tint logSetFile(const char *path);\n> -\tint logSetStream(std::ostream *stream);\n> +\tint logSetFile(const char *path, bool color);\n> +\tint logSetStream(std::ostream *stream, bool color);\n> \tint logSetTarget(LoggingTarget target);\n> \tvoid logSetLevel(const char *category, const char *level);\n> \n> @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> /**\n> * \\brief Direct logging to a file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to the file identified by \\a path. The\n> * previous log target, if any, is closed, and all new log messages will be\n> * written to the new log file.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log target is not changed.\n> *\n> * \\return Zero on success, or a negative error code otherwise\n> */\n> -int logSetFile(const char *path)\n> +int logSetFile(const char *path, bool color)\n> {\n> -\treturn Logger::instance()->logSetFile(path);\n> +\treturn Logger::instance()->logSetFile(path, color);\n> }\n> \n> /**\n> * \\brief Direct logging to a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to \\a stream. The previous log target,\n> * if any, is closed, and all new log messages will be written to the new log\n> * stream.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log file is not changed\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int logSetStream(std::ostream *stream)\n> +int logSetStream(std::ostream *stream, bool color)\n> {\n> -\treturn Logger::instance()->logSetStream(stream);\n> +\treturn Logger::instance()->logSetStream(stream, color);\n> }\n> \n> /**\n> @@ -437,14 +494,16 @@ void Logger::backtrace()\n> /**\n> * \\brief Set the log file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetFile()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetFile(const char *path)\n> +int Logger::logSetFile(const char *path, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(path);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(path, color);\n> \tif (!output->isValid())\n> \t\treturn -EINVAL;\n> \n> @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> /**\n> * \\brief Set the log stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetStream()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetStream(std::ostream *stream)\n> +int Logger::logSetStream(std::ostream *stream, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(stream);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(stream, color);\n> \tstd::atomic_store(&output_, output);\n> \treturn 0;\n> }\n> @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> \n> /**\n> * \\brief Construct a logger\n> + *\n> + * If the environment variable is not set, log to std::cerr. The log messages\n> + * are then colored by default. This can be overridden by setting the\n> + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> */\n> Logger::Logger()\n> {\n> -\tlogSetStream(&std::cerr);\n> +\tbool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> +\tlogSetStream(&std::cerr, color);\n> \n> \tparseLogFile();\n> \tparseLogLevels();\n> @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> \t\treturn;\n> \t}\n> \n> -\tlogSetFile(file);\n> +\tlogSetFile(file, false);\n> }\n> \n> /**\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 70d73822193b..d934596e4145 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> */\n> int CameraManager::start()\n> {\n> -\tLOG(Camera, Info) << \"libcamera \" << version_;\n> +\tLOG(Camera, Debug) << \"libcamera \" << version_;\n\n\nunrelated maybe ? Should be mentioned in commit message?\n\nMinors....\n\nReviewed-by: Umang Jain \n\n> \n> \tint ret = _d()->start();\n> \tif (ret)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9360DBD160\n\tfor ;\n\tSun, 29 May 2022 12:25:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1E7765635;\n\tSun, 29 May 2022 14:25:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A39F360415\n\tfor ;\n\tSun, 29 May 2022 14:25:25 +0200 (CEST)","from [192.168.1.68] (235.red-81-36-186.dynamicip.rima-tde.net\n\t[81.36.186.235])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 23CE06DC;\n\tSun, 29 May 2022 14:25:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653827126;\n\tbh=xnQg+TS3hc/b7yl3xqhdhPkX5Wrk/5NoXwQ4uRinB8M=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=D0y5cfhcA24/C46zC7KemZ2WklmkWsNBAXXCCyNr1hvlTj52AUECR6fxLEExAQ7dJ\n\tSt63dAU5tLPV+XjAHSf9eS2iXuUaf76rvXdrYQPITkDMB/rWgkU3NKSi0or3pgHeRz\n\tmky5uYf4QuP+za9boHwNXzr3xYXheOkJtfF/rpfRp5Lw4N5cRaBnKDKabzNrPyRHpn\n\tpM9OE2K/43TcmK8dswnXomzGHO/MUiEY3zy7Gho8B7ph2OtXop0Edd3RzjFW51oHFL\n\tIbt2q3RzQV8BS0JrIydUpxmNwOb2TL/Tc1jBcbn7Tpn2bKy13iI1E8t7PlqF4Sb3WZ\n\tZr0FQ/4GOuUIg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653827125;\n\tbh=xnQg+TS3hc/b7yl3xqhdhPkX5Wrk/5NoXwQ4uRinB8M=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=YzYvS0/hW1R63B4DZ7sDL3PSX+hy4J48anvPi6A3xK8z8nohA3wajaoC0Gfkbri3j\n\thaEU7B+Z6bMdYXmzLOFLdnqtArx9b+9zklCxd7xDkNBnPjXZYmlRntFhTnuMbsRAKU\n\tPB7OSRZI7AKtRJkI8VOE+2eSmNSDsEyxc3Rzm9l4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YzYvS0/h\"; dkim-atps=neutral","Message-ID":"","Date":"Sun, 29 May 2022 14:25:22 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23216,"web_url":"https://patchwork.libcamera.org/comment/23216/","msgid":"<165382765292.2208220.10293907564106615238@Monstersaurus>","date":"2022-05-29T12:34:12","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-05-29 09:59:09)\n> Hi Kieran,\n> \n> On Sat, May 28, 2022 at 11:31:29PM +0100, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart via libcamera-devel (2022-05-26 10:09:17)\n> > > On Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > Extend the logger to support coloring messages. The log level is\n> > > > colorized with per-level colors, and the category with a fixed color.\n> > > > This makes the log output more readable.\n> > > > \n> > > > Coloring is enabled by default when logging to std::cerr, and can be\n> > > > disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> > > > When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> > > > can be enabled for file logging using the logSetFile() function.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > Documentation/environment_variables.rst | 21 +++--\n> > > > include/libcamera/logging.h | 4 +-\n> > > > src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> > > > src/libcamera/camera_manager.cpp | 2 +-\n> > > > 4 files changed, 109 insertions(+), 32 deletions(-)\n> > > > \n> > > > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > > > index fa703a726845..7028c024b14e 100644\n> > > > --- a/Documentation/environment_variables.rst\n> > > > +++ b/Documentation/environment_variables.rst\n> > > > @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> > > > \n> > > > Example value: ``*:DEBUG``\n> > > > \n> > > > +LIBCAMERA_LOG_NO_COLOR\n> > > > + Disable coloring of log messages (`more `__).\n> > > > +\n> > > > LIBCAMERA_IPA_CONFIG_PATH\n> > > > Define custom search locations for IPA configurations (`more `__).\n> > > > \n> > > > @@ -40,12 +43,20 @@ Further details\n> > > > Notes about debugging\n> > > > ~~~~~~~~~~~~~~~~~~~~~\n> > > > \n> > > > -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> > > > -are used to modify the destination and verbosity of messages provided by\n> > > > -libcamera.\n> > > > +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> > > > +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> > > > +libcamera logger.\n> > > > \n> > > > -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> > > > -'category:level' pairs.\n> > > > +By default, libcamera logs all messages to the standard error (std::cerr).\n> > > > +Messages are colored by default depending on the log level. Coloring can be\n> > > > +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> > > > +\n> > > > +The default log destination can also be directed to a file by setting the\n> > > > +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> > > > +disables coloring.\n> > > > +\n> > > > +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> > > > +accepts a comma-separated list of 'category:level' pairs.\n> > > > \n> > > > The `level `__ part is mandatory and can either be specified by\n> > > > name or by numerical index associated with each level.\n> > > > diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> > > > index c36882b91974..cd842f67d553 100644\n> > > > --- a/include/libcamera/logging.h\n> > > > +++ b/include/libcamera/logging.h\n> > > > @@ -16,8 +16,8 @@ enum LoggingTarget {\n> > > > LoggingTargetStream,\n> > > > };\n> > > > \n> > > > -int logSetFile(const char *path);\n> > > > -int logSetStream(std::ostream *stream);\n> > > > +int logSetFile(const char *path, bool color = false);\n> > > > +int logSetStream(std::ostream *stream, bool color = false);\n> > > > int logSetTarget(LoggingTarget target);\n> > > > void logSetLevel(const char *category, const char *level);\n> > > > \n> > > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > > > index 26f1420703b9..a9f5bbbd36f7 100644\n> > > > --- a/src/libcamera/base/log.cpp\n> > > > +++ b/src/libcamera/base/log.cpp\n> > > > @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> > > > class LogOutput\n> > > > {\n> > > > public:\n> > > > - LogOutput(const char *path);\n> > > > - LogOutput(std::ostream *stream);\n> > > > + LogOutput(const char *path, bool color);\n> > > > + LogOutput(std::ostream *stream, bool color);\n> > > > LogOutput();\n> > > > ~LogOutput();\n> > > > \n> > > > @@ -119,14 +119,16 @@ private:\n> > > > \n> > > > std::ostream *stream_;\n> > > > LoggingTarget target_;\n> > > > + bool color_;\n> > > > };\n> > > > \n> > > > /**\n> > > > * \\brief Construct a log output based on a file\n> > > > * \\param[in] path Full path to log file\n> > > > + * \\param[in] color True to output colored messages\n> > > > */\n> > > > -LogOutput::LogOutput(const char *path)\n> > > > - : target_(LoggingTargetFile)\n> > > > +LogOutput::LogOutput(const char *path, bool color)\n> > > > + : target_(LoggingTargetFile), color_(color)\n> > > > {\n> > > > stream_ = new std::ofstream(path);\n> > > > }\n> > > > @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> > > > /**\n> > > > * \\brief Construct a log output based on a stream\n> > > > * \\param[in] stream Stream to send log output to\n> > > > + * \\param[in] color True to output colored messages\n> > > > */\n> > > > -LogOutput::LogOutput(std::ostream *stream)\n> > > > - : stream_(stream), target_(LoggingTargetStream)\n> > > > +LogOutput::LogOutput(std::ostream *stream, bool color)\n> > > > + : stream_(stream), target_(LoggingTargetStream), color_(color)\n> > > > {\n> > > > }\n> > > > \n> > > > @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> > > > * \\brief Construct a log output to syslog\n> > > > */\n> > > > LogOutput::LogOutput()\n> > > > - : stream_(nullptr), target_(LoggingTargetSyslog)\n> > > > + : stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> > > > {\n> > > > openlog(\"libcamera\", LOG_PID, 0);\n> > > > }\n> > > > @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> > > > }\n> > > > }\n> > > > \n> > > > +namespace {\n> > > > +\n> > > > +constexpr const char *kColorReset = \"\\033[0m\";\n> > > > +constexpr const char *kColorRed = \"\\033[31m\";\n> > > \n> > > clang complains about unused variables, so I'll have to drop a few, or\n> > > add [[maybe_unused]]. I think I'll go for the former in this case.\n> > \n> > Having these as handy to reference would help for anyone who wants to\n> > tweak the colour schemes.\n> > \n> > I'd almost go as far to say maybe they should be #define'd to keep the\n> > compiler ignorant and still available ...\n> > \n> > Or pragma out the unused variable warning just for this table...\n> \n> I've thought about all that, but I'm not sure if it's worth it given how\n> easy the information is to find:\n> https://en.wikipedia.org/wiki/ANSI_escape_code#Colors.\n\nEasy to find if you know what they are ;-) I knew what they are but\nthat's not always a given.\n\nA reference to that link would help I guess.\n\n--\nKieran\n\n\n> \n> > > > +constexpr const char *kColorGreen = \"\\033[32m\";\n> > > > +constexpr const char *kColorYellow = \"\\033[33m\";\n> > > > +constexpr const char *kColorBlue = \"\\033[34m\";\n> > > > +constexpr const char *kColorMagenta = \"\\033[35m\";\n> > > > +constexpr const char *kColorCyan = \"\\033[36m\";\n> > > > +constexpr const char *kColorWhite = \"\\033[37m\";\n> > > > +constexpr const char *kColorGrey = \"\\033[90m\";\n> > > > +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> > > > +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> > > > +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> > > > +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> > > > +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> > > > +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> > > > +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> > > > +\n> > > > +} /* namespace */\n> > > > +\n> > > > /**\n> > > > * \\brief Write message to log output\n> > > > * \\param[in] msg Message to write\n> > > > */\n> > > > void LogOutput::write(const LogMessage &msg)\n> > > > {\n> > > > + static const char *const severityColors[] = {\n> > > > + kColorBrightCyan,\n> > > > + kColorBrightGreen,\n> > > > + kColorBrightYellow,\n> > > > + kColorBrightRed,\n> > > > + kColorBrightMagenta,\n> > > > + };\n> > > > +\n> > > > + const char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> > > > + const char *fileColor = color_ ? kColorBrightBlue : \"\";\n> > > > + const char *msgColor = color_ ? kColorReset : \"\";\n> > > > + const char *severityColor = \"\";\n> > > > + LogSeverity severity = msg.severity();\n> > > > std::string str;\n> > > > \n> > > > + if (color_) {\n> > > > + if (static_cast(severity) < std::size(severityColors))\n> > > > + severityColor = severityColors[severity];\n> > > > + else\n> > > > + severityColor = kColorBrightWhite;\n> > > > + }\n> > > > +\n> > > > switch (target_) {\n> > > > case LoggingTargetSyslog:\n> > > > - str = std::string(log_severity_name(msg.severity())) + \" \"\n> > > > + str = std::string(log_severity_name(severity)) + \" \"\n> > > > + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > > > + msg.msg();\n> > > > - writeSyslog(msg.severity(), str);\n> > > > + writeSyslog(severity, str);\n> > > > break;\n> > > > case LoggingTargetStream:\n> > > > case LoggingTargetFile:\n> > > > str = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > > > + std::to_string(Thread::currentId()) + \"] \"\n> > > > - + log_severity_name(msg.severity()) + \" \"\n> > > > - + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > > > - + msg.msg();\n> > > > + + severityColor + log_severity_name(severity) + \" \"\n> > > > + + categoryColor + msg.category().name() + \" \"\n> > > > + + fileColor + msg.fileInfo() + \" \"\n> > > > + + msgColor + msg.msg();\n> > > > writeStream(str);\n> > > > break;\n> > > > default:\n> > > > @@ -253,8 +298,8 @@ public:\n> > > > void write(const LogMessage &msg);\n> > > > void backtrace();\n> > > > \n> > > > - int logSetFile(const char *path);\n> > > > - int logSetStream(std::ostream *stream);\n> > > > + int logSetFile(const char *path, bool color);\n> > > > + int logSetStream(std::ostream *stream, bool color);\n> > > > int logSetTarget(LoggingTarget target);\n> > > > void logSetLevel(const char *category, const char *level);\n> > > > \n> > > > @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> > > > /**\n> > > > * \\brief Direct logging to a file\n> > > > * \\param[in] path Full path to the log file\n> > > > + * \\param[in] color True to output colored messages\n> > > > *\n> > > > * This function directs the log output to the file identified by \\a path. The\n> > > > * previous log target, if any, is closed, and all new log messages will be\n> > > > * written to the new log file.\n> > > > *\n> > > > + * \\a color controls whether or not the messages will be colored with standard\n> > > > + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> > > > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > > > + * not suitable for the log target.\n> > > > + *\n> > > > * If the function returns an error, the log target is not changed.\n> > > > *\n> > > > * \\return Zero on success, or a negative error code otherwise\n> > > > */\n> > > > -int logSetFile(const char *path)\n> > > > +int logSetFile(const char *path, bool color)\n> > > > {\n> > > > - return Logger::instance()->logSetFile(path);\n> > > > + return Logger::instance()->logSetFile(path, color);\n> > > > }\n> > > > \n> > > > /**\n> > > > * \\brief Direct logging to a stream\n> > > > * \\param[in] stream Stream to send log output to\n> > > > + * \\param[in] color True to output colored messages\n> > > > *\n> > > > * This function directs the log output to \\a stream. The previous log target,\n> > > > * if any, is closed, and all new log messages will be written to the new log\n> > > > * stream.\n> > > > *\n> > > > + * \\a color controls whether or not the messages will be colored with standard\n> > > > + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> > > > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > > > + * not suitable for the log target.\n> > > > + *\n> > > > * If the function returns an error, the log file is not changed\n> > > > *\n> > > > * \\return Zero on success, or a negative error code otherwise.\n> > > > */\n> > > > -int logSetStream(std::ostream *stream)\n> > > > +int logSetStream(std::ostream *stream, bool color)\n> > > > {\n> > > > - return Logger::instance()->logSetStream(stream);\n> > > > + return Logger::instance()->logSetStream(stream, color);\n> > > > }\n> > > > \n> > > > /**\n> > > > @@ -437,14 +494,16 @@ void Logger::backtrace()\n> > > > /**\n> > > > * \\brief Set the log file\n> > > > * \\param[in] path Full path to the log file\n> > > > + * \\param[in] color True to output colored messages\n> > > > *\n> > > > * \\sa libcamera::logSetFile()\n> > > > *\n> > > > * \\return Zero on success, or a negative error code otherwise.\n> > > > */\n> > > > -int Logger::logSetFile(const char *path)\n> > > > +int Logger::logSetFile(const char *path, bool color)\n> > > > {\n> > > > - std::shared_ptr output = std::make_shared(path);\n> > > > + std::shared_ptr output =\n> > > > + std::make_shared(path, color);\n> > > > if (!output->isValid())\n> > > > return -EINVAL;\n> > > > \n> > > > @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> > > > /**\n> > > > * \\brief Set the log stream\n> > > > * \\param[in] stream Stream to send log output to\n> > > > + * \\param[in] color True to output colored messages\n> > > > *\n> > > > * \\sa libcamera::logSetStream()\n> > > > *\n> > > > * \\return Zero on success, or a negative error code otherwise.\n> > > > */\n> > > > -int Logger::logSetStream(std::ostream *stream)\n> > > > +int Logger::logSetStream(std::ostream *stream, bool color)\n> > > > {\n> > > > - std::shared_ptr output = std::make_shared(stream);\n> > > > + std::shared_ptr output =\n> > > > + std::make_shared(stream, color);\n> > > > std::atomic_store(&output_, output);\n> > > > return 0;\n> > > > }\n> > > > @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > > > \n> > > > /**\n> > > > * \\brief Construct a logger\n> > > > + *\n> > > > + * If the environment variable is not set, log to std::cerr. The log messages\n> > > > + * are then colored by default. This can be overridden by setting the\n> > > > + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> > > > */\n> > > > Logger::Logger()\n> > > > {\n> > > > - logSetStream(&std::cerr);\n> > > > + bool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> > > > + logSetStream(&std::cerr, color);\n> > > > \n> > > > parseLogFile();\n> > > > parseLogLevels();\n> > > > @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> > > > return;\n> > > > }\n> > > > \n> > > > - logSetFile(file);\n> > > > + logSetFile(file, false);\n> > > > }\n> > > > \n> > > > /**\n> > > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > > index 70d73822193b..d934596e4145 100644\n> > > > --- a/src/libcamera/camera_manager.cpp\n> > > > +++ b/src/libcamera/camera_manager.cpp\n> > > > @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> > > > */\n> > > > int CameraManager::start()\n> > > > {\n> > > > - LOG(Camera, Info) << \"libcamera \" << version_;\n> > > > + LOG(Camera, Debug) << \"libcamera \" << version_;\n> > > > \n> > > > int ret = _d()->start();\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A5F87BD161\n\tfor ;\n\tSun, 29 May 2022 12:34:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E21E565633;\n\tSun, 29 May 2022 14:34:17 +0200 (CEST)","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 C168E60415\n\tfor ;\n\tSun, 29 May 2022 14:34:15 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 63F116DC;\n\tSun, 29 May 2022 14:34:15 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653827658;\n\tbh=awMUUvQxez05uQjzE4IeB1/uxnDhAsNPZ+ZNeFB/gvc=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hF0UCsCCXZF12aW8P1yzl/cQLjYDlthxBmk4p4RVvowXu1MJGddOvRRF0S8qRXE0h\n\tmDNRcLtfLTlEsbaJWfO8oGr+EIGHTMvRklb5FYsEj1QfBWdokUTfSvBYSiGq1xXcF8\n\tF/n9ZkLjWkdCWbXSUDPLNqP38GRce2X1kDX+d56YDOBBvVEaInXOvBOl7udc2E5y52\n\tnMJMm1i9rgraqU8HjbJ/UYyc8T7ZQvE16DAWw2Tz04yS8D2+O+EYeBeoR/4XUBjw5y\n\tn5YrTU9DvnsOGc+YzO4D5LP8RMCDrPyOzB4WWY/xShY4oihjahyDM232KM2wrguJSX\n\toxd8AEdSCXF5w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653827655;\n\tbh=awMUUvQxez05uQjzE4IeB1/uxnDhAsNPZ+ZNeFB/gvc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uBOVTcHkD7ocOf+JqVYy3HrrNB3PVgrpSE1kLuSlGMYCxRFUZrEGOKKK+eRxjvh04\n\tbLs2TUBx9sahqJhp8wtPv3jGiqUpdb+5k36x5tlOQqwcndWpT19y9OH0mD4NkmceBE\n\tSEmZFy1KlFl9Njb2TJUfn/payuqC5LrWNyxF4StU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uBOVTcHk\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>\n\t\n\t<165377708944.2093890.15129535625982732642@Monstersaurus>\n\t","To":"Laurent Pinchart ","Date":"Sun, 29 May 2022 13:34:12 +0100","Message-ID":"<165382765292.2208220.10293907564106615238@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23218,"web_url":"https://patchwork.libcamera.org/comment/23218/","msgid":"","date":"2022-05-29T15:56:09","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Sun, May 29, 2022 at 02:25:22PM +0200, Umang Jain wrote:\n> On 5/26/22 00:25, Laurent Pinchart via libcamera-devel wrote:\n> > Extend the logger to support coloring messages. The log level is\n> > colorized with per-level colors, and the category with a fixed color.\n> > This makes the log output more readable.\n> >\n> > Coloring is enabled by default when logging to std::cerr, and can be\n> > disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> > When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> > can be enabled for file logging using the logSetFile() function.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/environment_variables.rst | 21 +++--\n> > include/libcamera/logging.h | 4 +-\n> > src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> > src/libcamera/camera_manager.cpp | 2 +-\n> > 4 files changed, 109 insertions(+), 32 deletions(-)\n> >\n> > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > index fa703a726845..7028c024b14e 100644\n> > --- a/Documentation/environment_variables.rst\n> > +++ b/Documentation/environment_variables.rst\n> > @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> > \n> > Example value: ``*:DEBUG``\n> > \n> > +LIBCAMERA_LOG_NO_COLOR\n> > + Disable coloring of log messages (`more `__).\n> > +\n> > LIBCAMERA_IPA_CONFIG_PATH\n> > Define custom search locations for IPA configurations (`more `__).\n> > \n> > @@ -40,12 +43,20 @@ Further details\n> > Notes about debugging\n> > ~~~~~~~~~~~~~~~~~~~~~\n> > \n> > -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> > -are used to modify the destination and verbosity of messages provided by\n> > -libcamera.\n> > +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> > +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> \n> \n> s/degault/default\n> \n> > +libcamera logger.\n> > \n> > -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> > -'category:level' pairs.\n> > +By default, libcamera logs all messages to the standard error (std::cerr).\n> > +Messages are colored by default depending on the log level. Coloring can be\n> > +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> > +\n> > +The default log destination can also be directed to a file by setting the\n> > +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> > +disables coloring.\n> > +\n> > +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> > +accepts a comma-separated list of 'category:level' pairs.\n> > \n> > The `level `__ part is mandatory and can either be specified by\n> > name or by numerical index associated with each level.\n> > diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> > index c36882b91974..cd842f67d553 100644\n> > --- a/include/libcamera/logging.h\n> > +++ b/include/libcamera/logging.h\n> > @@ -16,8 +16,8 @@ enum LoggingTarget {\n> > \tLoggingTargetStream,\n> > };\n> > \n> > -int logSetFile(const char *path);\n> > -int logSetStream(std::ostream *stream);\n> > +int logSetFile(const char *path, bool color = false);\n> > +int logSetStream(std::ostream *stream, bool color = false);\n> > int logSetTarget(LoggingTarget target);\n> > void logSetLevel(const char *category, const char *level);\n> > \n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 26f1420703b9..a9f5bbbd36f7 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> > class LogOutput\n> > {\n> > public:\n> > -\tLogOutput(const char *path);\n> > -\tLogOutput(std::ostream *stream);\n> > +\tLogOutput(const char *path, bool color);\n> > +\tLogOutput(std::ostream *stream, bool color);\n> > \tLogOutput();\n> > \t~LogOutput();\n> > \n> > @@ -119,14 +119,16 @@ private:\n> > \n> > \tstd::ostream *stream_;\n> > \tLoggingTarget target_;\n> > +\tbool color_;\n> > };\n> > \n> > /**\n> > * \\brief Construct a log output based on a file\n> > * \\param[in] path Full path to log file\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(const char *path)\n> > -\t: target_(LoggingTargetFile)\n> > +LogOutput::LogOutput(const char *path, bool color)\n> > +\t: target_(LoggingTargetFile), color_(color)\n> > {\n> > \tstream_ = new std::ofstream(path);\n> > }\n> > @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> > /**\n> > * \\brief Construct a log output based on a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(std::ostream *stream)\n> > -\t: stream_(stream), target_(LoggingTargetStream)\n> > +LogOutput::LogOutput(std::ostream *stream, bool color)\n> > +\t: stream_(stream), target_(LoggingTargetStream), color_(color)\n> > {\n> > }\n> > \n> > @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> > * \\brief Construct a log output to syslog\n> > */\n> > LogOutput::LogOutput()\n> > -\t: stream_(nullptr), target_(LoggingTargetSyslog)\n> > +\t: stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> > {\n> > \topenlog(\"libcamera\", LOG_PID, 0);\n> > }\n> > @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> > \t}\n> > }\n> > \n> > +namespace {\n> > +\n> > +constexpr const char *kColorReset = \"\\033[0m\";\n> > +constexpr const char *kColorRed = \"\\033[31m\";\n> > +constexpr const char *kColorGreen = \"\\033[32m\";\n> > +constexpr const char *kColorYellow = \"\\033[33m\";\n> > +constexpr const char *kColorBlue = \"\\033[34m\";\n> > +constexpr const char *kColorMagenta = \"\\033[35m\";\n> > +constexpr const char *kColorCyan = \"\\033[36m\";\n> > +constexpr const char *kColorWhite = \"\\033[37m\";\n> > +constexpr const char *kColorGrey = \"\\033[90m\";\n> > +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> > +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> > +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> > +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> > +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> > +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> > +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> > +\n> > +} /* namespace */\n> > +\n> > /**\n> > * \\brief Write message to log output\n> > * \\param[in] msg Message to write\n> > */\n> > void LogOutput::write(const LogMessage &msg)\n> > {\n> > +\tstatic const char *const severityColors[] = {\n> > +\t\tkColorBrightCyan,\n> > +\t\tkColorBrightGreen,\n> > +\t\tkColorBrightYellow,\n> > +\t\tkColorBrightRed,\n> > +\t\tkColorBrightMagenta,\n> > +\t};\n> > +\n> > +\tconst char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> > +\tconst char *fileColor = color_ ? kColorBrightBlue : \"\";\n> > +\tconst char *msgColor = color_ ? kColorReset : \"\";\n> > +\tconst char *severityColor = \"\";\n> > +\tLogSeverity severity = msg.severity();\n> > \tstd::string str;\n> > \n> > +\tif (color_) {\n> > +\t\tif (static_cast(severity) < std::size(severityColors))\n> > +\t\t\tseverityColor = severityColors[severity];\n> > +\t\telse\n> > +\t\t\tseverityColor = kColorBrightWhite;\n> > +\t}\n> > +\n> > \tswitch (target_) {\n> > \tcase LoggingTargetSyslog:\n> > -\t\tstr = std::string(log_severity_name(msg.severity())) + \" \"\n> > +\t\tstr = std::string(log_severity_name(severity)) + \" \"\n> > \t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > \t\t + msg.msg();\n> > -\t\twriteSyslog(msg.severity(), str);\n> > +\t\twriteSyslog(severity, str);\n> > \t\tbreak;\n> > \tcase LoggingTargetStream:\n> > \tcase LoggingTargetFile:\n> > \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > \t\t + std::to_string(Thread::currentId()) + \"] \"\n> > -\t\t + log_severity_name(msg.severity()) + \" \"\n> > -\t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > -\t\t + msg.msg();\n> > +\t\t + severityColor + log_severity_name(severity) + \" \"\n> > +\t\t + categoryColor + msg.category().name() + \" \"\n> > +\t\t + fileColor + msg.fileInfo() + \" \"\n> > +\t\t + msgColor + msg.msg();\n> > \t\twriteStream(str);\n> > \t\tbreak;\n> > \tdefault:\n> > @@ -253,8 +298,8 @@ public:\n> > \tvoid write(const LogMessage &msg);\n> > \tvoid backtrace();\n> > \n> > -\tint logSetFile(const char *path);\n> > -\tint logSetStream(std::ostream *stream);\n> > +\tint logSetFile(const char *path, bool color);\n> > +\tint logSetStream(std::ostream *stream, bool color);\n> > \tint logSetTarget(LoggingTarget target);\n> > \tvoid logSetLevel(const char *category, const char *level);\n> > \n> > @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> > /**\n> > * \\brief Direct logging to a file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to the file identified by \\a path. The\n> > * previous log target, if any, is closed, and all new log messages will be\n> > * written to the new log file.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log target is not changed.\n> > *\n> > * \\return Zero on success, or a negative error code otherwise\n> > */\n> > -int logSetFile(const char *path)\n> > +int logSetFile(const char *path, bool color)\n> > {\n> > -\treturn Logger::instance()->logSetFile(path);\n> > +\treturn Logger::instance()->logSetFile(path, color);\n> > }\n> > \n> > /**\n> > * \\brief Direct logging to a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to \\a stream. The previous log target,\n> > * if any, is closed, and all new log messages will be written to the new log\n> > * stream.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log file is not changed\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int logSetStream(std::ostream *stream)\n> > +int logSetStream(std::ostream *stream, bool color)\n> > {\n> > -\treturn Logger::instance()->logSetStream(stream);\n> > +\treturn Logger::instance()->logSetStream(stream, color);\n> > }\n> > \n> > /**\n> > @@ -437,14 +494,16 @@ void Logger::backtrace()\n> > /**\n> > * \\brief Set the log file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetFile()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetFile(const char *path)\n> > +int Logger::logSetFile(const char *path, bool color)\n> > {\n> > -\tstd::shared_ptr output = std::make_shared(path);\n> > +\tstd::shared_ptr output =\n> > +\t\tstd::make_shared(path, color);\n> > \tif (!output->isValid())\n> > \t\treturn -EINVAL;\n> > \n> > @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> > /**\n> > * \\brief Set the log stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetStream()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetStream(std::ostream *stream)\n> > +int Logger::logSetStream(std::ostream *stream, bool color)\n> > {\n> > -\tstd::shared_ptr output = std::make_shared(stream);\n> > +\tstd::shared_ptr output =\n> > +\t\tstd::make_shared(stream, color);\n> > \tstd::atomic_store(&output_, output);\n> > \treturn 0;\n> > }\n> > @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > \n> > /**\n> > * \\brief Construct a logger\n> > + *\n> > + * If the environment variable is not set, log to std::cerr. The log messages\n> > + * are then colored by default. This can be overridden by setting the\n> > + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> > */\n> > Logger::Logger()\n> > {\n> > -\tlogSetStream(&std::cerr);\n> > +\tbool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> > +\tlogSetStream(&std::cerr, color);\n> > \n> > \tparseLogFile();\n> > \tparseLogLevels();\n> > @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> > \t\treturn;\n> > \t}\n> > \n> > -\tlogSetFile(file);\n> > +\tlogSetFile(file, false);\n> > }\n> > \n> > /**\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 70d73822193b..d934596e4145 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> > */\n> > int CameraManager::start()\n> > {\n> > -\tLOG(Camera, Info) << \"libcamera \" << version_;\n> > +\tLOG(Camera, Debug) << \"libcamera \" << version_;\n> \n> \n> unrelated maybe ? Should be mentioned in commit message?\n\nShould be dropped :-) I changed this for testing, with\n\n\tLOG(Camera, Debug) << \"libcamera \" << version_;\n\tLOG(Camera, Info) << \"libcamera \" << version_;\n\tLOG(Camera, Warning) << \"libcamera \" << version_;\n\tLOG(Camera, Error) << \"libcamera \" << version_;\n\tLOG(Camera, Fatal) << \"libcamera \" << version_;\n\nand then deleted the wrong line.\n\n> Minors....\n> \n> Reviewed-by: Umang Jain \n> \n> > \n> > \tint ret = _d()->start();\n> > \tif (ret)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 77065BD160\n\tfor ;\n\tSun, 29 May 2022 15:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B30C65635;\n\tSun, 29 May 2022 17:56:15 +0200 (CEST)","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 9A3AB60415\n\tfor ;\n\tSun, 29 May 2022 17:56:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(static-5-51-54-220.ftth.abo.bbox.fr [5.51.54.220])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1B5545A4;\n\tSun, 29 May 2022 17:56:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653839775;\n\tbh=pUsGgM2r2MYnof4mJLB8KyhpLlJZKclpcPeQI4aM5P0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=KdW9Bas0pUasRpO9x8N7WQbqbAApa+9rqiobKzSbaEKewh696Gr3ha/s/ebI83CUW\n\tVQgvFwdbXVti0YTgxDGArTW06vVFF61cmelqiyyumfFotxG9DNuufaEhZL+cq9mCp0\n\tUkZ8z1dQcC+O9dB0ER0Ny2T7wHrY6l7zN00M6Fc5SYimbBpVZJI9s9EuHYL4EdQkxt\n\tqZ0rEVUBhD/HAB5di8AEaP1xueVXhco2G0Ch6ymGk4TPKVGUXF/Be4IbzRGY5zw8YN\n\t8zgKoap7aJCmp+B1ORShTcZ35ShPkZvw20xWfJJx2ygMB6Obr8acP6aR8rwS9HdQ98\n\t5j2CxgcMg9E0Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653839773;\n\tbh=pUsGgM2r2MYnof4mJLB8KyhpLlJZKclpcPeQI4aM5P0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CZ48vgOtvewIYdmzq7pr3OeXBp2/0wDQ2BHBdmDPmAwBvS3eZoNofEZu9gG9YQTxx\n\tWukSv1XLSQRQ3rRVFSSGxv4ju+82jrXkSOCb+IB/oXH9TFgc46k7z/3HDF9EJLt27q\n\t0Qy1pXyfU9XB/7/QaSMZ+gXe2KTDrUMAHh2LXWrQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CZ48vgOt\"; dkim-atps=neutral","Date":"Sun, 29 May 2022 18:56:09 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23260,"web_url":"https://patchwork.libcamera.org/comment/23260/","msgid":"<20220531044736.GG2630765@pyrite.rasen.tech>","date":"2022-05-31T04:47:36","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Laurent,\n\nOn Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Extend the logger to support coloring messages. The log level is\n> colorized with per-level colors, and the category with a fixed color.\n> This makes the log output more readable.\n> \n> Coloring is enabled by default when logging to std::cerr, and can be\n> disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> can be enabled for file logging using the logSetFile() function.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/environment_variables.rst | 21 +++--\n> include/libcamera/logging.h | 4 +-\n> src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> src/libcamera/camera_manager.cpp | 2 +-\n> 4 files changed, 109 insertions(+), 32 deletions(-)\n> \n> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> index fa703a726845..7028c024b14e 100644\n> --- a/Documentation/environment_variables.rst\n> +++ b/Documentation/environment_variables.rst\n> @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> \n> Example value: ``*:DEBUG``\n> \n> +LIBCAMERA_LOG_NO_COLOR\n> + Disable coloring of log messages (`more `__).\n> +\n> LIBCAMERA_IPA_CONFIG_PATH\n> Define custom search locations for IPA configurations (`more `__).\n> \n> @@ -40,12 +43,20 @@ Further details\n> Notes about debugging\n> ~~~~~~~~~~~~~~~~~~~~~\n> \n> -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> -are used to modify the destination and verbosity of messages provided by\n> -libcamera.\n> +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> +libcamera logger.\n> \n> -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> -'category:level' pairs.\n> +By default, libcamera logs all messages to the standard error (std::cerr).\n> +Messages are colored by default depending on the log level. Coloring can be\n> +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> +\n> +The default log destination can also be directed to a file by setting the\n> +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> +disables coloring.\n> +\n> +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> +accepts a comma-separated list of 'category:level' pairs.\n> \n> The `level `__ part is mandatory and can either be specified by\n> name or by numerical index associated with each level.\n> diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> index c36882b91974..cd842f67d553 100644\n> --- a/include/libcamera/logging.h\n> +++ b/include/libcamera/logging.h\n> @@ -16,8 +16,8 @@ enum LoggingTarget {\n> \tLoggingTargetStream,\n> };\n> \n> -int logSetFile(const char *path);\n> -int logSetStream(std::ostream *stream);\n> +int logSetFile(const char *path, bool color = false);\n> +int logSetStream(std::ostream *stream, bool color = false);\n> int logSetTarget(LoggingTarget target);\n> void logSetLevel(const char *category, const char *level);\n> \n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 26f1420703b9..a9f5bbbd36f7 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> class LogOutput\n> {\n> public:\n> -\tLogOutput(const char *path);\n> -\tLogOutput(std::ostream *stream);\n> +\tLogOutput(const char *path, bool color);\n> +\tLogOutput(std::ostream *stream, bool color);\n> \tLogOutput();\n> \t~LogOutput();\n> \n> @@ -119,14 +119,16 @@ private:\n> \n> \tstd::ostream *stream_;\n> \tLoggingTarget target_;\n> +\tbool color_;\n> };\n> \n> /**\n> * \\brief Construct a log output based on a file\n> * \\param[in] path Full path to log file\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(const char *path)\n> -\t: target_(LoggingTargetFile)\n> +LogOutput::LogOutput(const char *path, bool color)\n> +\t: target_(LoggingTargetFile), color_(color)\n> {\n> \tstream_ = new std::ofstream(path);\n> }\n> @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> /**\n> * \\brief Construct a log output based on a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> */\n> -LogOutput::LogOutput(std::ostream *stream)\n> -\t: stream_(stream), target_(LoggingTargetStream)\n> +LogOutput::LogOutput(std::ostream *stream, bool color)\n> +\t: stream_(stream), target_(LoggingTargetStream), color_(color)\n> {\n> }\n> \n> @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> * \\brief Construct a log output to syslog\n> */\n> LogOutput::LogOutput()\n> -\t: stream_(nullptr), target_(LoggingTargetSyslog)\n> +\t: stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> {\n> \topenlog(\"libcamera\", LOG_PID, 0);\n> }\n> @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> \t}\n> }\n> \n> +namespace {\n> +\n> +constexpr const char *kColorReset = \"\\033[0m\";\n> +constexpr const char *kColorRed = \"\\033[31m\";\n> +constexpr const char *kColorGreen = \"\\033[32m\";\n> +constexpr const char *kColorYellow = \"\\033[33m\";\n> +constexpr const char *kColorBlue = \"\\033[34m\";\n> +constexpr const char *kColorMagenta = \"\\033[35m\";\n> +constexpr const char *kColorCyan = \"\\033[36m\";\n> +constexpr const char *kColorWhite = \"\\033[37m\";\n> +constexpr const char *kColorGrey = \"\\033[90m\";\n> +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> +\n> +} /* namespace */\n> +\n> /**\n> * \\brief Write message to log output\n> * \\param[in] msg Message to write\n> */\n> void LogOutput::write(const LogMessage &msg)\n> {\n> +\tstatic const char *const severityColors[] = {\n> +\t\tkColorBrightCyan,\n> +\t\tkColorBrightGreen,\n> +\t\tkColorBrightYellow,\n> +\t\tkColorBrightRed,\n> +\t\tkColorBrightMagenta,\n> +\t};\n> +\n> +\tconst char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> +\tconst char *fileColor = color_ ? kColorBrightBlue : \"\";\n> +\tconst char *msgColor = color_ ? kColorReset : \"\";\n> +\tconst char *severityColor = \"\";\n> +\tLogSeverity severity = msg.severity();\n> \tstd::string str;\n> \n> +\tif (color_) {\n> +\t\tif (static_cast(severity) < std::size(severityColors))\n> +\t\t\tseverityColor = severityColors[severity];\n> +\t\telse\n> +\t\t\tseverityColor = kColorBrightWhite;\n> +\t}\n> +\n> \tswitch (target_) {\n> \tcase LoggingTargetSyslog:\n> -\t\tstr = std::string(log_severity_name(msg.severity())) + \" \"\n> +\t\tstr = std::string(log_severity_name(severity)) + \" \"\n> \t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> \t\t + msg.msg();\n> -\t\twriteSyslog(msg.severity(), str);\n> +\t\twriteSyslog(severity, str);\n> \t\tbreak;\n> \tcase LoggingTargetStream:\n> \tcase LoggingTargetFile:\n> \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> \t\t + std::to_string(Thread::currentId()) + \"] \"\n> -\t\t + log_severity_name(msg.severity()) + \" \"\n> -\t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> -\t\t + msg.msg();\n> +\t\t + severityColor + log_severity_name(severity) + \" \"\n> +\t\t + categoryColor + msg.category().name() + \" \"\n> +\t\t + fileColor + msg.fileInfo() + \" \"\n> +\t\t + msgColor + msg.msg();\n\nDon't you need to reset the color here?\n\n> \t\twriteStream(str);\n> \t\tbreak;\n> \tdefault:\n> @@ -253,8 +298,8 @@ public:\n> \tvoid write(const LogMessage &msg);\n> \tvoid backtrace();\n> \n> -\tint logSetFile(const char *path);\n> -\tint logSetStream(std::ostream *stream);\n> +\tint logSetFile(const char *path, bool color);\n> +\tint logSetStream(std::ostream *stream, bool color);\n> \tint logSetTarget(LoggingTarget target);\n> \tvoid logSetLevel(const char *category, const char *level);\n> \n> @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> /**\n> * \\brief Direct logging to a file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to the file identified by \\a path. The\n> * previous log target, if any, is closed, and all new log messages will be\n> * written to the new log file.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log target is not changed.\n> *\n> * \\return Zero on success, or a negative error code otherwise\n> */\n> -int logSetFile(const char *path)\n> +int logSetFile(const char *path, bool color)\n> {\n> -\treturn Logger::instance()->logSetFile(path);\n> +\treturn Logger::instance()->logSetFile(path, color);\n> }\n> \n> /**\n> * \\brief Direct logging to a stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * This function directs the log output to \\a stream. The previous log target,\n> * if any, is closed, and all new log messages will be written to the new log\n> * stream.\n> *\n> + * \\a color controls whether or not the messages will be colored with standard\n> + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> + * standard file or a TTY, the caller is responsible for disabling coloring when\n> + * not suitable for the log target.\n> + *\n> * If the function returns an error, the log file is not changed\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int logSetStream(std::ostream *stream)\n> +int logSetStream(std::ostream *stream, bool color)\n> {\n> -\treturn Logger::instance()->logSetStream(stream);\n> +\treturn Logger::instance()->logSetStream(stream, color);\n> }\n> \n> /**\n> @@ -437,14 +494,16 @@ void Logger::backtrace()\n> /**\n> * \\brief Set the log file\n> * \\param[in] path Full path to the log file\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetFile()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetFile(const char *path)\n> +int Logger::logSetFile(const char *path, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(path);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(path, color);\n> \tif (!output->isValid())\n> \t\treturn -EINVAL;\n> \n> @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> /**\n> * \\brief Set the log stream\n> * \\param[in] stream Stream to send log output to\n> + * \\param[in] color True to output colored messages\n> *\n> * \\sa libcamera::logSetStream()\n> *\n> * \\return Zero on success, or a negative error code otherwise.\n> */\n> -int Logger::logSetStream(std::ostream *stream)\n> +int Logger::logSetStream(std::ostream *stream, bool color)\n> {\n> -\tstd::shared_ptr output = std::make_shared(stream);\n> +\tstd::shared_ptr output =\n> +\t\tstd::make_shared(stream, color);\n> \tstd::atomic_store(&output_, output);\n> \treturn 0;\n> }\n> @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> \n> /**\n> * \\brief Construct a logger\n> + *\n> + * If the environment variable is not set, log to std::cerr. The log messages\n> + * are then colored by default. This can be overridden by setting the\n> + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n\ns/disabled/disable/\n\n\nPaul\n\n> */\n> Logger::Logger()\n> {\n> -\tlogSetStream(&std::cerr);\n> +\tbool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> +\tlogSetStream(&std::cerr, color);\n> \n> \tparseLogFile();\n> \tparseLogLevels();\n> @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> \t\treturn;\n> \t}\n> \n> -\tlogSetFile(file);\n> +\tlogSetFile(file, false);\n> }\n> \n> /**\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 70d73822193b..d934596e4145 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> */\n> int CameraManager::start()\n> {\n> -\tLOG(Camera, Info) << \"libcamera \" << version_;\n> +\tLOG(Camera, Debug) << \"libcamera \" << version_;\n> \n> \tint ret = _d()->start();\n> \tif (ret)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49FB2BD160\n\tfor ;\n\tTue, 31 May 2022 04:47:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BB2A65635;\n\tTue, 31 May 2022 06:47:45 +0200 (CEST)","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 9C4636040A\n\tfor ;\n\tTue, 31 May 2022 06:47:44 +0200 (CEST)","from pyrite.rasen.tech (softbank036240126034.bbtec.net\n\t[36.240.126.34])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 120796F0;\n\tTue, 31 May 2022 06:47:42 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653972465;\n\tbh=5yA2L85pcZNepyio8r6FFGQgazmN7DtIW0JUDxHPzyk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ZwYFCTmk5HHTt+JXafDoYW3K5b33Alutev6dx3wunR51N7El+aLkuFYKcpO45DP9s\n\tkyhj4ae05M45lUakyNz2pqo1G1L90fSyLazqZ+6C/6v/HXywBXiVeTHqFITur0JJ+f\n\tPL3TUCh3jx56HihU02bPfoB+5n5xexoHwIYf+j4mIysBZmHVwJct91NoAZNBnXs9nh\n\tpY9gDkyGxs0cqmqdiCS2KBijI7vGVbFy0u35UkBO62cwD6l/rMhJt6ioArkuCRFL59\n\tfTIOXWvG+vc0WLIO6yuDQSrIkpewTKD4hIi2hmRoGRnDdkMPG0tAoZ5deVFHmy+3wi\n\tkfoc9a1RtTFSw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653972464;\n\tbh=5yA2L85pcZNepyio8r6FFGQgazmN7DtIW0JUDxHPzyk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sk56gymS2MKOcUI9Qq+8f5KrC2kjPqrDb6QRj8uSJD5h0VDst0XVNKsDa1RND/uSY\n\tMrRj2Oam+xleza32dd7eMphDEy420RSx970H2LY7jMGUeyU0fRp5l2LVZsKvcsaXmJ\n\tRLVUZKAlJJEo2Me+MPMn3B6Ln3O9oLHudNdzZSig="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sk56gymS\"; dkim-atps=neutral","Date":"Tue, 31 May 2022 13:47:36 +0900","To":"Laurent Pinchart ","Message-ID":"<20220531044736.GG2630765@pyrite.rasen.tech>","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23275,"web_url":"https://patchwork.libcamera.org/comment/23275/","msgid":"","date":"2022-06-01T08:13:02","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Tue, May 31, 2022 at 01:47:36PM +0900, paul.elder@ideasonboard.com wrote:\n> On Thu, May 26, 2022 at 01:25:02AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Extend the logger to support coloring messages. The log level is\n> > colorized with per-level colors, and the category with a fixed color.\n> > This makes the log output more readable.\n> > \n> > Coloring is enabled by default when logging to std::cerr, and can be\n> > disabled by setting the LIBCAMERA_LOG_NO_COLOR environment variable.\n> > When logging to a file with LIBCAMERA_LOG_FILE, coloring is disabled. It\n> > can be enabled for file logging using the logSetFile() function.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/environment_variables.rst | 21 +++--\n> > include/libcamera/logging.h | 4 +-\n> > src/libcamera/base/log.cpp | 114 +++++++++++++++++++-----\n> > src/libcamera/camera_manager.cpp | 2 +-\n> > 4 files changed, 109 insertions(+), 32 deletions(-)\n> > \n> > diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst\n> > index fa703a726845..7028c024b14e 100644\n> > --- a/Documentation/environment_variables.rst\n> > +++ b/Documentation/environment_variables.rst\n> > @@ -19,6 +19,9 @@ LIBCAMERA_LOG_LEVELS\n> > \n> > Example value: ``*:DEBUG``\n> > \n> > +LIBCAMERA_LOG_NO_COLOR\n> > + Disable coloring of log messages (`more `__).\n> > +\n> > LIBCAMERA_IPA_CONFIG_PATH\n> > Define custom search locations for IPA configurations (`more `__).\n> > \n> > @@ -40,12 +43,20 @@ Further details\n> > Notes about debugging\n> > ~~~~~~~~~~~~~~~~~~~~~\n> > \n> > -The environment variables ``LIBCAMERA_LOG_FILE`` and ``LIBCAMERA_LOG_LEVELS``\n> > -are used to modify the destination and verbosity of messages provided by\n> > -libcamera.\n> > +The environment variables ``LIBCAMERA_LOG_FILE``, ``LIBCAMERA_LOG_LEVELS`` and\n> > +``LIBCAMERA_LOG_NO_COLOR`` are used to modify the degault configuration of the\n> > +libcamera logger.\n> > \n> > -The ``LIBCAMERA_LOG_LEVELS`` variable accepts a comma-separated list of\n> > -'category:level' pairs.\n> > +By default, libcamera logs all messages to the standard error (std::cerr).\n> > +Messages are colored by default depending on the log level. Coloring can be\n> > +disabled by setting the ``LIBCAMERA_LOG_NO_COLOR`` environment variable.\n> > +\n> > +The default log destination can also be directed to a file by setting the\n> > +``LIBCAMERA_LOG_FILE`` environment variable to the log file name. This also\n> > +disables coloring.\n> > +\n> > +Log levels are controlled through the ``LIBCAMERA_LOG_LEVELS`` variable, which\n> > +accepts a comma-separated list of 'category:level' pairs.\n> > \n> > The `level `__ part is mandatory and can either be specified by\n> > name or by numerical index associated with each level.\n> > diff --git a/include/libcamera/logging.h b/include/libcamera/logging.h\n> > index c36882b91974..cd842f67d553 100644\n> > --- a/include/libcamera/logging.h\n> > +++ b/include/libcamera/logging.h\n> > @@ -16,8 +16,8 @@ enum LoggingTarget {\n> > \tLoggingTargetStream,\n> > };\n> > \n> > -int logSetFile(const char *path);\n> > -int logSetStream(std::ostream *stream);\n> > +int logSetFile(const char *path, bool color = false);\n> > +int logSetStream(std::ostream *stream, bool color = false);\n> > int logSetTarget(LoggingTarget target);\n> > void logSetLevel(const char *category, const char *level);\n> > \n> > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> > index 26f1420703b9..a9f5bbbd36f7 100644\n> > --- a/src/libcamera/base/log.cpp\n> > +++ b/src/libcamera/base/log.cpp\n> > @@ -104,8 +104,8 @@ static const char *log_severity_name(LogSeverity severity)\n> > class LogOutput\n> > {\n> > public:\n> > -\tLogOutput(const char *path);\n> > -\tLogOutput(std::ostream *stream);\n> > +\tLogOutput(const char *path, bool color);\n> > +\tLogOutput(std::ostream *stream, bool color);\n> > \tLogOutput();\n> > \t~LogOutput();\n> > \n> > @@ -119,14 +119,16 @@ private:\n> > \n> > \tstd::ostream *stream_;\n> > \tLoggingTarget target_;\n> > +\tbool color_;\n> > };\n> > \n> > /**\n> > * \\brief Construct a log output based on a file\n> > * \\param[in] path Full path to log file\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(const char *path)\n> > -\t: target_(LoggingTargetFile)\n> > +LogOutput::LogOutput(const char *path, bool color)\n> > +\t: target_(LoggingTargetFile), color_(color)\n> > {\n> > \tstream_ = new std::ofstream(path);\n> > }\n> > @@ -134,9 +136,10 @@ LogOutput::LogOutput(const char *path)\n> > /**\n> > * \\brief Construct a log output based on a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > */\n> > -LogOutput::LogOutput(std::ostream *stream)\n> > -\t: stream_(stream), target_(LoggingTargetStream)\n> > +LogOutput::LogOutput(std::ostream *stream, bool color)\n> > +\t: stream_(stream), target_(LoggingTargetStream), color_(color)\n> > {\n> > }\n> > \n> > @@ -144,7 +147,7 @@ LogOutput::LogOutput(std::ostream *stream)\n> > * \\brief Construct a log output to syslog\n> > */\n> > LogOutput::LogOutput()\n> > -\t: stream_(nullptr), target_(LoggingTargetSyslog)\n> > +\t: stream_(nullptr), target_(LoggingTargetSyslog), color_(false)\n> > {\n> > \topenlog(\"libcamera\", LOG_PID, 0);\n> > }\n> > @@ -179,28 +182,70 @@ bool LogOutput::isValid() const\n> > \t}\n> > }\n> > \n> > +namespace {\n> > +\n> > +constexpr const char *kColorReset = \"\\033[0m\";\n> > +constexpr const char *kColorRed = \"\\033[31m\";\n> > +constexpr const char *kColorGreen = \"\\033[32m\";\n> > +constexpr const char *kColorYellow = \"\\033[33m\";\n> > +constexpr const char *kColorBlue = \"\\033[34m\";\n> > +constexpr const char *kColorMagenta = \"\\033[35m\";\n> > +constexpr const char *kColorCyan = \"\\033[36m\";\n> > +constexpr const char *kColorWhite = \"\\033[37m\";\n> > +constexpr const char *kColorGrey = \"\\033[90m\";\n> > +constexpr const char *kColorBrightRed = \"\\033[91m\";\n> > +constexpr const char *kColorBrightGreen = \"\\033[92m\";\n> > +constexpr const char *kColorBrightYellow = \"\\033[93m\";\n> > +constexpr const char *kColorBrightBlue = \"\\033[94m\";\n> > +constexpr const char *kColorBrightMagenta = \"\\033[95m\";\n> > +constexpr const char *kColorBrightCyan = \"\\033[96m\";\n> > +constexpr const char *kColorBrightWhite = \"\\033[97m\";\n> > +\n> > +} /* namespace */\n> > +\n> > /**\n> > * \\brief Write message to log output\n> > * \\param[in] msg Message to write\n> > */\n> > void LogOutput::write(const LogMessage &msg)\n> > {\n> > +\tstatic const char *const severityColors[] = {\n> > +\t\tkColorBrightCyan,\n> > +\t\tkColorBrightGreen,\n> > +\t\tkColorBrightYellow,\n> > +\t\tkColorBrightRed,\n> > +\t\tkColorBrightMagenta,\n> > +\t};\n> > +\n> > +\tconst char *categoryColor = color_ ? kColorBrightWhite : \"\";\n> > +\tconst char *fileColor = color_ ? kColorBrightBlue : \"\";\n> > +\tconst char *msgColor = color_ ? kColorReset : \"\";\n> > +\tconst char *severityColor = \"\";\n> > +\tLogSeverity severity = msg.severity();\n> > \tstd::string str;\n> > \n> > +\tif (color_) {\n> > +\t\tif (static_cast(severity) < std::size(severityColors))\n> > +\t\t\tseverityColor = severityColors[severity];\n> > +\t\telse\n> > +\t\t\tseverityColor = kColorBrightWhite;\n> > +\t}\n> > +\n> > \tswitch (target_) {\n> > \tcase LoggingTargetSyslog:\n> > -\t\tstr = std::string(log_severity_name(msg.severity())) + \" \"\n> > +\t\tstr = std::string(log_severity_name(severity)) + \" \"\n> > \t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > \t\t + msg.msg();\n> > -\t\twriteSyslog(msg.severity(), str);\n> > +\t\twriteSyslog(severity, str);\n> > \t\tbreak;\n> > \tcase LoggingTargetStream:\n> > \tcase LoggingTargetFile:\n> > \t\tstr = \"[\" + utils::time_point_to_string(msg.timestamp()) + \"] [\"\n> > \t\t + std::to_string(Thread::currentId()) + \"] \"\n> > -\t\t + log_severity_name(msg.severity()) + \" \"\n> > -\t\t + msg.category().name() + \" \" + msg.fileInfo() + \" \"\n> > -\t\t + msg.msg();\n> > +\t\t + severityColor + log_severity_name(severity) + \" \"\n> > +\t\t + categoryColor + msg.category().name() + \" \"\n> > +\t\t + fileColor + msg.fileInfo() + \" \"\n> > +\t\t + msgColor + msg.msg();\n> \n> Don't you need to reset the color here?\n\nmsgColor resets it. I'll make this more explicit by renaming the\nvariable to resetColor.\n\n> > \t\twriteStream(str);\n> > \t\tbreak;\n> > \tdefault:\n> > @@ -253,8 +298,8 @@ public:\n> > \tvoid write(const LogMessage &msg);\n> > \tvoid backtrace();\n> > \n> > -\tint logSetFile(const char *path);\n> > -\tint logSetStream(std::ostream *stream);\n> > +\tint logSetFile(const char *path, bool color);\n> > +\tint logSetStream(std::ostream *stream, bool color);\n> > \tint logSetTarget(LoggingTarget target);\n> > \tvoid logSetLevel(const char *category, const char *level);\n> > \n> > @@ -298,35 +343,47 @@ bool Logger::destroyed_ = false;\n> > /**\n> > * \\brief Direct logging to a file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to the file identified by \\a path. The\n> > * previous log target, if any, is closed, and all new log messages will be\n> > * written to the new log file.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a path refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log target is not changed.\n> > *\n> > * \\return Zero on success, or a negative error code otherwise\n> > */\n> > -int logSetFile(const char *path)\n> > +int logSetFile(const char *path, bool color)\n> > {\n> > -\treturn Logger::instance()->logSetFile(path);\n> > +\treturn Logger::instance()->logSetFile(path, color);\n> > }\n> > \n> > /**\n> > * \\brief Direct logging to a stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * This function directs the log output to \\a stream. The previous log target,\n> > * if any, is closed, and all new log messages will be written to the new log\n> > * stream.\n> > *\n> > + * \\a color controls whether or not the messages will be colored with standard\n> > + * ANSI escape codes. This is done regardless of whether \\a stream refers to a\n> > + * standard file or a TTY, the caller is responsible for disabling coloring when\n> > + * not suitable for the log target.\n> > + *\n> > * If the function returns an error, the log file is not changed\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int logSetStream(std::ostream *stream)\n> > +int logSetStream(std::ostream *stream, bool color)\n> > {\n> > -\treturn Logger::instance()->logSetStream(stream);\n> > +\treturn Logger::instance()->logSetStream(stream, color);\n> > }\n> > \n> > /**\n> > @@ -437,14 +494,16 @@ void Logger::backtrace()\n> > /**\n> > * \\brief Set the log file\n> > * \\param[in] path Full path to the log file\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetFile()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetFile(const char *path)\n> > +int Logger::logSetFile(const char *path, bool color)\n> > {\n> > -\tstd::shared_ptr output = std::make_shared(path);\n> > +\tstd::shared_ptr output =\n> > +\t\tstd::make_shared(path, color);\n> > \tif (!output->isValid())\n> > \t\treturn -EINVAL;\n> > \n> > @@ -455,14 +514,16 @@ int Logger::logSetFile(const char *path)\n> > /**\n> > * \\brief Set the log stream\n> > * \\param[in] stream Stream to send log output to\n> > + * \\param[in] color True to output colored messages\n> > *\n> > * \\sa libcamera::logSetStream()\n> > *\n> > * \\return Zero on success, or a negative error code otherwise.\n> > */\n> > -int Logger::logSetStream(std::ostream *stream)\n> > +int Logger::logSetStream(std::ostream *stream, bool color)\n> > {\n> > -\tstd::shared_ptr output = std::make_shared(stream);\n> > +\tstd::shared_ptr output =\n> > +\t\tstd::make_shared(stream, color);\n> > \tstd::atomic_store(&output_, output);\n> > \treturn 0;\n> > }\n> > @@ -514,10 +575,15 @@ void Logger::logSetLevel(const char *category, const char *level)\n> > \n> > /**\n> > * \\brief Construct a logger\n> > + *\n> > + * If the environment variable is not set, log to std::cerr. The log messages\n> > + * are then colored by default. This can be overridden by setting the\n> > + * LIBCAMERA_LOG_NO_COLOR environment variable to disabled coloring.\n> \n> s/disabled/disable/\n\nOops. Fixed.\n\n> > */\n> > Logger::Logger()\n> > {\n> > -\tlogSetStream(&std::cerr);\n> > +\tbool color = !utils::secure_getenv(\"LIBCAMERA_LOG_NO_COLOR\");\n> > +\tlogSetStream(&std::cerr, color);\n> > \n> > \tparseLogFile();\n> > \tparseLogLevels();\n> > @@ -543,7 +609,7 @@ void Logger::parseLogFile()\n> > \t\treturn;\n> > \t}\n> > \n> > -\tlogSetFile(file);\n> > +\tlogSetFile(file, false);\n> > }\n> > \n> > /**\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 70d73822193b..d934596e4145 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -290,7 +290,7 @@ CameraManager::~CameraManager()\n> > */\n> > int CameraManager::start()\n> > {\n> > -\tLOG(Camera, Info) << \"libcamera \" << version_;\n> > +\tLOG(Camera, Debug) << \"libcamera \" << version_;\n> > \n> > \tint ret = _d()->start();\n> > \tif (ret)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 48AAFBD161\n\tfor ;\n\tWed, 1 Jun 2022 08:13:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A2D4633A7;\n\tWed, 1 Jun 2022 10:13:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D3E7A60414\n\tfor ;\n\tWed, 1 Jun 2022 10:13:07 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lmontsouris-659-1-41-236.w92-154.abo.wanadoo.fr [92.154.76.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76F0630A;\n\tWed, 1 Jun 2022 10:13:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654071189;\n\tbh=xADKcjBrfB4e2eeI2+iZOte3Aco+dnL8Xb3bW+3Zq9E=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ttZF/k1MKfSENKieGBJGcSfcwtNchsmt7wIIoAe/XX/wi+zQwrbGf3Vasf8q6SzL1\n\tfWHU/HqcbwkU7WanGARpAPqjYWWbWnXiB6cqQF1yL8qWXPpN7QZX2ob4TSjtWtpntC\n\t+s62U6ql+sFP8mWicgDxgUNHrrMDQEGv5oxy8bisZqSAiCCbalLoGpX6DRSgjaZMwI\n\tiKulJ9oaouVuFZGpbcQCf4AH9nEOsVQRMeniKQ2B5ePDo8t9dSSHq2Rvvajk2QoA/i\n\tKg53zVP2dbXRjl/ciCPIxHaYrYZY3s7rrr73xXStVV6N2lLlq8sXCqrqRfrEEEl4Dv\n\ty/ADmQSGp34gA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654071187;\n\tbh=xADKcjBrfB4e2eeI2+iZOte3Aco+dnL8Xb3bW+3Zq9E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LgP1HQSUs9SsSHrVffl+Db9d3dSzp72+X+WC94PGIlPjKvfcXK4aAnJaUIrQFFiOD\n\tFc0SaaBQEo/f/Eufopl6bk2wU1COsPPLx3i19dQbvM3EAUS4rzq0SvXw1fE+1rfACB\n\tmi5a4dNTzfCyvNvw8783U51CgjlOdpSLkKZXykdA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LgP1HQSU\"; dkim-atps=neutral","Date":"Wed, 1 Jun 2022 11:13:02 +0300","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20220525222503.6460-1-laurent.pinchart@ideasonboard.com>\n\t<20220525222503.6460-5-laurent.pinchart@ideasonboard.com>\n\t<20220531044736.GG2630765@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220531044736.GG2630765@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: base: log: Add\n\tcoloring to the log output","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]