Bug 76509

Summary: Lazy init for DefaultAudioDestinationNode?
Product: WebKit Reporter: Raymond <rgbbones>
Component: Web AudioAssignee: 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 Flags
Patch none

Description Raymond 2012-01-17 18:29:40 PST
Hi


In AudioContext's lazyInitialize m_destinationNode->initialize() is called, so is it necessary to call initialize in DefaultAudioDestinationNode's constructor? Will that help the lazyInitialize conception by not call it in DefaultAudioDestinationNode's constructor?

If so, I will be glad to provide this one line patch ;)
Comment 1 Chris Rogers 2012-01-19 11:28:16 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.
Comment 2 Raymond 2012-01-20 19:22:45 PST
Created attachment 123427 [details]
Patch
Comment 3 Raymond 2012-01-20 19:29:02 PST
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 4 Raymond 2012-01-20 19:32:20 PST
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 5 Eric Seidel (no email) 2012-02-16 13:29:39 PST
Comment on attachment 123427 [details]
Patch

If Chris is OK with this then r=me.
Comment 6 Chris Rogers 2012-02-16 15:04:23 PST
Looks good.
Comment 7 WebKit Review Bot 2012-02-16 20:37:07 PST
Comment on attachment 123427 [details]
Patch

Clearing flags on attachment: 123427

Committed r108022: <http://trac.webkit.org/changeset/108022>
Comment 8 WebKit Review Bot 2012-02-16 20:37:11 PST
All reviewed patches have been landed.  Closing bug.