Bug 45206 - Add AudioProcessor.h
Summary: Add AudioProcessor.h
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-09-03 15:18 PDT by Chris Rogers
Modified: 2010-09-09 12:52 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.81 KB, patch)
2010-09-03 15:20 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff
Patch (3.86 KB, patch)
2010-09-07 13:17 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-09-03 15:18:53 PDT
Add AudioProcessor.h
Comment 1 Chris Rogers 2010-09-03 15:20:32 PDT
Created attachment 66556 [details]
Patch
Comment 2 chris fleizach 2010-09-03 20:03:14 PDT
Comment on attachment 66556 [details]
Patch

>          Reviewed by Darin Adler.
> Index: WebCore/platform/audio/AudioProcessor.h
> ===================================================================
> --- WebCore/platform/audio/AudioProcessor.h	(revision 0)
> +++ WebCore/platform/audio/AudioProcessor.h	(revision 0)
> @@ -0,0 +1,73 @@
> +/*
> + * 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.
> + */
> +

wrong license
http://trac.webkit.org/browser/trunk/WebKit/LICENSE


> +#ifndef AudioProcessor_h
> +#define AudioProcessor_h
> +
> +namespace WebCore {
> +
> +class AudioBus;
> +
> +// AudioProcessor is an abstract base class which can be used as one part of a complex DSP algorithm.
> +// Or, it can be used as the processor for a basic (one input - one output) AudioNode.
> +// It's assumed that the number of input channels equals the number of output channels.
> +

if it's assumed, then you should say "output must equal input" so there's no confusion

> +class AudioProcessor {
> +public:
> +    AudioProcessor(double sampleRate)
> +        : m_initialized(false)
> +        , m_sampleRate(sampleRate)
> +    {
> +    }
> +
> +    virtual ~AudioProcessor() { }
> +
> +    // Full initialization can be done here instead of in the constructor.
> +    virtual void initialize() = 0;
> +    virtual void uninitialize() = 0;
> +

Why do you want initialize and uninitialize methods instead of using constructor/destructor?

> +    virtual void setNumberOfChannels(unsigned numberOfChannels) = 0;
> +

You don't need to name the argument here, it is clear from the function name.

> +    bool isInitialized() { return m_initialized; }
> +

can this be const?
Comment 3 Chris Rogers 2010-09-07 13:17:43 PDT
Created attachment 66756 [details]
Patch
Comment 4 Chris Rogers 2010-09-07 13:20:37 PDT
Hi Chris, thanks for having a look.  The license I changed slightly to be the one we use for Google contributed code (please see similar copyright notices in WebCore/workers, WebCore/websockets, and other places).

I tried to address all of your other comments.
Comment 5 Chris Rogers 2010-09-07 13:26:05 PDT
The reason I have explicit initialize() and uninitialize() methods is to follow the pattern used in Apple's AudioUnit plugin architecture where there is an explicit AudioUnitInitialize() and AudioUnitUninitialize().  Because audio processing objects can allocate significant resources, it is useful to be able to be able to have access to an object without it being in a fully initialized state.  There are useful things which can be done with these objects without them being fully initialized.
Comment 6 chris fleizach 2010-09-09 00:35:53 PDT
Comment on attachment 66756 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2010-09-09 12:52:54 PDT
Comment on attachment 66756 [details]
Patch

Clearing flags on attachment: 66756

Committed r67106: <http://trac.webkit.org/changeset/67106>
Comment 8 WebKit Commit Bot 2010-09-09 12:52:59 PDT
All reviewed patches have been landed.  Closing bug.