[FFmpeg-devel] [PATCH 1/5] libavutil: prefix all log messages with thread id
Nicolas George
nicolas.george at normalesup.org
Tue Jul 3 10:00:04 CEST 2012
Le quintidi 15 messidor, an CCXX, Martin Carroll a écrit :
> To make it easier to see which thread is doing what, the logger can
> now be configured to prefix all log messages with either the numeric
> id or the name of the calling thread. The name (instead of numeric
> id) is printed if the thread was previously registered with
> av_log_set_threadname(). To enable this feature, the variable
> print_threadid in libavutil/log.c must be set to 1.
>
> Signed-off-by: Martin Carroll <martin.carroll at alcatel-lucent.com>
> ---
> libavutil/log.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++-------
> libavutil/log.h | 2 +-
> 2 files changed, 91 insertions(+), 13 deletions(-)
>
> diff --git a/libavutil/log.c b/libavutil/log.c
> index a08223e..448843f 100644
> --- a/libavutil/log.c
> +++ b/libavutil/log.c
> @@ -1,4 +1,4 @@
> -/*
> +/*
> * log functions
> * Copyright (c) 2003 Michel Bardiaux
> *
> @@ -30,14 +30,30 @@
> #include <unistd.h>
> #endif
> #include <stdlib.h>
> +#include <sys/types.h>
> #include "avutil.h"
> #include "log.h"
>
> #define LINE_SZ 1024
>
> +#define MAX_THREAD_INFOS 10
> +#define MAX_THREAD_NAMELEN 128
> +
> +typedef struct {
> + pthread_t pid;
> + char name[MAX_THREAD_NAMELEN];
> + void *fcn;
> + int instance_num_of_fcn;
> + int sole_instance_of_fcn;
> +} Thread_info;
> +
> +static Thread_info thread_info[MAX_THREAD_INFOS];
> +static int num_thread_infos = 0;
> static int av_log_level = AV_LOG_INFO;
> static int flags;
>
> +static int print_threadid = 0; // set to 1 to prefix all log messages with name or id of thread
> +
> #if defined(_WIN32) && !defined(__MINGW32CE__)
> #include <windows.h>
> #include <io.h>
> @@ -154,11 +170,56 @@ static int get_category(void *ptr){
> return avc->category + 16;
> }
>
> +void av_log_set_threadname(pthread_t pid, const char * name, void *fcn)
Please, here and everywhere else, declare pointers as "type *pointer", not
"type * pointer" nor "type* pointer".
> +{
> +#if 0
> + char str[300];
> + snprintf(str, sizeof(str), "XXXXXXXXXXXXXXXX: %08x => %s\n", pid, name);
> + colored_fputs(0, str);
> +#endif
Leftover debug.
> + if (num_thread_infos < MAX_THREAD_INFOS) {
> + Thread_info *info;
> + int instance_num_of_fcn = 0;
> + int sole_instance_of_fcn = 1;
> + int i;
> +
> + for (i = 0; i < num_thread_infos; ++i) {
> + info = &thread_info[i];
> + if (info->fcn == fcn) {
> + info->sole_instance_of_fcn = 0;
> + sole_instance_of_fcn = 0;
> + ++instance_num_of_fcn;
> + }
> + }
> + info = &thread_info[num_thread_infos];
> + info->pid = pid;
> + info->fcn = fcn;
> + info->instance_num_of_fcn = instance_num_of_fcn;
> + info->sole_instance_of_fcn = sole_instance_of_fcn;
> + snprintf(info->name, MAX_THREAD_NAMELEN, "%s", name);
> + info->name[MAX_THREAD_NAMELEN-1] = 0;
> + ++num_thread_infos;
> + }
> +}
I see a global structure meant to be accessed from several threads and
nothing to make a memory barrier.
Also, it seems to me that using thread-local storage for that would be much
simpler and more efficient.
> +
> +static Thread_info* lookup_thread_info()
> +{
> + int i;
> +
> + pthread_t pid = pthread_self();
> + for (i = 0; i < num_thread_infos; ++i) {
> + Thread_info* info = &thread_info[i];
> + if (info->pid == pid)
You can not do that, pthread_t is allowed to be an arbitrary structure.
> + return info;
> + }
> + return NULL;
> +}
> +
> static void format_line(void *ptr, int level, const char *fmt, va_list vl,
> - char part[3][LINE_SZ], int part_size, int *print_prefix, int type[2])
> + char part[5][LINE_SZ], int part_size, int *print_prefix, int type[2])
> {
> AVClass* avc = ptr ? *(AVClass **) ptr : NULL;
> - part[0][0] = part[1][0] = part[2][0] = 0;
> + part[0][0] = part[1][0] = part[2][0] = part[3][0] = part[4][0] = 0;
> if(type) type[0] = type[1] = AV_CLASS_CATEGORY_NA + 16;
> if (*print_prefix && avc) {
> if (avc->parent_log_context_offset) {
> @@ -174,18 +235,29 @@ static void format_line(void *ptr, int level, const char *fmt, va_list vl,
> avc->item_name(ptr), ptr);
> if(type) type[1] = get_category(ptr);
> }
> -
> - vsnprintf(part[2], part_size, fmt, vl);
> -
> - *print_prefix = strlen(part[2]) && part[2][strlen(part[2]) - 1] == '\n';
> + if (print_threadid) {
> + Thread_info* info = lookup_thread_info();
> + if (info) {
> + int i;
> +
> + if (info->sole_instance_of_fcn)
> + snprintf(part[2], part_size, "%s: ", info->name);
> + else
> + snprintf(part[2], part_size, "%s(%d): ", info->name, info->instance_num_of_fcn);
> + }
> + else
> + snprintf(part[2], part_size, "%08x: ", pthread_self());
> + }
> + vsnprintf(part[4], part_size, fmt, vl);
> + *print_prefix = strlen(part[4]) && part[4][strlen(part[4]) - 1] == '\n';
> }
>
> void av_log_format_line(void *ptr, int level, const char *fmt, va_list vl,
> char *line, int line_size, int *print_prefix)
> {
> - char part[3][LINE_SZ];
> + char part[5][LINE_SZ];
> format_line(ptr, level, fmt, vl, part, sizeof(part[0]), print_prefix, NULL);
> - snprintf(line, line_size, "%s%s%s", part[0], part[1], part[2]);
> + snprintf(line, line_size, "%s%s%s%s%s", part[0], part[1], part[2], part[3], part[4]);
> }
>
> void av_log_default_callback(void* ptr, int level, const char* fmt, va_list vl)
> @@ -193,7 +265,7 @@ void av_log_default_callback(void* ptr, int level, const char* fmt, va_list vl)
> static int print_prefix = 1;
> static int count;
> static char prev[LINE_SZ];
> - char part[3][LINE_SZ];
> + char part[5][LINE_SZ];
> char line[LINE_SZ];
> static int is_atty;
> int type[2];
> @@ -201,7 +273,7 @@ void av_log_default_callback(void* ptr, int level, const char* fmt, va_list vl)
> if (level > av_log_level)
> return;
> format_line(ptr, level, fmt, vl, part, sizeof(part[0]), &print_prefix, type);
> - snprintf(line, sizeof(line), "%s%s%s", part[0], part[1], part[2]);
> + snprintf(line, sizeof(line), "%s%s%s%s%s", part[0], part[1], part[2], part[3], part[4]);
>
> #if HAVE_ISATTY
> if (!is_atty)
> @@ -225,7 +297,12 @@ void av_log_default_callback(void* ptr, int level, const char* fmt, va_list vl)
> sanitize(part[1]);
> colored_fputs(type[1], part[1]);
> sanitize(part[2]);
> - colored_fputs(av_clip(level >> 3, 0, 6), part[2]);
> + int lev = av_clip(level >> 3, 0, 6);
> + colored_fputs(lev, part[2]);
> + sanitize(part[3]);
> + colored_fputs(lev, part[3]);
> + sanitize(part[4]);
> + colored_fputs(lev, part[4]);
> }
>
> static void (*av_log_callback)(void*, int, const char*, va_list) =
> @@ -268,3 +345,4 @@ void av_log_set_callback(void (*callback)(void*, int, const char*, va_list))
> {
> av_log_callback = callback;
> }
> +
> diff --git a/libavutil/log.h b/libavutil/log.h
> index 8d9601a..ea85224 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -167,7 +167,7 @@ typedef struct AVClass {
> * @see av_vlog
> */
> void av_log(void *avcl, int level, const char *fmt, ...) av_printf_format(3, 4);
> -
> +void av_log_set_threadname(pthread_t pid, const char* name, void *fcn);
> void av_vlog(void *avcl, int level, const char *fmt, va_list);
> int av_log_get_level(void);
> void av_log_set_level(int);
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120703/a4a0b790/attachment.asc>
More information about the ffmpeg-devel
mailing list