Bug 44995 – Add AudioParam files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add AudioParam files

[Chris Rogers]

Created attachment 66119 [details]
Patch

[Chris Rogers]

This implements AudioParam as defined in the web audio specification:

http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioParam-section

[Kenneth Russell]

Comment on attachment 66119 [details]
Patch

Marking r+ with a few minor comments. Fix upon committing, or upload new patch.

> Index: WebCore/webaudio/AudioParam.h
> ===================================================================
> --- WebCore/webaudio/AudioParam.h	(revision 0)
> +++ WebCore/webaudio/AudioParam.h	(revision 0)
> @@ -0,0 +1,115 @@
> +/*
> + * 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 AudioParam_h
> +#define AudioParam_h
> +
> +#include "PlatformString.h"
> +#include <math.h>
> +#include <sys/types.h>
> +#include <wtf/PassRefPtr.h>
> +#include <wtf/RefCounted.h>
> +
> +namespace WebCore {
> +
> +class AudioParam : public RefCounted<AudioParam> {
> +public:
> +    static const double DefaultSmoothingConstant = 0.05;
> +    static const double SnapThreshold = 0.001;

I think you're going to have problems getting this to link on all platforms (or maybe not, since you're developing on the Mac and clearly things are linking for you). I believe you need the following in an AudioParam.cc for correctness.

// GCC requires these declarations, but MSVC requires they not be present
#ifndef COMPILER_MSVC
const double AudioParam::DefaultSmoothingConstant;
const double AudioParam::SnapThreshold;
#endif

It might only be necessary if these are referenced from another .cpp file; not sure. It looks like SnapThreshold doesn't need to be accessed from outside this class in which case it should be private. If the same is true for DefaultSmoothingConstant then making them both private may solve the problem.

> +
> +    static PassRefPtr<AudioParam> create(const String& name, double defaultValue, double minValue, double maxValue, unsigned units = 0)
> +    {
> +        return adoptRef(new AudioParam(name, defaultValue, minValue, maxValue, units));
> +    }
> +
> +    AudioParam(const String& name, double defaultValue, double minValue, double maxValue, unsigned units = 0)
> +        : m_name(name)
> +        , m_value(defaultValue)
> +        , m_defaultValue(defaultValue)
> +        , m_minValue(minValue)
> +        , m_maxValue(maxValue)
> +        , m_units(units)
> +        , m_smoothedValue(defaultValue)
> +        , m_smoothingConstant(DefaultSmoothingConstant)
> +    {
> +    }
> +
> +    float value() const { return static_cast<float>(m_value); }
> +    void setValue(float value) { m_value = value; }
> +
> +    String name() const { return m_name; }
> +
> +    float minValue() const { return static_cast<float>(m_minValue); }
> +    float maxValue() const { return static_cast<float>(m_maxValue); }
> +    float defaultValue() const { return static_cast<float>(m_defaultValue); }
> +    unsigned units() const { return m_units; }

I'd add a FIXME here too about defining units.

> +
> +    // Value smoothing:
> +
> +    // When a new value is set with setValue(), in our internal use of the parameter we don't immediately jump to it.
> +    // Instead we smoothly approach this value to avoid glitching.
> +    float smoothedValue() const { return static_cast<float>(m_smoothedValue); }

These casts between floats and doubles are verbose. I think you should consider changing all of these implementing classes to use floats internally rather than doubles, and pass floats around in the API.

> +
> +    // Smoothly exponentially approaches to (de-zippers) the desired value.
> +    // Returns true if smoothed value has already snapped exactly to value.
> +    bool smooth()
> +    {
> +        if (m_smoothedValue == m_value) {
> +            // Smoothed value has already approached and snapped to value.
> +            return true;
> +        }
> +
> +        // Exponential approach
> +        m_smoothedValue += (m_value - m_smoothedValue) * m_smoothingConstant;
> +
> +        // If we get close enough then snap to actual value.
> +        if (fabs(m_smoothedValue - m_value) < SnapThreshold) // FIXME: the threshold needs to be adjustable depending on range - but this is OK general purpose value.
> +            m_smoothedValue = m_value;
> +
> +        return false;
> +    }
> +
> +    void resetSmoothedValue() { m_smoothedValue = m_value; }
> +    void setSmoothingConstant(double k) { m_smoothingConstant = k; }
> +
> +private:
> +    String m_name;
> +    double m_value;
> +    double m_defaultValue;
> +    double m_minValue;
> +    double m_maxValue;
> +    unsigned m_units;
> +
> +    // Smoothing (de-zippering)
> +    double m_smoothedValue;
> +    double m_smoothingConstant;
> +};
> +
> +} // namespace WebCore
> +
> +#endif // AudioParam_h
> Index: WebCore/webaudio/AudioParam.idl
> ===================================================================
> --- WebCore/webaudio/AudioParam.idl	(revision 0)
> +++ WebCore/webaudio/AudioParam.idl	(revision 0)
> @@ -0,0 +1,43 @@
> +/*
> + * 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.
> + */
> +
> +module webaudio {
> +    interface [
> +        Conditional=WEB_AUDIO
> +    ] AudioParam {
> +        attribute float value;
> +        readonly attribute float minValue;
> +        readonly attribute float maxValue;
> +        readonly attribute float defaultValue;
> +        
> +        readonly attribute DOMString name;
> +
> +        // FIXME: Could define units constants here (seconds, decibels, cents, etc.)...
> +        readonly attribute unsigned short units;
> +    };
> +}

[Chris Rogers]

Comment on attachment 66119 [details]
Patch

Clearing flags on attachment: 66119

Committed r66759: <http://trac.webkit.org/changeset/66759>

[Chris Rogers]

All reviewed patches have been landed.  Closing bug.