Add vsma in VectorMath to handle vector scale multiply and add and use it in AudioBus
Created attachment 121630 [details] Patch
Comment on attachment 121630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121630&action=review > Source/WebCore/platform/audio/VectorMath.cpp:142 > + Are we missing the case where n (framesToProcess) is not a multiple of 4? If framesToProcess is 5, tailFrames = 1 and endP = destP + 4. The while loop will be executed once, processing 4 frames. We're missing the last frame, right?
Comment on attachment 121630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121630&action=review >> Source/WebCore/platform/audio/VectorMath.cpp:142 >> + > > Are we missing the case where n (framesToProcess) is not a multiple of 4? If framesToProcess is 5, tailFrames = 1 and endP = destP + 4. The while loop will be executed once, processing 4 frames. We're missing the last frame, right? if the n is not a multiple of 4, in the line 148, n will be assigned to tailFrames, and the code after the #endif will be executed to handle the last frame.
Comment on attachment 121630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121630&action=review >>> Source/WebCore/platform/audio/VectorMath.cpp:142 >>> + >> >> Are we missing the case where n (framesToProcess) is not a multiple of 4? If framesToProcess is 5, tailFrames = 1 and endP = destP + 4. The while loop will be executed once, processing 4 frames. We're missing the last frame, right? > > if the n is not a multiple of 4, in the line 148, n will be assigned to tailFrames, and the code after the #endif will be executed to handle the last frame. Sorry about that. Looks good, then.
Looks good.
The patch in bug #74345 also adds vsma support. If both patches are approved, we'll have to make sure not to step on each others' toes.
(In reply to comment #6) > The patch in bug #74345 also adds vsma support. If both patches are approved, we'll have to make sure not to step on each others' toes. Agreed. I think it's easiest if we can land this patch first. And then #74345 can be simplified quite a bit. Does that seem reasonable?
(In reply to comment #7) > (In reply to comment #6) > > The patch in bug #74345 also adds vsma support. If both patches are approved, we'll have to make sure not to step on each others' toes. > > Agreed. I think it's easiest if we can land this patch first. And then #74345 can be simplified quite a bit. Does that seem reasonable? Yup!
Comment on attachment 121630 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=121630&action=review > Source/WebCore/platform/audio/VectorMath.cpp:95 > +#if defined(__ppc__) || defined(__i386__) Sorry for the last minute change, but Jer Noble just mentioned that some of these vector functions don't have a problem with vec_translate.h. So we should simplify this, by removing #if defined(__ppc__) || defined(__i386__) and always simply and directly call: vDSP_vsma(sourceP, sourceStride, scale, destP, destStride, destP, destStride, framesToProcess);
Created attachment 122338 [details] Patch
(In reply to comment #9) > (From update of attachment 121630 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=121630&action=review > > > Source/WebCore/platform/audio/VectorMath.cpp:95 > > +#if defined(__ppc__) || defined(__i386__) > > Sorry for the last minute change, but Jer Noble just mentioned that some of these vector functions don't have a problem with vec_translate.h. > > So we should simplify this, by removing #if defined(__ppc__) || defined(__i386__) and always simply and directly call: > > vDSP_vsma(sourceP, sourceStride, scale, destP, destStride, destP, destStride, framesToProcess); thanks. patch updated.
Thanks James, looks good.
Comment on attachment 122338 [details] Patch rs=me
Comment on attachment 122338 [details] Patch Clearing flags on attachment: 122338 Committed r104893: <http://trac.webkit.org/changeset/104893>
All reviewed patches have been landed. Closing bug.