| Message ID | CAG2_C8AKtvC0ZgP03Cp-Q6HQG6V9WSLpAX6Jd5B-mjdURueBeg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] use proper macro to avoid issue with prior avutil/timestamp.c | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
On 8/17/2024 12:09 AM, Mike Lieman wrote: > From b2ddfdd9ed695a1f47ed6251369abca08864e3ab Mon Sep 17 00:00:00 2001 > From: Mike Lieman <mikelieman@gmail.com> > Date: Fri, 16 Aug 2024 23:05:51 -0400 > Subject: [PATCH] use proper macro to avoid issue with prior > avutil/timestamp.c > patch causing long startup times with some files under mplayer > (https://trac.mplayerhq.hu/ticket/2425) > > --- > libavutil/timestamp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c > index 6c231a517d..eab2531538 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 ? FP_INFINITE : FP_INFINITE is a return value from fpclassify(), not a double. Does maybe using av_int2double(UINT64_C(0xFFF) << 52) help your slow startup? > 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;
> > FP_INFINITE is a return value from fpclassify(), not a double. > > Does maybe using av_int2double(UINT64_C(0xFFF) << 52) help your slow > startup? > Sadly, no. double log = (fpclassify(val) == FP_ZERO ? av_int2double(UINT64_C(0xFFF) << 52) : floor(log10(fabs(val)))); gives the same long-startup time.
Le 17 août 2024 06:09:16 GMT+03:00, Mike Lieman <mikelieman@gmail.com> a écrit : >From b2ddfdd9ed695a1f47ed6251369abca08864e3ab Mon Sep 17 00:00:00 2001 >From: Mike Lieman <mikelieman@gmail.com> >Date: Fri, 16 Aug 2024 23:05:51 -0400 >Subject: [PATCH] use proper macro to avoid issue with prior >avutil/timestamp.c > patch causing long startup times with some files under mplayer > (https://trac.mplayerhq.hu/ticket/2425) > >--- > libavutil/timestamp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c >index 6c231a517d..eab2531538 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 ? FP_INFINITE : You're papering over whatever the real issue is here. As noted by James and the Trac issue already, FP_INFINITE is *not* a floating point value and using it this way makes no sense. >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;
I now understand that and am trying to work through the logic to see why we are getting this strange behaviour. When I have some free time, I think I'm going to have to step through this with gdb since without the context of what's actually in buf, ts, and tb I can't understand what's going on in the for loops, but it seems something's going on in the for loops, since it's not happening when I'm using the incorrect constant. On Sat, Aug 17, 2024 at 10:07 AM Rémi Denis-Courmont <remi@remlab.net> wrote: > > > Le 17 août 2024 06:09:16 GMT+03:00, Mike Lieman <mikelieman@gmail.com> a > écrit : > >From b2ddfdd9ed695a1f47ed6251369abca08864e3ab Mon Sep 17 00:00:00 2001 > >From: Mike Lieman <mikelieman@gmail.com> > >Date: Fri, 16 Aug 2024 23:05:51 -0400 > >Subject: [PATCH] use proper macro to avoid issue with prior > >avutil/timestamp.c > > patch causing long startup times with some files under mplayer > > (https://trac.mplayerhq.hu/ticket/2425) > > > >--- > > libavutil/timestamp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c > >index 6c231a517d..eab2531538 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 ? FP_INFINITE : > > You're papering over whatever the real issue is here. As noted by James > and the Trac issue already, FP_INFINITE is *not* a floating point value and > using it this way makes no sense. > > >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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sat, Aug 17, 2024 at 3:27 PM Mike Lieman <mikelieman@gmail.com> wrote: > > > > > FP_INFINITE is a return value from fpclassify(), not a double. > > > > Does maybe using av_int2double(UINT64_C(0xFFF) << 52) help your slow > > startup? > > > > Sadly, no. > > double log = (fpclassify(val) == FP_ZERO ? av_int2double(UINT64_C(0xFFF) << > 52) : floor(log10(fabs(val)))); > For the logic in this particular function - even if not accurate, any non-negative value will result in the same outcome as using -INFINITY. So using even the value 0 would work. My simple guess would be that the infinity check fails for some reason (maybe -ffast-math?), thus treating the infinite as a finite value, and using an extreme amount of precision that almost breaks printf? We could avoid the infinity entirely here without any downsides, really. Or you could check if your build uses ffast-math, not sure we support that. - Hendrik
> > On Sat, Aug 17, 2024 at 11:29 AM Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > Or you could check if your build uses ffast-math, not sure we support that > Hot Dog! We have a weiner! Removing -ffast-math from my mplayer build appears to have resolved the issue. Thank you for your time and guidance.
diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c index 6c231a517d..eab2531538 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 ? FP_INFINITE : floor(log10(fabs(val))));
From b2ddfdd9ed695a1f47ed6251369abca08864e3ab Mon Sep 17 00:00:00 2001 From: Mike Lieman <mikelieman@gmail.com> Date: Fri, 16 Aug 2024 23:05:51 -0400 Subject: [PATCH] use proper macro to avoid issue with prior avutil/timestamp.c patch causing long startup times with some files under mplayer (https://trac.mplayerhq.hu/ticket/2425) --- libavutil/timestamp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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;