Summary: | Add vsma in VectorMath to handle vector scale multiply and add and use it in AudioBus | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wei James (wistoch) <james.wei> | ||||||
Component: | Web Audio | Assignee: | Wei James (wistoch) <james.wei> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | crogers, james.wei, jer.noble, kbr, rtoy, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 74345 | ||||||||
Attachments: |
|
Description
Wei James (wistoch)
2012-01-09 01:03:23 PST
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. |