Bug 44705 - Add audio distance effect files
Summary: Add audio distance effect files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-26 12:04 PDT by Chris Rogers
Modified: 2010-09-03 11:33 PDT (History)
6 users (show)

See Also:


Attachments
Patch (7.57 KB, patch)
2010-08-26 12:11 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff
Patch (7.53 KB, patch)
2010-08-30 15:31 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Rogers 2010-08-26 12:04:41 PDT
Add audio distance effect files
Comment 1 Chris Rogers 2010-08-26 12:11:34 PDT
Created attachment 65595 [details]
Patch
Comment 2 Chris Rogers 2010-08-26 12:38:39 PDT
For use with the web audio API, these files implement the distance rolloff models as defined in OpenAL.
Comment 3 Kenneth Russell 2010-08-30 14:12:24 PDT
Comment on attachment 65595 [details]
Patch

Generally looks good, but a few minor issues.

> Index: WebCore/platform/audio/Distance.cpp
> ===================================================================
> --- WebCore/platform/audio/Distance.cpp	(revision 0)
> +++ WebCore/platform/audio/Distance.cpp	(revision 0)
> @@ -0,0 +1,90 @@
> +/*
> + * 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"
> +
> +#if ENABLE(WEB_AUDIO)
> +
> +#include "Distance.h"
> +
> +#include <math.h>
> +
> +namespace WebCore {
> +
> +DistanceEffect::DistanceEffect()
> +    : m_model(ModelInverse)
> +    , m_isClamped(true)
> +    , m_refDistance(1.0)
> +    , m_maxDistance(10000.0)
> +    , m_rolloffFactor(1.0)
> +{
> +}
> +
> +double DistanceEffect::gain(double distance)
> +{
> +    // don't go beyond maximum distance
> +    distance = distance < m_maxDistance ? distance : m_maxDistance;

Write this as std::min(distance, m_maxDistance). Requires #include <algorithm>.

> +    // if clamped, don't get closer than reference distance
> +    if (m_isClamped)
> +        distance = distance > m_refDistance ? distance : m_refDistance;

Similarly, std::max(distance, m_refDistance).

> +    switch (m_model) {
> +    case ModelLinear:
> +        return linearGain(distance);
> +        break;
> +    case ModelInverse:
> +        return inverseGain(distance);
> +        break;
> +    case ModelExponential:
> +        return exponentialGain(distance);
> +        break;
> +
> +    default:
> +        return 0.0;
> +    }
> +}
> +
> +double DistanceEffect::linearGain(double distance)
> +{
> +    return (1.0 - m_rolloffFactor * (distance - m_refDistance)) / (m_maxDistance - m_refDistance);
> +}
> +
> +double DistanceEffect::inverseGain(double distance)
> +{
> +    return m_refDistance / (m_refDistance + m_rolloffFactor * (distance - m_refDistance));
> +}
> +
> +double DistanceEffect::exponentialGain(double distance)
> +{
> +    return pow(distance / m_refDistance, -m_rolloffFactor);
> +}
> +
> +} // namespace WebCore
> +
> +#endif // ENABLE(WEB_AUDIO)
> Index: WebCore/platform/audio/Distance.h
> ===================================================================
> --- WebCore/platform/audio/Distance.h	(revision 0)
> +++ WebCore/platform/audio/Distance.h	(revision 0)
> @@ -0,0 +1,80 @@
> +/*
> + * 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 Distance_h
> +#define Distance_h
> +
> +namespace WebCore {
> +
> +// Distance models are defined according to the OpenAL specification
> +
> +class DistanceEffect {
> +public:
> +    enum {
> +        ModelLinear = 0,
> +        ModelInverse = 1,
> +        ModelExponential = 2
> +    };

This enum should have a name, for example ModelType.

> +    DistanceEffect();
> +
> +    // Returns scalar gain for the given distance the current distance model is used
> +    double gain(double distance);
> +
> +    int model() { return m_model; }

Should return enum name (e.g. ModelType).

> +    void setModel(int model, bool clamped)
> +    {
> +        m_model = model;
> +        m_isClamped = clamped;
> +    }

Should take e.g. ModelType as first argument.

> +    // Distance params
> +    void setRefDistance(double refDistance) { m_refDistance = refDistance; }
> +    void setMaxDistance(double maxDistance) { m_maxDistance = maxDistance; }
> +    void setRolloffFactor(double rolloffFactor) { m_rolloffFactor = rolloffFactor; }
> +
> +    double refDistance() const { return m_refDistance; }
> +    double maxDistance() const { return m_maxDistance; }
> +    double rolloffFactor() const { return m_rolloffFactor; }
> +
> +protected:
> +    double linearGain(double distance);
> +    double inverseGain(double distance);
> +    double exponentialGain(double distance);
> +
> +    int m_model;

Should use enum type.

> +    bool m_isClamped;
> +    double m_refDistance;
> +    double m_maxDistance;
> +    double m_rolloffFactor;
> +};
> +
> +} // namespace WebCore
> +
> +#endif // Distance_h
Comment 4 Chris Rogers 2010-08-30 15:31:27 PDT
Created attachment 65970 [details]
Patch
Comment 5 Chris Rogers 2010-09-01 12:10:52 PDT
Addressed all comments with last patch.
Comment 6 Kenneth Russell 2010-09-02 16:13:03 PDT
Comment on attachment 65970 [details]
Patch

Looks good; r=me.
Comment 7 WebKit Commit Bot 2010-09-02 18:01:10 PDT
Comment on attachment 65970 [details]
Patch

Rejecting patch 65970 from commit-queue.

crogers@google.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 8 Chris Rogers 2010-09-03 11:33:15 PDT
Comment on attachment 65970 [details]
Patch

Clearing flags on attachment: 65970

Committed r66745: <http://trac.webkit.org/changeset/66745>
Comment 9 Chris Rogers 2010-09-03 11:33:19 PDT
All reviewed patches have been landed.  Closing bug.