Closed Bug 1266112 Opened 4 years ago Closed 4 years ago

Handle unaligned buffers in AudioNodeStream::AccumulateInputChunk

Categories

(Core :: Web Audio, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(1 file)

We're seeing unaligned input or output buffers in AudioNodeStream::AccumulateInputChunk which is causing a crash in SSE2 code (see Bug 1266047) in the wild, but not in automation.

I'm planning to land checks for alignment and fallback to scalar operations in the interests of getting Bug 1266047 fixed as quickly as possible. We should still either find and fix the cause of the unaligned buffers or rework the code so that as many of the operations are done with SSE2 as possible and fallback to scalar for the rest.
Looks like there is another code path which will pass unaligned buffers into this function. See Bug 1266302.
Digging into Bug 1266302, I strongly suspect that Bug 1266405 is the underlying cause here.

Although it didn't repro on my system, we were definitely getting borrowed (and potentially unaligned) buffers on the code path which lead to that crash.
Assignee: nobody → dminor
If Bug 1266405 was the underlying cause, we should be able to remove the quick fix applied in Bug 1266047.
Status: NEW → ASSIGNED
Assuming that Bug 1266405 fixed the underlying cause for the unaligned
buffers we were seeing, the checks in AudioBufferAddWithScale and
AudioBlockCopyWithScale shoudl no longer be necessary.

Review commit: https://reviewboard.mozilla.org/r/49699/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49699/
Attachment #8747042 - Flags: review?(padenot)
Comment on attachment 8747042 [details]
MozReview Request: Bug 1266112 - Remove unnecessary alignment checks from AudioNodeEngine.cpp; r?padenot

https://reviewboard.mozilla.org/r/49699/#review46453
Attachment #8747042 - Flags: review?(padenot) → review+
https://hg.mozilla.org/mozilla-central/rev/28bba443ce38
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.