Video files suddenly take 100s before playback starts

[Rixa]

Summary of the bug:

I built mplayer from fresh svn sources (r38637) the other day on Xubuntu 22.04 and several video file formats now seem to hang for about 100 seconds before playback starts.

-demuxer mov played mp4 files without delay (though presenting other issues) so I figured it has to do with the lavfpref demuxer and bisected the included ffmpeg code to this commit ​https://github.com/FFmpeg/FFmpeg/commit/0d5e3f5a4034b6c9312b7c621e25aa4303a00b6f . If I revert it, playback starts promptly again.

How to reproduce:

The input file is the 480x270 one from
​https://file-examples.com/index.php/sample-video-files/sample-mp4-files/

$ mplayer file_example_MP4_480_1_5MG.mp4 
MPlayer SVN-r38637-11 (C) 2000-2024 MPlayer Team

Playing file_example_MP4_480_1_5MG.mp4.
libavformat version 61.5.101 (internal)
libavformat file format detected.
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x6426620c8a60]Protocol name not provided, cannot determine if input is local or a network protocol, buffers and access patterns cannot be configured optimally without knowing the protocol

Nothing further for about 100s, then playback starts.

[Mike Lieman]

Thank you for this. The past few days I was trying to get profile data out of mplayer, to try and understand what was going on, but there's something about the way mplayer exits that prevents triggering the output of profile data.

[Mike Lieman]

patch for ffmpeg/libavutil/timestamp.c

[Mike Lieman]

Hey, I patched timestamp.c to use the right constant, and it appears to resolve the issue.

-        double log = (fpclassify(val) == FP_ZERO ? -INFINITY : floor(log10(fabs(val))));
+        double log = (fpclassify(val) == FP_ZERO ? FP_INFINITE : floor(log10(fabs(val))));

I attached a patch file. If you could test it and verify that it fixes it, that would be great. Then I just have to figure out how to get it into ffmpeg.

[Rixa]

While strictly speaking it does make the problem away, I think it also breaks the code as FP_INFINITE appears to be an enumeration value for fpclassify() so you have essentially changed -INFINITY to 2 and its not the same thing at all.

I think the problematic commit is meant to get around log10(0.0) being undefined but approaching negative infinity and address it as -INFINITY instead of -HUGE_VAL as log10() is supposed to return. But does it even?

Infinities are tricky to handle so something somewhere might go wrong because of it but as it seems to be only used in the next statement, I don't really understand why the commit is doing much at all..

[Mike Lieman]

Replying to Rixa:

While strictly speaking it does make the problem away, I think it also breaks the code as FP_INFINITE appears to be an enumeration value for fpclassify() so you have essentially changed -INFINITY to 2 and its not the same thing at all.

I have been corresponding with the ffmpeg-devel list and now understand exactly what is happening.

Mplayer's use of -ffast-math is messing with the floating point operations in ffmpeg's libavutil/timestamp.c. By removing it from mplayer's configure script, the issue appears to be resolved.

Could you please verify?

Index: configure
===================================================================
--- configure   (revision 38637)
+++ configure   (working copy)
@@ -2969,10 +2969,10 @@
   elif test "$cc_vendor" != "gnu" ; then
     CFLAGS="-O2 $_march $_mcpu $_pipe"
   else
-    CFLAGS="-O4 $_march $_mcpu $_pipe -ffast-math -fomit-frame-pointer"
+    CFLAGS="-O4 $_march $_mcpu $_pipe -fomit-frame-pointer"
     WARNFLAGS="-Wall -Wno-switch -Wno-parentheses -Wpointer-arith -Wredundant-decls -Werror=format-security"
     WARN_CFLAGS="-Werror-implicit-function-declaration"
-    extra_ldflags="$extra_ldflags -ffast-math"
+    extra_ldflags="$extra_ldflags"
   fi
 
   if test "$_profile" != "" || test "$_debug" != ""; then

[Rixa]

Yes you're right, removing -ffast-math seems to get around the problem. So it's an optimization thing then.

It feels rather heavy-handed to remove the flag, though, if it has worked so far. Perhaps there are ways to rewrite the problem-bringing commit in a way that would survive -ffast-math? I can't help with that, unfortunately.

[Mike Lieman]

There really isn't anything wrong with ffmpeg's code, the root-cause of this is fast-math's handling of infinities, and the fix is "do floating point math the correct way".

[mike@orion mplayer]$ svn log  configure --search fast-math
------------------------------------------------------------------------
r28694 | diego | 2009-02-21 15:25:02 -0500 (Sat, 21 Feb 2009) | 3 lines

Add -ffast-math to LDFLAGS as well as to CFLAGS.
patch by Piotr Kaczuba, pepe attika.ath cx

------------------------------------------------------------------------
r15592 | diego | 2005-05-29 19:16:22 -0400 (Sun, 29 May 2005) | 5 lines

The default CFLAGS settings include -ffast-math and GCC 3.4.3 therefore
optimizes the lrintf call in the test away, effectively turning it into
a NOP. The attached patch forces -ffast-math of for this test.
patch by Joerg Sonnenberger <joerg - at -britannica - dot - bec - dot - de>

------------------------------------------------------------------------

Does it actually bring anything to the table today?

[Rixa]

I don't know, it could add up when processing large video files I suppose but would require benchmarking to find out. On the other hand having read about it a bit now it does seem to be a very risky flag to enable globally, especially for external components like ffmpeg. So perhaps you're right.

[Rixa]

For what it's worth, it should be perfectly doable in this instance to address the special case of log10(0) without involving infinities, and survive -ffast-math that way:

diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c
index 6c231a517..68466d2a3 100644
--- a/libavutil/timestamp.c
+++ b/libavutil/timestamp.c
@@ -24,8 +24,13 @@ 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))));
-        int precision = (isfinite(log) && log < 0) ? -log + 5 : 6;
+        int precision;
+        if (fpclassify(val) == FP_ZERO) {
+            precision = 6;
+        } else {
+            double log = floor(log10(fabs(val)));
+            precision = (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;
         for (; last && buf[last] == '0'; last--);