[libcamera-devel,v2,0/2] libcamera: Add colors to the log
mbox series

Message ID 20220602073405.28168-1-laurent.pinchart@ideasonboard.com
Headers show
Series
  • libcamera: Add colors to the log
Related show

Message

Laurent Pinchart June 2, 2022, 7:34 a.m. UTC 
Hello,

This patch series addresses a shortcoming of libcamera compared to
GStreamer or PipeWire, namely colored logs.

This was attempted by Marco Felsch in "[PATCH] libcamera: log: add
colors to log levels" but suffered from coloring message
unconditionally for streams, regardless of whether the stream was backed
by a TTY, a file or something else.

This turned out impossible to solve automatically (as far as I can tell)
as the C++ stream API doesn't expose the underlying file descriptor.
I've thus taken another approach, by exposing coloring through the
libcamera logger API and only enabling it automatically when logging to
std::cerr. This can be disabled with the new LIBCAMERA_LOG_NO_COLOR
environment variable.

Compared to v1, the first 3 patch have already been merged and are thus
not included in this version. The most notable other change is in patch
1/2, where the escape codes used for bright colors have changed to work
with minicom.

Laurent Pinchart (2):
  libcamera: base: log: Add coloring to the log output
  libcamera: base: log: Color the log prefix

 Documentation/environment_variables.rst |  21 +++-
 include/libcamera/base/log.h            |   5 +-
 include/libcamera/logging.h             |   4 +-
 src/libcamera/base/log.cpp              | 144 ++++++++++++++++++------
 4 files changed, 131 insertions(+), 43 deletions(-)


base-commit: dfcf638a0a6d26dbfab5348e646e5cb3c45ec51e

Comments

Kieran Bingham June 6, 2022, 12:05 p.m. UTC | #1 
Quoting Laurent Pinchart via libcamera-devel (2022-06-02 08:34:03)
> Hello,
> 
> This patch series addresses a shortcoming of libcamera compared to
> GStreamer or PipeWire, namely colored logs.
> 
> This was attempted by Marco Felsch in "[PATCH] libcamera: log: add
> colors to log levels" but suffered from coloring message
> unconditionally for streams, regardless of whether the stream was backed
> by a TTY, a file or something else.
> 
> This turned out impossible to solve automatically (as far as I can tell)
> as the C++ stream API doesn't expose the underlying file descriptor.
> I've thus taken another approach, by exposing coloring through the
> libcamera logger API and only enabling it automatically when logging to
> std::cerr. This can be disabled with the new LIBCAMERA_LOG_NO_COLOR
> environment variable.
> 
> Compared to v1, the first 3 patch have already been merged and are thus
> not included in this version. The most notable other change is in patch
> 1/2, where the escape codes used for bright colors have changed to work
> with minicom.

Aha, interesting - so there are terminal specific requirements. I'm sure
we'll learn more when it's in use too, but I like this, and I think it
helps readability for parsing the logs. I know there will be those who
find color schemes more difficult to parse, or who have color blind
requirements, but the env flag can disable for that.


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

> 
> Laurent Pinchart (2):
>   libcamera: base: log: Add coloring to the log output
>   libcamera: base: log: Color the log prefix
> 
>  Documentation/environment_variables.rst |  21 +++-
>  include/libcamera/base/log.h            |   5 +-
>  include/libcamera/logging.h             |   4 +-
>  src/libcamera/base/log.cpp              | 144 ++++++++++++++++++------
>  4 files changed, 131 insertions(+), 43 deletions(-)
> 
> 
> base-commit: dfcf638a0a6d26dbfab5348e646e5cb3c45ec51e
> -- 
> Regards,
> 
> Laurent Pinchart
>