Created attachment 518356 [details] Screenshot A Debian user reported a problem with beta 12 that he verified to happen with Mozilla builds. He mentioned his CPU doesn't support SSE2, and that ogv rendering was working fine with older versions. In fact, he narrowed down to a day it broke using nightlies: The last working nightly is: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/12/2010-12-15-03-mozilla-central/firefox-4.0b9pre.en-US.linux-i686.tar.bz2 and the first broken one is: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/12/2010-12-16-03-mozilla-central/firefox-4.0b9pre.en-US.linux-i686.tar.bz2
So that would be somewhere between f11f7ed625ba and a5413c3c1013
This could be related to bug 616469.
I could reproduce by forcing my X server to use a 16-bits display instead of 24.
Summary: Bad rendering of webm and ogv under some circumstances on Linux → Bad rendering of webm and ogv with 16-bits displays on Linux
The GTK reimplementation of GetOffscreenSurface will return ImageFormatRGB16_565 here: http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatformGtk.cpp#511 Then we'll hit this path: http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicImages.cpp#206 And fire the assertion because have_ycbcr_to_rgb565() is only true if you have ARM NEON. Since the assertion is non-fatal, we'll call ConvertYCbCrToRGB565 anyway, which does nothing unless HAVE_YCBCR_TO_RGB565 is defined (which is conditional on HAVE_ARM_NEON).
Technically that's not true, because above at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/basic/BasicImages.cpp#155 we change the format to ImageFormatRGB24, since RGB565 isn't supported. However, "format" is not "mOffscreenFormat", and so the eventual call to BasicPlanarYCbCrImage::GetAsSurface() will get data in a format that it is not at all expecting. It seems like we should be calling SetOffscreenFormat() if we change the format. I don't know if that's guaranteed to be correct in all cases, but I've already tested it as part of my upcoming patch for bug 634557 and it appears to work.
Ah yeah, silly me, sorry.
This also happens on Maemo.
Really? It should only happen in Maemo if NEON detection fails. Or are you testing on something other than an N900?
Not testing on n900.
Created attachment 518864 [details] [diff] [review] Fix This one line change is enough to fix this for me.
Assignee: nobody → tterribe
Status: NEW → ASSIGNED
Attachment #518864 - Flags: review+
Attachment #518864 - Flags: approval2.0?
Comment on attachment 518864 [details] [diff] [review] Fix I should point out, for the purposes of evaluating approval, that this also fixes the crash in bug 641019 for systems without NEON.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Can someone please point me to a test video I can use to verify this bug is fixed?
Any video at all should do, for example the one linked in bug 641019: http://people.xiph.org/~tterribe/theora/coastguard_335-V500.ogv
(In reply to comment #14) > Any video at all should do, for example the one linked in bug 641019: > http://people.xiph.org/~tterribe/theora/coastguard_335-V500.ogv Thanks -- works for me using Firefox 4.0rc1. However, I'm using a fairly modern machine. Perhaps someone could confirm this video working on a mobile device or computer which exhibited this bug before it was fixed.
1) I don't think this patch is in RC1 (it landed on m-c, but not on mozilla-2.0), so you'd probably have to use a recent nightly to confirm (I assume nightlies are still being built from m-c? I don't actually know). 2) You'd have to set your display to 16 bits. You know you're testing the right code path if RC1 or a nightly prior to 2011-03-15 crashes on the above URL.
I can confirm that rc1+patch works properly on 16-bits display.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.