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

resume audio context [WIP] #283

Closed
wants to merge 4 commits into from
Closed

Conversation

therewasaguy
Copy link
Member

@therewasaguy therewasaguy commented May 29, 2018

Add an example of how it would work

function draw() {
  background(255);
  textAlign(CENTER);

  var audioContext = getAudioContext();
  if (audioContext.state !== 'running') {
    text('click to start audio', width/2, height/2);
  } else {
    text('audio is enabled', width/2, height/2);
  }
}
function touchStarted() {
  startAudioContext();
}

...but if (getAudioContext.state !== 'running') is a bit clunky. It might make more sense to expose a wrapper for the audio context that returns a boolean for running, rather than adding methods like getAudioContext startAudioContext etc

@JunShern
Copy link
Contributor

JunShern commented Jun 1, 2018

Hey @therewasaguy , thanks for doing this!

Seems like a reasonable approach, but I'm slightly hesitant about that code example you gave. It would be quite bothersome for any developer (will every single one of our examples, even those in the documentation, need to have this?), and quite a high barrier in situations where we might want to introduce p5-sound to someone new to programming. Imagine having to explain (1) The AudioContext, (2) How to check if it's already running, (3) How to check for a touch event, all before they get their hands on anything fun.

I wonder what the alternative would be like, maybe if we tried to encapsulate the whole context-starting issue into a userInitSound function or something like that. The idea is that we then only need to have

function setup() {
  userInitSound(); // This is a blocking statement, waits until the user has touched the screen to start
}

Which could place a click-to-start menu over the entire canvas, blocking execution of the sketch until after the user has clicked in. I feel like this is quite an acceptable UI convention, for example:

codepen-click-to-load

Of course, we would need to handle noCanvas separately, perhaps with an alert()? And then of course we can handle that click-in event in isolation, so the user doesn't have to think about touchStarted() events unless he/she wants to.

This would save the amount of boilerplate necessary to get started, and as far as I can see the only downside (compared to the exposed alternative) is that the default touch-to-start menu might not be in the style that the developer wants. In which case the exposed version would still be an option for users who wish to customize the interaction.

What do you think?

@datramt
Copy link

datramt commented Jun 1, 2018

On one hand, I think it’s important to keep exposed two aspects fo the issue:

  1. an event listener is necessary in order to have any audio in any sketch work on the web
  2. the audio context needs to be resumed.

and that is what @therewasaguy 's proposal does with

function touchStarted() { 
  startAudioContext();
}

I like this because it really does seem to me that resuming audio contexts is a part of web audio life starting now, and that it’s the dev’s responsibility to structure sketches exactly like the codepen embed example with the blocking statement, and new programmers ought to learn about this newly imposed convention.

That being said, I'm also in favor for abstracting away multiple lines of code that I would have to write out for every single sketch, so...

Something like Yotam Mann's startAudioContext https://github.com/tambien/StartAudioContext seems like a nice compromise between both of your proposals. Plus I really like that it removes event listeners when resumed. I can easily see this sort of thing implemented in p5 and achieved using one function such as startAudioContext() or indeed userInitSound(), with optional arguments:

StartAudioContext(AudioContext, (Elements), (Callback)) => Promise

I personally think it’s more in the spirit of p5 to have devs make their own visual block or alert, rather than relying on a pre-built one. with startAudioContext you can define a callback to easily do that, while also eliminating the need of checking the audiocontext state.

@therewasaguy
Copy link
Member Author

good points all! Some good news: The changes have been rolled back in Chrome 66, and are now postponed until Chrome 70 (October 2018). Definitely want to get this resolved soon, though!

@JunShern I like the idea of a library default, but many users will include p5.sound without actually using it, so it might get in the way. The CodePen example is good inspiration for our documentation, though—maybe we can put something like this in a shared library that we use in all example sketches without distracting from the primary goal of each sketch.

function setup() {
userInitSound(); // This is a blocking statement, waits until the user has touched the screen to start
}

This is sort of possible with async/await, as long as userInitSound returns a Promise. Here's an example.

I do like Yotam Mann's StartAudioContext, @datramt. Maybe we should just add that as a dependency, perhaps with a wrapper so that users don't need to think about the audio context. I like how it returns a Promise and offers a callback. With the callback, I think this could be used in p5's preload(), where if the user doesn't provide a callback, setup() becomes the callback...

@therewasaguy
Copy link
Member Author

I'm going to change this PR to just be the refactor-y stuff, and will resume the "resume AudioContext" stuff in another PR.

It'll help to have Chrome Canary version 70 (they're currently at 69). Or, if someone happens to have a copy of Chrome 66 with the sneak peak at their upcoming autoplay policy, lmk!

@therewasaguy therewasaguy changed the title [WIP] separate the audiocontext module and add a startAudioContext method separate the audiocontext module Jun 15, 2018
@therewasaguy therewasaguy changed the title separate the audiocontext module resume audio context [WIP] Jun 15, 2018
@therewasaguy therewasaguy mentioned this pull request Sep 25, 2018
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.

3 participants