Bug 46291 - Add AudioChannelMerger files
Summary: Add AudioChannelMerger 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-09-22 12:15 PDT by Chris Rogers
Modified: 2010-09-29 17:55 PDT (History)
8 users (show)

See Also:

Attachments
Patch (9.62 KB, patch)
2010-09-22 12:16 PDT, Chris Rogers 		no flags Details | Formatted Diff | Diff
Patch (9.38 KB, patch)
2010-09-28 16:13 PDT, Chris Rogers 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Rogers 2010-09-22 12:15:24 PDT 
Add AudioChannelMerger files
Comment 1 Chris Rogers 2010-09-22 12:16:16 PDT 
Created attachment 68415 [details]
Patch
Comment 2 Chris Rogers 2010-09-22 12:18:51 PDT 
This is the implementation for AudioChannelMerger as described in the web audio API:
http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioChannelMerger-section
Comment 3 Kenneth Russell 2010-09-28 10:36:14 PDT 
Comment on attachment 68415 [details]
Patch

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

Looks good overall, but r- for similar issues as in AudioChannelSplitter.

> WebCore/webaudio/AudioChannelMerger.cpp:40
> +// This is considering that 5.1 (6 channels is the largest we'll ever deal with).

Same issue with comment as in AudioChannelSplitter.

> WebCore/webaudio/AudioChannelMerger.cpp:86
> +                outputChannelIndex++;

Use preincrement.

> WebCore/webaudio/AudioChannelMerger.cpp:98
> +void AudioChannelMerger::initialize()

Refactor these into AudioNode.
Comment 4 Chris Rogers 2010-09-28 16:13:15 PDT 
Created attachment 69125 [details]
Patch
Comment 5 Chris Rogers 2010-09-28 16:14:05 PDT 
Comment on attachment 68415 [details]
Patch

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

>> WebCore/webaudio/AudioChannelMerger.cpp:40
>> +// This is considering that 5.1 (6 channels is the largest we'll ever deal with).
> 
> Same issue with comment as in AudioChannelSplitter.

FIXED

>> WebCore/webaudio/AudioChannelMerger.cpp:86
>> +                outputChannelIndex++;
> 
> Use preincrement.

FIXED

>> WebCore/webaudio/AudioChannelMerger.cpp:98
>> +void AudioChannelMerger::initialize()
> 
> Refactor these into AudioNode.

FIXED
Comment 6 Kenneth Russell 2010-09-29 15:14:09 PDT 
Comment on attachment 69125 [details]
Patch

Looks good.
Comment 7 WebKit Commit Bot 2010-09-29 17:55:13 PDT 
Comment on attachment 69125 [details]
Patch

Clearing flags on attachment: 69125

Committed r68732: <http://trac.webkit.org/changeset/68732>
Comment 8 WebKit Commit Bot 2010-09-29 17:55:19 PDT 
All reviewed patches have been landed.  Closing bug.