[Webkit-unassigned] [Bug 93321] Exception should be thrown when connect() creates a cycle without DelayNode

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=93321





--- Comment #4 from Xingnan Wang <xingnan.wang at intel.com>  2012-08-14 01:47:04 PST ---
(In reply to comment #2)
> (From update of attachment 156833 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156833&action=review
> 
> Thanks for the nice patch to fix this issue.  Just a few small comments.
> 
> > Source/WebCore/Modules/webaudio/AudioNode.cpp:73
> > +bool AudioNode::checkCycle(AudioNode* startNode, AudioNode* endNode)
> 
> Can we make checkCycle more explicit.  Maybe checkForDelayFreeCycle?

Done.

> 
> Perhaps a short comment can also be added saying we're looking for a path between startNode and endNode that does not contain a delayNode.
>

Done.

> Also, have you considered what happens if the delayNode has 0 delay?  Convolvers could also have an arbitrarily long delay. (These are probably outside the scope of this fix.)
> 

That`s an interesting question. Will the sound grow gradually larger and larger and never end if delayNOde has 0 delay? Not sure what`s the result if a convolver in a cycle, I`d love to have a try.

But now in spec only delayNode is allowed in cycle, do we have more use-cases to use a convolver in a cycle?  

> > LayoutTests/webaudio/audionode-connect-cycle-expected.txt:1
> > +This tests that we should throw an exception if a cycle is created without DelayNode in it.
> 
> Typos:  "we should throw" -> "we throw" and "without DelayNode" -> "without a DelayNode"
>

Done.

> > LayoutTests/webaudio/audionode-connect-cycle-expected.txt:5
> > +PASS connect() is OK.
> 
> These aren't very informative.  Is there an easy way to provide a little more context?
>

Find a way to print the node name.

> > LayoutTests/webaudio/audionode-connect-cycle.html:43
> > +    
> 
> Is there any significance to the fact that the audio context length is 411 but the buffer length is 441?  Or did you intend both to be 0.01 sec long?

My fault. Both should be 441.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.