Skip to content

Fix #2248: Clean up ABSN start algorithm - #2286

Merged
rtoy merged 5 commits into
WebAudio:mainfrom
rtoy:2248-absn-start-message
Mar 8, 2021
Merged

rtoy merged 5 commits into
WebAudio:mainfrom
rtoy:2248-absn-start-message

Conversation

@rtoy

@rtoy rtoy commented Dec 16, 2020 •

Copy link
Copy Markdown
Member

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.


Preview | Diff

Raymond Toy added 2 commits December 16, 2020 13:01
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.
Make the algorithms ABSN.start and AudioScheduledSourceNode.start use the
same text, more or less.  This means:

- Use the `[[source started]]` flag in ABNS, and add an additional
  step in ABSN to set the flag, just like for ASSN.
- Rephrase the last step to mention more clearly we're sending a
  message to start the context when possible.
- Add a NOTE to the ABSN step to state what this is meant to do
- Update the last step in ASSN to use the same algorithm as ABSN since
  these sources are all supposed to start a context that is currently
  suspended by autoplay
@rtoy

rtoy commented Dec 17, 2020

Copy link
Copy Markdown
Member Author

My original change is wrong and I've updated it with more text and more changes including the following (as noted in the commit):

Make the algorithms ABSN.start and AudioScheduledSourceNode.start use the same text, more or less. This means:

  • Use the [[source started]] flag in ABSN, and add an additional step in ABSN to set the flag, just like for ASSN.
  • Rephrase the last step to mention more clearly we're sending a message to start the context when possible.
  • Add a NOTE to the ABSN step to state what this is meant to do
  • Update the last step in ASSN to use the same algorithm as ABSN since these sources are all supposed to start a context that is currently suspended by autoplay

Add quotes around the state because the state is a enum (string).
@padenot
padenot self-requested a review January 7, 2021 17:16
@rtoy
rtoy requested a review from hoch March 3, 2021 22:48

@hoch hoch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
@rtoy
rtoy merged commit d360708 into WebAudio:main Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants