Bug 74372 – WebAudio: Optimize calculateNormalizationScale().

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

WebAudio: Optimize calculateNormalizationScale().

[Jer Noble]

Created attachment 118916 [details]
Patch

[Chris Rogers]

Comment on attachment 118916 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118916&action=review

> Source/WebCore/platform/audio/Reverb.cpp:67
> +        float channelPower = 0.0;

WebKit style: 0 instead of 0.0

> Source/WebCore/platform/audio/Reverb.cpp:68
> +        vsvesq(response->channel(i)->data(), 1, &power, length);

You need to pass in &channelPower instead of &power

> Source/WebCore/platform/audio/VectorMath.cpp:90
> +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess)

nit: I'd use destP instead of dest to be consistent with the other functions

> Source/WebCore/platform/audio/VectorMath.cpp:92
> +    vDSP_svesq(const_cast<float*>(sourceP), sourceStride, dest, framesToProcess);

Might need the #if defined(__ppc__) || defined(__i386__) work-around like the other functions...

> Source/WebCore/platform/audio/VectorMath.cpp:309
> +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess);

nit: I'd use destP instead of dest to be consistent with the other functions

> Source/WebCore/platform/audio/VectorMath.h:40
> +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess);

Simple comment describing what function does.

consistency nit: dest -> destP

[Jer Noble]

(In reply to comment #2)
> (From update of attachment 118916 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118916&action=review
> 
> > Source/WebCore/platform/audio/Reverb.cpp:67
> > +        float channelPower = 0.0;
> 
> WebKit style: 0 instead of 0.0

Will change.

> > Source/WebCore/platform/audio/Reverb.cpp:68
> > +        vsvesq(response->channel(i)->data(), 1, &power, length);
> 
> You need to pass in &channelPower instead of &power

Whoops!  Good catch.

> > Source/WebCore/platform/audio/VectorMath.cpp:90
> > +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess)
> 
> nit: I'd use destP instead of dest to be consistent with the other functions

How about "sumP"?

> > Source/WebCore/platform/audio/VectorMath.cpp:92
> > +    vDSP_svesq(const_cast<float*>(sourceP), sourceStride, dest, framesToProcess);
> 
> Might need the #if defined(__ppc__) || defined(__i386__) work-around like the other functions...

Nope, this is one of the functions not in vDSP_translate.h.

> > Source/WebCore/platform/audio/VectorMath.cpp:309
> > +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess);
> 
> nit: I'd use destP instead of dest to be consistent with the other functions

Ditto.

> > Source/WebCore/platform/audio/VectorMath.h:40
> > +void vsvesq(const float* sourceP, int sourceStride, float* dest, size_t framesToProcess);
> 
> Simple comment describing what function does.

Sure thing.

> consistency nit: dest -> destP

Ditto.

[Jer Noble]

Created attachment 122330 [details]
Patch

[Chris Rogers]

Looks ok to me, but it seems like your patch is not applying.  You might need to update your source and re-upload a new patch.

Ok with me once that's done and the bots pass.  I'm not a reviewer so I can't R+

[Jer Noble]

Created attachment 122462 [details]
Patch

[WebKit Review Bot]

Comment on attachment 122462 [details]
Patch

Attachment 122462 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11236142

[Jer Noble]

Created attachment 122480 [details]
Patch

Fixed a compile error in the non-Darwin section of VectorMath.cpp.

[Eric Carlson]

Comment on attachment 122480 [details]
Patch

rs=me based on Chris' "unofficial" review

[Jer Noble]

Committed r105004: <http://trac.webkit.org/changeset/105004>