1062277 - Crash in mozilla::WMFVideoMFTManager::Init when hw acceleration is enabled

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

Attachment: stack trace

RyanVM reported this one on irc, I think he can repro 100% of the time on his  windows 8.1 machine.

The stack is attached, it crashes when we do:

> hr = decoder->SendMFTMessage(MFT_MESSAGE_SET_D3D_MANAGER, manager);

during the initialization.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Yes, 100% reproducible.

Graphics
--------

Adapter Description: Intel(R) HD Graphics 4000
Adapter Description (GPU #2): NVIDIA Quadro K1000M
Adapter Drivers: igdumdim64 igd10iumd64 igd10iumd64 igdumdim32 igd10iumd32 igd10iumd32
Adapter Drivers (GPU #2): nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM: Unknown
Adapter RAM (GPU #2): 2048
Device ID: 0x0166
Device ID (GPU #2): 0x0ffc
Direct2D Enabled: true
DirectWrite Enabled: true (6.3.9600.17111)
Driver Date: 5-17-2014
Driver Date (GPU #2): 8-4-2014
Driver Version: 10.18.10.3621
Driver Version (GPU #2): 9.18.13.4066
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 21f517aa
Subsys ID (GPU #2): 21f517aa
Vendor ID: 0x8086
Vendor ID (GPU #2): 0x10de
WebGL Renderer: Google Inc. -- ANGLE (Intel(R) HD Graphics 4000 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d
AzureContentBackend: direct2d
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Disabling DXVA does make the crash go away.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

And this is a home-brew Win64 build, if that matters. I haven't attempted to reproduce with TBPL builds.

Comment 4

[Rob North]

This is also happening to me on 64bit nightly builds.

See for instance bug report:
    bp-b6ca7697-6eef-4d44-a899-5e96b2140914.

Also it is clear *what* the bug is:
    The function takes SendMFTMessage() takes a second parameter of 32bits (UINT32)
   , but is on x86-64 passed a 64 bit pointer. 
    The Windows API function IMFTransform::ProcessMessage() 
    take a ULONG_PTR, which is also 64bits, on x86-64.

   Fix would be to change type to whatever Firefox uses to store pointers...
   uintptr_t, or ULONG_PTR would both be good candidates.

Comment 5

[Rob North]

And here's a new signature from today's nightly.
    bp-2e7d1444-b401-435d-9a36-d12c02140914

Comment 6

[Rob North]

Moving severity to critical, as can prevent a x86-64 nightly from starting.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Chris, does the analysis in comment 4 look right to you?

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #7)
> Chris, does the analysis in comment 4 look right to you?

Yes, that seems a good bet. There will be a test build with that fixed here in a couple of hours:
https://tbpl.mozilla.org/?tree=Try&rev=ac9eb4b2f299

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Works nicely!

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch

Use ULONG_PTR instead of UINT32, fix 64bit builds.

We'll need this on Aurora too.

Comment 11

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

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

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

ouch.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7be93a252737

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7be93a252737

Comment 14

[Ryan VanderMeulen [:RyanVM]]

(In reply to Rob North from comment #6)
> Moving severity to critical, as can prevent a x86-64 nightly from starting.

Thanks for the pointer, Rob! While we technically don't support Win64 builds off the release branches, it's probably still not a bad idea to get this on Aurora since it's a nice correctness fix anyway.

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

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

Approval Request Comment
[Feature/regressing bug #]: bug 1057879, switch to new MP4 demuxer on Windows
[User impact if declined]: Users in 64bit Nightly builds may crash playing video
[Describe test coverage new/current, TBPL]: Been verified. No 64bit test coverage, but 32bit Win builds all pass existing video mochitests
[Risks and why]: Low risk; tests pass.
[String/UUID change made/needed]: None.

Comment 16

[Sladky Vladimir]

Canot test, but latest build canot start (crashes before SM start) and in safe mode too :-(

https://crash-stats.mozilla.com/report/index/228c73f3-421d-44bc-89ad-085b92140917

Comment 17

[Ryan VanderMeulen [:RyanVM]]

(In reply to Sladky Vladimir from comment #16)
> Canot test, but latest build canot start (crashes before SM start) and in
> safe mode too :-(
> 
> https://crash-stats.mozilla.com/report/index/228c73f3-421d-44bc-89ad-
> 085b92140917

Not sure what the story is with that given the lack of symbols in that report. But I can confirm that m-c tip is playing videos fine for me again with dxva enabled.

Comment 18

[Sladky Vladimir]

Build  ID:20140917135317 CSet: 641d450532be is now O.K. and with DXVA enabled too not crashed.

Comment 19

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Chris Pearce (:cpearce) from comment #15)
> Comment on attachment 8489772 [details] [diff] [review]
> Patch
> 
> Approval Request Comment
> [Feature/regressing bug #]: bug 1057879, switch to new MP4 demuxer on Windows
> [User impact if declined]: Users in 64bit Nightly builds may crash playing
> video

Ryan said in comment 14 that this is a correctness fix but does this have any impact on 32bit Windows users?

Comment 20

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Lawrence Mandel [:lmandel] from comment #19)
> (In reply to Chris Pearce (:cpearce) from comment #15)
> > Comment on attachment 8489772 [details] [diff] [review]
> > Patch
> > 
> > Approval Request Comment
> > [Feature/regressing bug #]: bug 1057879, switch to new MP4 demuxer on Windows
> > [User impact if declined]: Users in 64bit Nightly builds may crash playing
> > video
> 
> Ryan said in comment 14 that this is a correctness fix but does this have
> any impact on 32bit Windows users?

This does not affect 32bit Windows, the crash is only in 64bit Windows, so will only affect Nightly builds there. A large proportion of our Nightly users run 64bit builds apparently. Bug was introduced in Firefox 34.

Comment 21

[Lawrence Mandel [:lmandel] (use needinfo)]

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

Given comment 20, even though this is a small patch, there is no need to uplift. Aurora-

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Not sure how much we care, but we'll also break PaleMoon. They ship 64bit releases.