[FFmpeg-devel] [PATCH v2] lavfi/drawtext: Add localtime_ms for millisecond precision
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Fri Jan 14 20:05:33 EET 2022
Thilo Borgmann:
> Am 14.01.22 um 14:17 schrieb "zhilizhao(赵志立)":
>>
>>
>>> On Jan 14, 2022, at 8:14 PM, Thilo Borgmann <thilo.borgmann at 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
More information about the ffmpeg-devel
mailing list