WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(3.58 KB, patch)
2013-02-12 13:36 PST
,
Raymond Toy
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Raymond Toy
Comment 1
2013-02-12 11:18:02 PST
Created
attachment 187903
[details]
Patch
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
Created
attachment 187922
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug