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 libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel