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

[Andreas Rheinhardt]

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

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

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

- Andreas