The default logging callback in lavu currently contains several
"advanced" features, such as
- suppressing repeated messages
- automatically hiding the log prefix
- color support
They add significant complexity to the logging callback and - more
importantly - global state, making logging not thread-safe (and strictly
speaking introducing UB).
Many (perhaps most) of our callers either do not care for such fancy
features, or already use a custom logging callback. Therefore, it is
better to move them to cmdutils (for use by avtools) and leave the
default logging callback simple, straightforward and safe.
---
I hope everyone agrees that having global state in the default logging callback
is a BadThing(tm).
One thing for discussion is the logging prefix (the [mp3 @ 0xdeadface] thingy).
The current code checks whether the message contains a newline, and when it
doesn't it toggles a flag to skip the prefix on the next invocation. Since we
don't want such global flags, I see two possibilities:
* just abolish the prefices (what I did)
* print them always
Comments/other suggestions welcome.
---
avtools/avconv.c | 2 +-
avtools/avplay.c | 2 +-
avtools/avprobe.c | 1 +
avtools/cmdutils.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++--
avtools/cmdutils.h | 2 +
libavutil/log.c | 134 +--------------------------------------------
libavutil/version.h | 2 +-
7 files changed, 156 insertions(+), 140 deletions(-)
diff --git a/avtools/avconv.c b/avtools/avconv.c
index 719d289..6a1186d 100644
--- a/avtools/avconv.c
+++ b/avtools/avconv.c
@@ -2899,7 +2899,7 @@ int main(int argc, char **argv)
register_exit(avconv_cleanup);
- av_log_set_flags(AV_LOG_SKIP_REPEATED);
+ logging_init();
parse_loglevel(argc, argv, options);
avcodec_register_all();
diff --git a/avtools/avplay.c b/avtools/avplay.c
index b6dbc52..0c7cdf0 100644
--- a/avtools/avplay.c
+++ b/avtools/avplay.c
@@ -3009,7 +3009,7 @@ int main(int argc, char **argv)
{
int flags;
- av_log_set_flags(AV_LOG_SKIP_REPEATED);
+ logging_init();
parse_loglevel(argc, argv, options);
/* register all codecs, demux and protocols */
diff --git a/avtools/avprobe.c b/avtools/avprobe.c
index a9ca193..3a98519 100644
--- a/avtools/avprobe.c
+++ b/avtools/avprobe.c
@@ -1170,6 +1170,7 @@ int main(int argc, char **argv)
exit(1);
register_exit(avprobe_cleanup);
+ logging_init();
options = real_options;
parse_loglevel(argc, argv, options);
diff --git a/avtools/cmdutils.c b/avtools/cmdutils.c
index a19df30..070b5ff 100644
--- a/avtools/cmdutils.c
+++ b/avtools/cmdutils.c
@@ -19,17 +19,34 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include "config.h"
+
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <errno.h>
#include <math.h>
+#if HAVE_VALGRIND_VALGRIND_H
+#include <valgrind/valgrind.h>
+/* this is the log level at which valgrind will output a full backtrace */
+#define BACKTRACE_LOGLEVEL AV_LOG_ERROR
+#endif
+
+#if HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#if HAVE_IO_H
+#include <io.h>
+#endif
+
+#if HAVE_SETCONSOLETEXTATTRIBUTE
+#include <windows.h>
+#endif
+
/* Include only the enabled headers since some compilers (namely, Sun
Studio) will not omit unused inline functions and create undefined
references to libraries that are not being built. */
-
-#include "config.h"
#include "libavformat/avformat.h"
#include "libavfilter/avfilter.h"
#include "libavdevice/avdevice.h"
@@ -57,6 +74,8 @@ AVDictionary *format_opts, *codec_opts, *resample_opts;
static const int this_year = 2017;
+static int log_level = AV_LOG_INFO;
+
void init_opts(void)
{
#if CONFIG_SWSCALE
@@ -729,7 +748,7 @@ int opt_loglevel(void *optctx, const char *opt, const char
*arg)
for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
if (!strcmp(log_levels[i].name, arg)) {
- av_log_set_level(log_levels[i].level);
+ log_level = log_levels[i].level;
return 0;
}
}
@@ -742,7 +761,7 @@ int opt_loglevel(void *optctx, const char *opt, const char
*arg)
av_log(NULL, AV_LOG_FATAL, "\"%s\"\n", log_levels[i].name);
exit_program(1);
}
- av_log_set_level(level);
+ log_level = level;
return 0;
}
@@ -1697,3 +1716,129 @@ const char *media_type_string(enum AVMediaType
media_type)
default: return "unknown";
}
}
+
+#define NB_LEVELS 8
+#if HAVE_SETCONSOLETEXTATTRIBUTE
+static const uint8_t color[NB_LEVELS] = { 12, 12, 12, 14, 7, 10, 11, 8};
+static int16_t background, attr_orig;
+static HANDLE con;
+#define set_color(x) SetConsoleTextAttribute(con, background | color[x])
+#define reset_color() SetConsoleTextAttribute(con, attr_orig)
+#define print_256color(x)
+#else
+static const uint8_t color[NB_LEVELS] = {
+ 0x41, 0x41, 0x11, 0x03, 9, 0x02, 0x06, 0x07
+};
+#define set_color(x) fprintf(stderr, "\033[%d;3%dm", color[x] >> 4,
color[x]&15)
+#define print_256color(x) fprintf(stderr, "\033[38;5;%dm", x)
+#define reset_color() fprintf(stderr, "\033[0m")
+#endif
+static int use_color = -1;
+
+static void check_color_terminal(void)
+{
+#if HAVE_SETCONSOLETEXTATTRIBUTE
+ CONSOLE_SCREEN_BUFFER_INFO con_info;
+ con = GetStdHandle(STD_ERROR_HANDLE);
+ use_color = (con != INVALID_HANDLE_VALUE) && !getenv("NO_COLOR") &&
+ !getenv("AV_LOG_FORCE_NOCOLOR");
+ if (use_color) {
+ GetConsoleScreenBufferInfo(con, &con_info);
+ attr_orig = con_info.wAttributes;
+ background = attr_orig & 0xF0;
+ }
+#elif HAVE_ISATTY
+ char *term = getenv("TERM");
+ use_color = !getenv("NO_COLOR") && !getenv("AV_LOG_FORCE_NOCOLOR") &&
+ (getenv("TERM") && isatty(2) || getenv("AV_LOG_FORCE_COLOR"));
+ if (use_color)
+ use_color += term && strstr(term, "256color");
+#else
+ use_color = getenv("AV_LOG_FORCE_COLOR") && !getenv("NO_COLOR") &&
+ !getenv("AV_LOG_FORCE_NOCOLOR");
+#endif
+}
+
+static void colored_fputs(int level, int tint, const char *str)
+{
+ if (use_color < 0)
+ check_color_terminal();
+
+ switch (use_color) {
+ case 1:
+ set_color(level);
+ break;
+ case 2:
+ set_color(level);
+ if (tint)
+ print_256color(tint);
+ break;
+ default:
+ break;
+ }
+ fputs(str, stderr);
+ if (use_color) {
+ reset_color();
+ }
+}
+
+static void log_callback(void *avcl, int level, const char *fmt, va_list vl)
+{
+ static int print_prefix = 1;
+ static int count;
+ static char prev[1024];
+ char line[1024];
+ static int is_atty;
+ AVClass *avc = avcl ? *(AVClass **) avcl : NULL;
+ unsigned tint = level & 0xff00;
+
+ level &= 0xff;
+
+ if (level > log_level)
+ return;
+ line[0] = 0;
+ if (print_prefix && avc) {
+ if (avc->parent_log_context_offset) {
+ AVClass **parent = *(AVClass ***) (((uint8_t *) avcl) +
+ avc->parent_log_context_offset);
+ if (parent && *parent) {
+ snprintf(line, sizeof(line), "[%s @ %p] ",
+ (*parent)->item_name(parent), parent);
+ }
+ }
+ snprintf(line + strlen(line), sizeof(line) - strlen(line), "[%s @ %p]
",
+ avc->item_name(avcl), avcl);
+ }
+
+ vsnprintf(line + strlen(line), sizeof(line) - strlen(line), fmt, vl);
+
+ print_prefix = strlen(line) && line[strlen(line) - 1] == '\n';
+
+#if HAVE_ISATTY
+ if (!is_atty)
+ is_atty = isatty(2) ? 1 : -1;
+#endif
+
+ if (print_prefix && !strncmp(line, prev, sizeof line)) {
+ count++;
+ if (is_atty == 1)
+ fprintf(stderr, " Last message repeated %d times\r", count);
+ return;
+ }
+ if (count > 0) {
+ fprintf(stderr, " Last message repeated %d times\n", count);
+ count = 0;
+ }
+ colored_fputs(av_clip(level >> 3, 0, NB_LEVELS - 1), tint >> 8, line);
+ av_strlcpy(prev, line, sizeof line);
+
+#if CONFIG_VALGRIND_BACKTRACE
+ if (level <= BACKTRACE_LOGLEVEL)
+ VALGRIND_PRINTF_BACKTRACE("");
+#endif
+}
+
+void logging_init(void)
+{
+ av_log_set_callback(log_callback);
+}
diff --git a/avtools/cmdutils.h b/avtools/cmdutils.h
index cc78ac5..2490e54 100644
--- a/avtools/cmdutils.h
+++ b/avtools/cmdutils.h
@@ -542,6 +542,8 @@ void *grow_array(void *array, int elem_size, int *size, int
new_size);
*/
const char *media_type_string(enum AVMediaType media_type);
+void logging_init(void);
+
#define GROW_ARRAY(array, nb_elems)\
array = grow_array(array, sizeof(*array), &nb_elems, nb_elems + 1)
diff --git a/libavutil/log.c b/libavutil/log.c
index 37427ef..99c7b7a 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -24,14 +24,6 @@
* logging functions
*/
-#include "config.h"
-
-#if HAVE_UNISTD_H
-#include <unistd.h>
-#endif
-#if HAVE_IO_H
-#include <io.h>
-#endif
#include <stdarg.h>
#include <stdlib.h>
#include "avstring.h"
@@ -40,80 +32,7 @@
#include "internal.h"
#include "log.h"
-#if HAVE_VALGRIND_VALGRIND_H
-#include <valgrind/valgrind.h>
-/* this is the log level at which valgrind will output a full backtrace */
-#define BACKTRACE_LOGLEVEL AV_LOG_ERROR
-#endif
-
static int av_log_level = AV_LOG_INFO;
-static int flags;
-
-#define NB_LEVELS 8
-#if HAVE_SETCONSOLETEXTATTRIBUTE
-#include <windows.h>
-static const uint8_t color[NB_LEVELS] = { 12, 12, 12, 14, 7, 10, 11, 8};
-static int16_t background, attr_orig;
-static HANDLE con;
-#define set_color(x) SetConsoleTextAttribute(con, background | color[x])
-#define reset_color() SetConsoleTextAttribute(con, attr_orig)
-#define print_256color(x)
-#else
-static const uint8_t color[NB_LEVELS] = {
- 0x41, 0x41, 0x11, 0x03, 9, 0x02, 0x06, 0x07
-};
-#define set_color(x) fprintf(stderr, "\033[%d;3%dm", color[x] >> 4,
color[x]&15)
-#define print_256color(x) fprintf(stderr, "\033[38;5;%dm", x)
-#define reset_color() fprintf(stderr, "\033[0m")
-#endif
-static int use_color = -1;
-
-static void check_color_terminal(void)
-{
-#if HAVE_SETCONSOLETEXTATTRIBUTE
- CONSOLE_SCREEN_BUFFER_INFO con_info;
- con = GetStdHandle(STD_ERROR_HANDLE);
- use_color = (con != INVALID_HANDLE_VALUE) && !getenv("NO_COLOR") &&
- !getenv("AV_LOG_FORCE_NOCOLOR");
- if (use_color) {
- GetConsoleScreenBufferInfo(con, &con_info);
- attr_orig = con_info.wAttributes;
- background = attr_orig & 0xF0;
- }
-#elif HAVE_ISATTY
- char *term = getenv("TERM");
- use_color = !getenv("NO_COLOR") && !getenv("AV_LOG_FORCE_NOCOLOR") &&
- (getenv("TERM") && isatty(2) || getenv("AV_LOG_FORCE_COLOR"));
- if (use_color)
- use_color += term && strstr(term, "256color");
-#else
- use_color = getenv("AV_LOG_FORCE_COLOR") && !getenv("NO_COLOR") &&
- !getenv("AV_LOG_FORCE_NOCOLOR");
-#endif
-}
-
-static void colored_fputs(int level, int tint, const char *str)
-{
- if (use_color < 0)
- check_color_terminal();
-
- switch (use_color) {
- case 1:
- set_color(level);
- break;
- case 2:
- set_color(level);
- if (tint)
- print_256color(tint);
- break;
- default:
- break;
- }
- fputs(str, stderr);
- if (use_color) {
- reset_color();
- }
-}
const char *av_default_item_name(void *ptr)
{
@@ -122,59 +41,9 @@ const char *av_default_item_name(void *ptr)
void av_log_default_callback(void *avcl, int level, const char *fmt, va_list
vl)
{
- static int print_prefix = 1;
- static int count;
- static char prev[1024];
- char line[1024];
- static int is_atty;
- AVClass* avc = avcl ? *(AVClass **) avcl : NULL;
- unsigned tint = level & 0xff00;
-
- level &= 0xff;
-
if (level > av_log_level)
return;
- line[0] = 0;
- if (print_prefix && avc) {
- if (avc->parent_log_context_offset) {
- AVClass** parent = *(AVClass ***) (((uint8_t *) avcl) +
- avc->parent_log_context_offset);
- if (parent && *parent) {
- snprintf(line, sizeof(line), "[%s @ %p] ",
- (*parent)->item_name(parent), parent);
- }
- }
- snprintf(line + strlen(line), sizeof(line) - strlen(line), "[%s @ %p]
",
- avc->item_name(avcl), avcl);
- }
-
- vsnprintf(line + strlen(line), sizeof(line) - strlen(line), fmt, vl);
-
- print_prefix = strlen(line) && line[strlen(line) - 1] == '\n';
-
-#if HAVE_ISATTY
- if (!is_atty)
- is_atty = isatty(2) ? 1 : -1;
-#endif
-
- if (print_prefix && (flags & AV_LOG_SKIP_REPEATED) &&
- !strncmp(line, prev, sizeof line)) {
- count++;
- if (is_atty == 1)
- fprintf(stderr, " Last message repeated %d times\r", count);
- return;
- }
- if (count > 0) {
- fprintf(stderr, " Last message repeated %d times\n", count);
- count = 0;
- }
- colored_fputs(av_clip(level >> 3, 0, NB_LEVELS - 1), tint >> 8, line);
- av_strlcpy(prev, line, sizeof line);
-
-#if CONFIG_VALGRIND_BACKTRACE
- if (level <= BACKTRACE_LOGLEVEL)
- VALGRIND_PRINTF_BACKTRACE("");
-#endif
+ vfprintf(stderr, fmt, vl);
}
static void (*av_log_callback)(void*, int, const char*, va_list) =
@@ -209,7 +78,6 @@ void av_log_set_level(int level)
void av_log_set_flags(int arg)
{
- flags = arg;
}
void av_log_set_callback(void (*callback)(void*, int, const char*, va_list))
diff --git a/libavutil/version.h b/libavutil/version.h
index 7779755..38e1906 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -55,7 +55,7 @@
#define LIBAVUTIL_VERSION_MAJOR 56
#define LIBAVUTIL_VERSION_MINOR 1
-#define LIBAVUTIL_VERSION_MICRO 1
+#define LIBAVUTIL_VERSION_MICRO 2
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
LIBAVUTIL_VERSION_MINOR, \
--
2.0.0
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
