Closed Bug 1096716 Opened 5 years ago Closed 5 years ago

Delay buffer frame calculation in WMF audio decoder until after UpdateOutputType

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: kinetik, Assigned: kinetik)

References

Details

Attachments

(1 file)

Spun off from bug 1091704 comment 2 and bug 1091704 comment 4, I'm not convinced this is the cause of bug 1091704 but it seems worth addressing.  If the crashes drop off, we'll know the culprit.  Otherwise, the search will continue.

(In reply to Matthew Gregan [:kinetik] from comment #4)
> Running with my (possibly incorrect) guess about this being related to WMF,
> bug 1073792 looks suspicious in that WMFAudioMFTMananger::UpdateOutputType
> may change mAudioRate and mAudioChannels, and WMFAudioMFTManager::Output
> computes values using those member variables before *and* after calling
> UpdateOutputType without handling the possibility that they've changed.  In
> this case, it seems to be possible for the size of the memory owned by the
> AudioData to be different to the length passed to it as numFrames, which is
> used in AudioStream::Write to compute bytesToCopy which flows into the
> memcpy in CircularByteBuffer::AppendElements.
Move numSamples and numFrames calculation below the call to UpdateOutputType because that may update mAudioChannels.  Also move the UpdateOutputType call above a use of mAudioRate for the same reason.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c8c7d483df4f
Attachment #8520330 - Flags: review?(cpearce)
Comment on attachment 8520330 [details] [diff] [review]
bug1096716_v0.patch

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

d'oh...
Attachment #8520330 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/0995ecffadcb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment on attachment 8520330 [details] [diff] [review]
bug1096716_v0.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1073792
[User impact if declined]: crash during MP4 playback; this is potentially a fix for topcrash bug 1091704
[Describe test coverage new/current, TBPL]: 
[Risks and why]: simple change, very low risk
[String/UUID change made/needed]: none
Attachment #8520330 - Flags: approval-mozilla-beta?
Attachment #8520330 - Flags: approval-mozilla-aurora?
Attachment #8520330 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8520330 [details] [diff] [review]
bug1096716_v0.patch

Beta+
Attachment #8520330 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.