[Webkit-unassigned] [Bug 34827] audio engine: add FFTFrame files

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 12 04:08:20 PST 2010


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


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #48666|review?                     |review-
               Flag|                            |




--- Comment #4 from Jeremy Orlow <jorlow at chromium.org>  2010-03-12 04:08:21 PST ---
(From update of attachment 48666)
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 866f6d8..a11dd7a 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,35 @@
> +2010-02-12  Chris Rogers  <crogers at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        audio engine: add FFTFrame files
> +        https://bugs.webkit.org/show_bug.cgi?id=34827
> +
> +        No tests - no javascript API

Add a "yet"


> diff --git a/WebCore/platform/audio/FFTFrame.cpp b/WebCore/platform/audio/FFTFrame.cpp
> new file mode 100644
> index 0000000..17f6c8e
> --- /dev/null
> +++ b/WebCore/platform/audio/FFTFrame.cpp
> @@ -0,0 +1,268 @@
> +/*
> + * 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.
> + */
> +
> +// Cross-platform FFTFrame implementation

This comment seems out of place and not particularly useful.

> +
> +#include "config.h"
> +#include "FFTFrame.h"
> +
> +#include <wtf/Complex.h>
> +#include <wtf/MathExtras.h>
> +
> +namespace WebCore {
> +
> +void FFTFrame::doPaddedFFT(float* data, size_t dataSize)
> +{
> +    // Zero-pad the impulse response
> +    AudioFloatArray paddedResponse(fftSize());
> +    
> +    assert(dataSize <= fftSize());
> +    memcpy(paddedResponse, data, sizeof(float) * dataSize);
> +
> +    // Get the frequency-domain version of padded response
> +    doFFT(paddedResponse);
> +}
> +
> +FFTFrame* FFTFrame::createInterpolatedFrame(FFTFrame& frame1, FFTFrame& frame2, double x)

Use PassOwnPtr<>

> +{
> +    FFTFrame* newFrame = new FFTFrame(frame1.fftSize());

Use OwnPtr<>

> +
> +    newFrame->interpolateFrequencyComponents(frame1, frame2, x);
> +
> +    // In the time-domain, the 2nd half of the response must be zero, to avoid circular convolution aliasing...
> +    int fftSize = newFrame->fftSize();
> +    AudioFloatArray buffer(fftSize);
> +    newFrame->doInverseFFT(buffer);
> +
> +    memset(buffer.data() + fftSize / 2, 0, sizeof(float) * fftSize / 2);
> +
> +    newFrame->doFFT(buffer);
> +
> +    return newFrame;
> +}
> +
> +void FFTFrame::interpolateFrequencyComponents(FFTFrame& frame1,
> +                                              FFTFrame& frame2,
> +                                              double interp)
> +{
> +    // FIXME : with some work, this method could be optimized
> +
> +    float* realP = realData();
> +    float* imagP = imagData();
> +
> +    float* realP1 = frame1.realData();
> +    float* imagP1 = frame1.imagData();
> +    float* realP2 = frame2.realData();
> +    float* imagP2 = frame2.imagData();
> +
> +    m_FFTSize = frame1.fftSize();
> +    m_log2FFTSize = frame1.log2FFTSize();
> +
> +    double s1base = (1.0 - interp);
> +    double s2base = interp;
> +
> +    double phaseAccum = 0.0;
> +    double lastPhase1 = 0.0;
> +    double lastPhase2 = 0.0;
> +
> +    realP[0] = s1base * realP1[0] + s2base * realP2[0];
> +    imagP[0] = s1base * imagP1[0] + s2base * imagP2[0];
> +
> +    int n = m_FFTSize / 2;
> +
> +    for (int i = 1; i < n; ++i) {
> +        Complex c1(realP1[i], imagP1[i]);
> +        Complex c2(realP2[i], imagP2[i]);
> +
> +        double mag1 = abs(c1);
> +        double mag2 = abs(c2);
> +
> +        // Interpolate magnitudes in decibels
> +        double mag1db = 20.0 * log10(mag1);
> +        double mag2db = 20.0 * log10(mag2);
> +
> +        double s1 = s1base;
> +        double s2 = s2base;
> +
> +        double magdbdiff = mag1db - mag2db;
> +
> +        // Empirical tweak to retain higher-frequency zeroes
> +        double threshold =  (i > 16) ? 5.0 : 2.0;
> +
> +        if (magdbdiff < -threshold && mag1db < 0.0) {
> +            s1 = pow(s1, 0.75);
> +            s2 = 1.0 - s1;
> +        } else if (magdbdiff > threshold && mag2db < 0.0) {
> +            s2 = pow(s2, 0.75);
> +            s1 = 1.0 - s2;
> +        }
> +
> +        // Average magnitude by decibels instead of linearly
> +        double magdb = s1 * mag1db + s2 * mag2db;
> +        double mag = pow(10.0, 0.05 * magdb);
> +
> +        // Now, deal with phase
> +        double phase1 = arg(c1);
> +        double phase2 = arg(c2);
> +
> +        double deltaPhase1 = phase1 - lastPhase1;
> +        double deltaPhase2 = phase2 - lastPhase2;
> +        lastPhase1 = phase1;
> +        lastPhase2 = phase2;
> +
> +        // Unwrap phase deltas
> +        if (deltaPhase1 > M_PI)
> +            deltaPhase1 -= 2.0 * M_PI;
> +        if (deltaPhase1 < -M_PI)
> +            deltaPhase1 += 2.0 * M_PI;
> +        if (deltaPhase2 > M_PI)
> +            deltaPhase2 -= 2.0 * M_PI;
> +        if (deltaPhase2 < -M_PI)
> +            deltaPhase2 += 2.0 * M_PI;
> +
> +        // Blend group-delays
> +        double deltaPhaseBlend;
> +
> +        if (deltaPhase1 - deltaPhase2 > M_PI)
> +            deltaPhaseBlend = s1 * deltaPhase1 + s2 * (2.0 * M_PI + deltaPhase2);
> +        else if (deltaPhase2 - deltaPhase1 > M_PI)
> +            deltaPhaseBlend = s1 * (2.0 * M_PI + deltaPhase1) + s2 * deltaPhase2;
> +        else
> +            deltaPhaseBlend = s1 * deltaPhase1 + s2 * deltaPhase2;
> +
> +        phaseAccum += deltaPhaseBlend;
> +
> +        // Unwrap
> +        if (phaseAccum > M_PI)
> +            phaseAccum -= 2.0 * M_PI;
> +        if (phaseAccum < -M_PI)
> +            phaseAccum += 2.0 * M_PI;
> +
> +        Complex c = complexFromMagnitudePhase(mag, phaseAccum);
> +
> +        realP[i] = c.real();
> +        imagP[i] = c.imag();
> +    }
> +}
> +
> +double  FFTFrame::extractAverageGroupDelay()
> +{
> +    float* realP = realData();
> +    float* imagP = imagData();
> +
> +    double aveSum = 0.0;
> +    double weightSum = 0.0;
> +    double lastPhase = 0.0;
> +
> +    int halfSize = fftSize() / 2;
> +
> +    const double kSamplePhaseDelay = (2.0 * M_PI) / double(fftSize());
> +
> +    // Calculate weighted average group delay
> +    for (int i = 0; i < halfSize; i++) {
> +        Complex c(realP[i], imagP[i]);
> +        double mag = abs(c);
> +        double phase = arg(c);
> +
> +        double deltaPhase = phase - lastPhase;
> +        lastPhase = phase;
> +
> +        // Unwrap
> +        if (deltaPhase < -M_PI)
> +            deltaPhase += 2.0 * M_PI;
> +        if (deltaPhase > M_PI)
> +            deltaPhase -= 2.0 * M_PI;
> +
> +        aveSum += mag * deltaPhase;
> +        weightSum += mag;
> +    }
> +
> +    // Note how we invert the phase delta wrt frequency since this is how group delay is defined
> +    double ave = aveSum / weightSum;
> +    double aveSampleDelay = -ave / kSamplePhaseDelay;
> +
> +    // Leave 20 sample headroom (for leading edge of impulse)
> +    if (aveSampleDelay > 20.0)
> +        aveSampleDelay -= 20.0;
> +
> +    // Remove average group delay (minus 20 samples for headroom)
> +    addConstantGroupDelay(-aveSampleDelay);
> +
> +    // Remove DC offset
> +    realP[0] = 0.0;
> +
> +    return aveSampleDelay;
> +}
> +
> +void  FFTFrame::addConstantGroupDelay(double sampleFrameDelay)
> +{
> +    int halfSize = fftSize() / 2;
> +
> +    float* realP = realData();
> +    float* imagP = imagData();
> +
> +    const double kSamplePhaseDelay = (2.0 * M_PI) / double(fftSize());
> +
> +    double phaseAdj = -sampleFrameDelay * kSamplePhaseDelay;
> +
> +    // Add constant group delay
> +    for (int i = 1; i < halfSize; i++) {
> +        Complex c(realP[i], imagP[i]);
> +        double mag = abs(c);
> +        double phase = arg(c);
> +
> +        phase += i * phaseAdj;
> +
> +        Complex c2 = complexFromMagnitudePhase(mag, phase);
> +
> +        realP[i] = c2.real();
> +        imagP[i] = c2.imag();
> +    }
> +}
> +
> +// For debugging

#ifndef NDEBUG then?

> +void FFTFrame::print()
> +{
> +    FFTFrame& frame = *this;
> +    float* realP = frame.realData();
> +    float* imagP = frame.imagData();
> +    printf("**** \n");
> +    printf("DC = %f : nyquist = %f\n", realP[0], imagP[0]);
> +
> +    int n = m_FFTSize / 2;
> +
> +    for (int i = 1; i < n; i++) {
> +        double mag = sqrt(realP[i] * realP[i] + imagP[i] * imagP[i]);
> +        double phase = atan2(realP[i], imagP[i]);
> +
> +        printf("[%d] (%f %f)\n", i, mag, phase);
> +    }
> +    printf("****\n");
> +}
> +
> +} // namespace WebCore
> diff --git a/WebCore/platform/audio/FFTFrame.h b/WebCore/platform/audio/FFTFrame.h
> new file mode 100644
> index 0000000..2130eb0
> --- /dev/null
> +++ b/WebCore/platform/audio/FFTFrame.h
> @@ -0,0 +1,52 @@
> +/*
> + * 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.
> + */
> +
> +#ifndef FFTFrame_h
> +#define FFTFrame_h
> +
> +#include <wtf/Platform.h>
> +
> +// Defines the interface for an "FFT frame", an object which is able to perform a forward
> +// and reverse FFT, internally storing the resultant frequency-domain data
> +//
> +// The reason we don't create an abstract base class and subclass for Mac, MKL, etc.
> +// is because then it would not be possible to use the object in a cross-platform way
> +// as an ivarof a class or as a stack-based object, since we would have to resort to

s/ivar/instance variable/

> +// factory methods, etc.  Since it turns out to be quite useful to use these as ivars
> +// and stack based objects, and since the exact type *is* known at compile-time, we use
> +// this approach.

I don't understand.  You can just name all of the cross platform libraries
FFTFrameBase or FFTFramePlatform or something and then have FFTFrame inherit
from that.  The only reason not to do this is if there's a lot of 2 way
dependencies between the halves (and thus you'd need to use virtual
calls..which is not desirable).  You could also do this by having the platform
specific parts be in their own class and have that class be owned by the cross
platform bits or visa versa.  Or you can even split this into multiple layers. 
I haven't looked closely 

The thing I dislike the most is that the .h file is 100% per platform but then
pulls in some common CPP files.

> +
> +#if OS(DARWIN)
> +    #include "FFTFrameMac.h"
> +#elif OS(LINUX) || OS(WINDOWS)
> +    #include "FFTFrameMKL.h"
> +#else
> +    #error "OS not supported"
> +#endif

Don't indent.

> +
> +#endif // FFTFrame_h
> diff --git a/WebCore/platform/audio/mac/FFTFrameMac.cpp b/WebCore/platform/audio/mac/FFTFrameMac.cpp
> new file mode 100644
> index 0000000..e99b573
> --- /dev/null
> +++ b/WebCore/platform/audio/mac/FFTFrameMac.cpp
> @@ -0,0 +1,215 @@
> +/*
> + * 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.
> + */
> +
> +// Mac OS X - specific FFTFrame implementation
> +
> +#include "config.h"
> +#include "FFTFrameMac.h"
> +
> +namespace WebCore {
> +
> +const int kMaxFFTPow2Size = 24;
> +
> +FFTSetup* FFTFrame::fftSetups = 0;
> +
> +// Normal constructor: allocates for a given fftSize
> +FFTFrame::FFTFrame(size_t fftSize)
> +    : m_realData(fftSize)
> +    , m_imagData(fftSize)
> +{
> +    m_FFTSize = fftSize;
> +    m_log2FFTSize = log2(fftSize);
> +
> +    // We only allow power of two
> +    assert(pow(2.0, m_log2FFTSize) == m_FFTSize);

ASSERT not assert.

And I don't like that you're using a float function here.  Why not just use <<?

> +
> +    // Lazily create and share fftSetup with other frames
> +    m_FFTSetup = fftSetupForSize(fftSize);
> +
> +    // Setup frame data
> +    m_frame.realp = m_realData;
> +    m_frame.imagp = m_imagData;
> +}
> +
> +// Creates a blank/empty frame (interpolate() must later be called)
> +FFTFrame::FFTFrame()
> +    : m_realData(0)
> +    , m_imagData(0)
> +{
> +    // Later will be set to correct values when interpolate() is called
> +    m_frame.realp = 0;
> +    m_frame.imagp = 0;
> +
> +    m_FFTSize = 0;
> +    m_log2FFTSize = 0;
> +}
> +
> +// Copy constructor
> +FFTFrame::FFTFrame(const FFTFrame& frame)
> +    : m_FFTSize(frame.m_FFTSize)
> +    , m_log2FFTSize(frame.m_log2FFTSize)
> +    , m_FFTSetup(frame.m_FFTSetup)
> +    , m_realData(frame.m_FFTSize)
> +    , m_imagData(frame.m_FFTSize)
> +{
> +    // Copy/setup frame data
> +    size_t nbytes = sizeof(float) * m_FFTSize;
> +    FFTFrame& safeFrame = const_cast<FFTFrame&>(frame);
> +    memcpy(realData(), safeFrame.realData(), nbytes);
> +    memcpy(imagData(), safeFrame.imagData(), nbytes);

You don't need to do this.  You can just do frame->realp and such.  In general,
we should do everything we can to avoid const_cast.

> +
> +    m_frame.realp = realData();
> +    m_frame.imagp = imagData();
> +}
> +
> +FFTFrame::~FFTFrame()
> +{
> +}
> +
> +// Uses Accelerate.framework highly-optimized zvmul() function

Not sure if this comment adds any value.

> +void FFTFrame::multiply(const FFTFrame& frame)
> +{
> +    FFTFrame& frame1 = *this;
> +    FFTFrame& frame2 = const_cast<FFTFrame&>(frame);
> +
> +    float* realP1 = frame1.realData();
> +    float* imagP1 = frame1.imagData();
> +    float* realP2 = frame2.realData();
> +    float* imagP2 = frame2.imagData();
> +
> +    // Scale accounts for vecLib's peculiar scaling
> +    // This ensures the right scaling all the way back to inverse FFT
> +    float scale = 0.5;
> +
> +    // Multiply packed DC/nyquist component
> +    realP1[0] *= scale * realP2[0];
> +    imagP1[0] *= scale * imagP2[0];
> +
> +    // Multiply the rest, skipping packed DC/Nyquist components
> +    DSPSplitComplex sc1 = frame1.dspSplitComplex();
> +    sc1.realp++;
> +    sc1.imagp++;
> +
> +    DSPSplitComplex sc2 = frame2.dspSplitComplex();
> +    sc2.realp++;
> +    sc2.imagp++;
> +
> +    size_t halfSize = m_FFTSize / 2;
> +
> +    // Complex multiply
> +    zvmul(&sc1,
> +          1,
> +          &sc2,
> +          1,
> +          &sc1,
> +          1,
> +          halfSize - 1,
> +          1 /* normal multiplication */);
> +
> +    // We've previously scaled the packed part, now scale the rest.....
> +    vsmul(sc1.realp, 1, &scale, sc1.realp, 1, halfSize - 1);
> +    vsmul(sc1.imagp, 1, &scale, sc1.imagp, 1, halfSize - 1);
> +
> +    // Scalar code for reference

I'm leaning towards saying this should removed so it won't go stale.  Would you
mind terribly?

> +#if 0
> +    float scale = 0.5;
> +
> +    // multiply packed DC/nyquist component
> +    realP2[0] *= scale *realP1[0];
> +    imagP2[0] *= scale *imagP1[0];
> +
> +    // scalar code
> +    for (int i = 1; i < halfSize; i++) {
> +        // scale by 1/2 since we're not handling the scaling in the FFT -> IFFT
> +        // (we're handling it all in the IFFT stage)
> +
> +
> +        float r1 = realP1[i];
> +        float c1 = imagP1[i];
> +
> +        float r2 = realP2[i];
> +        float c2 = imagP2[i];
> +
> +        // multiply in the frequency domain
> +        float r3 = r1*r2 - c1*c2;
> +        float c3 = r1*c2 + r2*c1;
> +
> +        realP2[i] = scale * r3;
> +        imagP2[i] = scale * c3;
> +    }
> +#endif
> +}
> +
> +void FFTFrame::doFFT(float* data)
> +{
> +    ctoz((DSPComplex*)data, 2, &m_frame, 1, m_FFTSize / 2);
> +    fft_zrip(m_FFTSetup, &m_frame, 1, m_log2FFTSize, FFT_FORWARD);
> +}
> +
> +void FFTFrame::doInverseFFT(float* data)
> +{
> +    fft_zrip(m_FFTSetup, &m_frame, 1, m_log2FFTSize, FFT_INVERSE);
> +    ztoc(&m_frame, 1, (DSPComplex*)data, 2, m_FFTSize / 2);
> +
> +    // Do final scaling so that x == IFFT(FFT(x))
> +    float scale = 0.5 / m_FFTSize;
> +    vsmul(data, 1, &scale, data, 1, m_FFTSize);
> +}
> +
> +FFTSetup FFTFrame::fftSetupForSize(size_t fftSize)
> +{
> +    if (!fftSetups) {
> +        fftSetups = (FFTSetup*)malloc(sizeof(FFTSetup) * kMaxFFTPow2Size);
> +        memset(fftSetups, 0, sizeof(FFTSetup) * kMaxFFTPow2Size);
> +    }
> +
> +    int pow2size = log2(fftSize);
> +
> +    assert(pow2size < kMaxFFTPow2Size);
> +
> +    if (!fftSetups[pow2size])
> +        fftSetups[pow2size] = create_fftsetup(pow2size, FFT_RADIX2);
> +
> +    return fftSetups[pow2size];
> +}

Nit, but it seems like vertical whitespace could be condensed a bit here
without making it much harder to read.

> +
> +void FFTFrame::cleanupFFTSetups()
> +{
> +    if (!fftSetups)
> +        return;
> +
> +    for (int i = 0; i < kMaxFFTPow2Size; ++i) {
> +        if (fftSetups[i])
> +            destroy_fftsetup(fftSetups[i]);
> +    }
> +
> +    free(fftSetups);
> +    fftSetups = 0;
> +}
> +
> +} // namespace WebCore
> diff --git a/WebCore/platform/audio/mac/FFTFrameMac.h b/WebCore/platform/audio/mac/FFTFrameMac.h
> new file mode 100644
> index 0000000..40f976b
> --- /dev/null
> +++ b/WebCore/platform/audio/mac/FFTFrameMac.h
> @@ -0,0 +1,91 @@
> +/*
> + * 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.
> + */
> +
> +// FFT abstraction implemented using Mac OS X Accelerate.framework (vecLib)
> +//
> +
> +#ifndef FFTFrameMac_h
> +#define FFTFrameMac_h
> +
> +#include "AudioFloatArray.h"
> +#include <Accelerate/Accelerate.h>
> +
> +namespace WebCore {
> +
> +class FFTFrame {
> +public:
> +    FFTFrame(size_t fftSize);
> +    FFTFrame(); // creates a blank/empty frame for later use with interpolate()

You mean createInterpolatedFrame or something in some other class?  If the
former, correct.  If the latter, make it ClassName::interpolate()

Btw, all of these should be private and you should expose constructors for only
what needs to be created outside of this class.

> +    FFTFrame(const FFTFrame& frame);
> +    virtual ~FFTFrame();

Why does this need to be virtual?

> +
> +    void doFFT(float* data);
> +    void doPaddedFFT(float* data, size_t dataSize); // zero-padding with dataSize <= fftSize
> +    void doInverseFFT(float* data);
> +
> +    // Multiplies ourself with |frame| : effectively operator*=()
> +    void multiply(const FFTFrame& frame);
> +
> +    // Interpolates from frame1 -> frame2 as |x| goes from 0.0 -> 1.0
> +    static FFTFrame* createInterpolatedFrame(FFTFrame& frame1, FFTFrame& frame2, double x);

Can the inputs be const?

This should return a PassOwnPtr<FFTFrame>.

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



More information about the webkit-unassigned mailing list