Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ABSN.start sends a control message on a suspended context? #2248

Closed
rtoy opened this issue Sep 17, 2020 · 7 comments
Closed

ABSN.start sends a control message on a suspended context? #2248

rtoy opened this issue Sep 17, 2020 · 7 comments

Comments

@rtoy
Copy link
Member

rtoy commented Sep 17, 2020

Where Is It

Describe the issue
In the algorithm for start(), step 4.1 says the control message is sent to the rendering thread only if the control thread state is suspended. And control thread state says the state corresponds to the states of an AudioContext state.

Do we really mean that the message is only sent if the context is suspended? Should this not say running instead? Or maybe the state isn't the context state? Or maybe it should say something altogether different?

@rtoy
Copy link
Member Author

rtoy commented Sep 17, 2020

Also, the algorithm step 2 says check for errors. This differs from the algorithm for AudioScheduledSourceNode.start() which has step 2 aborting the steps if any exception is thrown. We should update this for ABSN.

@rtoy
Copy link
Member Author

rtoy commented Sep 17, 2020

Oops. Wrong repo. This is v1, I think.

@rtoy rtoy transferred this issue from WebAudio/web-audio-api-v2 Sep 17, 2020
@rtoy
Copy link
Member Author

rtoy commented Sep 24, 2020

Teleconf: Simple typo that should be fixed.

@rtoy
Copy link
Member Author

rtoy commented Sep 24, 2020

Do we need to make the distinction about the context state at all? Perhaps the messages should always be sent, letting the context deal with them appropriately?

@rtoy
Copy link
Member Author

rtoy commented Oct 2, 2020

I'm confused by the conditions for sending a control message from start() for either an ABSN or other scheduled source. The text says:

  1. Send a control message to the associated AudioContext to run it in the rendering thread only when the following conditions are met:

    1. The context’s control thread state is suspended.
    2. The context is allowed to start.
    3. [[suspended by user]] flag is false.

Based on these conditions, if I explicitly called context.suspend(), then any calls to start() don't send messages. When the context is resumed, none of the sources play because the control message wasn't sent.

I find this unexpected. I expect all of the sources to start playing either immediately or eventually if they were scheduled to start in the future.

If this interpretation is correct, I think the message should always be sent, implying we can remove all of the conditions.

@rtoy
Copy link
Member Author

rtoy commented Oct 2, 2020

I'm further confused. Step 3 says:

Queue a control message to start the AudioBufferSourceNode, including the parameter values in the messsage.

(Note to self: Fix typo in "messsage".)

Step 5 says we send a control message to the audio thread to run it (under some conditions). But presumably, the message in 3 is eventually delivered to the audio thread, so we tell the audio thread to start the source twice. That seems wrong.

Do we really want step 5 to send directly the message to the audio thread? Shouldn't this be handled in the normal processing of the control message queue?

rtoy pushed a commit to rtoy/web-audio-api that referenced this issue Dec 16, 2020
Remove step 5; step 3 already queues a control message to start the
buffer source so we don't need a step 5 to send another.  Plus step 5
sends another control message under certaiin conditions.  These
conditions aren't useful since the start message should always be
sent.  The context will handle them in the right way when it is
resumed or already running.
@rtoy
Copy link
Member Author

rtoy commented Dec 16, 2020

I think I was misunderstanding what step 5 was really trying to do. I think this step is basically to allow start() to allow the context to start running if start() were called from an user gesture. I think to make it clearer, we might want to steal a bit from resume() because that's basically what this step is intended to do: resume a context that has been suspended by the autoplay policy.'

May also want to add a Note that this allows calling start() from a user gesture to start the context that is otherwise suspended.

@rtoy rtoy closed this as completed in d360708 Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant