Summary: | Lazy init for DefaultAudioDestinationNode? | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Raymond <rgbbones> | ||||
Component: | Web Audio | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | crogers, kbr, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Raymond
2012-01-17 18:29:40 PST
Sure, sounds reasonable to me. I had a quick look through the code and couldn't see any obvious problems. It would be good if you could quickly test your change on a couple of the web audio demos just as a sanity check. Created attachment 123427 [details]
Patch
Well, it's not that simple as I thought before. The sampleRate() function is called in the AudioContext constructor, which required the m_destination to be available. While, I think that sampleRate() function is already provided by AudioNode, so why not just use it, after all , the current AudioDestination implementation just return the same sampleRate passed in during construction. Thus AudioDestinationNode and AudioDestination share the same sampleRate. And if later we need to dynamic adjust sampleRate, we might set sample rate from AudioDestination to AudioDestinationNode. Anyway, the result is the patch attached Comment on attachment 123427 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123427&action=review > Source/WebCore/webaudio/DefaultAudioDestinationNode.cpp:-38 > - initialize(); I also considered to move this empty one to head file, but then I think again, due to it is a private function, and who knows we might add something here later. So I choose to leave it here. Comment on attachment 123427 [details]
Patch
If Chris is OK with this then r=me.
Looks good. Comment on attachment 123427 [details] Patch Clearing flags on attachment: 123427 Committed r108022: <http://trac.webkit.org/changeset/108022> All reviewed patches have been landed. Closing bug. |