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

[Thilo Borgmann <thilo.borgmann@mail.de>]

Am 20.01.22 um 16:03 schrieb Andreas Rheinhardt:
> Thilo Borgmann:
>> Am 20.01.22 um 13:04 schrieb Thilo Borgmann:
>>> Am 19.01.22 um 04:16 schrieb "zhilizhao(赵志立)":
>>>>
>>>>
>>>>> 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).
>>>
>>> Indeed, simplified.
>>>
>>>
>>>>> +
>>>>> +            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.
>>>
>>> As well, also added to skip "%%".
>>>
>>>
>>>>> +    }
>>>>> +
>>>>>        av_bprint_strftime(bp, fmt, &tm);
>>>>>        return 0;
>>>>>    }
>>>>> -- 
>>>
>>> v8 attached.
>>
>> Fixed off-by-one bug.
>> Allows for several occurrences of %N parameter now.
>>
>> v9 attached.
>>
>> Thanks,
>> Thilo
>>
>>
>> +    // manually parse format for %N (fractional seconds)
>> +    begin = fmt;
>> +    while ((begin = av_stristr(begin, "%"))) {
>> +        tmp = begin + 1;
> 
> begin = strchr(begin, '%')

Done.
Also fixed buggy freep() for non user-supplied format string.

v10 attached.

-Thilo
_______________________________________________
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".