[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