Re: [FFmpeg-devel] [PATCH v2] lavfi/drawtext: Add localtime_ms for millisecond precision

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> On Jan 18, 2022, at 8:52 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
> 
> Am 16.01.22 um 12:06 schrieb Nicolas George:
>> Thilo Borgman (12022-01-14):
>>> v6 does:
>>> 
>>> $> ffmpeg ... drawtext="fontfile=...:text='%{localtime   \:%a %b %d %Y %S}'"           (seconds)
>>> $> ffmpeg ... drawtext="fontfile=...:text='%{localtime_ms\:%a %b %d %Y %S}'"           (milliseconds)
>>> 
>>> I suggest v7 should according to your remark:
>>> 
>>> $> ffmpeg ... drawtext="fontfile=...:text='%{localtime   \:%a %b %d %Y %S}'"           (seconds)
>>> $> ffmpeg ... drawtext="fontfile=...:text='%{localtime   \:%a %b %d %Y %S}':show_ms=1" (milliseconds)
>>> 
>>> Good?
>> 
>> I dislike both versions, from a user interface point of view: if there
>> is a format string, then it stands to reason, for the user, that the
>> resulting text is governed by the format string, not by an extra option
>> somewhere else.
>> 
>> There is no "use_four_digit_year=1" option, there is %Y instead of %y.
>> 
>> There is no "use_slashes=1" option, you write %Y/%m/%d instead of
>> %Y-%m-%d.
>> 
>> There are no "omit_date=1" and "omit_hour=1" options, you just write
>> what you want in the format string.
>> 
>> My proposal goes the same way:
>> 
>> $> ffmpeg ... drawtext="fontfile=...:text='%{localtime   \:%a %b %d %Y %S.%3N}'"
>> 
>> It has several merits over your proposal:
>> 
>> - It can be extended later to support printing the milliseconds at
>>  another place than the end (for example to put the time in brackets).
>> 
>> - It can be extended to support microseconds or centiseconds (%6N, %2N).
>> 
>> - It is somewhat compatible with GNU date and possibly a few others.
>> 
>> And I do not think it is harder to implement.
> 
> Ok, did introduce a variable: %[1-6]N
> Parsing and clipping value to valid range of 1-6.
> Default 3.
> 
> That way it is position independent and can show any number of decimals from 1 to 6.
> 

> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 2a88692cbd..448b174dbb 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -51,6 +51,7 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/random_seed.h"
>  #include "libavutil/parseutils.h"
> +#include "libavutil/time.h"
>  #include "libavutil/timecode.h"
>  #include "libavutil/time_internal.h"
>  #include "libavutil/tree.h"
> @@ -1045,14 +1046,82 @@ static int func_strftime(AVFilterContext *ctx, AVBPrint *bp,
>                           char *fct, unsigned argc, char **argv, int tag)
>  {
>      const char *fmt = argc ? argv[0] : "%Y-%m-%d %H:%M:%S";
> +    int64_t unow;
>      time_t now;
>      struct tm tm;
> -
> -    time(&now);
> -    if (tag == 'L')
> +    char *begin;
> +    char *tmp;
> +    int len;
> +    char *fmt_new;
> +    const char *fmt_tmp;
> +    int div;
> +
> +    unow = av_gettime();
> +    now  = unow / 1000000;
> +    if (tag == 'L' || tag == 'm')
>          localtime_r(&now, &tm);
>      else
>          tm = *gmtime_r(&now, &tm);
> +
> +    // manually parse format for %N (fractional seconds)
> +    begin = (char*)fmt;

Make begin and tmp const char *, so the cast can be removed.

> +    while ((begin = av_stristr(begin, "%"))) {

How about strstr() since ‘%’ is caseless?

> +        tmp = begin + 1;
> +        len = 0;
> +        // count digits between % and possible N
> +        while (*tmp != '\0' && av_isdigit((int)*tmp)) {
> +            len++;
> +            tmp++;
> +        }
> +        // N encountered, insert time
> +        if (*tmp == 'N') {
> +            int num_digits = 3; // default show millisecond [1,6]
> +
> +            // if digits given, parse as number in [1,6]
> +            if (len > 0) {
> +                av_sscanf(begin + 1, "%i", &num_digits);
> +                num_digits = av_clip(num_digits, 1, 6); // ensure valid value

We can ignore len > 1, then the code can be simplified as

if (len == 1)
    num_digits = av_clip(*(begin + 1) - ‘\0’, 1, 6)


> +            }
> +
> +            len += 2; // add % and N to get length of string part
> +
> +            switch(num_digits) {
> +            case 1:
> +                fmt_tmp = "%.*s%01d%s";
> +                div     = 100000;
> +                break;
> +            case 2:
> +                fmt_tmp = "%.*s%02d%s";
> +                div     = 10000;
> +                break;
> +            case 3:
> +                fmt_tmp = "%.*s%03d%s";
> +                div     = 1000;
> +                break;
> +            case 4:
> +                fmt_tmp = "%.*s%04d%s";
> +                div     = 100;
> +                break;
> +            case 5:
> +                fmt_tmp = "%.*s%05d%s";
> +                div     = 10;
> +                break;
> +            case 6:
> +                fmt_tmp = "%.*s%06d%s";
> +                div     = 1;
> +                break;
> +            }

The switch-case can be replaced by “%0*d” and pow(10, 6 - num_digits).

> +
> +            fmt_new = av_asprintf(fmt_tmp, begin - fmt, fmt, (int)(unow % 1000000) / div, begin + len);
> +            if (!fmt_new)
> +                return AVERROR(ENOMEM);
> +            av_bprint_strftime(bp, fmt_new, &tm);
> +            av_freep(&fmt_new);
> +            return 0;
> +        }
> +        begin++;

Progress faster by taking account of len.

> +    }
> +
>      av_bprint_strftime(bp, fmt, &tm);
>      return 0;
>  }
> -- 
> 2.20.1 (Apple Git-117)
> 


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".