Crash in [@ mozilla::MozPromise<T>::Private::Resolve<T>] on OSX
Categories
(Core :: Audio/Video: Playback, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr68 | --- | unaffected |
| firefox75 | --- | unaffected |
| firefox76 | --- | unaffected |
| firefox77 | --- | unaffected |
| firefox78 | blocking | verified |
People
(Reporter: mccr8, Assigned: bwc)
References
(Regression)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
This bug is for crash report bp-357d3ef4-a78d-477b-805a-9fc8c0200507.
Top 10 frames of crashing thread:
0 XUL void mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>::Private::Resolve<nsTArray<RefPtr<mozilla::MediaData> > > xpcom/threads/MozPromise.h:1071
1 XUL mozilla::AppleVTDecoder::OutputFrame dom/media/platforms/apple/AppleVTDecoder.cpp:440
2 XUL mozilla::PlatformCallback dom/media/platforms/apple/AppleVTDecoder.cpp:303
3 VideoToolbox VTDecoderSessionSetPixelBufferAttributes
4 VideoToolbox VTDecoderSessionSetPixelBufferAttributes
5 VideoToolbox VTRemoteVideoDecoderGetClassID
6 VideoToolbox VCH263VideoDecoder_CreateInstance
7 VideoToolbox VCH263VideoDecoder_CreateInstance
8 VideoToolbox VTRemoteVideoDecoderGetClassID
9 CoreMedia figThreadMain
This is a frequent OSX-only crash that started on with the 20200506093716 build and is happening inside mozilla::AppleVTDecoder::OutputFrame.
This is the set of patches for that build: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=385f49adaf00d02fc8e04da1e0031e3477182d67&tochange=1fa1d4f4b0e247804927d474cfecdf6c6f185203
Bug 1626364 is in the range and touched AppleVTDecoder.cpp so I'm going to guess that is related to this regression.
|
Reporter | |
Comment 2•4 years ago
|
||
[Tracking Requested - why for this release]: OSX-specific top crash regression.
|
||
Comment 3•4 years ago
•
|
||
What this crash indicates is that we resolved the promise either there https://searchfox.org/mozilla-central/source/dom/media/platforms/apple/AppleVTDecoder.cpp#191 or there: https://searchfox.org/mozilla-central/source/dom/media/platforms/apple/AppleVTDecoder.cpp#201
Yet decoding started and we would have normally returned a frame.
It's likely playback is now broken on mac.
|
Assignee | |
Comment 4•4 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #3)
What this crash indicates is that we resolved the promise either there https://searchfox.org/mozilla-central/source/dom/media/platforms/apple/AppleVTDecoder.cpp#191 or there: https://searchfox.org/mozilla-central/source/dom/media/platforms/apple/AppleVTDecoder.cpp#201
Yet decoding started and we would have normally returned a frame.
It's likely playback is now broken on mac.
This seems to only effect OS X versions 10.12 and earlier, so maybe VideoToolbox on those versions still decodes frames sometimes when there's an error. This comment hints that this can happen:
However, bug 1626364 did not change how we treat most errors returned synchronously by VTDecompressionSessionDecodeFrame. It did change how we process the dropped frame error:
We used to ignore the kVTDecodeInfo_FrameDropped error entirely. Perhaps it is possible (or even expected) to still get a frame when this error occurs on older versions of VideoToolbox.
|
|
Assignee | |
Comment 5•4 years ago
|
||
The other change from bug 1626364 was handling of errors here:
It may be possible that older versions of VideoToolbox will call this callback again sometimes after calling it with an error.
|
|
Assignee | |
Comment 6•4 years ago
|
||
I guess we could make this call a ResolveIfExists, but that seems a bit of a cop-out:
It may be the only thing we can do in this situation though, if VideoToolbox is behaving erratically. The question is whether VideoToolbox only behaves this way occasionally. Looking at the crash-stats, I see the same install crashing multiple times in a number of cases, which implies that it is a pretty reproducible problem (at least for those users).
|
|
Assignee | |
Comment 7•4 years ago
|
||
So this is interesting. Here's a report with a more complete stack (all of the complete stacks I've found look like this):
https://crash-stats.mozilla.org/report/index/497872fa-b71f-4fb1-9651-825470200508
Note the appearance of VCH263VideoDecoder_CreateInstance in that stack. That hints that this is the first frame. That says to me that re-initting when we see this problem may lead to a re-occurrance of the problem. Maybe we should simply use ResolveIfExists, or put a check in PlatformCallback that bails if mPromise is already settled.
Thoughts?
|
|
||
Comment 8•4 years ago
|
||
(In reply to Byron Campen [:bwc] from comment #7)
So this is interesting. Here's a report with a more complete stack (all of the complete stacks I've found look like this):
https://crash-stats.mozilla.org/report/index/497872fa-b71f-4fb1-9651-825470200508
Note the appearance of VCH263VideoDecoder_CreateInstance in that stack. That hints that this is the first frame. That says to me that re-initting when we see this problem may lead to a re-occurrance of the problem. Maybe we should simply use ResolveIfExists, or put a check in PlatformCallback that bails if mPromise is already settled.
Thoughts?
This would be the simplest workaround, but I'm concerned that it could be a more generic issue and somehow we end up dropping video frames that would otherwise play properly.
So the easy fix may not be the right one. We should only be dropping frames that need to be dropped.
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 9•4 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #8)
(In reply to Byron Campen [:bwc] from comment #7)
So this is interesting. Here's a report with a more complete stack (all of the complete stacks I've found look like this):
https://crash-stats.mozilla.org/report/index/497872fa-b71f-4fb1-9651-825470200508
Note the appearance of VCH263VideoDecoder_CreateInstance in that stack. That hints that this is the first frame. That says to me that re-initting when we see this problem may lead to a re-occurrance of the problem. Maybe we should simply use ResolveIfExists, or put a check in PlatformCallback that bails if mPromise is already settled.
Thoughts?
This would be the simplest workaround, but I'm concerned that it could be a more generic issue and somehow we end up dropping video frames that would otherwise play properly.
So the easy fix may not be the right one. We should only be dropping frames that need to be dropped.
True, but if the decoder tells us it has dropped the frame when it has not, what should we do? We would need to ignore VT when it tells us it has dropped a frame, which would mean we would need to have a backup plan for rejecting that promise. We can't just set a timeout, because blocking decode for 30ms (just an example) every time a frame is dropped would probably not be a good idea. We would probably need to first get rid of the sync wait, and shift over to an API that uses a separate promise for each frame (multiple of which can be unsettled at a time). Then maybe we could reject old unsettled promises when a more recent frame is decoded (or an error occurs). Of course, we still aren't sure whether webrtc.org is compatible with such an approach.
|
|
Assignee | |
Comment 10•4 years ago
|
||
|
|
Assignee | |
Comment 11•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec95522b70ff6e63c7351abac1d50dce17826d40
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 12•4 years ago
|
||
|
|
Assignee | |
Comment 13•4 years ago
|
||
Try looks fine.
|
|
Assignee | |
Updated•4 years ago
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 14•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=33eeb05306e411343868dabf6476dcba5928ec7c
|
|
Assignee | |
Comment 15•4 years ago
|
||
We do this by keeping the unexpected frame in mReorderQueue, and outputting it later when the caller is expecting frames.
|
|
Assignee | |
Comment 16•4 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9cfa237a3949106907b17bea976762ab77f9290
|
|
Assignee | |
Comment 17•4 years ago
|
||
Try looks good. Doing some manual testing on my end.
|
|
Assignee | |
Comment 18•4 years ago
|
||
Seems to work ok. I'll be able to do some testing on old OS X next week once some hardware arrives.
|
||
Comment 19•4 years ago
|
||
Pushed by bcampen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eb882d1719d2 Work around Apple VT decoder saying it dropped a frame, and then decoding that frame anyway. r=jya
|
||
Comment 20•4 years ago
|
||
| bugherder | ||
|
|
||
Updated•4 years ago
|
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 21•4 years ago
|
||
I've verified that the crash happens very reliably when using Amazon Chime on OS X 10.12, and have also verified that the crash goes away with the changeset from this bug. H264 decode on Amazon Chime seems to work well using the try push in comment 16. I am seeing some strange behavior with current nightly, however. Might be an unrelated regression.
|
|
Assignee | |
Comment 22•4 years ago
|
||
Verified that this revision (which landed very soon after the fix from this bug; this was a backout that un-busted a crash on newtab) works ok.
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=332e06a545d2
|
|
Assignee | |
Comment 23•4 years ago
•
|
||
The fix for bug 1632489 is causing the weirdness I'm seeing. Have filed bug 1638942 for this.
|
||
Updated•4 years ago
|
Description
•