865233 - Implement the ended event for AudioBufferSourceNode

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[(no longer active)]

I need to write the spec for it first...

Comment 1

[Jean-Yves Perrier [:teoli]]

Naive question: is 'finished' the appropriate name? WebRTC MediaStream fire an 'ended' event at the end of the stream.

Comment 2

[(no longer active)]

(In reply to comment #1)
> Naive question: is 'finished' the appropriate name? WebRTC MediaStream fire an
> 'ended' event at the end of the stream.

Hmm, finished is what we talked about on public-audio, please feel free to propose ended there.

Comment 3

[(no longer active)]

On the Audio WG call today, people were open to the idea of using the name "ended".  :-)

Comment 4

[(no longer active)]

Attachment: Patch (v1)

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 745581 [details] [diff] [review]
Patch (v1)

Review of attachment 745581 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/AudioBufferSourceNode.cpp
@@ +560,5 @@
>  void
>  AudioBufferSourceNode::NotifyMainThreadStateChanged()
>  {
>    if (mStream->IsFinished()) {
> +    DispatchTrustedEvent(NS_LITERAL_STRING("ended"));

This runs during a stable state and is not a safe time to run script. Please dispatch this asynchronously.

Comment 6

[(no longer active)]

Attachment: Patch (v2)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 745621 [details] [diff] [review]
Patch (v2)

Review of attachment 745621 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/AudioBufferSourceNode.cpp
@@ +569,5 @@
> +        : mNode(aNode) {}
> +      NS_IMETHODIMP Run()
> +      {
> +        // If it's not safe to run scripts right now, schedule this to run later
> +        if (!nsContentUtils::IsSafeToRunScript()) {

This check is unnecessary. It's always safe to run script from a main-thread runnable.

Comment 8

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> Comment on attachment 745621 [details] [diff] [review]
> Patch (v2)
> 
> Review of attachment 745621 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/webaudio/AudioBufferSourceNode.cpp
> @@ +569,5 @@
> > +        : mNode(aNode) {}
> > +      NS_IMETHODIMP Run()
> > +      {
> > +        // If it's not safe to run scripts right now, schedule this to run later
> > +        if (!nsContentUtils::IsSafeToRunScript()) {
> 
> This check is unnecessary. It's always safe to run script from a main-thread
> runnable.

Nope, since they may run from nested event loops!  I actually hit an assertion about this when I was debugging my ScriptProcessorNode implementation using alert()s.

Comment 9

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb926cf3a068

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fb926cf3a068

Comment 11

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.