[webkit-reviews] review granted: [Bug 34907] audio engine: add FFTConvolver class : [Attachment 50849] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has granted Chris Rogers
<crogers at google.com>'s request for review:
Bug 34907: audio engine: add FFTConvolver class
https://bugs.webkit.org/show_bug.cgi?id=34907

Attachment 50849: Patch
https://bugs.webkit.org/attachment.cgi?id=50849&action=review

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
> diff --git a/WebCore/platform/audio/FFTConvolver.cpp
b/WebCore/platform/audio/FFTConvolver.cpp
> new file mode 100644
> index 0000000..16a0f48
> --- /dev/null
> +++ b/WebCore/platform/audio/FFTConvolver.cpp
> @@ -0,0 +1,101 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1.  Redistributions of source code must retain the above copyright
> + *	  notice, this list of conditions and the following disclaimer.
> + * 2.  Redistributions in binary form must reproduce the above copyright
> + *	  notice, this list of conditions and the following disclaimer in the
> + *	  documentation and/or other materials provided with the distribution.
> + * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> + *	  its contributors may be used to endorse or promote products derived
> + *	  from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED

> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY

> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "config.h"
> +#include "FFTConvolver.h"
> +
> +namespace WebCore {
> +
> +FFTConvolver::FFTConvolver(size_t fftSize)
> +    : m_frame(fftSize)
> +    , m_readWriteIndex(0)
> +    , m_inputBuffer(fftSize) // 2nd half of buffer is always zeroed
> +    , m_outputBuffer(fftSize)
> +    , m_lastOverlapBuffer(fftSize / 2)
> +{
> +}
> +
> +void FFTConvolver::process(FFTFrame* fftKernel, float* sourceP, float*
destP, size_t framesToProcess)
> +{
> +    // FIXME: make so |framesToProcess| is not required to fit evenly into
fftSize/2

Get rid of |'s per earlier comment.


Looks good, but we'll probably want to land this in the branch rather than
trunk...we'll know for sure soon, hopefully.