SurfaceCaps assertion failure in GLContext::UpdatePixelFormat() when playing Flash video

RESOLVED FIXED in Firefox 23

Status

()

Firefox for Android
Plugins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

({regression})

unspecified
Firefox 23
All
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox22 wontfix, firefox23 fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
When playing the following embedded YouTube video on my Nexus 4, I hit an GL assertion failure because GLContext's SurfaceCaps don't match the PixelBufferFormat. I believe this is a regression from bug 716859.

http://www.qiwireless.com/nokia-md-51w-jbl-playup-unboxing-and-demo-video/


F/MOZ_Assert( 5455): Assertion failure: caps.color == !!format.red, at gfx/gl/GLContext.cpp:877
F/libc    ( 5455): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1), thread 5473 (FP_DoPlay)


(gdb) bt
#0  0x7a652408 in mozilla::gl::GLContext::UpdatePixelFormat (this=0x78e9e400) at gfx/gl/GLContext.cpp:878
#1  0x7a65f614 in mozilla::gl::GLContext::InitOffscreen (this=0x78e9e400, size=..., caps=...) at gfx/gl/GLContext.h:1201
#2  0x7a65ffb2 in mozilla::gl::GLContextProviderEGL::CreateOffscreen (size=..., caps=..., flags=<optimized out>) at gfx/gl/GLContextProviderEGL.cpp:2361
#3  0x7a1594f2 in EnsureGLContext () at dom/plugins/base/nsNPAPIPluginInstance.cpp:87
#4  0x7a15b1ac in nsNPAPIPluginInstance::CreateSurfaceTexture (this=0x78257980) at dom/plugins/base/nsNPAPIPluginInstance.cpp:976
#5  0x7a15b246 in nsNPAPIPluginInstance::AcquireContentWindow (this=0x78257980) at dom/plugins/base/nsNPAPIPluginInstance.cpp:1001
#6  0x81ef9122 in ?? ()


(gdb) print this->mCaps
$4 = {any = false, color = false, alpha = false, bpp16 = false, depth = false, stencil = false, antialias = false, preserve = false}

(gdb) print format
$6 = {red = 8, green = 8, blue = 8, alpha = 0, depth = 0, stencil = 0, samples = 1}
(Assignee)

Comment 1

5 years ago
Created attachment 739315 [details] [diff] [review]
RGBA-dummyCaps.patch

Use RGBA SurfaceCaps to avoid GLContext::UpdatePixelFormat() assertion failure when playing Flash video on Android. In bug 827407 comment 20, snorp suggested initializing dummyCaps with SurfaceCaps::ForRGBA().
Attachment #739315 - Flags: review?(jgilbert)
I think that for this case, we should be asking for SurfaceCaps::Any(), as we're not actually interested in using the offscreen buffer. We only want a GLContext to use, nothing more.
Comment on attachment 739315 [details] [diff] [review]
RGBA-dummyCaps.patch

Review of attachment 739315 [details] [diff] [review]:
-----------------------------------------------------------------

This problem should go away more properly if we use `SurfaceCaps::Any()`.
Attachment #739315 - Flags: review?(jgilbert) → review-
(Assignee)

Comment 4

5 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> I think that for this case, we should be asking for SurfaceCaps::Any(), as
> we're not actually interested in using the offscreen buffer. We only want a
> GLContext to use, nothing more.

With SurfaceCaps::Any(), the GLContext::UpdatePixelFormat() assertions still fail. We're comparing SurfaceCaps::Any() to the QueryPixelFormat() of the offscreen GLContext. Perhaps the assertions in UpdatePixelFormat() are themselves incorrect?

https://hg.mozilla.org/mozilla-central/annotate/d8202613aaea/gfx/gl/GLContext.cpp#l872

(gdb) bt
#0  0x7a8fcd70 in mozilla::gl::GLContext::UpdatePixelFormat (this=0x815f9c00) at central/gfx/gl/GLContext.cpp:873
#1  0x7a908fcc in mozilla::gl::GLContext::InitOffscreen (this=0x815f9c00, size=..., caps=...) at central/gfx/gl/GLContext.h:1199
#2  0x7a90983e in mozilla::gl::GLContextProviderEGL::CreateOffscreen (size=..., caps=..., flags=<optimized out>) at central/gfx/gl/GLContextProviderEGL.cpp:2008
#3  0x7a49e1b2 in EnsureGLContext () at central/dom/plugins/base/nsNPAPIPluginInstance.cpp:87
#4  0x7a49fd28 in nsNPAPIPluginInstance::CreateSurfaceTexture (this=0x7f2663e0) at central/dom/plugins/base/nsNPAPIPluginInstance.cpp:976
#5  0x7a49fdc2 in nsNPAPIPluginInstance::AcquireContentWindow (this=0x7f2663e0) at central/dom/plugins/base/nsNPAPIPluginInstance.cpp:1001
#6  0x82639122 in ?? ()

(gdb) print format
$3 = {red = 8, green = 8, blue = 8, alpha = 0, depth = 0, stencil = 0, samples = 1}

(gdb) print mCaps
$4 = {any = true, color = false, alpha = false, bpp16 = false, depth = false, stencil = false, antialias = false, preserve = false}
Created attachment 741114 [details] [diff] [review]
Use SurfaceCaps::Any, and make offscreen GLContexts DetermineCaps().

I think this is what we want. Does this work?
Attachment #739315 - Attachment is obsolete: true
Attachment #741114 - Flags: review?(cpeterson)
(Assignee)

Comment 6

5 years ago
Comment on attachment 741114 [details] [diff] [review]
Use SurfaceCaps::Any, and make offscreen GLContexts DetermineCaps().

Review of attachment 741114 [details] [diff] [review]:
-----------------------------------------------------------------

I still hit MOZ_ASSERT(caps.color == !!format.red) because format.red == 8, but gdb can't see much more because I'm debugging an optimized debug build. I'll need to debug this in an unoptimized build for more info.
Attachment #741114 - Flags: review?(cpeterson) → review-
(In reply to Chris Peterson (:cpeterson) from comment #6)
> Comment on attachment 741114 [details] [diff] [review]
> Use SurfaceCaps::Any, and make offscreen GLContexts DetermineCaps().
> 
> Review of attachment 741114 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I still hit MOZ_ASSERT(caps.color == !!format.red) because format.red == 8,
> but gdb can't see much more because I'm debugging an optimized debug build.
> I'll need to debug this in an unoptimized build for more info.

This should not be possible with the patch. DetermineCaps and UpdatePixelFormat both use QueryPixelFormat. Please step through DetermineCaps to assure that the `format` there matches the `format` we later check ourselves against in UpdatePixelFormat.
(Assignee)

Comment 8

5 years ago
Comment on attachment 741114 [details] [diff] [review]
Use SurfaceCaps::Any, and make offscreen GLContexts DetermineCaps().

Review of attachment 741114 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Your patch works for me this morning. I must have been debugging the wrong build yesterday.
Attachment #741114 - Flags: review- → review+
(Assignee)

Comment 10

5 years ago
Jeff, I can land your patch tomorrow. The try builds look good.
(In reply to Chris Peterson (:cpeterson) from comment #10)
> Jeff, I can land your patch tomorrow. The try builds look good.

That will work fine.
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad5badac802f
status-firefox21: ? → ---
status-firefox22: affected → wontfix
status-firefox23: affected → fixed
https://hg.mozilla.org/mozilla-central/rev/ad5badac802f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
You need to log in before you can comment on or make changes to this bug.