The SSE implementation of AudioBufferSumOfSquares handles leading and trailing unaligned input incorrectly. There is probably currently no leading unaligned input in practice due to bug 1024182 making the buffer at least 128 samples long, if the allocator aligns larger allocations on 64 bytes. The issue with trailing unaligned data is a regression from bug 881587. Given this means only a few incorrect samples from what is usually a considerably longer buffer, the regression is probably not too bad. P3 because this is not too bad a regression, though it is really higher priority because this blocks 1024182, which makes the normalization scale completely wrong.

Bug 1024182 makes this difficult to test, and so I'll add a test case there.

Comment on attachment 8896482 [details] bug 1389638 declare length of array for AudioBufferSumOfSquares_SSE as an integral type https://reviewboard.mozilla.org/r/167722/#review173424

Comment on attachment 8896483 [details] bug 1389638 include leading unaligned AudioBufferSumOfSquares input in length count and check for end of buffer https://reviewboard.mozilla.org/r/167724/#review173432 ::: commit-message-39625:4 (Diff revision 1) > +bug 1389638 include leading unaligned AudioBufferSumOfSquares input in length count and check for end of buffer r?dminor > + > +The length of the array for AudioBufferSumOfSquares_SSE() is calculated from > +available aligned input, instead of total input. Please make the change you have made clearer, e.g. "This fixes the calculation for the length of the array for AudioBufferSumOfSquares_SSE to use the available aligned input rather than the total input"

Comment on attachment 8896484 [details] bug 1389638 use correct input elements for summing squares of trailing unaligned input https://reviewboard.mozilla.org/r/167726/#review173440

Comment on attachment 8896483 [details] bug 1389638 include leading unaligned AudioBufferSumOfSquares input in length count and check for end of buffer https://reviewboard.mozilla.org/r/167724/#review173432 > Please make the change you have made clearer, e.g. "This fixes the calculation for the length of the array for AudioBufferSumOfSquares_SSE to use the available aligned input rather than the total input" The patch does more than that, and so I've added comments to clarify.

(In reply to Karl Tomlinson (:karlt) from comment #11) > The patch does more than that, and so I've added comments to clarify. That seems very terse, sorry. I actually replaced that reviewboard comment before publishing with a long description of what I changed in the comment and why. Reviewboard has decided to revert to an earlier version on publishing. As it is going to be turned off soon, I won't bother filing another reviewboard bug about it losing comment changes. Thank you for the review. I hope I've made the changes clearer. The comments are at least more detailed.

Pushed by ktomlinson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c307e5ea3488 declare length of array for AudioBufferSumOfSquares_SSE as an integral type r=dminor https://hg.mozilla.org/integration/autoland/rev/46a45f39fc9d include leading unaligned AudioBufferSumOfSquares input in length count and check for end of buffer r=dminor https://hg.mozilla.org/integration/autoland/rev/b95c2146d637 use correct input elements for summing squares of trailing unaligned input r=dminor

FWIW closing the reviewboard page now presents "This page is asking you to confirm that you want to leave - data you have entered may not be saved.", but selecting "Stay on Page" is showing the previous version of the comment. I can't find the more recent version anywhere.

https://hg.mozilla.org/mozilla-central/rev/c307e5ea3488 https://hg.mozilla.org/mozilla-central/rev/46a45f39fc9d https://hg.mozilla.org/mozilla-central/rev/b95c2146d637

Doesn't sound like this is worth backporting. Feel free to set status back to affected and request approval if you feel otherwise, however.