Closed Bug 950026 Opened 6 years ago Closed 6 years ago

Bail out safely when we get an unexpected value from AudioClient::IsFormatSupported

Categories

(Core :: Audio/Video, defect, critical)

27 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla29
Tracking Status
firefox27 + fixed
firefox28 + fixed
firefox29 --- fixed

People

(Reporter: padenot, Assigned: padenot)

Details

(Keywords: crash, topcrash-win, Whiteboard: [qa-])

Crash Data

Attachments

(1 file)

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> roc, are you talking about bug 833161? That appears to be a linux-only bug,
> and this is Windows, so it's not clear to me that it is the same.
> 
> cpearce the assert location is well-known in this case:
> http://hg.mozilla.org/releases/mozilla-aurora/annotate/731470a7c4ca/media/
> libcubeb/src/cubeb_wasapi.cpp#l665
> 
> See e.g. the stack at
> https://crash-stats.mozilla.com/report/index/83dcae38-7539-41ea-a342-
> c10902131204 or
> https://crash-stats.mozilla.com/report/index/a4999c2b-0cca-4f0d-ab1f-
> 7ffe02131205

----

(In reply to Matthew Gregan [:kinetik] from comment #6)
> Crashes in cubeb_wasapi need a different bug, this one has always been about
> cubeb_winmm code.
This will fall back to our own channel mapping code, instead of trying to use
the platform's.
Attachment #8347305 - Flags: review?(kinetik)
Summary: Crash due to fatal assert in cubeb_refill_stream → Bail out safely when we get an unexpected value from AudioClient::IsFormatSupported
Attachment #8347305 - Flags: review?(kinetik) → review+
https://hg.mozilla.org/mozilla-central/rev/5c1ca8f7cf88
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Worth to nominate it for uplifting?
Comment on attachment 8347305 [details] [diff] [review]
Bail out when we don't get an expected return value from AudioClient::IsFormatSupported, instead of asserting. r=

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 866675 (compiled in from firefox 27)
User impact if declined: Crashes on an assert (even in opt/non-debug) on some specific sound hardware/OS configuration.
Testing completed (on m-c, etc.): Baked for a while on m-c.
Risk to taking this patch (and alternatives if risky): Not risky, we just fallback to some other part of the code that is used a lot instead of trying to rely on system facilities.
String or IDL/UUID changes made by this patch: none.
Attachment #8347305 - Flags: approval-mozilla-beta?
Attachment #8347305 - Flags: approval-mozilla-aurora?
Attachment #8347305 - Flags: approval-mozilla-beta?
Attachment #8347305 - Flags: approval-mozilla-beta+
Attachment #8347305 - Flags: approval-mozilla-aurora?
Attachment #8347305 - Flags: approval-mozilla-aurora+
Do we have tests to ensure this is working correctly?
Flags: in-testsuite?
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #7)
> Do we have tests to ensure this is working correctly?

The test would require specific hardware/driver combination, I'm afraid. But since we were crashing on our own assertion, and because we already had a fallback path for this exact case, I'm confident with uplifting/shipping this.
Marking in-testsuite- based on comment 8. Please let me know if there's something QA can do manually to raise confidence that the fallback mechanism is working correctly.
Flags: in-testsuite? → in-testsuite-
Whiteboard: [qa-]
I do not believe this is fixed,

I can crash 26, 27, 28 and 29 by simply  opening https://vine.co/v/ManB7AzZjtP

The issue also occurs with tumblr video's.
Issue is confirmed NOT resolved with Creative Soundblaster and Realtek hardware when vendor specific enhancements are enabled.

Crash can be reproduced 100% until "Disable [Product name] Enhancements" is ticked in Speaker device properties

I expect the enhancement modules provided with Recon and Z(x/R) series soundblaster cards will exhibit the same issue.
(In reply to Danial Horton from comment #10)
> I do not believe this is fixed,
> 
> I can crash 26, 27, 28 and 29 by simply  opening
> https://vine.co/v/ManB7AzZjtP
> 
> The issue also occurs with tumblr video's.

Note that the fix in this bug is only present in Firefox 27 onwards, so it is expected that 26 would crash if you were hitting the same bug.

However, looking at the stacks, you're hitting a different assert and thus have a different bug.  I'll file a new bug and CC you.
(In reply to Matthew Gregan [:kinetik] from comment #13)
> I'll file a new bug and CC you.

Bug 970147.
Matthew, my apologies. Opened up bug# 970519, and closed it (incorrectly) as a duplicate of bug# 791112. Reading the latter more carefully, I understand that my issue is more likely related to either bug# 950026 or bug# 970147.

On which bug report should I continue to provide crash reports/feedback, considering my own problem is as listed in bug# 970519?

Apologies to all for littering the bugtracked not with one, but with 3 posts, and your patience pls; I'll try to be more careful next time...
No problem, thanks for the bug reports Michail.  It looks like bug 970147 is the correct one for your issue.
You need to log in before you can comment on or make changes to this bug.