[FFmpeg-devel] [PATCH v7 2/2] fftools: Add option to log timing

[Andreas Rheinhardt]

Soft Works:
> This commit adds two logging flags: 'time' and 'datetime'.
> 
> Usage:
> 
> ffmpeg -loglevel +time
> 
> or
> 
> ffmpeg -loglevel +datetime
> 
> Signed-off-by: softworkz <softworkz at hotmail.com>
> ---
>  doc/fftools-common-opts.texi |  4 ++++
>  fftools/cmdutils.c           | 21 +++++++++++++++++++++
>  fftools/ffmpeg.c             |  6 +++++-
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
> index 7643dd8396..aae26c28d0 100644
> --- a/doc/fftools-common-opts.texi
> +++ b/doc/fftools-common-opts.texi
> @@ -198,6 +198,10 @@ and the "Last message repeated n times" line will be omitted.
>  Indicates that log output should add a @code{[level]} prefix to each message
>  line. This can be used as an alternative to log coloring, e.g. when dumping the
>  log to file.
> + at item time
> +Prefixes each log line with the local time.
> + at item datetime
> +Same as time but also prints the current date in each line.
>  @end table
>  Flags can also be used alone by adding a '+'/'-' prefix to set/reset a single
>  flag without affecting other @var{flags} or changing @var{loglevel}. When
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 912e881174..7918078766 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -918,6 +918,27 @@ int opt_loglevel(void *optctx, const char *opt, const char *arg)
>                  flags |= AV_LOG_PRINT_LEVEL;
>              }
>              arg = token + 5;
> +        } else if (av_strstart(token, "time", NULL)) {
> +            if (cmd == '-') {
> +                flags &= ~AV_LOG_PRINT_TIME;
> +            } else {
> +                flags |= AV_LOG_PRINT_TIME;
> +            }
> +            arg = token + 4;
> +        } else if (av_strstart(token, "timing", NULL)) {

Why are you recognizing an undocumented and redundant command? Just
remove this.

> +            if (cmd == '-') {
> +                flags &= ~AV_LOG_PRINT_TIME;
> +            } else {
> +                flags |= AV_LOG_PRINT_TIME;
> +            }
> +            arg = token + 6;
> +        } else if (av_strstart(token, "datetime", NULL)) {
> +            if (cmd == '-') {
> +                flags &= ~AV_LOG_PRINT_DATETIME;
> +            } else {
> +                flags |= AV_LOG_PRINT_DATETIME;
> +            }
> +            arg = token + 8;

This still hardcodes the lengths of the strings, resulting in potential
breakage if one is updated and the other is forgotten. See my patch for
how to avoid this.

>          } else {
>              break;
>          }
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index b0ce7c7c32..b0b105be1c 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -4983,7 +4983,7 @@ static void log_callback_null(void *ptr, int level, const char *fmt, va_list vl)
>  
>  int main(int argc, char **argv)
>  {
> -    int i, ret;
> +    int i, ret, log_flags;
>      BenchmarkTimeStamps ti;
>  
>      init_dynload();
> @@ -5049,6 +5049,10 @@ int main(int argc, char **argv)
>      if ((decode_error_stat[0] + decode_error_stat[1]) * max_error_rate < decode_error_stat[1])
>          exit_program(69);
>  
> +    log_flags = av_log_get_flags();
> +    if (log_flags & AV_LOG_PRINT_TIME || log_flags & AV_LOG_PRINT_DATETIME)

You can check both flags at once (as Nicolas has already pointed out).

> +        av_log(NULL, AV_LOG_INFO, "\n"); /* end with line break to avoid mixing up with shell prompt */
> +
>      exit_program(received_nb_signals ? 255 : main_return_code);
>      return main_return_code;
>  }
>