Bug 84639 – Re-factor scheduling logic from AudioBufferSourceNode into AudioScheduledSourceNode

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Chris Rogers]

Re-factor scheduling logic from AudioBufferSourceNode into AudioScheduledSourceNode

[Chris Rogers]

Created attachment 138434 [details]
Patch

[Build Bot]

Comment on attachment 138434 [details]
Patch

Attachment 138434 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12530041

[Chris Rogers]

Created attachment 138453 [details]
Patch

[Chris Rogers]

Fixed simple compile error from first patch caused by last minute cleanup.

[Chris Rogers]

Hi Eric and Jer, would one of you be able to look over this patch?  It's not as bad as it looks since it's essentially just cutting some code from AudioBufferSourceNode and putting it into a new base-class.  Thanks!

[Jer Noble]

Comment on attachment 138453 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138453&action=review

> Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h:-85
> -    void noteOn(double when);
>      void noteGrainOn(double when, double grainOffset, double grainDuration);
> -    void noteOff(double when);

Why wasn't noteGrainOn() included in this patch?  It appears that updateSchedulingInfo() does much of the same calculations as noteGrainOn().  Is there additional refactoring which could be done here?  It seems that it's concepts (i.e. mucking around with the m_playbackState variable) belong in AudioScheduledSourceNode and not in AudioBufferSourceNode.

I would suggest adding a pure virtual "double maxGrainDuration() const = 0;" function in AudioScheduledSourceNode.h, overridden by AudioBufferSourceNode as "buffer()->duration()", and use the result of that function to do the necessary calculations in noteGrainOn().

[Chris Rogers]

(In reply to comment #6)
> (From update of attachment 138453 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138453&action=review
> 
> > Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h:-85
> > -    void noteOn(double when);
> >      void noteGrainOn(double when, double grainOffset, double grainDuration);
> > -    void noteOff(double when);
> 
> Why wasn't noteGrainOn() included in this patch?  It appears that updateSchedulingInfo() does much of the same calculations as noteGrainOn().  Is there additional refactoring which could be done here?  It seems that it's concepts (i.e. mucking around with the m_playbackState variable) belong in AudioScheduledSourceNode and not in AudioBufferSourceNode.
> 
> I would suggest adding a pure virtual "double maxGrainDuration() const = 0;" function in AudioScheduledSourceNode.h, overridden by AudioBufferSourceNode as "buffer()->duration()", and use the result of that function to do the necessary calculations in noteGrainOn().

Hi Jer, the reason is because noteGrainOn() is a concept specific to AudioBufferSourceNodes, since it accesses parts of an AudioBuffer.  The Oscillator AudioSourceNode does not have this concept of noteGrainOn(), but only noteOn() and noteOff() for basic starting/stopping playback, and going through specific playback states.

[Chris Rogers]

Committed r115485: <http://trac.webkit.org/changeset/115485>

[Chris Rogers]

Thanks Eric!