Bug 76509 - Lazy init for DefaultAudioDestinationNode?
Summary: Lazy init for DefaultAudioDestinationNode?
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2012-01-17 18:29 PST by Raymond
Modified: 2012-02-16 20:37 PST (History)
3 users (show)

See Also:

Patch (5.04 KB, patch)
2012-01-20 19:22 PST, Raymond
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raymond 2012-01-17 18:29:40 PST

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]
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]

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]

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]

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.