791731 - Animations on apple.com are not smooth, spend a lot of CPU time in the JPEG decoder

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Daniel]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20120914030538

Steps to reproduce:

1. Visit http://www.apple.com/macbook-pro/features/#seq
2. Watch animation of lid opening


Actual results:

The animation flickers


Expected results:

The animation should be smooth

The animation looks good in current stable (Firefox 15), and also in Chrome and in IE, but not in Aurora or Nightly.

This bug is quite possibly related to Bug 683290 and perhaps Bug 685516.

I also came across this bug while "streaming" video from an IP-camera. A jpeg image from the camera was loaded onto the page. I then loaded a new image in the background and on onload I switched the old image src out for the just loaded. Then the next frame started loading in the background. The point is this simple solution works great in 15 (and Chrome etc.) but the video flickers a lot in Nightly.

Comment 1

[Timothy Nikkel (:tnikkel)]

Yeah, this regressed when those bugs landed.

Comment 3

[Justin Lebar (not reading bugmail)]

On my powerful Linux x64 desktop machine, the animation in comment 0 is smooth, but the following STR shows the same flickering effect:

* http://www.apple.com/ipad/features/
* Scroll down to the smart cover section, second from the bottom.
* "Unwrap" and "wrap" the cover by dragging your mouse.

Profiling with perf shows that we're spending all our time in the JPEG decoder, so this is very likely a discarding issue.

See also https://docs.google.com/document/pub?id=1GWTMLjqQsQS45FWwqNG9ztQTdGF48hQYpjQHR_d1WsI for a description of what Apple is doing here, which is pretty rad.

Comment 4

[Justin Lebar (not reading bugmail)]

Like I asked in bug 791779, I don't get why we're discarding these images immediately.  Shouldn't we hold on to them until the discard timer fires?

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Yeah, we shouldn't discard immediately.  Part 1 of Bug 784591 gives us the option, and then we'll just need a small patch here to flip the switch for this case.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Don't RequestDiscard from UnbindFromTree.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1797401e19ce

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1797401e19ce

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 667032 [details] [diff] [review]
Patch

We should take this patch on aurora too.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 683290
User impact if declined: Increased flickering and CPU usage when switching between certain websites.
Testing completed (on m-c, etc.): Landed on m-c.
Risk to taking this patch (and alternatives if risky): Low risk
String or UUID changes made by this patch: None.

Comment 10

[Jim Jeffery not reading bug-mail 1/2/11]

Still flickers here using latest hourly m-c win32 build on win7 x64

https://hg.mozilla.org/mozilla-central/rev/5ac283a12f02

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Did if flicker for you before Bug 683290 landed?  I don't see any flicker here.

Comment 12

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #11)
> Did if flicker for you before Bug 683290 landed?  I don't see any flicker
> here.

Yes it does, I see no difference between builds without the patch and a build with.

Could be just my crappy video card I guess.

Comment 13

[Alex Keybl [:akeybl]]

Comment on attachment 667032 [details] [diff] [review]
Patch

[Triage Comment]
Low risk fix for a FF17 regression. Approving for Aurora.

Comment 14

[Justin Wood (:Callek)]

No luck transplanting this without conflicts:

Justin@ORION /d/sources/mozilla-aurora
$ hg transplant -s http://hg.mozilla.org/mozilla-central 1797401e19ce
searching for changes
applying 1797401e19ce
patching file content/base/src/nsImageLoadingContent.cpp
Hunk #1 FAILED at 1182
1 out of 1 hunks FAILED -- saving rejects to file content/base/src/nsImageLoadingContent.cpp.rej
patch failed to apply
abort: Fix up the merge and run hg transplant --continue

Comment 15

[Justin Lebar (not reading bugmail)]

(In reply to Alex Keybl [:akeybl] from comment #13)
> Comment on attachment 667032 [details] [diff] [review]
> Patch
> 
> [Triage Comment]
> Low risk fix for a FF17 regression. Approving for Aurora.

Kyle, did you forget about landing this on Aurora?  I can merge this for you if you'd like.

Comment 16

[Justin Lebar (not reading bugmail)]

Oh, merge day is tomorrow!  That's why we're doing this.

Anyway, the transplant here fails with conflicts on the one hunk in the patch.  And the conflict is that the existing code matches the patched code.

Comment 17

[Justin Lebar (not reading bugmail)]

Aha.  I needed bug 784519 first.

Fixed in Aurora for FF17: https://hg.mozilla.org/releases/mozilla-aurora/rev/8aac40fc9890

Comment 19

[Manuela Muntean [Away]]

I couldn't reproduce this issue, after trying the STR from comment 3. The "unwrap" and "wrap"  by dragging the mouse works smooth for me.

Nightly 2012-08-10
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20120810030512

Comment 20

[Manuela Muntean [Away]]

Any suggestions?

Comment 21

[Justin Lebar (not reading bugmail)]

(In reply to Manuela Muntean from comment #20)
> Any suggestions?

It's possible that Apple's website has changed since then.  Also, I don't know why you're testing with that particular Nightly, since it looks like this bug was reproduced with nightlies from one month after.

Comment 22

[Manuela Muntean [Away]]

(In reply to Justin Lebar [:jlebar] from comment #21)
> (In reply to Manuela Muntean from comment #20)
> > Any suggestions?
> 
> It's possible that Apple's website has changed since then.  Also, I don't
> know why you're testing with that particular Nightly, since it looks like
> this bug was reproduced with nightlies from one month after.

I've tried the STR from comment 3 on another Nightly, but I still can't reproduce the issue. Everything works smoothly.

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20120905030555