p5.sound 0.3.12

[therewasaguy]

Hello! We have an update to p5 sound library:

• AudioWorklet replaces the deprecated ScriptProcessorNode (when available) as part of @oshoham's GSoC project https://github.com/processing/p5.js/blob/master/developer_docs/project_wrapups/orenshoham_gsoc_2019.md
• remove bad console call (Remove bad console call p5.js-sound#378)
• p5 library compiled with webpack so it looks a little different and we had to remove comments that were tripping up YUIDoc here in the p5.js repo

BUT it is currently blocked by https://github.com/processing/p5.js-sound/issues/382: AudioWorklet preload conflicts with example 'render' script

AudioWorklet modules need to be loaded asynchronously, and this poses a potential problem if they have not finished loading by the time setup is called and we need to use them.

In processing/p5.js-sound#369, @oshoham came up with a really innovative solution to tap into p5's preload system to load the audioworklet modules, ensuring they have loaded before setup is called.

This is clashing with the way documentation example sketches are rendered for the p5 website. The website uses a custom render script that looks for lifecycle methods like "setup" and "preload" and hoists them if they exists, otherwise wraps the example code in a setup function (so that examples don't all need to have this boilerplate and can instead focus on the thing they are trying to document/exemplify). On this line in Chrome and Firefox https://github.com/processing/p5.js/blob/master/docs/yuidoc-p5-theme/assets/js/render.js#L222 and on this line in Safari https://github.com/processing/p5.js/blob/master/docs/yuidoc-p5-theme/assets/js/render.js#L219 the sketch fails because "preload" doesn't exist in "runnable" which is the parsed text from the editable example.

I'm not sure if we should fix this in the p5.sound library, or in that render script...

^ Anyone have ideas for how to address that? cc @limzykenneth I thought you might since you've worked on the render.js script recently (if the fix should happen there). Or @meiamsome since you've recently added a Promise-based preload mechanism?

[lmccart]

thanks @therewasaguy I'm looking at website stuff this week, so I will check this one out and see if I can figure anything out on this

[limzykenneth]

The render.js in this repo is actually a copy of the one in the website repo and may be outdated (though I think probably not as not much has changed in that file since I copied it over). So in actual fact, I don't understand how the rendering for reference examples work either 😝

I don't fully understand the problem you have (no idea what AudioWorklet does or how it works), but that's just me not understanding WebAudio enough, so I'm not sure I will be of much help here. In any case if the example rendering code is to be changed, the changes should be made over at the website repo as well.

[therewasaguy]

Thank you @lmccart! Let me know if you figure anything out.

@limzykenneth that's really good info, thank you!

The problem is basically that we need to ensure that some Promises (AudioWorklet module loaders) have resolved before we try to use those modules. We can hack into p5 preload to do that here https://github.com/processing/p5.js-sound/pull/369/files#diff-2a7f5646d78d97940147769cd4a2fc21R25 . But that causes issues with the website's custom render method for sketches that don't have a preload as part of their "runnable" code.

[limzykenneth]

Is it possible to save the promise onto a private instance variable (eg. something attached as p5._audioWorkletReady) and the code that needs that promise to resovle before running will check the status of that promise saved in the private instance variable before executing its code?

One potential problem I see with the above is if there's a chain of dependencies throughout the whole code base and because of this all of them needs to refer to upstream promises, but if there isn't such a thing it may work?

[oshoham]

Hi @therewasaguy @lmccart @limzykenneth, sorry for not chiming in on this thread earlier.

I've done some local testing with the p5.js website and the latest build of p5.sound, and I think I have a solution on the p5.js-website side of things in the render.js file.

I think that the problem is that when eval(p) attempts to evaluate a sketch that contains a preload() function, that results in a conflict with the existing preload key on the p object, which causes eval() to throw a ReferenceError.

This can be solved by deleting the preload key from p before calling eval(), and then adding it back afterwards if the sketch doesn't contain a preload() function. This shouldn't cause any problems, because p.preload only exists in the first place if p5.sound adds it, and it's just an empty placeholder function.

Here's what that would look like (these lines would replace the block of code in render.js starting at line 234):

// Actually runs the code to get functions into scope.
if (p.preload) {
  delete p.preload;
}
with (p) {
  eval(runnable);
}
_found.forEach(function(name) {
  p[name] = eval(name);
});
p.setup = p.setup || function() {
  p.createCanvas(100, 100);
  p.background(200);
}
p.preload = p.preload || function() {}

I've tested this fix locally on Chrome, Firefox, and Safari, and it seems to work across all 3 browsers. If you're okay with this proposal, I'll open a PR to the p5.js-website repo. Let me know what you think!

[lmccart]

@oshoham thanks for looking into this. this fix seems reasonable to me, we would welcome a PR to the p5.js-website repo. when you make the change, please include some notes about why this is being done, so others that access it in the future can follow. thank you!

[therewasaguy]

🎉 thank you @oshoham!!! I would +1 that PR to the p5.js-website repo.

A comment linking to this might help clarify why this line p.preload = p.preload || function() {} is necessary to always ensure that there is a preload method.

In the long run, maybe we can address this from another angle in the sound library. If we only create the AudioContext on a user gesture, and wait for that Promise/callback before creating any p5.sound objects, we could also wait for the AudioWorklet modules to load as part of that sound initialization...I'm just starting to think this through in https://github.com/processing/p5.js-sound/issues/389, please chime in!

[therewasaguy]

wait, I'm not sure if this solution works when loading a page directly for an example that does not have a "preload" method but uses the sound library. For example, try typing /reference/#/p5.Oscillator into the browser's address bar, rather than navigating via a link within the reference pages. That results in "preload" showing up in the array of _found functions by this line:

_found.forEach(function(name) {
    p[name] = eval(name);
});

But it wasn't actually found, so it fails on eval(name) with a ReferenceError: Can't find variable: preload

[oshoham]

@therewasaguy You're totally right, thanks for catching that!

Sorry for taking so long to get around to this, but I finally found the time to open a PR on the p5-website repo with a fix that accounts for the problem that Jason brought up: processing/p5.js-website#524

[therewasaguy]

I think this is ready, thanks to processing/p5.js-website#524

Also! In the meantime, add a few bugfixes

• p5.SoundFile: stop() won't stop a soundfile loop processing/p5.js-sound#401
• p5.SoundFile: addCue() not triggering function calls processing/p5.js-sound#371
• p5.SoundFile: jump() only called if soundfile is playing soundfile.jump: only jump if currently playing p5.js-sound#404
• p5.SoundFile: save() bugfix fix p5.soundFile.save() p5.js-sound#406
• remove bad console call (Remove bad console call p5.js-sound#378)

and updated inline documentation examples so that every example starts audio on a user gesture and abides by autoplay policies (processing/p5.js-sound#403)

[lmccart]

awesome, thanks @therewasaguy and @oshosham!

[fadookie]

Cool, do you think this might fix https://github.com/processing/p5.js-sound/issues/387 ?

[endurance21]

@fadookie

Cool, do you think this might fix processing/p5.js-sound#387 ?

visiting this link as provided in
processing/p5.js-sound#387 ? , i can still see that the problem prevails , AudioContext is not able to start !

however resuming the audioContext does the job !
getAudioContext().resume() in console
@therewasaguy i am curious if there is another way around we can do it !

[limzykenneth]

@endurance21 The sketch in the provided link is still using v0.3.11 of p5.Sound so any fix would not be present in there. Can you try with the latest version and open a new issue if the problem is still there?

[endurance21]

@limzykenneth
is p5.js-web editor updated to use latest update of p5.js? , because I tried a fresh sketch on the web editor and still getting the same issue on chrome !!

[limzykenneth]

The editor has not been updated to use the latest version of the library yet. You can change that yourself by editing the index.html file of the sketch on the editor itself.

[endurance21]

@limzykenneth i tried
with
https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.0.0/addons/p5.sound.min.js
and the problem is still there !

[endurance21]

@limzykenneth is this the problem with web editor or with p5.sound , because it works well with localhost but problem exists when the sketch is shared or downloaded for presentation ! i think this might be problem with web-editor as well ?

[limzykenneth]

@endurance21 Can you post your findings over at https://github.com/processing/p5.js-sound/issues/387 so we can track it centrally instead of discussing on this PR. Thanks.