[FFmpeg-devel] [PATCH] avutil/timestamp: avoid using INFINITY for log10 result in av_ts_make_time_string2
Marton Balint
cus at passwd.hu
Tue Aug 27 10:00:08 EEST 2024
On Tue, 27 Aug 2024, Rémi Denis-Courmont wrote:
>
>
> Le 27 août 2024 03:03:37 GMT+03:00, Marton Balint <cus at passwd.hu> a écrit :
>> Using INFINITY can cause issues with -ffast-math, and since we only use this
>> value to decide the formatting, we can just as easily use 0 for log10 of zero.
>
> FFmpeg does not enable fast-math and last I tried it won't work at all anyway: FATE commits seppuku by OoM.
>
> The statement above is completely false if applied to the whole code
> base rather than just this function. Notably FFMAX and FFMIN behave
> differently under -fno-infinite-math (which is implied by -ffast-math).
I meant the statement to the code in question, not generically.
>
> I suspect that *this* code is just the tip of the iceberg. It's not hard
> to imagine that ignoring infinities will break stuff, for one: reading
> raw binary content as float can lead to infinities and other NaNs.
Agreed.
>
> So IMO this patch is wrong. Either we've audited the whole code base,
> and then we should enable -ffast-math, or at least -fno-infinite-math by
> default, or we haven't and then we should not be papering over the issue
> like this.
>
IMHO the patch itself is harmless (simple, easy to follow), and if it
helps somebody (no matter how unupported its use-case is), then why the
hell not. But I can understand your point of view, that we should not
bother with it, if it is unsupported in the first place, so I will just
drop this, did not feel strongly about it anyway.
Regards,
Marton
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>> libavutil/timestamp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c
>> index 6c231a517d..be4540d4c8 100644
>> --- a/libavutil/timestamp.c
>> +++ b/libavutil/timestamp.c
>> @@ -24,7 +24,7 @@ char *av_ts_make_time_string2(char *buf, int64_t ts, AVRational tb)
>> snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
>> } else {
>> double val = av_q2d(tb) * ts;
>> - double log = (fpclassify(val) == FP_ZERO ? -INFINITY : floor(log10(fabs(val))));
>> + double log = (fpclassify(val) == FP_ZERO ? 0 : floor(log10(fabs(val))));
>> int precision = (isfinite(log) && log < 0) ? -log + 5 : 6;
>> int last = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.*f", precision, val);
>> last = FFMIN(last, AV_TS_MAX_STRING_SIZE - 1) - 1;
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list