1234023 - certain animated gif becomes gray after within 10sec

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect, P3)

Description

[Alice0775 White]

It seems calculation of memory usage is too safe side.

Reproducible: always

Steps to reproduce:
1. Clear cache
2. Open URL http://www.squarefree.com/bug523950/523950.gif
3. Wait for 10sec

Actual results:
Becomes gray after within 10sec

Expected results:
Animated properly (but memory usage is approx. 2.5GB)

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7a63a3b1bd61&tochange=914258721d1f

Triggered by:
914258721d1f	Seth Fowler — Bug 1155332 - If we don't have enough memory to fully decode an image, discard it immediately. r=tn
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=04a3f1ef4fbb&tochange=bbe25ecbebda

Comment 1

[Roman]

This happens on Firefox 64-bit, with plenty of spare RAM available (about 8GB unused). It's nowhere close to actually running out of memory when this happens.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Bas, did you intend to NI Seth?

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Jet, can you help get movement here?

Comment 4

[Jet Villegas (inactive)]

(In reply to Alice0775 White from comment #0)
> Expected results:
> Animated properly (but memory usage is approx. 2.5GB)

Really? I don't agree that allowing any GIF to bloat to 2.5GB is a good user experience. This behavior certainly led to OOM crashes in the past that we've fixed in bug 1155332. Before that bug was fixed, the GIF decoder was already returning memory errors that we'd ignore and destabilize Firefox later in the session.

Comment 5

[Seth Fowler [:seth] [:s2h]]

Yeah, we will not fix this by increasing the memory limit for decoded images.

The ongoing work to decode animated images in the media framework (which means that we only need to allocate memory for a few frames, not every frame in the image) should fix this. I'm blocking this bug appropriately. (I'm forced to remove the block on bug 1155332, because it would create a circular dependency.)

Comment 6

[Johnny Stenback (:jst)]

Removing the regression keyword because this was due to an intentional change, not an unintended regression.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

We put a hard maximum of 4GB (as in, 32-bit pointer), regardless of the OS.

Comment 8

[Kuba Niewiarowski]

Cannot reproduce, is it resolved?

Comment 9

[Alice0775 White]

no longer reproduced on Nightly67.0a1 Windows10