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

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

["zhilizhao(赵志立)"]

> On Dec 13, 2021, at 4:12 AM, Nicolas George <george@nsup.org> wrote:
> 
> Thilo Borgman (12021-12-12):
>> +@item localtime_ms
>> +Same as @code{localtime} but with millisecond precision.
>> +It can accept an argument: a strftime() format string.
> 
> What happens if the format string is something like '%H:%M:%S%z'? My
> guess it it prints 21:12:42+0100.1234 instead of 21:12:42.1234+0100.

How about add a restriction like this:

if (format.endsWith(“%S"))
    enable the feature
else
    warning message

It’s a useful feature, it shouldn't create unexpected results, but
doesn’t need to support every use case.

> 
> Regards,
> 
> -- 
>  Nicolas George
> _______________________________________________
> 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".

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

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

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 690 bytes --]

"zhilizhao(赵志立)" (12021-12-29):
> How about add a restriction like this:
> 
> if (format.endsWith(“%S"))
>     enable the feature
> else
>     warning message
> 
> It’s a useful feature, it shouldn't create unexpected results, but
> doesn’t need to support every use case.

I would not oppose it, but I find it inelegant, especially because it
requires a different expansion function, localtime_ms instead of
localtime.

What about this: with the original function "localtime", if the format
ends in "%3N", then append the millisecond. It can later be expanded to
support %xN at any place in the format for any value of x.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

Am 29.12.21 um 12:46 schrieb Nicolas George:
> "zhilizhao(赵志立)" (12021-12-29):
>> How about add a restriction like this:
>>
>> if (format.endsWith(“%S"))
>>     enable the feature
>> else
>>     warning message
>>
>> It’s a useful feature, it shouldn't create unexpected results, but
>> doesn’t need to support every use case.
> 
> I would not oppose it, but I find it inelegant, especially because it
> requires a different expansion function, localtime_ms instead of
> localtime.
> 
> What about this: with the original function "localtime", if the format
> ends in "%3N", then append the millisecond. It can later be expanded to
> support %xN at any place in the format for any value of x.

I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.

Just need to find time to actually implement it. 

Thanks,
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".

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
> Am 29.12.21 um 12:46 schrieb Nicolas George:
>> "zhilizhao(赵志立)" (12021-12-29):
>>> How about add a restriction like this:
>>>
>>> if (format.endsWith(“%S"))
>>>     enable the feature
>>> else
>>>     warning message
>>>
>>> It’s a useful feature, it shouldn't create unexpected results, but
>>> doesn’t need to support every use case.
>>
>> I would not oppose it, but I find it inelegant, especially because it
>> requires a different expansion function, localtime_ms instead of
>> localtime.
>>
>> What about this: with the original function "localtime", if the format
>> ends in "%3N", then append the millisecond. It can later be expanded to
>> support %xN at any place in the format for any value of x.
> 
> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
> 
> Just need to find time to actually implement it. 

Like v5 as attached.

Thanks,
Thilo

[-- Attachment #2: v5-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch --]
[-- Type: text/plain, Size: 3237 bytes --]

From c7f7c7a1cedc4ccc51977fc92645e1131608ac95 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Thu, 6 Jan 2022 12:24:46 +0100
Subject: [PATCH v5] lavfi/drawtext: Add localtime_ms for millisecond precision

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  8 ++++++++
 libavfilter/vf_drawtext.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..967021e48b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11379,10 +11379,18 @@ It can be used to add padding with zeros from the left.
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
 
+@item gmtime_ms
+Same as @code{gmtime} but with millisecond precision.
+It can accept an argument: a strftime() format string.
+
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
 
+@item localtime_ms
+Same as @code{localtime} but with millisecond precision.
+It can accept an argument: a strftime() format string.
+
 @item metadata
 Frame metadata. Takes one or two arguments.
 
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..723473f299 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,35 @@ 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')
+    unow = av_gettime();
+    now  = unow / 1000000;
+    if (tag == 'L' || tag == 'm')
         localtime_r(&now, &tm);
     else
         tm = *gmtime_r(&now, &tm);
+
+    if (tag == 'M' || tag == 'm') {
+        char *seconds = av_stristr(fmt, "%S");
+        if (seconds) {
+            seconds += 2;
+            int len = seconds - fmt + 1;
+            char *tmp = av_malloc(len);
+            av_strlcpy(tmp, fmt, len);
+
+            char *fmt_new = av_asprintf("%s.%03d%s", tmp, (int)(unow % 1000000) / 1000, seconds);
+            av_bprint_strftime(bp, fmt_new, &tm);
+
+            av_freep(&tmp);
+            av_freep(&fmt_new);
+
+            return 0;
+        }
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
     return 0;
 }
@@ -1152,7 +1174,9 @@ static const struct drawtext_function {
     { "pict_type", 0, 0, 0,   func_pict_type },
     { "pts",       0, 3, 0,   func_pts      },
     { "gmtime",    0, 1, 'G', func_strftime },
+    { "gmtime_ms", 0, 1, 'M', func_strftime },
     { "localtime", 0, 1, 'L', func_strftime },
+    { "localtime_ms", 0, 1, 'm', func_strftime },
     { "frame_num", 0, 0, 0,   func_frame_num },
     { "n",         0, 0, 0,   func_frame_num },
     { "metadata",  1, 2, 0,   func_metadata },
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>> "zhilizhao(赵志立)" (12021-12-29):
>>>> How about add a restriction like this:
>>>>
>>>> if (format.endsWith(“%S"))
>>>>     enable the feature
>>>> else
>>>>     warning message
>>>>
>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>> doesn’t need to support every use case.
>>>
>>> I would not oppose it, but I find it inelegant, especially because it
>>> requires a different expansion function, localtime_ms instead of
>>> localtime.
>>>
>>> What about this: with the original function "localtime", if the format
>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>> support %xN at any place in the format for any value of x.
>>
>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>
>> Just need to find time to actually implement it. 
> 
> Like v5 as attached.

Ping ^

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

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

["zhilizhao(赵志立)"]

> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
> 
> Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
>> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>>> "zhilizhao(赵志立)" (12021-12-29):
>>>>> How about add a restriction like this:
>>>>> 
>>>>> if (format.endsWith(“%S"))
>>>>>    enable the feature
>>>>> else
>>>>>    warning message
>>>>> 
>>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>>> doesn’t need to support every use case.
>>>> 
>>>> I would not oppose it, but I find it inelegant, especially because it
>>>> requires a different expansion function, localtime_ms instead of
>>>> localtime.
>>>> 
>>>> What about this: with the original function "localtime", if the format
>>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>>> support %xN at any place in the format for any value of x.
>>> 
>>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>> 
>>> Just need to find time to actually implement it. 
>> 
>> Like v5 as attached.


> +    if (tag == 'M' || tag == 'm') {
> +        char *seconds = av_stristr(fmt, "%S");
> +        if (seconds) {
> +            seconds += 2;
> +            int len = seconds - fmt + 1;
> +            char *tmp = av_malloc(len);
> +            av_strlcpy(tmp, fmt, len);

Firstly, mixed variable declaration and statements.

Secondly, I think you don’t need ’tmp’, something like

av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);

> +
> +            char *fmt_new = av_asprintf("%s.%03d%s", tmp, (int)(unow % 1000000) / 1000, seconds);
> +            av_bprint_strftime(bp, fmt_new, &tm);
> +
> +            av_freep(&tmp);
> +            av_freep(&fmt_new);
> +
> +            return 0;
> +        }
> +    }


> 
> Ping ^
> 
> -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".

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]

Am 14.01.22 um 14:17 schrieb "zhilizhao(赵志立)":
> 
> 
>> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>
>> Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
>>> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>>>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>>>> "zhilizhao(赵志立)" (12021-12-29):
>>>>>> How about add a restriction like this:
>>>>>>
>>>>>> if (format.endsWith(“%S"))
>>>>>>    enable the feature
>>>>>> else
>>>>>>    warning message
>>>>>>
>>>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>>>> doesn’t need to support every use case.
>>>>>
>>>>> I would not oppose it, but I find it inelegant, especially because it
>>>>> requires a different expansion function, localtime_ms instead of
>>>>> localtime.
>>>>>
>>>>> What about this: with the original function "localtime", if the format
>>>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>>>> support %xN at any place in the format for any value of x.
>>>>
>>>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>>>
>>>> Just need to find time to actually implement it. 
>>>
>>> Like v5 as attached.
> 
> 
>> +    if (tag == 'M' || tag == 'm') {
>> +        char *seconds = av_stristr(fmt, "%S");
>> +        if (seconds) {
>> +            seconds += 2;
>> +            int len = seconds - fmt + 1;
>> +            char *tmp = av_malloc(len);
>> +            av_strlcpy(tmp, fmt, len);
> 
> Firstly, mixed variable declaration and statements.
> 
> Secondly, I think you don’t need ’tmp’, something like
> 
> av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);

You know your printf format-string :)

Thanks, v6 attached.
-Thilo


[-- Attachment #2: v6-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch --]
[-- Type: text/plain, Size: 3065 bytes --]

From 603a52a2bb86e558acdd754d8322e89e984dad08 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Fri, 14 Jan 2022 18:38:14 +0100
Subject: [PATCH v6] lavfi/drawtext: Add localtime_ms for millisecond precision

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  8 ++++++++
 libavfilter/vf_drawtext.c | 20 ++++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..967021e48b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11379,10 +11379,18 @@ It can be used to add padding with zeros from the left.
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
 
+@item gmtime_ms
+Same as @code{gmtime} but with millisecond precision.
+It can accept an argument: a strftime() format string.
+
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
 
+@item localtime_ms
+Same as @code{localtime} but with millisecond precision.
+It can accept an argument: a strftime() format string.
+
 @item metadata
 Frame metadata. Takes one or two arguments.
 
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..7e3771fdca 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,27 @@ 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')
+    unow = av_gettime();
+    now  = unow / 1000000;
+    if (tag == 'L' || tag == 'm')
         localtime_r(&now, &tm);
     else
         tm = *gmtime_r(&now, &tm);
+
+    if (tag == 'M' || tag == 'm') {
+        char *seconds = av_stristr(fmt, "%S");
+        if (seconds) {
+            int len = seconds + 2 - fmt;
+            char *fmt_new = av_asprintf("%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds + 2);
+            av_bprint_strftime(bp, fmt_new, &tm);
+            return 0;
+        }
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
     return 0;
 }
@@ -1152,7 +1166,9 @@ static const struct drawtext_function {
     { "pict_type", 0, 0, 0,   func_pict_type },
     { "pts",       0, 3, 0,   func_pts      },
     { "gmtime",    0, 1, 'G', func_strftime },
+    { "gmtime_ms", 0, 1, 'M', func_strftime },
     { "localtime", 0, 1, 'L', func_strftime },
+    { "localtime_ms", 0, 1, 'm', func_strftime },
     { "frame_num", 0, 0, 0,   func_frame_num },
     { "n",         0, 0, 0,   func_frame_num },
     { "metadata",  1, 2, 0,   func_metadata },
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Zhao Zhili]

> On Jan 15, 2022, at 1:40 AM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>> 
>>> +    if (tag == 'M' || tag == 'm') {
>>> +        char *seconds = av_stristr(fmt, "%S");
>>> +        if (seconds) {
>>> +            seconds += 2;
>>> +            int len = seconds - fmt + 1;
>>> +            char *tmp = av_malloc(len);
>>> +            av_strlcpy(tmp, fmt, len);
>> 
>> Firstly, mixed variable declaration and statements.
>> 
>> Secondly, I think you don’t need ’tmp’, something like
>> 
>> av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);
> 
> You know your printf format-string :)
> 
> Thanks, v6 attached.

LGTM now. Looking forward to use it for end-to-end latency test.

> -Thilo
> 
> 
> <v6-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

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

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

[Andreas Rheinhardt]

Thilo Borgmann:
> Am 14.01.22 um 14:17 schrieb "zhilizhao(赵志立)":
>>
>>
>>> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>
>>> Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
>>>> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>>>>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>>>>> "zhilizhao(赵志立)" (12021-12-29):
>>>>>>> How about add a restriction like this:
>>>>>>>
>>>>>>> if (format.endsWith(“%S"))
>>>>>>>    enable the feature
>>>>>>> else
>>>>>>>    warning message
>>>>>>>
>>>>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>>>>> doesn’t need to support every use case.
>>>>>>
>>>>>> I would not oppose it, but I find it inelegant, especially because it
>>>>>> requires a different expansion function, localtime_ms instead of
>>>>>> localtime.
>>>>>>
>>>>>> What about this: with the original function "localtime", if the format
>>>>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>>>>> support %xN at any place in the format for any value of x.
>>>>>
>>>>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>>>>
>>>>> Just need to find time to actually implement it. 
>>>>
>>>> Like v5 as attached.
>>
>>
>>> +    if (tag == 'M' || tag == 'm') {
>>> +        char *seconds = av_stristr(fmt, "%S");
>>> +        if (seconds) {
>>> +            seconds += 2;
>>> +            int len = seconds - fmt + 1;
>>> +            char *tmp = av_malloc(len);
>>> +            av_strlcpy(tmp, fmt, len);
>>
>> Firstly, mixed variable declaration and statements.
>>
>> Secondly, I think you don’t need ’tmp’, something like
>>
>> av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);
> 
> You know your printf format-string :)
> 
> Thanks, v6 attached.
> -Thilo
> 
> 

> 
> +            int len = seconds + 2 - fmt;
> +            char *fmt_new = av_asprintf("%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds + 2);
> +            av_bprint_strftime(bp, fmt_new, &tm);
> +            return 0;
> +        }

I see an unchecked allocation and a leak. And it seems you are using a
format string that comes from the user. This is undefined behaviour if
this string contains an invalid conversion specifier.

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

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

[Thilo Borgmann]

Am 14.01.22 um 19:05 schrieb Andreas Rheinhardt:
> Thilo Borgmann:
>> Am 14.01.22 um 14:17 schrieb "zhilizhao(赵志立)":
>>>
>>>
>>>> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>
>>>> Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
>>>>> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>>>>>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>>>>>> "zhilizhao(赵志立)" (12021-12-29):
>>>>>>>> How about add a restriction like this:
>>>>>>>>
>>>>>>>> if (format.endsWith(“%S"))
>>>>>>>>    enable the feature
>>>>>>>> else
>>>>>>>>    warning message
>>>>>>>>
>>>>>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>>>>>> doesn’t need to support every use case.
>>>>>>>
>>>>>>> I would not oppose it, but I find it inelegant, especially because it
>>>>>>> requires a different expansion function, localtime_ms instead of
>>>>>>> localtime.
>>>>>>>
>>>>>>> What about this: with the original function "localtime", if the format
>>>>>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>>>>>> support %xN at any place in the format for any value of x.
>>>>>>
>>>>>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>>>>>
>>>>>> Just need to find time to actually implement it. 
>>>>>
>>>>> Like v5 as attached.
>>>
>>>
>>>> +    if (tag == 'M' || tag == 'm') {
>>>> +        char *seconds = av_stristr(fmt, "%S");
>>>> +        if (seconds) {
>>>> +            seconds += 2;
>>>> +            int len = seconds - fmt + 1;
>>>> +            char *tmp = av_malloc(len);
>>>> +            av_strlcpy(tmp, fmt, len);
>>>
>>> Firstly, mixed variable declaration and statements.
>>>
>>> Secondly, I think you don’t need ’tmp’, something like
>>>
>>> av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);
>>
>> You know your printf format-string :)
>>
>> Thanks, v6 attached.
>> -Thilo
>>
>>
> 
>>
>> +            int len = seconds + 2 - fmt;
>> +            char *fmt_new = av_asprintf("%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds + 2);
>> +            av_bprint_strftime(bp, fmt_new, &tm);
>> +            return 0;
>> +        }
> 
> I see an unchecked allocation and a leak. 

Ok fmt_new might be null, where is the leak?


> And it seems you are using a
> format string that comes from the user. This is undefined behaviour if
> this string contains an invalid conversion specifier.

I think that was unfortunately true before the patch as well, was it not?
And if true or not, do we have something in place to check a user string?

Thanks,
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".

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

[Andreas Rheinhardt]

Thilo Borgmann:
> Am 14.01.22 um 19:05 schrieb Andreas Rheinhardt:
>> Thilo Borgmann:
>>> Am 14.01.22 um 14:17 schrieb "zhilizhao(赵志立)":
>>>>
>>>>
>>>>> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>>
>>>>> Am 06.01.22 um 12:27 schrieb Thilo Borgmann:
>>>>>> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>>>>>>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>>>>>>> "zhilizhao(赵志立)" (12021-12-29):
>>>>>>>>> How about add a restriction like this:
>>>>>>>>>
>>>>>>>>> if (format.endsWith(“%S"))
>>>>>>>>>    enable the feature
>>>>>>>>> else
>>>>>>>>>    warning message
>>>>>>>>>
>>>>>>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>>>>>>> doesn’t need to support every use case.
>>>>>>>>
>>>>>>>> I would not oppose it, but I find it inelegant, especially because it
>>>>>>>> requires a different expansion function, localtime_ms instead of
>>>>>>>> localtime.
>>>>>>>>
>>>>>>>> What about this: with the original function "localtime", if the format
>>>>>>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>>>>>>> support %xN at any place in the format for any value of x.
>>>>>>>
>>>>>>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>>>>>>
>>>>>>> Just need to find time to actually implement it. 
>>>>>>
>>>>>> Like v5 as attached.
>>>>
>>>>
>>>>> +    if (tag == 'M' || tag == 'm') {
>>>>> +        char *seconds = av_stristr(fmt, "%S");
>>>>> +        if (seconds) {
>>>>> +            seconds += 2;
>>>>> +            int len = seconds - fmt + 1;
>>>>> +            char *tmp = av_malloc(len);
>>>>> +            av_strlcpy(tmp, fmt, len);
>>>>
>>>> Firstly, mixed variable declaration and statements.
>>>>
>>>> Secondly, I think you don’t need ’tmp’, something like
>>>>
>>>> av_asprintf(“%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds);
>>>
>>> You know your printf format-string :)
>>>
>>> Thanks, v6 attached.
>>> -Thilo
>>>
>>>
>>
>>>
>>> +            int len = seconds + 2 - fmt;
>>> +            char *fmt_new = av_asprintf("%.*s.%03d%s", len, fmt, (int)(unow % 1000000) / 1000, seconds + 2);
>>> +            av_bprint_strftime(bp, fmt_new, &tm);
>>> +            return 0;
>>> +        }
>>
>> I see an unchecked allocation and a leak. 
> 
> Ok fmt_new might be null, where is the leak?
> 

Where is fmt_new freed?

> 
>> And it seems you are using a
>> format string that comes from the user. This is undefined behaviour if
>> this string contains an invalid conversion specifier.
> 
> I think that was unfortunately true before the patch as well, was it not?

Seems so.

> And if true or not, do we have something in place to check a user string?
> 

Afaik no.

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

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

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 624 bytes --]

Thilo Borgman (12022-01-14):
> >> I think best will be to scan the format string for %S and extend it
> >> there with .ms part before expanding the rest of it, not? Shouldn't
> >> be too expensive for the filter.
> >>
> >> Just need to find time to actually implement it. 

Sorry, I completely missed you reply.

No, I do not think it is best: your solution requires the user to use a
different function, I find this inelegant and not user friendly. I like
the solution where the option is enabled by the format string itself
much more user friendly. But we can discuss it.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

Am 14.01.22 um 18:57 schrieb Nicolas George:
> Thilo Borgman (12022-01-14):
>>>> I think best will be to scan the format string for %S and extend it
>>>> there with .ms part before expanding the rest of it, not? Shouldn't
>>>> be too expensive for the filter.
>>>>
>>>> Just need to find time to actually implement it. 
> 
> Sorry, I completely missed you reply.
> 
> No, I do not think it is best: your solution requires the user to use a
> different function, I find this inelegant and not user friendly. I like
> the solution where the option is enabled by the format string itself
> much more user friendly. But we can discuss it.

Ah and I misunderstood your other remark about it.

What about adding an option for ms precision and stick to the existing expansions localtime/gmtime, then add .ms if set by option?

Thanks,
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".

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

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 365 bytes --]

Thilo Borgman (12022-01-14):
> Ah and I misunderstood your other remark about it.
> 
> What about adding an option for ms precision and stick to the existing
> expansions localtime/gmtime, then add .ms if set by option?

I am not sure what you are suggesting.

How does it look like from the point of view of the user?

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

Am 14.01.22 um 19:08 schrieb Nicolas George:
> Thilo Borgman (12022-01-14):
>> Ah and I misunderstood your other remark about it.
>>
>> What about adding an option for ms precision and stick to the existing
>> expansions localtime/gmtime, then add .ms if set by option?
> 
> I am not sure what you are suggesting.
> 
> How does it look like from the point of view of the user?

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?

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

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

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 1563 bytes --]

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.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

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.

Thanks,
Thilo

[-- Attachment #2: v7-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch --]
[-- Type: text/plain, Size: 4240 bytes --]

From 5bcb05253440bae44af01882485e1973f5b9045a Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Tue, 18 Jan 2022 13:51:18 +0100
Subject: [PATCH v7] lavfi/drawtext: Add localtime_ms for millisecond precision

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 75 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
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;
+    while ((begin = av_stristr(begin, "%"))) {
+        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
+            }
+
+            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;
+            }
+
+            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++;
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
     return 0;
 }
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

["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).

> +
> +            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".

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 5746 bytes --]

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.

Thanks,
Thilo

[-- Attachment #2: v8-0001-lavfi-drawtext-Add-N-for-drawing-fractions-of-a-s.patch --]
[-- Type: text/plain, Size: 3757 bytes --]

From 2f42ca23b35a5e2ecedfd60203298cf7dcafdba5 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Thu, 20 Jan 2022 13:02:05 +0100
Subject: [PATCH v8] lavfi/drawtext: Add %N for drawing fractions of a second

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 58 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..06d0c77c55 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,65 @@ 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')
+    const char *begin;
+    const char *tmp;
+    int len;
+    char *fmt_new;
+    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 = fmt;
+    while ((begin = av_stristr(begin, "%"))) {
+        tmp = begin + 1;
+        len = 0;
+
+        // skip escaped "%%"
+        if (*tmp == '%') {
+            begin = tmp + 1;;
+            continue;
+        }
+
+        // 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 digit given, expect [1,6], warn & clamp otherwise
+            if (len == 1) {
+                num_digits = av_clip(*(begin + 1) - '0', 1, 6);
+            } else if (len > 1) {
+                av_log(ctx, AV_LOG_WARNING, "Invalid number of decimals for %%N, using default of %i\n", num_digits);
+            }
+
+            len += 2; // add % and N to get length of string part
+
+            div = pow(10, 6 - num_digits);
+            fmt_new = av_asprintf("%.*s%0*d%s", (int)(begin - fmt), fmt, num_digits, (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 = tmp + 1; 
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
     return 0;
 }
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 6854 bytes --]

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

[-- Attachment #2: v9-0001-lavfi-drawtext-Add-N-for-drawing-fractions-of-a-s.patch --]
[-- Type: text/plain, Size: 3908 bytes --]

From 066ca3d644daea88803b0b7ab1d3c3c66480ddfe Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Thu, 20 Jan 2022 15:57:14 +0100
Subject: [PATCH v9] lavfi/drawtext: Add %N for drawing fractions of a second

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 66 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..49414a3c0d 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,15 +1046,74 @@ 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')
+    const char *begin;
+    const char *tmp;
+    int len;
+    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 = fmt;
+    while ((begin = av_stristr(begin, "%"))) {
+        tmp = begin + 1;
+        len = 0;
+
+        // skip escaped "%%"
+        if (*tmp == '%') {
+            begin = tmp + 1;
+            continue;
+        }
+
+        // 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]
+            char *fmt_new = NULL;
+
+            // if digit given, expect [1,6], warn & clamp otherwise
+            if (len == 1) {
+                num_digits = av_clip(*(begin + 1) - '0', 1, 6);
+            } else if (len > 1) {
+                av_log(ctx, AV_LOG_WARNING, "Invalid number of decimals for %%N, using default of %i\n", num_digits);
+            }
+
+            len += 2; // add % and N to get length of string part
+
+            div = pow(10, 6 - num_digits);
+
+            if (fmt_new && fmt_new != argv[0])
+                av_freep(&fmt_new);
+
+            fmt_new = av_asprintf("%.*s%0*d%s", (int)(begin - fmt), fmt, num_digits, (int)(unow % 1000000) / div, begin + len);
+
+            if (!fmt_new)
+                return AVERROR(ENOMEM);
+
+            begin = fmt_new + (begin - fmt);
+            fmt = fmt_new;
+            continue;
+        }
+
+        begin = tmp;
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
+    if (fmt && fmt != argv[0])
+        av_freep(&fmt);
+
     return 0;
 }
 
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[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, '%')

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

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

[Thilo Borgmann]

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

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

[Thilo Borgmann]

Am 20.01.22 um 16:32 schrieb Thilo Borgmann:
> 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.

Also going to apply soon if there are no more comments.

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

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

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 194 bytes --]

Thilo Borgman (12022-01-31):
> > v10 attached.
> 
> Also going to apply soon if there are no more comments.

I think you neglected to attach the file.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 240 bytes --]

Am 31.01.22 um 14:08 schrieb Nicolas George:
> Thilo Borgman (12022-01-31):
>>> v10 attached.
>>
>> Also going to apply soon if there are no more comments.
> 
> I think you neglected to attach the file.

omg stupid me. Here it is...

-Thilo

[-- Attachment #2: v10-0001-lavfi-drawtext-Add-N-for-drawing-fractions-of-a-.patch --]
[-- Type: text/plain, Size: 4120 bytes --]

From 56e3af9f9039aa09b9daea3a47dea9fb24b19cec Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Thu, 20 Jan 2022 16:31:01 +0100
Subject: [PATCH v10] lavfi/drawtext: Add %N for drawing fractions of a second

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 69 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..2323643ee8 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"
@@ -1044,16 +1045,76 @@ static int func_metadata(AVFilterContext *ctx, AVBPrint *bp,
 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";
+    const char *fmt_default = "%Y-%m-%d %H:%M:%S";
+    const char *fmt = argc ? argv[0] : fmt_default;
+    int64_t unow;
     time_t now;
     struct tm tm;
-
-    time(&now);
-    if (tag == 'L')
+    const char *begin;
+    const char *tmp;
+    int len;
+    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 = fmt;
+    while ((begin = strchr(begin, '%'))) {
+        tmp = begin + 1;
+        len = 0;
+
+        // skip escaped "%%"
+        if (*tmp == '%') {
+            begin = tmp + 1;
+            continue;
+        }
+
+        // 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]
+            char *fmt_new = NULL;
+
+            // if digit given, expect [1,6], warn & clamp otherwise
+            if (len == 1) {
+                num_digits = av_clip(*(begin + 1) - '0', 1, 6);
+            } else if (len > 1) {
+                av_log(ctx, AV_LOG_WARNING, "Invalid number of decimals for %%N, using default of %i\n", num_digits);
+            }
+
+            len += 2; // add % and N to get length of string part
+
+            div = pow(10, 6 - num_digits);
+
+            if (fmt_new && fmt_new != argv[0] && fmt_new != fmt_default)
+                av_freep(&fmt_new);
+
+            fmt_new = av_asprintf("%.*s%0*d%s", (int)(begin - fmt), fmt, num_digits, (int)(unow % 1000000) / div, begin + len);
+
+            if (!fmt_new)
+                return AVERROR(ENOMEM);
+
+            begin = fmt_new + (begin - fmt);
+            fmt = fmt_new;
+            continue;
+        }
+
+        begin = tmp;
+    }
+
     av_bprint_strftime(bp, fmt, &tm);
+    if (fmt && fmt != argv[0] && fmt != fmt_default)
+        av_freep(&fmt);
+
     return 0;
 }
 
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Andreas Rheinhardt]

Thilo Borgmann:
> Am 31.01.22 um 14:08 schrieb Nicolas George:
>> Thilo Borgman (12022-01-31):
>>>> v10 attached.
>>>
>>> Also going to apply soon if there are no more comments.
>>
>> I think you neglected to attach the file.
> 
> omg stupid me. Here it is...
> 
> -Thilo
> 

> 
> +
> +            if (fmt_new && fmt_new != argv[0] && fmt_new != fmt_default)
> +                av_freep(&fmt_new);
> +

fmt_new is always NULL at this point, so the above is unnecessary.

> 
> +    if (fmt && fmt != argv[0] && fmt != fmt_default)
> +        av_freep(&fmt);
> +

Don't free a const char*. Instead add char *fmt_new = NULL; directly
besides the definition of fmt and free fmt_new after
av_bprint_strftime(). This fmt_new should obviously be used instead of
the current fmt_new with the more limited scope.
You can then also remove fmt_default.

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

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

[Andreas Rheinhardt]

Thilo Borgmann:
> Am 31.01.22 um 14:08 schrieb Nicolas George:
>> Thilo Borgman (12022-01-31):
>>>> v10 attached.
>>>
>>> Also going to apply soon if there are no more comments.
>>
>> I think you neglected to attach the file.
> 
> omg stupid me. Here it is...
> 
> -Thilo
> 

Seems like I misunderstood your code and ignored the outer while. Your
code can leak if there are multiple 'N', because (as I said)

> 
> +
> +            if (fmt_new && fmt_new != argv[0] && fmt_new != fmt_default)
> +                av_freep(&fmt_new);
> +

does not free fmt if it is already allocated. It is possible to fix this
by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
the block that is executed if a 'N' is executed. (You have to free
fmt_allocated immediately after av_asprintf().)
But maybe it would be best to use an AVBPrint to write the new string
instead of allocating a new string for every %N encountered.

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
> Thilo Borgmann:
>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>> Thilo Borgman (12022-01-31):
>>>>> v10 attached.
>>>>
>>>> Also going to apply soon if there are no more comments.
>>>
>>> I think you neglected to attach the file.
>>
>> omg stupid me. Here it is...
>>
>> -Thilo
>>
> 
> Seems like I misunderstood your code and ignored the outer while. Your
> code can leak if there are multiple 'N', because (as I said)
> 
>>
>> +
>> +            if (fmt_new && fmt_new != argv[0] && fmt_new != fmt_default)
>> +                av_freep(&fmt_new);
>> +
> 
> does not free fmt if it is already allocated. It is possible to fix this
> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
> the block that is executed if a 'N' is executed. (You have to free
> fmt_allocated immediately after av_asprintf().)

> But maybe it would be best to use an AVBPrint to write the new string
> instead of allocating a new string for every %N encountered.

v11 does it that way.

Thanks,
Thilo

[-- Attachment #2: v11-0001-lavfi-drawtext-Add-N-for-drawing-fractions-of-a-.patch --]
[-- Type: text/plain, Size: 3916 bytes --]

From 1831153a5309502414c8639d1364930b305dd5dd Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Tue, 8 Feb 2022 10:15:13 +0100
Subject: [PATCH v11] lavfi/drawtext: Add %N for drawing fractions of a second

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 67 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..75ce97a2a6 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,15 +1046,77 @@ 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";
+    const char *fmt_begin = fmt;
+    int64_t unow;
     time_t now;
     struct tm tm;
+    const char *begin;
+    const char *tmp;
+    int len;
+    int div;
+    AVBPrint fmt_bp;
 
-    time(&now);
-    if (tag == 'L')
+    av_bprint_init(&fmt_bp, 0, AV_BPRINT_SIZE_UNLIMITED);
+
+    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 = fmt;
+    while ((begin = strchr(begin, '%'))) {
+        tmp = begin + 1;
+        len = 0;
+
+        // skip escaped "%%"
+        if (*tmp == '%') {
+            begin = tmp + 1;
+            continue;
+        }
+
+        // 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 digit given, expect [1,6], warn & clamp otherwise
+            if (len == 1) {
+                num_digits = av_clip(*(begin + 1) - '0', 1, 6);
+            } else if (len > 1) {
+                av_log(ctx, AV_LOG_WARNING, "Invalid number of decimals for %%N, using default of %i\n", num_digits);
+            }
+
+            len += 2; // add % and N to get length of string part
+
+            div = pow(10, 6 - num_digits);
+
+            av_bprintf(&fmt_bp, "%.*s%0*d", (int)(begin - fmt_begin), fmt_begin, num_digits, (int)(unow % 1000000) / div);
+
+            begin += len;
+            fmt_begin = begin;
+
+            continue;
+        }
+
+        begin = tmp;
+    }
+
+    av_bprintf(&fmt_bp, "%s", fmt_begin);
+    av_bprint_finalize(&fmt_bp, (char**)&fmt);
+    if (!fmt)
+        return AVERROR(ENOMEM);
+
     av_bprint_strftime(bp, fmt, &tm);
+    av_freep(&fmt);
+
     return 0;
 }
 
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Andreas Rheinhardt]

Thilo Borgmann:
> Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
>> Thilo Borgmann:
>>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>>> Thilo Borgman (12022-01-31):
>>>>>> v10 attached.
>>>>>
>>>>> Also going to apply soon if there are no more comments.
>>>>
>>>> I think you neglected to attach the file.
>>>
>>> omg stupid me. Here it is...
>>>
>>> -Thilo
>>>
>>
>> Seems like I misunderstood your code and ignored the outer while. Your
>> code can leak if there are multiple 'N', because (as I said)
>>
>>>
>>> +
>>> +            if (fmt_new && fmt_new != argv[0] && fmt_new !=
>>> fmt_default)
>>> +                av_freep(&fmt_new);
>>> +
>>
>> does not free fmt if it is already allocated. It is possible to fix this
>> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
>> the block that is executed if a 'N' is executed. (You have to free
>> fmt_allocated immediately after av_asprintf().)
> 
>> But maybe it would be best to use an AVBPrint to write the new string
>> instead of allocating a new string for every %N encountered.
> 
> v11 does it that way.
> 

> 
> +    av_bprintf(&fmt_bp, "%s", fmt_begin);
> +    av_bprint_finalize(&fmt_bp, (char**)&fmt);
> +    if (!fmt)
> +        return AVERROR(ENOMEM);
> +
>      av_bprint_strftime(bp, fmt, &tm);
> +    av_freep(&fmt);
> +

This is not how one uses an AVBPrint: You are loosing the
small-string-optimization that way.
Furthermore, you do not check for truncation.

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

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

[Thilo Borgmann]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

Am 08.02.22 um 10:27 schrieb Andreas Rheinhardt:
> Thilo Borgmann:
>> Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
>>> Thilo Borgmann:
>>>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>>>> Thilo Borgman (12022-01-31):
>>>>>>> v10 attached.
>>>>>>
>>>>>> Also going to apply soon if there are no more comments.
>>>>>
>>>>> I think you neglected to attach the file.
>>>>
>>>> omg stupid me. Here it is...
>>>>
>>>> -Thilo
>>>>
>>>
>>> Seems like I misunderstood your code and ignored the outer while. Your
>>> code can leak if there are multiple 'N', because (as I said)
>>>
>>>>
>>>> +
>>>> +            if (fmt_new && fmt_new != argv[0] && fmt_new !=
>>>> fmt_default)
>>>> +                av_freep(&fmt_new);
>>>> +
>>>
>>> does not free fmt if it is already allocated. It is possible to fix this
>>> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
>>> the block that is executed if a 'N' is executed. (You have to free
>>> fmt_allocated immediately after av_asprintf().)
>>
>>> But maybe it would be best to use an AVBPrint to write the new string
>>> instead of allocating a new string for every %N encountered.
>>
>> v11 does it that way.
>>
> 
>>
>> +    av_bprintf(&fmt_bp, "%s", fmt_begin);
>> +    av_bprint_finalize(&fmt_bp, (char**)&fmt);
>> +    if (!fmt)
>> +        return AVERROR(ENOMEM);
>> +
>>       av_bprint_strftime(bp, fmt, &tm);
>> +    av_freep(&fmt);
>> +
> 
> This is not how one uses an AVBPrint: You are loosing the
> small-string-optimization that way.
> Furthermore, you do not check for truncation.

v12 including IRC comments.

-Thilo

[-- Attachment #2: v12-0001-lavfi-drawtext-Add-N-for-drawing-fractions-of-a-.patch --]
[-- Type: text/plain, Size: 4038 bytes --]

From bb53ac8d924ed7d674f254091d2c7a92fe2ea41d Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Tue, 8 Feb 2022 11:39:46 +0100
Subject: [PATCH v12] lavfi/drawtext: Add %N for drawing fractions of a second

Suggested-By: ffmpeg@fb.com
---
 doc/filters.texi          |  4 +++
 libavfilter/vf_drawtext.c | 70 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 05d4b1a56e..c3895138e0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11378,10 +11378,14 @@ It can be used to add padding with zeros from the left.
 @item gmtime
 The time at which the filter is running, expressed in UTC.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item localtime
 The time at which the filter is running, expressed in the local time zone.
 It can accept an argument: a strftime() format string.
+The format string is extended to support the variable @var{%[1-6]N}
+which prints fractions of the second with optionally specified number of digits.
 
 @item metadata
 Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index 2a88692cbd..d3ec68004d 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,15 +1046,78 @@ 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";
+    const char *fmt_begin = fmt;
+    int64_t unow;
     time_t now;
     struct tm tm;
+    const char *begin;
+    const char *tmp;
+    int len;
+    int div;
+    AVBPrint fmt_bp;
 
-    time(&now);
-    if (tag == 'L')
+    av_bprint_init(&fmt_bp, 0, AV_BPRINT_SIZE_UNLIMITED);
+
+    unow = av_gettime();
+    now  = unow / 1000000;
+    if (tag == 'L' || tag == 'm')
         localtime_r(&now, &tm);
     else
         tm = *gmtime_r(&now, &tm);
-    av_bprint_strftime(bp, fmt, &tm);
+
+    // manually parse format for %N (fractional seconds)
+    begin = fmt;
+    while ((begin = strchr(begin, '%'))) {
+        tmp = begin + 1;
+        len = 0;
+
+        // skip escaped "%%"
+        if (*tmp == '%') {
+            begin = tmp + 1;
+            continue;
+        }
+
+        // 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 digit given, expect [1,6], warn & clamp otherwise
+            if (len == 1) {
+                num_digits = av_clip(*(begin + 1) - '0', 1, 6);
+            } else if (len > 1) {
+                av_log(ctx, AV_LOG_WARNING, "Invalid number of decimals for %%N, using default of %i\n", num_digits);
+            }
+
+            len += 2; // add % and N to get length of string part
+
+            div = pow(10, 6 - num_digits);
+
+            av_bprintf(&fmt_bp, "%.*s%0*d", (int)(begin - fmt_begin), fmt_begin, num_digits, (int)(unow % 1000000) / div);
+
+            begin += len;
+            fmt_begin = begin;
+
+            continue;
+        }
+
+        begin = tmp;
+    }
+
+    av_bprintf(&fmt_bp, "%s", fmt_begin);
+    if (!av_bprint_is_complete(&fmt_bp)) {
+        av_log(ctx, AV_LOG_WARNING, "Format string truncated at %u/%u.", fmt_bp.size, fmt_bp.len);
+    }
+
+    av_bprint_strftime(bp, fmt_bp.str, &tm);
+
+    av_bprint_finalize(&fmt_bp, NULL);
+
     return 0;
 }
 
-- 
2.20.1 (Apple Git-117)


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

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

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

[Thilo Borgmann]

Am 08.02.22 um 11:41 schrieb Thilo Borgmann:
> Am 08.02.22 um 10:27 schrieb Andreas Rheinhardt:
>> Thilo Borgmann:
>>> Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
>>>> Thilo Borgmann:
>>>>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>>>>> Thilo Borgman (12022-01-31):
>>>>>>>> v10 attached.
>>>>>>>
>>>>>>> Also going to apply soon if there are no more comments.
>>>>>>
>>>>>> I think you neglected to attach the file.
>>>>>
>>>>> omg stupid me. Here it is...
>>>>>
>>>>> -Thilo
>>>>>
>>>>
>>>> Seems like I misunderstood your code and ignored the outer while. Your
>>>> code can leak if there are multiple 'N', because (as I said)
>>>>
>>>>>
>>>>> +
>>>>> +            if (fmt_new && fmt_new != argv[0] && fmt_new !=
>>>>> fmt_default)
>>>>> +                av_freep(&fmt_new);
>>>>> +
>>>>
>>>> does not free fmt if it is already allocated. It is possible to fix this
>>>> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
>>>> the block that is executed if a 'N' is executed. (You have to free
>>>> fmt_allocated immediately after av_asprintf().)
>>>
>>>> But maybe it would be best to use an AVBPrint to write the new string
>>>> instead of allocating a new string for every %N encountered.
>>>
>>> v11 does it that way.
>>>
>>
>>>
>>> +    av_bprintf(&fmt_bp, "%s", fmt_begin);
>>> +    av_bprint_finalize(&fmt_bp, (char**)&fmt);
>>> +    if (!fmt)
>>> +        return AVERROR(ENOMEM);
>>> +
>>>       av_bprint_strftime(bp, fmt, &tm);
>>> +    av_freep(&fmt);
>>> +
>>
>> This is not how one uses an AVBPrint: You are loosing the
>> small-string-optimization that way.
>> Furthermore, you do not check for truncation.
> 
> v12 including IRC comments.

Ping.

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

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

[Thilo Borgmann]

Am 22.02.22 um 12:36 schrieb Thilo Borgmann:
> Am 08.02.22 um 11:41 schrieb Thilo Borgmann:
>> Am 08.02.22 um 10:27 schrieb Andreas Rheinhardt:
>>> Thilo Borgmann:
>>>> Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
>>>>> Thilo Borgmann:
>>>>>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>>>>>> Thilo Borgman (12022-01-31):
>>>>>>>>> v10 attached.
>>>>>>>>
>>>>>>>> Also going to apply soon if there are no more comments.
>>>>>>>
>>>>>>> I think you neglected to attach the file.
>>>>>>
>>>>>> omg stupid me. Here it is...
>>>>>>
>>>>>> -Thilo
>>>>>>
>>>>>
>>>>> Seems like I misunderstood your code and ignored the outer while. Your
>>>>> code can leak if there are multiple 'N', because (as I said)
>>>>>
>>>>>>
>>>>>> +
>>>>>> +            if (fmt_new && fmt_new != argv[0] && fmt_new !=
>>>>>> fmt_default)
>>>>>> +                av_freep(&fmt_new);
>>>>>> +
>>>>>
>>>>> does not free fmt if it is already allocated. It is possible to fix this
>>>>> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
>>>>> the block that is executed if a 'N' is executed. (You have to free
>>>>> fmt_allocated immediately after av_asprintf().)
>>>>
>>>>> But maybe it would be best to use an AVBPrint to write the new string
>>>>> instead of allocating a new string for every %N encountered.
>>>>
>>>> v11 does it that way.
>>>>
>>>
>>>>
>>>> +    av_bprintf(&fmt_bp, "%s", fmt_begin);
>>>> +    av_bprint_finalize(&fmt_bp, (char**)&fmt);
>>>> +    if (!fmt)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>>       av_bprint_strftime(bp, fmt, &tm);
>>>> +    av_freep(&fmt);
>>>> +
>>>
>>> This is not how one uses an AVBPrint: You are loosing the
>>> small-string-optimization that way.
>>> Furthermore, you do not check for truncation.
>>
>> v12 including IRC comments.
> 
> Ping.

Will push soon if there are no more comments.

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

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

[Thilo Borgmann]

Am 06.03.22 um 21:38 schrieb Thilo Borgmann:
> Am 22.02.22 um 12:36 schrieb Thilo Borgmann:
>> Am 08.02.22 um 11:41 schrieb Thilo Borgmann:
>>> Am 08.02.22 um 10:27 schrieb Andreas Rheinhardt:
>>>> Thilo Borgmann:
>>>>> Am 01.02.22 um 00:40 schrieb Andreas Rheinhardt:
>>>>>> Thilo Borgmann:
>>>>>>> Am 31.01.22 um 14:08 schrieb Nicolas George:
>>>>>>>> Thilo Borgman (12022-01-31):
>>>>>>>>>> v10 attached.
>>>>>>>>>
>>>>>>>>> Also going to apply soon if there are no more comments.
>>>>>>>>
>>>>>>>> I think you neglected to attach the file.
>>>>>>>
>>>>>>> omg stupid me. Here it is...
>>>>>>>
>>>>>>> -Thilo
>>>>>>>
>>>>>>
>>>>>> Seems like I misunderstood your code and ignored the outer while. Your
>>>>>> code can leak if there are multiple 'N', because (as I said)
>>>>>>
>>>>>>>
>>>>>>> +
>>>>>>> +            if (fmt_new && fmt_new != argv[0] && fmt_new !=
>>>>>>> fmt_default)
>>>>>>> +                av_freep(&fmt_new);
>>>>>>> +
>>>>>>
>>>>>> does not free fmt if it is already allocated. It is possible to fix this
>>>>>> by adding an char *fmt_allocated = NULL; at outer scope and a fmt_new in
>>>>>> the block that is executed if a 'N' is executed. (You have to free
>>>>>> fmt_allocated immediately after av_asprintf().)
>>>>>
>>>>>> But maybe it would be best to use an AVBPrint to write the new string
>>>>>> instead of allocating a new string for every %N encountered.
>>>>>
>>>>> v11 does it that way.
>>>>>
>>>>
>>>>>
>>>>> +    av_bprintf(&fmt_bp, "%s", fmt_begin);
>>>>> +    av_bprint_finalize(&fmt_bp, (char**)&fmt);
>>>>> +    if (!fmt)
>>>>> +        return AVERROR(ENOMEM);
>>>>> +
>>>>>       av_bprint_strftime(bp, fmt, &tm);
>>>>> +    av_freep(&fmt);
>>>>> +
>>>>
>>>> This is not how one uses an AVBPrint: You are loosing the
>>>> small-string-optimization that way.
>>>> Furthermore, you do not check for truncation.
>>>
>>> v12 including IRC comments.
>>
>> Ping.
> 
> Will push soon if there are no more comments.

Pushed, thanks!

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

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

[James Almer]

On 1/6/2022 8:27 AM, Thilo Borgmann wrote:
> Am 03.01.22 um 16:22 schrieb Thilo Borgmann:
>> Am 29.12.21 um 12:46 schrieb Nicolas George:
>>> "zhilizhao(赵志立)" (12021-12-29):
>>>> How about add a restriction like this:
>>>>
>>>> if (format.endsWith(“%S"))
>>>>      enable the feature
>>>> else
>>>>      warning message
>>>>
>>>> It’s a useful feature, it shouldn't create unexpected results, but
>>>> doesn’t need to support every use case.
>>>
>>> I would not oppose it, but I find it inelegant, especially because it
>>> requires a different expansion function, localtime_ms instead of
>>> localtime.
>>>
>>> What about this: with the original function "localtime", if the format
>>> ends in "%3N", then append the millisecond. It can later be expanded to
>>> support %xN at any place in the format for any value of x.
>>
>> I think best will be to scan the format string for %S and extend it there with .ms part before expanding the rest of it, not? Shouldn't be too expensive for the filter.
>>
>> Just need to find time to actually implement it.
> 
> Like v5 as attached.
> 
> Thanks,
> Thilo


> From c7f7c7a1cedc4ccc51977fc92645e1131608ac95 Mon Sep 17 00:00:00 2001
> From: Thilo Borgmann <thilo.borgmann@mail.de>
> Date: Thu, 6 Jan 2022 12:24:46 +0100
> Subject: [PATCH v5] lavfi/drawtext: Add localtime_ms for millisecond precision
> 
> Suggested-By: ffmpeg@fb.com
> ---
>  doc/filters.texi          |  8 ++++++++
>  libavfilter/vf_drawtext.c | 28 ++++++++++++++++++++++++++--
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 05d4b1a56e..967021e48b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -11379,10 +11379,18 @@ It can be used to add padding with zeros from the left.
>  The time at which the filter is running, expressed in UTC.
>  It can accept an argument: a strftime() format string.
>  
> +@item gmtime_ms
> +Same as @code{gmtime} but with millisecond precision.
> +It can accept an argument: a strftime() format string.
> +
>  @item localtime
>  The time at which the filter is running, expressed in the local time zone.
>  It can accept an argument: a strftime() format string.
>  
> +@item localtime_ms
> +Same as @code{localtime} but with millisecond precision.
> +It can accept an argument: a strftime() format string.
> +
>  @item metadata
>  Frame metadata. Takes one or two arguments.
>  
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 2a88692cbd..723473f299 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,35 @@ 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')
> +    unow = av_gettime();
> +    now  = unow / 1000000;
> +    if (tag == 'L' || tag == 'm')
>          localtime_r(&now, &tm);
>      else
>          tm = *gmtime_r(&now, &tm);
> +
> +    if (tag == 'M' || tag == 'm') {
> +        char *seconds = av_stristr(fmt, "%S");
> +        if (seconds) {
> +            seconds += 2;
> +            int len = seconds - fmt + 1;

Should be size_t. Also, mixed declarations and code.

> +            char *tmp = av_malloc(len);
> +            av_strlcpy(tmp, fmt, len);
> +
> +            char *fmt_new = av_asprintf("%s.%03d%s", tmp, (int)(unow % 1000000) / 1000, seconds);

Same.

> +            av_bprint_strftime(bp, fmt_new, &tm);
> +
> +            av_freep(&tmp);
> +            av_freep(&fmt_new);
> +
> +            return 0;
> +        }
> +    }
> +
>      av_bprint_strftime(bp, fmt, &tm);
>      return 0;
>  }
> @@ -1152,7 +1174,9 @@ static const struct drawtext_function {
>      { "pict_type", 0, 0, 0,   func_pict_type },
>      { "pts",       0, 3, 0,   func_pts      },
>      { "gmtime",    0, 1, 'G', func_strftime },
> +    { "gmtime_ms", 0, 1, 'M', func_strftime },
>      { "localtime", 0, 1, 'L', func_strftime },
> +    { "localtime_ms", 0, 1, 'm', func_strftime },
>      { "frame_num", 0, 0, 0,   func_frame_num },
>      { "n",         0, 0, 0,   func_frame_num },
>      { "metadata",  1, 2, 0,   func_metadata },
> -- 
> 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".