[Webkit-unassigned] [Bug 74592] Optimize AudioBufferSourceNode process by avoiding interpolation when pitchRate==1

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74592





--- Comment #3 from Raymond Toy <rtoy at chromium.org>  2011-12-16 10:25:03 PST ---
(From update of attachment 119391)
View in context: https://bugs.webkit.org/attachment.cgi?id=119391&action=review

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:255
> +    if (!needInterpolation) {

needInterpolation isn't used anywhere else, so remove it, and change to if (pitchRate == 1).

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:258
> +        virtualReadIndex += framesToProcess;

virtualReadIndex isn't used in the if block below.  It would be nice to move this down near line 298 where it's actually used.

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:261
> +            memcpy(destinationL, sourceL + readIndex, sizeof(float) * framesToProcess);

I think sizeof(*sourceL) or sizeof(*destinationL) is better than sizeof(float).  Same applies to other memcpy places below.

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:273
> +            framesToProcess -= framesToEnd;

Maybe add ASSERT(framesToProcess >= 0) here?  Currently, this is always true, but this is an important invariant for the memcpy's below so we don't suddenly copy a huge number of samples to the destination.

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:277
> +                for (unsigned k = 0; k < rounds; ++k) {

Why not just use a while loop:

while (framesToProcess > 0) {
 ...
|

instead of the for loop with rounds?

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:279
> +                    destinationL += deltaFrames;

Is this right?  sourceL and startFrame are not modified in the for loop so we keep copying the same data from sourceL+startFrame to consecutive places in destinationL.

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:298
>          if (virtualReadIndex >= endFrame) {

Maybe move this into the else case of the if above (place near line 295)?  I think this condition can only happen in that case.

> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:300
> +            virtualReadIndex -= ((virtualReadIndex - endFrame) / deltaFrames + 1) * deltaFrames;

Is this correct?  If virtualReadIndex - endFrame = 1, we get (1/deltaFrames + 1)*deltaFrames = deltaFrames + 1 (because virtualReadIndex is a float and ignoring roundoff).  I think we want to subtract just deltaFrames in this case.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.