IE seems to have a minumum frame delay of about 0.1 seconds. Thus, any frames set to 0.01 through 0.09 (1 - 9 in .gif "display time" terms) gets slowed down to 0.1. Mozilla should have a Minumum Frame Delay Setting, and it possibly should be defaulted to 0.1. The major reason for a 0.1 default is that many animated .gif makers set their .gifs to 0.01, test it on IE, and see it going the speed they want it to go. Little do they realize IE is slowing down their animation. Then, when someone loads the .gif in Mozilla, it goes the proper 100 frames a second, and the animation blazes by, sometimes causing high CPU usage. This feature would effect the following bugs: Bug 116853 - animated gif too fast when in background of inactive window. Bug 105851 - no (incorrect) delay in gif animation -- speedy -- when moving cursor around. Bug 99636 - animated gif causes high (sometimes 100%) CPU usage. Bug 86319 - (large) animated GIF has terrible performance This feature request is very similar but not the same as Bug 71829 ([rfe] Let the user set a default delay for animated gifs that don't specify one). Bug 71829 requests a default delay, and this bug requests a minimum delay. These two could be combined into a "minumum/default delay" setting and kill both birds with one stone. When users see their CPU hit 99%, they generally blame the browser instead of the real culprit, the web designer that thought IE actually displayed their animation at 0.01 (setting 1 in most animation programs) The URL shows 4 animated gifs, each at 10, 50, 100, and 200ms delay respectively. IE displays the first 3 at the same speed, and the last one takes twice as long. Mozilla (Build 2002021203, Win2k) displays the gif (close to) the real speed.
Summary: Minimum frame rate delay on animatited images → Minimum frame rate delay on animated images
Can you try this patch out and post the results? It seems to make things better for me.
Excellent! I don't have the source to compile.. could someone else try it out? From the looks of it, that will probably fix it up to "looking like IE". It still would be nice to allow faster animation.. mind you, I can't think of any reason one would need 100 frames per second. But I can see some people wanting 30fps. If we are looking for a quick fix, how about something like: if (mTimeout = 0) *aTimeout = 100; // Ensure a minimal time between updates so we don't throttle the UI thread. elseif (mTimeout < 30) *aTimeout = 30; // Set minimum frame display length to save CPU else *aTimeout = mTimeout; (excuse my bad C grammer, it's been a while) This would force each frame to be shown at least for 30ms, which gives the user approx. 33 1/3 FPS. Mozilla would be more capable than IE, but limiting (hopefully) enough to save the CPU. The preferable method of course is to have a configurable setting, but that's hardly a priority.
Can this bug be moved under Bug 119597 (animated GIF tracking bug) if my guess of where it belongs is correct? First glaring bug in my previous comment is the first line of code. It should be of course: if (mTimeout == 0) Would gfxImageFrameWin.cpp also need this patch?
sorry for the spam, I just noticed my edit to your patch would introduce a horid bug. Here's my 3rd try: if (mTimeout == 0) *aTimeout = 100; // Ensure a minimal time between updates so we don't throttle the UI thread. elseif (mTimeout >= 0 && mTimeout < 30) *aTimeout = 30; // Set a minimum delay of 30ms else *aTimeout = mTimeout; I really need to figure out how to make patch files someday :)
A proper "minimum" timeout is a tricky thing. Some people may make anims with a rate of around screen-refresh rate to get smooth animations. (See bug 124999). 30ms ~= 30Hz, which is approaching smooth, but still way under refresh rates of CRT monitors of 60-90+ Hz (mostly 72-85). LCD's are 25-40Hz generally. 60Hz would be around 16ms; I'd be tempted to target that and see how it affects the bugs reported. I have a patch that makes it a pref, but we have a lot of prefs in the tree.... I agree if the time isn't specified, set it to something safe, like 50 or 100ms. (100 is good I think)
Target Milestone: mozilla0.9.9 → ---
Sets default of 100ms if animtime <= 0, otherwise limits animation speed to no faster than 20ms (50Hz). If someone wants, I can create prefs for this trivially (I have them in my local tree).
Attachment #69177 - Attachment is obsolete: true
Nominating - trivial patch. We can change the defaults if other values are preferred.
I don't know how much effect having a minimum of 20ms will be on large animated gifs. Most of them are using 99% CPU @ 10ms, and will continue to use 99% @ 20ms due to the sheer volume of the image trying to be moved. However, it's a good starting point, as as pointed out, it can be tweaked if need be later. Let's get it into a build and see what it does :D
Since this can eat 100% cpu and cause UI starvation with anims with <10-15 ms repeat times, raising from enhancement to normal. Already nominated for 0.9.9 - r/sr? we could change the max rate to 100ms (10Hz, same as IE), 16ms (60+Hz) or 10ms (100Hz) if people feel that's needed. Read the initial report for details.
Severity: enhancement → normal
Mmmh, I feel we should limit the frame rate if no value is specified, but not try to "improve" the animation settings to what we believe the creator wanted them to be in other cases. It looks to me like a rather bad, Microsoftish approach. There *may* be reasonable uses for higher frame rates (e.g. by improved monitors in the future).
Part of the problem is that there are many animations out there that animate faster with mozilla than the author intended; far more than where the author intended it animate faster than IE lets it (100ms). As I said, we can quibble over whether the limit should be 10, 16, 20, 50 or 100 (etc) ms. We could make it a pref (I have the code, it's trivial). But I think we should check this fix in with some delay > 10ms. The current limitation is set by the GIF decoder at 10ms (#define MINIMUM_DELAY_TIME 10). Thus far, there are no other sources of animated frames, but I'm sure eventually some other anim format will appear, given GIF's inadequacies.
I believe IE and Opera set the minimum delay to 100ms for a reason. That reason being anything shorter uses up way too much CPU (not because of the monitor). That was a while ago. Today, we have faster computers, and probably can afford less of a minumum delay. But not 10ms as it's set to now. (See the bugs in the first comment -- they all use 99% CPU). 10ms = bad, 99% CPU 100ms = like Microsoft & Opera, but gif makers are still in ignorant bliss about correct settings for delay. <100ms = better than other browsers. Gif makers will learn they can do more with Mozilla. (Although IMO, if they are making high frame rate animations, they should be using something more designed for it -- like flash or quicktime) Let's get the patch in :)
Example of a gif I ran across today that animates so fast the red-and-blue flashing appears purple on my machine (I'm running a 10ms limit). Obviously not as the author intended. The point that lots of images around the web cause 99% cpu usage and possible UI starvation is a very strong one. I advise nothing below 20ms, and I think 40ms (same as cheaper LCD displays) or 50ms may make sense - unless we want to go with IE's default of 100ms, given that that's what all authors use/assume in their testing, whether they know it or not. If we want to preserve the ability for other (future) formats to provide faster animation, we can move the limit from here into the gif decoder (where it's already a #define).
IE has created a _de facto_ standard. I think we can live with it. We might argue that to beat the 10Hz perceptual limit, we should use 50ms, but at this point, why be different? /be
We can always add a preference to specify the minimum frame delay, with a default of 100 mSec, just like IE. I think this is real low hanging fruit, which can dramatically improve the performance of Zilla (together with bug 125025). In fact, it's so low that you need a dig a hole to grab the fruit!
I agree somewhat with comment #11. Or... at the very least, 100ms is too tardy a compromise.
Okay, the patch has been around for 2 weeks now, and has been tested with Bug 99636 (reducing the CPU usage of the bug's gif from 88% to 50% if I read it correctly) What's the next step? Can we get this patch into Mozilla?
Comment on attachment 70180 [details] [diff] [review] Updated patch r=pavlov
Attachment #70180 - Flags: review+
This is a simple patch which we should check in for 1.0
Target Milestone: --- → mozilla1.0
reassigning to rjesup
Updated patch for 100ms minimum delay (IE/etc compatible), per discussion with pavlov. r/sr please.
Attachment #70180 - Attachment is obsolete: true
Comment on attachment 78242 [details] [diff] [review] Updated patch for 100ms as per pavlov r=pavlov
Attachment #78242 - Flags: review+
Assignee: pavlov → rjesup
Status: ASSIGNED → NEW
Comment on attachment 78242 [details] [diff] [review] Updated patch for 100ms as per pavlov sr=jst
Attachment #78242 - Flags: superreview+
New Patch may cause problems. if (mTimeout < 100) // 100 is compatible with IE and Opera among others should be: if (mTimeout >= 0 && mTimeout < 100) // 100 is compatible with IE and Opera among others If I read the code correctly, functions such as imgContainer::AppendFrame(gfxIImageFrame *item) and imgContainer::StartAnimation() want a -1 returned from GetTimeout if the frame should be displayed forever, so we shouldn't set timeout to 100 if it's < 0 And, as I asked earlier, will the GetTimeout function in gfx2/src/windows/gfxImageFrameWin.cpp need updating too?
ArronM is correct (-1 is a flag to display forever, and gfx2/windows has a copy of the routine (why? I don't care right now). Updated patch to match. Fast r/sr and I'll get an a= from Asa
Attachment #78242 - Attachment is obsolete: true
Comment on attachment 78254 [details] [diff] [review] Allow negative values through, fix windows as well sr=jst
Attachment #78254 - Flags: superreview+
Comment on attachment 78254 [details] [diff] [review] Allow negative values through, fix windows as well r=pavlov (fyi, the windows gfx2 code isn't built)
Attachment #78254 - Flags: review+
Comment on attachment 78254 [details] [diff] [review] Allow negative values through, fix windows as well a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #78254 - Flags: approval+
Fix checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
I have not been following this bug because I had never experienced any problems with GIF animations eating too much CPU time on any of my Macs. I am wonder if this was a Windows only problem? This morning I looked at http://animecity.nu/Bugzilla/Anim.html with OS X Mozilla build 2002040512, IE 5.1 for Mac OS X, and iCab 2.7 for OS X. All of them displayed the 4 images at http://animecity.nu/Bugzilla/Anim.html at the frame rates specified by the animations (or at least a close approximation since I have no way to determine the actual frame rate that I am seeing). Of course Mozilla build 20020400903 which has the patch applied now shows the first 3 animations at the 100ms speed. Since IE 5.1 and iCab do not follow the "de facto standard" of minimum of 100ms and since it looks like Mac builds of Mozilla may never have had the problems that this patch addresses, I am wondering if the Mac's ability to show animations at the author's requested speed should be restricted. Someone might also want to check and see if this was an issue for Linux. I am happy for all the Windows users whose system performance issues were aided by this patch, but as a Mac user who likes 30fps animations to be displayed at 30fps, I hope that someone will consider adjusting this patch.
It's a tough issue. Lots of web designers never realize that their animations are being played back at up to 10x the intended speed. I've seen very few anims that purposely selected values less than 100ms. If they did, they'd then have a problem with Win32-IE, Opera, etc (plus in most cases ns4.x) users being annoyed that the animations are slower than intended. We talked about different values. Increasing from 10ms (for GIFs) to 20, 30, 50 and 100 were discussed. The final determination was to go with 100ms for compatibility. We also could put in Yet Another Pref (in fact, I have done that for the wgate product because I need to crank it _down_ to 2000 or 3000ms). Too many prefs is a bad thing though. If you'd like to request 100 be changed to something smaller, open a bug, reference this one, and suggest reasoning for a different limit. One argument is that we should respect the author's request, though it may have been unintentional.
*** Bug 105851 has been marked as a duplicate of this bug. ***
*** Bug 205170 has been marked as a duplicate of this bug. ***
*** Bug 210626 has been marked as a duplicate of this bug. ***
For reference purposes : Bug 139677 : Request to Remove Unnecessary Minimum frame rate delay for GIF anim on OS X since Mozilla is the only Mac browser imposing this restriction. Bug 71829 : Request for a pref to allow the user set a default delay for animated gifs that do not specify one.
*** Bug 342680 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.