929009 - crash in `anonymous namespace''::downmix_to_stereo<float>(float*, long, float*, int)

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[u279076]

This bug was filed from the Socorro interface and is 
report bp-cd256988-392b-4c12-8ef7-08ca72131021.
=============================================================
0 	gkmedias.dll 	`anonymous namespace'::downmix_to_stereo<float>(float *,long,float *,int) 	media/libcubeb/src/cubeb_wasapi.cpp
1 	gkmedias.dll 	`anonymous namespace'::refill(cubeb_stream *,float *,long) 	media/libcubeb/src/cubeb_wasapi.cpp
2 	gkmedias.dll 	`anonymous namespace'::wasapi_stream_render_loop 	media/libcubeb/src/cubeb_wasapi.cpp
3 	msvcr100.dll 	_callthreadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c
4 	msvcr100.dll 	_threadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c
5 	kernel32.dll 	BaseThreadInitThunk 	
6 	ntdll.dll 	__RtlUserThreadStart 	
7 	ntdll.dll 	_RtlUserThreadStart 	
=============================================================
More reports:
https://crash-stats.mozilla.com/report/list?signature=%60anonymous%20namespace%27%27%3A%3Adownmix_to_stereo%3Cfloat%3E%28float*%2C%20long%2C%20float*%2C%20int%29

Crash is explosive and first appeared with Firefox 25.0b8, currently the #7 top crash in Firefox 25.

Comment 1

[u279076]

Looking at the changelog for 25b8 the only change I see related to "stereo was bug 899050. Paul, is it possible your patch caused this topcrash?

Comment 2

[Paul Adenot (:padenot)]

Yeah, It is certain this patch is causing this crash. I'm going to back it out and disable WASAPI for this cycle.

Comment 4

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Paul Adenot (:padenot) from comment #2)
> Yeah, It is certain this patch is causing this crash. I'm going to back it
> out and disable WASAPI for this cycle.

Paul, we're building the last FF25 beta today, how soon can you backout? We could maybe take a re-landing attempt on Aurora if low-risk and well tested within the week.

Comment 5

[Alex Keybl [:akeybl]]

For future reference:

1:40 PM <akeybl> is that safer than going for another forward fix?
1:40 PM <akeybl> even though WASAPI has been enabled for the majority of this pre-release?
1:40 PM <padenot> WASAPI is vista+ only
1:40 PM <padenot> so it has been tested during pre-release as well

Paul's comment 2 is our best path forward.

Comment 6

[Paul Adenot (:padenot)]

Attachment: disable-wasapi

Comment 7

[Paul Adenot (:padenot)]

https://hg.mozilla.org/releases/mozilla-beta/rev/627dda566b79

Comment 8

[u279076]

It might be too early to make any final calls on this but looking at the data this was at #4 in 25b9 and does not show up in 25b10. I'll continue to monitor.

Comment 9

[Paul Adenot (:padenot)]

Well, I can tell you, the code has been disabled in b10.

Comment 10

[u279076]

It's been a few days and I'm not seeing any instances of this crash reported beyond Firefox 25.0b9. Calling this verified fixed for Firefox 25.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Paul - any chance of a forward fix coming in the next week or two for FF26 or are we looking at backing this out again?

Comment 12

[Robert Kaiser]

this should actually be marked disabled for 25 as what was done was disabling the feature that crashes.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Matthew, can you take this?

Comment 14

[Matthew Gregan [:kinetik]]

I spent a while trying to debug this via code inspection yesterday, but no luck so far.  Having a way to reproduce it would be helpful, although it's possible that it's specific to the audio configuration on the host.

Comment 15

[Matthew Gregan [:kinetik]]

Attachment: bug929009_v0.patch

No great ideas yet.  libcubeb uses fatal asserts in all builds to aid in catching bugs, but the requisite #undef NDEBUG is missing from the WASAPI backend.  This patch adds that, plus a couple of extra asserts and also fixes a typo in a function name and removes an unused field.  I'll land this on trunk and keep an eye on incoming crash reports while working on other options.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=b1b06f79d14b

Comment 16

[Matthew Gregan [:kinetik]]

https://tbpl.mozilla.org/?tree=Try&rev=2d155430b1b4

Comment 17

[Matthew Gregan [:kinetik]]

Attachment: bug929009_v1.patch

Build fix.

Comment 18

[Matthew Gregan [:kinetik]]

Attachment: bug929009_mono_v0.patch

Chromium SVN cset 156851 implies the device can be mono, but we seem to assume the minimum device configuration is at least stereo (which is supported by random Googling, but nothing concrete either way).

So we might need a patch like this.  Try: https://tbpl.mozilla.org/?tree=Try&rev=91b5b7a3b2fe

Comment 19

[Tracy Walker [:tracy]]

(In reply to Matthew Gregan [:kinetik] from comment #14)
> I spent a while trying to debug this via code inspection yesterday, but no
> luck so far.  Having a way to reproduce it would be helpful, although it's
> possible that it's specific to the audio configuration on the host.

Total Count 	URL
376 	https://www.facebook.com/
104 	http://battlelog.battlefield.com/bf4/
52 	http://battlelog.battlefield.com/bf4/servers/
33 	https://www.facebook.com/?ref=tn_tnmn
30 	http://battlelog.battlefield.com/bf3/
29 	https://plus.google.com/hangouts/_/pre?authuser=0
29 	https://player.vimeo.com/v2/receiver.html
27 	http://battlelog.battlefield.com/bf4/de/

followed by a mix of mostly facebook apps and battlelog.battlefield.com urls

Comment 20

[Matthew Gregan [:kinetik]]

Attachment: bug929009_mono_v1.patch

Comment 21

[Matthew Gregan [:kinetik]]

Comment on attachment 830530 [details] [diff] [review]
bug929009_v1.patch

Paul's back, so I'll get him to review.

Comment 22

[Matthew Gregan [:kinetik]]

Comment on attachment 831294 [details] [diff] [review]
bug929009_mono_v1.patch

https://tbpl.mozilla.org/?tree=Try&rev=d2592f66b270

Comment 24

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This reproduces 100% on a Friend's computer -- I'm going to get him to try the try build.  Is there any info that I can get by actually debugging his crash that would help?

Comment 25

[Paul Adenot (:padenot)]

Vlad, if we could have his setup: sample rate of the sound card, number of output channel, website on which he can repro, that would be very helpful. Or maybe kinetik's patch fixes it.

Comment 26

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e6ac3f67e79

Comment 27

[Matthew Gregan [:kinetik]]

Comment on attachment 830530 [details] [diff] [review]
bug929009_v1.patch

This patch is contained in the mono version, so obsoleting.

Comment 28

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1e6ac3f67e79

Comment 29

[Paul Adenot (:padenot)]

(Forgot to needinfo back in comment 25).

Comment 30

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ok; I'm asking him to try the tryserver build to see if he still crashes.  (It's 100% reproducible for him.)  I don't know how to easily get the other info; it's just generic onboard sound I think, is there anywhere in Windows that would display the info you need?

Comment 31

[Lukas Blakk [:lsblakk] use ?needinfo]

It looks like it's time in FF26 to look at disabling this again since it's at #6 in Beta and we don't want to explode in Release.

Comment 32

[Matthew Gregan [:kinetik]]

I'll land Paul's patch on beta as soon as the tree reopens.

Comment 33

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/releases/mozilla-beta/rev/502fdc1bba1a

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/502fdc1bba1a

Comment 35

[Matthew Gregan [:kinetik]]

Was your friend able to test the try (or nightly) builds yet?

The audio section of dxdiag *might* be useful, I'm not sure how else to get any useful audio hardware/configuration specific info from Windows without writing a new tool.

Comment 36

[Matthew Gregan [:kinetik]]

downmix_to_stereo was renamed to downmix in the patch landed on m-c.  I'm not seeing new crashes showing up in this area under top crashes (or by searching for occurrences of downmix in the call stack).

Comment 37

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Whoops, forgot to post results.  Yes, he can no longer reproduce with the tryserver build, so the patch looks good.  I'd uplift to Aurora IMO, it's a pretty huge papercut.

Comment 38

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: dxdiag from crashing system

here's dxdiag.  There looks to be useful info in the sound device section.

Comment 39

[Matthew Gregan [:kinetik]]

Comment on attachment 831294 [details] [diff] [review]
bug929009_mono_v1.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 899050
User impact if declined: crash playing audio on some Windows Vista or newer systems
Testing completed (on m-c, etc.): baked on m-c, fix verified by Vlad
Risk to taking this patch (and alternatives if risky): low, backup plan is to take the patch to disable the WASAPI backend that we've already landed on beta
String or IDL/UUID changes made by this patch: none

Is it worth considering this for beta too, or is it too late?

Comment 40

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

The crash isn't present on beta after the disabling, right? What's the downside to just leaving the disabling?

Comment 41

[Paul Adenot (:padenot)]

So, there is no user impact to let it disabled on Beta. winmm is perfectly fine to play high latency stream.

WASAPI exists solely to be able to play low-latency audio (for games, voip and the like). All the nice latency optimizations and tuning are currently in Aurora (not beta), iirc, and require WASAPI to be enabled to benefit from them. Without those latency optimization patches, winmm and WASAPI are equivalent.

Comment 42

[Matthew Gregan [:kinetik]]

Actually, there isn't any really.  It only starts to matter with bug 907817/bug 918861 for Web Audio and WebRTC, but those are only in Aurora.

Comment 43

[Matthew Gregan [:kinetik]]

Resolving based on comment 37.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/02294155afa9

Comment 45

[Tracy Walker [:tracy]]

Verified on 26 and 28 per crash reports no longer showing up after respective checkin dates. Will check crash stats when there is enough recent data to verify on 27.

Comment 46

[Tracy Walker [:tracy]]

no reports of this on builds after the fix was landed on Fx27