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

Hi,

add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter. Same 
as %{localtime}/%{gmtime} but with additional millisecond part.

sorry for delay, second version including review remarks:

-get timing once
-also add gmtime_ms instead of just localtime_ms

Thanks,
Thilo
From 07c940d63df392a592a72dc23fb48f6682284c65 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Thu, 9 Dec 2021 19:57:18 +0100
Subject: [PATCH v2] lavfi/drawtext: Add localtime_ms for millisecond precision

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

> On Dec 10, 2021, at 3:11 AM, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
> 
> Hi,
> 
> add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter. Same as %{localtime}/%{gmtime} but with additional millisecond part.
> 
> sorry for delay, second version including review remarks:
> 
> -get timing once
> -also add gmtime_ms instead of just localtime_ms
> 

> +    if (tag == 'M' || tag == 'm') {
> +        char ms[5] = {0};
> +        int64_t dnow = (unow - ((int64_t)now) * 1000000) / 1000;
> +        snprintf(ms, 5, ".%03d", (int)dnow);
> +        av_bprint_append_data(bp, ms, 4);
> +    }
> 


How about

    av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);

On 10 Dec 2021, at 3:47, zhilizhao(赵志立) wrote:

>> On Dec 10, 2021, at 3:11 AM, Thilo Borgmann <thilo.borgmann@mail.de> 
>> wrote:
>>
>> Hi,
>>
>> add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter. 
>> Same as %{localtime}/%{gmtime} but with additional millisecond part.
>>
>> sorry for delay, second version including review remarks:
>>
>> -get timing once
>> -also add gmtime_ms instead of just localtime_ms
>>
>
>> +    if (tag == 'M' || tag == 'm') {
>> +        char ms[5] = {0};
>> +        int64_t dnow = (unow - ((int64_t)now) * 1000000) / 1000;
>> +        snprintf(ms, 5, ".%03d", (int)dnow);
>> +        av_bprint_append_data(bp, ms, 4);
>> +    }
>>
>
>
> How about
>
>     av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);

Makes way too much sense. I need holidays…

Attached v3.

Thanks!
-Thilo
From fd34d1434e2243a881c24f6db4cc0db92289f4bb Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Fri, 10 Dec 2021 12:34:23 +0100
Subject: [PATCH v3] lavfi/drawtext: Add localtime_ms for millisecond precision

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

diff --git a/doc/filters.texi b/doc/filters.texi
index 78faf76..db75632 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -10949,10 +10949,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 382d589..36c9103 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -1045,15 +1045,21 @@ 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);
     av_bprint_strftime(bp, fmt, &tm);
+
+    if (tag == 'M' || tag == 'm') {
+        av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
+    }
     return 0;
 }
 
@@ -1152,7 +1158,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 },

On Fri, Dec 10, 2021 at 12:36:21PM +0100, Thilo Borgmann wrote:
> 
> 
> On 10 Dec 2021, at 3:47, zhilizhao(赵志立) wrote:
> 
> > > On Dec 10, 2021, at 3:11 AM, Thilo Borgmann <thilo.borgmann@mail.de>
> > > wrote:
> > > 
> > > Hi,
> > > 
> > > add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter.
> > > Same as %{localtime}/%{gmtime} but with additional millisecond part.
> > > 
> > > sorry for delay, second version including review remarks:
> > > 
> > > -get timing once
> > > -also add gmtime_ms instead of just localtime_ms
> > > 
> > 
> > > +    if (tag == 'M' || tag == 'm') {
> > > +        char ms[5] = {0};
> > > +        int64_t dnow = (unow - ((int64_t)now) * 1000000) / 1000;
> > > +        snprintf(ms, 5, ".%03d", (int)dnow);
> > > +        av_bprint_append_data(bp, ms, 4);
> > > +    }
> > > 
> > 
> > 
> > How about
> > 
> >     av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
> 
> Makes way too much sense. I need holidays…
> 
> Attached v3.
> 
> Thanks!
> -Thilo

>  doc/filters.texi          |    8 ++++++++
>  libavfilter/vf_drawtext.c |   12 ++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 87d34e4106b829d42c5e57c847c28bed08bf3a81  v3-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch
> From fd34d1434e2243a881c24f6db4cc0db92289f4bb Mon Sep 17 00:00:00 2001
> From: Thilo Borgmann <thilo.borgmann@mail.de>
> Date: Fri, 10 Dec 2021 12:34:23 +0100
> Subject: [PATCH v3] lavfi/drawtext: Add localtime_ms for millisecond precision

Iam missining something here as it doesnt build

AR	libavdevice/libavdevice.a
CC	libavfilter/vf_drawtext.o
libavfilter/vf_drawtext.c: In function ‘func_strftime’:
libavfilter/vf_drawtext.c:1052:12: error: implicit declaration of function ‘av_gettime’; did you mean ‘av_get_token’? [-Werror=implicit-function-declaration]
     unow = av_gettime();
            ^~~~~~~~~~
            av_get_token
libavfilter/vf_drawtext.c:1061:20: warning: passing argument 1 of ‘av_bprintf’ from incompatible pointer type [-Wincompatible-pointer-types]
         av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
                    ^
In file included from libavfilter/vf_drawtext.c:47:0:
./libavutil/bprint.h:127:6: note: expected ‘AVBPrint * {aka struct AVBPrint *}’ but argument is of type ‘AVBPrint ** {aka struct AVBPrint **}’
 void av_bprintf(AVBPrint *buf, const char *fmt, ...) av_printf_format(2, 3);
      ^~~~~~~~~~
cc1: some warnings being treated as errors
ffbuild/common.mak:70: recipe for target 'libavfilter/vf_drawtext.o' failed
make: *** [libavfilter/vf_drawtext.o] Error 1

[...]

On 10 Dec 2021, at 17:46, Michael Niedermayer wrote:

> On Fri, Dec 10, 2021 at 12:36:21PM +0100, Thilo Borgmann wrote:
>>
>>
>> On 10 Dec 2021, at 3:47, zhilizhao(赵志立) wrote:
>>
>>>> On Dec 10, 2021, at 3:11 AM, Thilo Borgmann 
>>>> <thilo.borgmann@mail.de>
>>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter.
>>>> Same as %{localtime}/%{gmtime} but with additional millisecond 
>>>> part.
>>>>
>>>> sorry for delay, second version including review remarks:
>>>>
>>>> -get timing once
>>>> -also add gmtime_ms instead of just localtime_ms
>>>>
>>>
>>>> +    if (tag == 'M' || tag == 'm') {
>>>> +        char ms[5] = {0};
>>>> +        int64_t dnow = (unow - ((int64_t)now) * 1000000) / 1000;
>>>> +        snprintf(ms, 5, ".%03d", (int)dnow);
>>>> +        av_bprint_append_data(bp, ms, 4);
>>>> +    }
>>>>
>>>
>>>
>>> How about
>>>
>>>     av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
>>
>> Makes way too much sense. I need holidays…
>>
>> Attached v3.
>>
>> Thanks!
>> -Thilo
>
>>  doc/filters.texi          |    8 ++++++++
>>  libavfilter/vf_drawtext.c |   12 ++++++++++--
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>> 87d34e4106b829d42c5e57c847c28bed08bf3a81  
>> v3-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch
>> From fd34d1434e2243a881c24f6db4cc0db92289f4bb Mon Sep 17 00:00:00 
>> 2001
>> From: Thilo Borgmann <thilo.borgmann@mail.de>
>> Date: Fri, 10 Dec 2021 12:34:23 +0100
>> Subject: [PATCH v3] lavfi/drawtext: Add localtime_ms for millisecond 
>> precision
>
> Iam missining something here as it doesnt build
>
> AR	libavdevice/libavdevice.a
> CC	libavfilter/vf_drawtext.o
> libavfilter/vf_drawtext.c: In function ‘func_strftime’:
> libavfilter/vf_drawtext.c:1052:12: error: implicit declaration of 
> function ‘av_gettime’; did you mean ‘av_get_token’? 
> [-Werror=implicit-function-declaration]
>      unow = av_gettime();
>             ^~~~~~~~~~
>             av_get_token
> libavfilter/vf_drawtext.c:1061:20: warning: passing argument 1 of 
> ‘av_bprintf’ from incompatible pointer type 
> [-Wincompatible-pointer-types]
>          av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
>                     ^
> In file included from libavfilter/vf_drawtext.c:47:0:
> ./libavutil/bprint.h:127:6: note: expected ‘AVBPrint * {aka struct 
> AVBPrint *}’ but argument is of type ‘AVBPrint ** {aka struct 
> AVBPrint **}’
>  void av_bprintf(AVBPrint *buf, const char *fmt, ...) 
> av_printf_format(2, 3);
>       ^~~~~~~~~~
> cc1: some warnings being treated as errors
> ffbuild/common.mak:70: recipe for target 'libavfilter/vf_drawtext.o' 
> failed
> make: *** [libavfilter/vf_drawtext.o] Error 1

Works for me on OSX.

av_gettime() is in lavu/time.c which gets included via 
lavu/time_internal.h….

make distclean?
configure?

-Thilo

Thilo Borgmann:
> 
> 
> On 10 Dec 2021, at 17:46, Michael Niedermayer wrote:
> 
>> On Fri, Dec 10, 2021 at 12:36:21PM +0100, Thilo Borgmann wrote:
>>>
>>>
>>> On 10 Dec 2021, at 3:47, zhilizhao(赵志立) wrote:
>>>
>>>>> On Dec 10, 2021, at 3:11 AM, Thilo Borgmann <thilo.borgmann@mail.de>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> add %{localtime_ms}, %{gmtime_ms} functions to the drawtext filter.
>>>>> Same as %{localtime}/%{gmtime} but with additional millisecond part.
>>>>>
>>>>> sorry for delay, second version including review remarks:
>>>>>
>>>>> -get timing once
>>>>> -also add gmtime_ms instead of just localtime_ms
>>>>>
>>>>
>>>>> +    if (tag == 'M' || tag == 'm') {
>>>>> +        char ms[5] = {0};
>>>>> +        int64_t dnow = (unow - ((int64_t)now) * 1000000) / 1000;
>>>>> +        snprintf(ms, 5, ".%03d", (int)dnow);
>>>>> +        av_bprint_append_data(bp, ms, 4);
>>>>> +    }
>>>>>
>>>>
>>>>
>>>> How about
>>>>
>>>>     av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
>>>
>>> Makes way too much sense. I need holidays…
>>>
>>> Attached v3.
>>>
>>> Thanks!
>>> -Thilo
>>
>>>  doc/filters.texi          |    8 ++++++++
>>>  libavfilter/vf_drawtext.c |   12 ++++++++++--
>>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>> 87d34e4106b829d42c5e57c847c28bed08bf3a81 
>>> v3-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch
>>> From fd34d1434e2243a881c24f6db4cc0db92289f4bb Mon Sep 17 00:00:00 2001
>>> From: Thilo Borgmann <thilo.borgmann@mail.de>
>>> Date: Fri, 10 Dec 2021 12:34:23 +0100
>>> Subject: [PATCH v3] lavfi/drawtext: Add localtime_ms for millisecond
>>> precision
>>
>> Iam missining something here as it doesnt build
>>
>> AR    libavdevice/libavdevice.a
>> CC    libavfilter/vf_drawtext.o
>> libavfilter/vf_drawtext.c: In function ‘func_strftime’:
>> libavfilter/vf_drawtext.c:1052:12: error: implicit declaration of
>> function ‘av_gettime’; did you mean ‘av_get_token’?
>> [-Werror=implicit-function-declaration]
>>      unow = av_gettime();
>>             ^~~~~~~~~~
>>             av_get_token
>> libavfilter/vf_drawtext.c:1061:20: warning: passing argument 1 of
>> ‘av_bprintf’ from incompatible pointer type
>> [-Wincompatible-pointer-types]
>>          av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
>>                     ^
>> In file included from libavfilter/vf_drawtext.c:47:0:
>> ./libavutil/bprint.h:127:6: note: expected ‘AVBPrint * {aka struct
>> AVBPrint *}’ but argument is of type ‘AVBPrint ** {aka struct AVBPrint
>> **}’
>>  void av_bprintf(AVBPrint *buf, const char *fmt, ...)
>> av_printf_format(2, 3);
>>       ^~~~~~~~~~
>> cc1: some warnings being treated as errors
>> ffbuild/common.mak:70: recipe for target 'libavfilter/vf_drawtext.o'
>> failed
>> make: *** [libavfilter/vf_drawtext.o] Error 1
> 
> Works for me on OSX.
> 
> av_gettime() is in lavu/time.c which gets included via
> lavu/time_internal.h….
> 

av_gettime() is public and resides in lavu/time.h, not
lavu/time_internal.h; the latter does not include the former in any way.
But compat/os2threads.h and compat/w32pthreads.h include lavu/time.h.
Maybe you have it from the former? It doesn't work here (Ubuntu 21.10)
either.

- Andreas

On 11 Dec 2021, at 23:17, Andreas Rheinhardt wrote:

> Thilo Borgmann:
>>
>>
>> On 10 Dec 2021, at 17:46, Michael Niedermayer wrote:
>>
>>> On Fri, Dec 10, 2021 at 12:36:21PM +0100, Thilo Borgmann wrote:
>>>>
>>>>
>>>> On 10 Dec 2021, at 3:47, zhilizhao(赵志立) wrote:
>>>>
>>>>>> On Dec 10, 2021, at 3:11 AM, Thilo Borgmann 
>>>>>> <thilo.borgmann@mail.de>
>>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> add %{localtime_ms}, %{gmtime_ms} functions to the drawtext 
>>>>>> filter.
>>>>>> Same as %{localtime}/%{gmtime} but with additional millisecond 
>>>>>> part.
>>>>>>
>>>>>> sorry for delay, second version including review remarks:
>>>>>>
>>>>>> -get timing once
>>>>>> -also add gmtime_ms instead of just localtime_ms
>>>>>>
>>>>>
>>>>>> +    if (tag == 'M' || tag == 'm') {
>>>>>> +        char ms[5] = {0};
>>>>>> +        int64_t dnow = (unow - ((int64_t)now) * 1000000) 
>>>>>> / 1000;
>>>>>> +        snprintf(ms, 5, ".%03d", (int)dnow);
>>>>>> +        av_bprint_append_data(bp, ms, 4);
>>>>>> +    }
>>>>>>
>>>>>
>>>>>
>>>>> How about
>>>>>
>>>>>     av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 1000);
>>>>
>>>> Makes way too much sense. I need holidays…
>>>>
>>>> Attached v3.
>>>>
>>>> Thanks!
>>>> -Thilo
>>>
>>>>  doc/filters.texi          |    8 ++++++++
>>>>  libavfilter/vf_drawtext.c |   12 ++++++++++--
>>>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>>> 87d34e4106b829d42c5e57c847c28bed08bf3a81 
>>>> v3-0001-lavfi-drawtext-Add-localtime_ms-for-millisecond-p.patch
>>>> From fd34d1434e2243a881c24f6db4cc0db92289f4bb Mon Sep 17 00:00:00 
>>>> 2001
>>>> From: Thilo Borgmann <thilo.borgmann@mail.de>
>>>> Date: Fri, 10 Dec 2021 12:34:23 +0100
>>>> Subject: [PATCH v3] lavfi/drawtext: Add localtime_ms for 
>>>> millisecond
>>>> precision
>>>
>>> Iam missining something here as it doesnt build
>>>
>>> AR    libavdevice/libavdevice.a
>>> CC    libavfilter/vf_drawtext.o
>>> libavfilter/vf_drawtext.c: In function ‘func_strftime’:
>>> libavfilter/vf_drawtext.c:1052:12: error: implicit declaration of
>>> function ‘av_gettime’; did you mean ‘av_get_token’?
>>> [-Werror=implicit-function-declaration]
>>>      unow = av_gettime();
>>>             ^~~~~~~~~~
>>>             av_get_token
>>> libavfilter/vf_drawtext.c:1061:20: warning: passing argument 1 of
>>> ‘av_bprintf’ from incompatible pointer type
>>> [-Wincompatible-pointer-types]
>>>          av_bprintf(&bp, ".%03d", (int)(unow % 1000000) / 
>>> 1000);
>>>                     ^
>>> In file included from libavfilter/vf_drawtext.c:47:0:
>>> ./libavutil/bprint.h:127:6: note: expected ‘AVBPrint * {aka struct
>>> AVBPrint *}’ but argument is of type ‘AVBPrint ** {aka struct 
>>> AVBPrint
>>> **}’
>>>  void av_bprintf(AVBPrint *buf, const char *fmt, ...)
>>> av_printf_format(2, 3);
>>>       ^~~~~~~~~~
>>> cc1: some warnings being treated as errors
>>> ffbuild/common.mak:70: recipe for target 'libavfilter/vf_drawtext.o'
>>> failed
>>> make: *** [libavfilter/vf_drawtext.o] Error 1
>>
>> Works for me on OSX.
>>
>> av_gettime() is in lavu/time.c which gets included via
>> lavu/time_internal.h….
>>
>
> av_gettime() is public and resides in lavu/time.h, not
> lavu/time_internal.h; the latter does not include the former in any 
> way.

Ups, <time.h> was it…


> But compat/os2threads.h and compat/w32pthreads.h include lavu/time.h.
> Maybe you have it from the former? It doesn't work here (Ubuntu 21.10)
> either.

Retested and I got no clue how this appeared to be working for me 
yesterday - sorry.

v4 attached, including lavu/time.h and fixed pointer type warning.

Thanks!
-Thilo
From 9b70c93c754fa4cd1b55fd3967910727f685a6b6 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann@mail.de>
Date: Sun, 12 Dec 2021 16:17:03 +0100
Subject: [PATCH v4] lavfi/drawtext: Add localtime_ms for millisecond precision

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

diff --git a/doc/filters.texi b/doc/filters.texi
index 78faf76..db75632 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -10949,10 +10949,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 382d589..d047a8c 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,21 @@ 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);
     av_bprint_strftime(bp, fmt, &tm);
+
+    if (tag == 'M' || tag == 'm') {
+        av_bprintf(bp, ".%03d", (int)(unow % 1000000) / 1000);
+    }
     return 0;
 }
 
@@ -1152,7 +1159,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 },

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.

Regards,

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

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

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

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;
+            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 },

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

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)
>

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

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
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 },

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,

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

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

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

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,

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

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

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

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,

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
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;
 }

> 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)
>

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
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;
 }

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
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;
 }

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

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

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

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,

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
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;
 }

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

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

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
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;
 }

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

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
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;
 }

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

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

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