RESOLVED FIXED 109599
Synchronize setting of panner node model and processing
https://bugs.webkit.org/show_bug.cgi?id=109599
Summary Synchronize setting of panner node model and processing
Raymond Toy
Reported 2013-02-12 11:01:51 PST
Synchronize setting of panner node model and processing
Attachments
Patch (2.84 KB, patch)
2013-02-12 11:18 PST, Raymond Toy
no flags
Patch (3.58 KB, patch)
2013-02-12 13:36 PST, Raymond Toy
no flags
Raymond Toy
Comment 1 2013-02-12 11:18:02 PST
Chris Rogers
Comment 2 2013-02-12 13:25:15 PST
Comment on attachment 187903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187903&action=review some nits - looks good overall > Source/WebCore/Modules/webaudio/PannerNode.cpp:115 > + } you need an else clause similar (but not identical) to AudioBufferSourceNode: } else { // Too bad - the tryLock() failed. We must be in the middle of changing buffers and were already outputting silence anyway. outputBus->zero(); } > Source/WebCore/Modules/webaudio/PannerNode.cpp:192 > + // This synchronizes with process() nit: period at end of sentence > Source/WebCore/Modules/webaudio/PannerNode.h:158 > + // Synchronize process() and setPanningModel() which can change the panner nit: period at end of sentence
Raymond Toy
Comment 3 2013-02-12 13:36:41 PST
Raymond Toy
Comment 4 2013-02-12 13:37:56 PST
(In reply to comment #2) > (From update of attachment 187903 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=187903&action=review > > some nits - looks good overall > > > Source/WebCore/Modules/webaudio/PannerNode.cpp:115 > > + } > > you need an else clause similar (but not identical) to AudioBufferSourceNode: > > } else { > // Too bad - the tryLock() failed. We must be in the middle of changing buffers and were already outputting silence anyway. > outputBus->zero(); > } > > > Source/WebCore/Modules/webaudio/PannerNode.cpp:192 > > + // This synchronizes with process() > > nit: period at end of sentence > > > Source/WebCore/Modules/webaudio/PannerNode.h:158 > > + // Synchronize process() and setPanningModel() which can change the panner > > nit: period at end of sentence Fixed according to your comments.
Chris Rogers
Comment 5 2013-02-12 14:35:55 PST
Comment on attachment 187922 [details] Patch thanks!
WebKit Review Bot
Comment 6 2013-02-12 14:44:04 PST
Comment on attachment 187922 [details] Patch Rejecting attachment 187922 [details] from commit-queue. rtoy@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
WebKit Review Bot
Comment 7 2013-02-12 16:12:23 PST
Comment on attachment 187922 [details] Patch Clearing flags on attachment: 187922 Committed r142687: <http://trac.webkit.org/changeset/142687>
WebKit Review Bot
Comment 8 2013-02-12 16:12:27 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.