WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 76772
Clean up m_processLock logic in AudioBasicProcessorNode and AudioGainNode
https://bugs.webkit.org/show_bug.cgi?id=76772
Summary
Clean up m_processLock logic in AudioBasicProcessorNode and AudioGainNode
Raymond
Reported
2012-01-20 23:07:25 PST
Clean up m_processLock logic in AudioBasicProcessorNode and AudioGainNode
Attachments
Patch
(5.73 KB, patch)
2012-01-20 23:13 PST
,
Raymond
no flags
Details
Formatted Diff
Diff
Patch
(7.80 KB, patch)
2012-01-29 18:22 PST
,
Raymond
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Raymond
Comment 1
2012-01-20 23:13:52 PST
Created
attachment 123435
[details]
Patch
Raymond
Comment 2
2012-01-20 23:24:22 PST
Comment on
attachment 123435
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=123435&action=review
> Source/WebCore/webaudio/AudioBasicProcessorNode.cpp:-128 > - // We need to be careful since we may be actively processing right now, so synchronize with process(). > - MutexLocker locker(m_processLock);
In AudioGainNode, all m_processLock related code is removed since no one use it anymore. While for AudioBasicProcessorNode, only remove the lock here, while reserve the lock mechanism in process(), since subclass might do some thing in main thread. ( Though, actually it seems to me they do not at present.)
Chris Rogers
Comment 3
2012-01-25 14:58:35 PST
(In reply to
comment #2
)
> (From update of
attachment 123435
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=123435&action=review
> > > Source/WebCore/webaudio/AudioBasicProcessorNode.cpp:-128 > > - // We need to be careful since we may be actively processing right now, so synchronize with process(). > > - MutexLocker locker(m_processLock); > > In AudioGainNode, all m_processLock related code is removed since no one use it anymore. While for AudioBasicProcessorNode, only remove the lock here, while reserve the lock mechanism in process(), since subclass might do some thing in main thread. ( Though, actually it seems to me they do not at present.)
Raymond, thanks for the patch. I think we should remove the lock in AudioBasicProcessorNode::process() as well. m_processLock is private, so no sub-class can do anything with it. Also, all changes should now happen with the AudioContext lock in the pre or post-render tasks anyway so we shouldn't need it. Does that make sense?
Raymond
Comment 4
2012-01-29 18:22:49 PST
Created
attachment 124481
[details]
Patch
Chris Rogers
Comment 5
2012-01-31 16:45:06 PST
Thanks Raymond, looks good.
Kenneth Russell
Comment 6
2012-01-31 17:09:29 PST
Comment on
attachment 124481
[details]
Patch rs=me
WebKit Review Bot
Comment 7
2012-01-31 19:05:04 PST
Comment on
attachment 124481
[details]
Patch Clearing flags on attachment: 124481 Committed
r106420
: <
http://trac.webkit.org/changeset/106420
>
WebKit Review Bot
Comment 8
2012-01-31 19:05:08 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