diff mbox

[FFmpeg-devel,RFC] lavu/log: Do not print pointer addresses for loglevel < debug

Message ID CAB0OVGpSr7j-XS+8vLbpCMfLL_8-fUV4qjsnmrGewe0yXMCM1A@mail.gmail.com
State Rejected
Headers show

Commit Message

Carl Eugen Hoyos Dec. 16, 2019, 8:05 p.m. UTC
Hi!

Attached patch reduces the verbosity of the console output, comments welcome.

Carl Eugen

Comments

Lou Logan Dec. 16, 2019, 8:47 p.m. UTC | #1
On Mon, 16 Dec 2019 21:05:55 +0100
Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> Attached patch reduces the verbosity of the console output, comments welcome.

I am for this patch. Results in less noise, and in my opinion the
addresses are unnecessary in anything < debug.
Michael Niedermayer Dec. 17, 2019, 10:22 a.m. UTC | #2
On Mon, Dec 16, 2019 at 09:05:55PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch reduces the verbosity of the console output, comments welcome.

with this it would no longer be possible to associate which of 2 instances
like 2 encoders or 2 decoders of the same type a message comes from

[...]
Nicolas George Dec. 17, 2019, 6:36 p.m. UTC | #3
Carl Eugen Hoyos (12019-12-16):
> From f164e22f185d6a3e69d3376246a41a6958dba215 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Mon, 16 Dec 2019 21:02:19 +0100
> Subject: [PATCH] lavu/log: Do not print pointer addresses for loglevel <
>  debug.
> 
> ---
>  libavutil/log.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)

It could make sense, but I think not exactly like that.

Right now, at loglevel info, we have:

[something @ 0x42] Info

And at loglevel debug, we have:

[something @ 0x42] Info
[something @ 0x42] Debug

With your change, that would become respectively:

(i) [something  Info

(d) [something] Info
(d) [something @ 0x42] Debug

It think it would be better to have:

(i) [something] Info

(d) [something @ 0x42] Info
(d) [something @ 0x42] Debug

In practice, it means testing av_log_level rather than level to
determine whether the address should be printed. That way, if the
address is needed (to distinguish different objects of the same type, as
Michael pointed), it can be: it is treating the address part itself as
level debug.

Also, maybe verbose rather than debug. I am not sure.

Regards,
Carl Eugen Hoyos Dec. 17, 2019, 8:44 p.m. UTC | #4
Am Di., 17. Dez. 2019 um 19:36 Uhr schrieb Nicolas George <george@nsup.org>:
>
> Carl Eugen Hoyos (12019-12-16):
> > From f164e22f185d6a3e69d3376246a41a6958dba215 Mon Sep 17 00:00:00 2001
> > From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> > Date: Mon, 16 Dec 2019 21:02:19 +0100
> > Subject: [PATCH] lavu/log: Do not print pointer addresses for loglevel <
> >  debug.
> >
> > ---
> >  libavutil/log.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
>
> It could make sense, but I think not exactly like that.
>
> Right now, at loglevel info, we have:
>
> [something @ 0x42] Info
>
> And at loglevel debug, we have:
>
> [something @ 0x42] Info
> [something @ 0x42] Debug
>
> With your change, that would become respectively:
>
> (i) [something  Info
>
> (d) [something] Info
> (d) [something @ 0x42] Debug
>
> It think it would be better to have:
>
> (i) [something] Info
>
> (d) [something @ 0x42] Info
> (d) [something @ 0x42] Debug
>
> In practice, it means testing av_log_level rather than level to
> determine whether the address should be printed.

Of course.

Carl Eugen
diff mbox

Patch

From f164e22f185d6a3e69d3376246a41a6958dba215 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Mon, 16 Dec 2019 21:02:19 +0100
Subject: [PATCH] lavu/log: Do not print pointer addresses for loglevel <
 debug.

---
 libavutil/log.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/libavutil/log.c b/libavutil/log.c
index e8a0db7716..9f246ff99c 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -260,13 +260,21 @@  static void format_line(void *avcl, int level, const char *fmt, va_list vl,
             AVClass** parent = *(AVClass ***) (((uint8_t *) avcl) +
                                    avc->parent_log_context_offset);
             if (parent && *parent) {
-                av_bprintf(part+0, "[%s @ %p] ",
-                         (*parent)->item_name(parent), parent);
+                if (level >= AV_LOG_DEBUG) {
+                    av_bprintf(part+0, "[%s @ %p] ",
+                               (*parent)->item_name(parent), parent);
+                } else {
+                    av_bprintf(part+0, "[%s ", (*parent)->item_name(parent));
+                }
                 if(type) type[0] = get_category(parent);
             }
         }
-        av_bprintf(part+1, "[%s @ %p] ",
-                 avc->item_name(avcl), avcl);
+        if (level >= AV_LOG_DEBUG) {
+            av_bprintf(part+1, "[%s @ %p] ",
+                       avc->item_name(avcl), avcl);
+        } else {
+            av_bprintf(part+1, "[%s] ", avc->item_name(avcl));
+        }
         if(type) type[1] = get_category(avcl);
     }
 
-- 
2.23.0