Bug 63586

Summary: Simplify AudioBufferSourceNode rendering
Product: WebKit Reporter: Chris Rogers <crogers>
Component: New BugsAssignee: Chris Rogers <crogers>
Status: RESOLVED FIXED    
Severity: Normal CC: kbr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch kbr: review+

Chris Rogers
Reported 2011-06-28 16:53:08 PDT
Simplify AudioBufferSourceNode rendering
Attachments
Patch (20.39 KB, patch)
2011-06-28 16:59 PDT, Chris Rogers 		no flags
DetailsFormatted DiffDiff
Patch (21.20 KB, patch)
2011-06-29 13:38 PDT, Chris Rogers 		kbr: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Rogers
Comment 1 2011-06-28 16:59:46 PDT
Created attachment 99003 [details] Patch
Kenneth Russell
Comment 2 2011-06-28 19:17:33 PDT
Comment on attachment 99003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99003&action=review Thanks for walking me through the code offline. Few comments as we discussed. > Source/WebCore/webaudio/AudioBufferSourceNode.cpp:121 > + size_t bufferFramesToProcess = framesToProcess - quantumFrameOffset; Missing zeroing from the beginning of the quantum to the quantumFrameOffset. > Source/WebCore/webaudio/AudioBufferSourceNode.cpp:246 > + // Final sanity check on buffer access. Should there be a FIXME to try to get rid of this check, and use assertions and guards outside the loop instead? > Source/WebCore/webaudio/AudioBufferSourceNode.h:54 > + void renderFromBuffer(AudioBus*, unsigned destinationFrameOffset, size_t numberOfFrames); This should be private.
Chris Rogers
Comment 3 2011-06-29 13:38:59 PDT
Created attachment 99140 [details] Patch
Chris Rogers
Comment 4 2011-06-29 13:43:37 PDT
Comment on attachment 99003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99003&action=review Also please note at line 250 of the new patch I've addressed the small clicks issue with the "pool" demo I discussed with you >> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:121 >> + size_t bufferFramesToProcess = framesToProcess - quantumFrameOffset; > > Missing zeroing from the beginning of the quantum to the quantumFrameOffset. FIXED: I've added the zeroing inside renderFromBuffer() >> Source/WebCore/webaudio/AudioBufferSourceNode.cpp:246 >> + // Final sanity check on buffer access. > > Should there be a FIXME to try to get rid of this check, and use assertions and guards outside the loop instead? Added FIXME >> Source/WebCore/webaudio/AudioBufferSourceNode.h:54 >> + void renderFromBuffer(AudioBus*, unsigned destinationFrameOffset, size_t numberOfFrames); > > This should be private. FIXED
Kenneth Russell
Comment 5 2011-06-29 14:08:54 PDT
Comment on attachment 99140 [details] Patch Looks great.
Chris Rogers
Comment 6 2011-06-29 14:20:20 PDT
Committed r90042: <http://trac.webkit.org/changeset/90042>
Note You need to log in before you can comment on or make changes to this bug.