GIF scaling limit should be applied to animated GIFs only
Closed, ResolvedPublic

Description

Due to recurring problems with scaling of large animated GIFs taking down the image scaling servers, we put in a hack to prevent server-side scaling of very large, then *all* animated GIFs.

Unfortunately this currently applies to *all* GIFs, which can be very nasty for cases like this page:

http://commons.wikimedia.org/wiki/Category:Neue_Gedichte_(Heine_1852)

This category on Commons has a gallery with a large number of scanned pages... uploaded as large GIFs... which are not being thumbnailed... so they're all being loaded inline...

My Firefox process was using 2.31GB of real memory after loading the page. Not so bad when you've got a loaded 4-gig box, but pretty crappy if you're on a more typical machine which will keel over and die.

We need to distinguish between animated and non-animated GIFs so still images can continue to be scaled cleanly.


Version: unspecified
Severity: normal
URL: http://commons.wikimedia.org/wiki/Category:Neue_Gedichte_(Heine_1852)

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz16451.
brion created this task.Nov 24 2008, 8:27 PM
demon added a comment.Nov 25 2008, 6:06 AM

Googling around for a solution, it seems the eZ Components framework guys have hit a solution (or they did? I can't find the same bit of code in their current trunk). In any case, check out the processGif() function at http://svn.ezcomponents.org/viewvc.cgi/packages/ImageAnalysis/trunk/src/analyzer.php?view=markup&pathrev=1474 (very bottom of page). Seems they count the frames based off the number of commas. Code seems to be under New BSD license.

brion added a comment.Dec 9 2008, 8:01 PM

A quick test of that extracted code seems to show incorrect results; I get multiple frames listed for a lot of images in my downloads folder which are not animated.

  • Bug 16892 has been marked as a duplicate of this bug. ***

Bug 16892 is saying that you are not thumbnailing images whose filenames start
with numbers, nothing to do with animation...

ayg wrote:

(In reply to comment #4)

Bug 16892 is saying that you are not thumbnailing images whose filenames start
with numbers, nothing to do with animation...

Your description is wrong. Plenty of files whose names start with numbers are being thumbnailed. Note that all of the individual files you had problems with are GIFs -- this is a more plausible explanation for the lack of thumbnailing.

The first comment in the user notes section at:

http://www.php.net/manual/en/function.imagecreatefromgif.php

provides instructions for determining whether a GIF is animated or not.

It looks for 6 specific bytes as part of a 10 byte sequence that denote the opening of a new frame header. Strictly speaking, I believe this will sometimes misidentify an image as animated when it is not, since the GIF spec appears to allow the same control bytes to occur within compressed image data (etc.) However, getting a 6 byte sequence by accident is a low probability event in random data. More importantly, a false positive (that causes a static image not to be scaled) would appear to be fine from a WMF point of view provided that one never has false negatives, which I think is correct for the code sample given.

If the issue is simply that one wants to the detect and disable scaling on animated GIFs then I believe the code sample at the linked page is adequate for that purpose.

brion added a comment.Mar 6 2009, 1:47 AM

This fix would make some people happy again. :)

ssanbeg wrote:

perl script to parse blocks from a GIF

Those PHP solutions seem a bit kludgy. A better solution would be parse the GIF, and count the frames; then only thumbnail single-frame images.

It needs < 100 lines of perl to do this, and that could be used as a wrapper around the thumbnailer, or called to check the value before thumbnailing. Feel free to use the example & credit me.

attachment parsegif.pl ignored as obsolete

This would be pretty easy to implement in PHP. :) Thanks for the sample code!

Created attachment 5956
Mostly finished patch

This is a mostly-done patch which I need to polish off at some point. Putting it here so it doesn't get lost.

Attached: gifpatch

ssanbeg wrote:

Cool, glad it's useful. BTW

+ ## Skip dimensions (Why is this 8 bytes and not 4?)

Maybe I should've called that a bounding box, since the frame overlays a part of the background, so that's the upper left corner + width & height as 4 unsigned shorts.

werdna added a comment.Apr 1 2009, 6:54 AM

(In reply to comment #11)

Cool, glad it's useful. BTW

+ ## Skip dimensions (Why is this 8 bytes and not
4?)

Maybe I should've called that a bounding box, since the frame overlays a part
of the background, so that's the upper left corner + width & height as 4
unsigned shorts.

I ended up pulling the information from the GIF spec.

ssanbeg wrote:

*** Bug 19089 has been marked as a duplicate of this bug. ***

(Batch change)

These are low-priority miniprojects that I can mop up at some point when I'm doing general code work as opposed to working on a specific projects.

buzz wrote:

Bug 19089 https://bugzilla.wikimedia.org/show_bug.cgi?id=19089 which was mine and marked as a duplicate is quite important though. What do you think ?

I have to agree with Jools that this doesn't really feel like a low priority issue. This bug may not be the end of the world, but certain GIF heavy categories and pages are highly burdensome because the client gets sent many megabytes of uncompressed GIFs rather than usable thumbnails. This is also one of the most frequently complained about bugs on wiki (once every few weeks in my experience).

ssanbeg wrote:

Upgrading severity; I don't think "not bringing down the entire site" is really in the spirit of an enhancement.

It would be helpful to have some information on what still needs to be done, and possibly what time frame it's expected. It's not clear to me whether there's still development work needed, or just testing. If the current behavior is sufficiently broken, and the time frame is long enough, people may want to migrate from gif to png just to avoid this bug; so they should have something to base that decision on.

When I closed Jools, bug we already had the basic functionality implemented, so I expected a better resolution to be quickly forthcoming, but in the meantime it's only been de-prioritized, so I may have been mistaken.

Note that my change of the priority was merely a tool to try and make sense of my bug list, distinguishing bugs that need urgent attention, my ongoing priorities, side projects that I need to get to on my bug day (currently Friday), and bugs that are best assigned to me but which I have no intention of expending the effort to fix.

This bug is in the penultimate category — I will get to it, but it is not urgent nor one of my day to day priorities.

The functionality requested has been implemented in the attached patch, but I need to test and apply the patch when I have the time to.

brion added a comment.Aug 2 2009, 4:41 PM

Bumping severity down and priority up; it's not a big deal but it's damned annoying and a fix has been a long time coming. :)

brion added a comment.Aug 2 2009, 4:47 PM

Patch looks pretty nice, as long as the metadata extraction works. :D

Quick note on UI/localization...

+ $info[] = wfMsgNoTrans( 'file-info-gif-frames', $metadata['frameCount'] );
...
+'file-info-gif-frames' => '$1 frames',

For some languages such as a lot of the Slavic languages, we'll still want to be able to use {{PLURAL:}} here even if we're guaranteed to be more than 1, as different numbers can require different plural forms.

werdna added a comment.Aug 3 2009, 3:49 PM

Fix has been committed, with modifications, in r54284.

brion added a comment.Aug 5 2009, 1:29 AM

I've got some GIF files that throw an exception reading the metadata, all "At position: 803". Will attach samples...

brion added a comment.Aug 5 2009, 1:29 AM

Created attachment 6419
pics/Spinning-Jimbo-head-of-doom.gif: FAILED: At position: 803, Unknown byte 135

Attached:

brion added a comment.Aug 5 2009, 1:30 AM

Created attachment 6420
unsorted/spider_wuv.gif: FAILED: At position: 803, Unknown byte 46

Attached:

brion added a comment.Aug 5 2009, 1:33 AM

And one more sample which is too big to upload to bugzilla:

http://leuksman.com/misc/giant.gif

evil/giant.gif: FAILED: At position: 803, Unknown byte 194

Warning -- this is a very large animated GIF which caused problems with our scalers last year. Your browser might not like it either. ;)

[Batch change]

Removing Dave McCabe from CC, who I somehow managed to add to the CC list of 12 bugs assigned to me.

Fixed bug with non-GCF, non-application extensions in r55015.

This bug is marked as "fixed", but when non animated GIF's on commons are still not being scaled. Today i was looking at [http://commons.wikimedia.org/wiki/File:Durer-vision-hires.gif] and the 100x100 thumbnail is being created out of 2000x1000 image. Is it fixed and not released yet, or the fix is not working?

Additional question about animated gif's: Are there any plans to thumbnail them? Current fix on commons to add NOGALLERY to all categories that might have any gif's, in order to prevent browser crashes, makes those categories impossible to browse. Since scaling of animated GIF's was disabled "due to recurring problems with scaling of large animated GIFs taking down the image scaling servers", is there a way to outsource this job to some other machine or even allow community to upload scaled versions of GIF file?

Correctly resized animated GIF images are showing up on last.fm pages:

Maybe last.fm is using open source code for this? Can someone ask them? People can load static or animated GIF images as profile/avatar images at the last.fm site. The uploaded image shows up very quickly in the various sizes. See also:
http://commons.wikimedia.org/wiki/Commons:Graphics_village_pump#Animated_GIF_resizing._Last.fm_resizes_them._Why_not_MediaWiki.3F

I used a throwaway email address here at Bugzilla. I don't check it more than once a week. I suggest making email addresses private at Bugzilla. Bugzilla would be much better for it.

werdna added a comment.Nov 3 2009, 3:09 PM

(In reply to comment #29)

Maybe last.fm is using open source code for this? Can someone ask them? People
can load static or animated GIF images as profile/avatar images at the last.fm
site. The uploaded image shows up very quickly in the various sizes. See also:
http://commons.wikimedia.org/wiki/Commons:Graphics_village_pump#Animated_GIF_resizing._Last.fm_resizes_them._Why_not_MediaWiki.3F

We use open-source code to resize animated GIFs, but it is very difficult to do for very very big images.

(In reply to comment #28)

This bug is marked as "fixed", but when non animated GIF's on commons are still
not being scaled. Today i was looking at
[http://commons.wikimedia.org/wiki/File:Durer-vision-hires.gif] and the 100x100
thumbnail is being created out of 2000x1000 image. Is it fixed and not released
yet, or the fix is not working?

Yes, for ordinary GIFs to render properly somebody needs to activate it on Wikimedia.

I'm reopening this bug.

Essentially, there's some configuration at Wikimedia that says that GIFs should not be scaled at all. Somebody needs to remove it.

Marking with shell keyword.

(In reply to comment #32)

I'm reopening this bug.

Essentially, there's some configuration at Wikimedia that says that GIFs should
not be scaled at all. Somebody needs to remove it.

It's the lines:

// Disable server-side GIF scaling
$wgMediaHandlers['image/gif'] = 'BitmapHandler_ClientOnly';

in CommonSettings.php which need to be removed.

lar wrote:

Do you need another but against a different component entered in order to get this enabled at Commons or is this bug sufficient? What about at other wikis (in case they want it too?). Thanks!

The Commons thread says that system administrators can make the change in CommonSettings.php which is linked from here:
http://noc.wikimedia.org/conf

http://meta.wikimedia.org/wiki/System_administrators#List says "Do not contact random people on this list if you want something done. Instead, go to the [[irc:wikimedia-tech|#wikimedia-tech]] channel on irc.freenode.net."

I see the part that needs to be removed:

// Disable server-side GIF scaling <br>
$wgMediaHandlers['image/gif'] = 'BitmapHandler_ClientOnly';

It is found here:
http://noc.wikimedia.org/conf/highlight.php?file=CommonSettings.php

I believe this is FIXED in MediaWiki now, and there's a dedicated request ( bug 20312 - Enable GIF thumbnail scaling on Wikimedia websites) to enable GIF scaling on Wikimedia websites.

Removing shell keyword and closing accordingly.

The fix was reverted, follow up on bug 22041

Gilles moved this task from Untriaged to Done on the Multimedia board.Dec 4 2014, 10:48 AM

Add Comment