Bug 45009 - Add AudioDestinationNode files
Summary: Add AudioDestinationNode 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-31 17:13 PDT by Chris Rogers
Modified: 2010-09-28 10:55 PDT (History)
8 users (show)

See Also:


Attachments
Patch (10.29 KB, patch)
2010-08-31 17:14 PDT, Chris Rogers
no flags Details | Formatted Diff | Diff
Patch (10.40 KB, patch)
2010-09-22 12:25 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-31 17:13:08 PDT
Add AudioDestinationNode files
Comment 1 Chris Rogers 2010-08-31 17:14:43 PDT
Created attachment 66145 [details]
Patch
Comment 2 Chris Rogers 2010-08-31 17:16:03 PDT
Implements AudioDestinationNode as described in the web audio specification:

http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioDestinationNode-section
Comment 3 Kenneth Russell 2010-09-09 14:42:05 PDT
Comment on attachment 66145 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66145&action=prettypatch

Generally looks fine; r- because of an inconsistency in the publicly visible attributes. Also a couple of minor suggestions.

> WebCore/webaudio/AudioDestinationNode.cpp:50
> +    initialize();
I certainly hope that AudioNode initializes the m_isInitialized flag to false in its constructor. It would be simpler if AudioNode were landed before this class which depends on it.

> WebCore/webaudio/AudioDestinationNode.cpp:71
> +    m_isInitialized = true;
I would suggest just calling AudioNode::initialize() here and making m_isInitialized private in AudioNode (assuming that's where it's declared).

> WebCore/webaudio/AudioDestinationNode.cpp:81
> +    m_isInitialized = false;
Same comment, but about calling AudioNode::uninitialize().

> WebCore/webaudio/AudioDestinationNode.idl:34
> +        readonly attribute short numberOfChannels;
This attribute isn't currently in the Web Audio spec.
Comment 4 Chris Rogers 2010-09-22 12:25:01 PDT
Created attachment 68416 [details]
Patch
Comment 5 Chris Rogers 2010-09-22 12:27:33 PDT
Ken, I've created a new patch to address some changes to AudioContext.  I've added the "numberOfChannels" attribute to the web audio specification document.  I spoke with you offline about keeping the initialize/uninitialize part as it is.  But I *did* add addInput() and addOutput() methods to AudioNode.
Comment 6 Kenneth Russell 2010-09-28 10:06:50 PDT
Comment on attachment 68416 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=68416&action=review

Looks fine. Tiny formatting issue.

> WebCore/webaudio/AudioDestinationNode.h:54
> +    virtual void reset() { m_currentTime = 0.0; };

No semicolons at end of lines.
Comment 7 WebKit Commit Bot 2010-09-28 10:55:32 PDT
Comment on attachment 68416 [details]
Patch

Clearing flags on attachment: 68416

Committed r68540: <http://trac.webkit.org/changeset/68540>
Comment 8 WebKit Commit Bot 2010-09-28 10:55:38 PDT
All reviewed patches have been landed.  Closing bug.