Summary: | JavaScriptAudioNode should not ASSERT if number of input channels is 0 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Raymond Toy <rtoy> | ||||||
Component: | Web Audio | Assignee: | Raymond Toy <rtoy> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | crogers, eric.carlson, feature-media-reviews, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 84743 | ||||||||
Attachments: |
|
Description
Raymond Toy
2012-05-07 12:50:06 PDT
Created attachment 140592 [details]
Patch
Comment on attachment 140592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140592&action=review Hi Ray, thanks very much for the fix and layout test. Just a few nits: > LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:7 > +<title>JavaScriptAudioNode</title> <title> line should be removed > LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:35 > + var context = new webkitAudioContext(1, 44100, 44100); Best not to use hard-coded values here, and instead use the exact same pattern as the other JavaScriptAudioNode tests such as "javascriptaudionode.html" > LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:36 > + var node = context.createJavaScriptNode(4096, 0, 2); Please put this creation in a try/catch block to check for exceptions here > LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:40 > + Please add short comment explaining that the onaudioprocess function is doing nothing since we simply want to make sure that the processing logic in the implementation can handle 0 input channels and don't care about doing any processing. You might want to add a FIXME explaining that we could check the .inputBuffer attribute of the AudioProcessingEvent appropriately (don't need to add that part right now). Created attachment 140753 [details]
Patch
Comment on attachment 140592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140592&action=review >> LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:7 >> +<title>JavaScriptAudioNode</title> > > <title> line should be removed Done. >> LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:35 >> + var context = new webkitAudioContext(1, 44100, 44100); > > Best not to use hard-coded values here, and instead use the exact same pattern as the other JavaScriptAudioNode tests such as "javascriptaudionode.html" Done. >> LayoutTests/webaudio/javascriptaudionode-zero-input-channels.html:40 >> + > > Please add short comment explaining that the onaudioprocess function is doing nothing since we simply want to make sure that the processing logic in the implementation can handle 0 input channels and don't care about doing any processing. > You might want to add a FIXME explaining that we could check the .inputBuffer attribute of the AudioProcessingEvent appropriately (don't need to add that part right now). Comment and FIXME added. Comment on attachment 140753 [details]
Patch
Seems reasonable.
Comment on attachment 140753 [details] Patch Clearing flags on attachment: 140753 Committed r116465: <http://trac.webkit.org/changeset/116465> All reviewed patches have been landed. Closing bug. |