search

phetsims / tambo

library containing code to support sonification of PhET simulations
MIT License
2 stars 4 forks source link

Create a peak detector #133

Open jbphet opened 3 years ago

jbphet commented 3 years ago

When PhET first started using Web Audio, I created a peak detector that would monitor the output of an audio node and periodically report the peak values of the output (it would log it to the console). This was useful for testing whether audio levels were getting too and, and for comparing perception to measured levels.

Unfortunately, this peak detector was based on ScriptProcessorNode, which has since been deprecated, so I got rid of it. However, there have been a couple of times recently where having such a thing would have been quite useful, so I think I should try to recreate it. It looks like the replacement for ScriptProcessorNode is AudioWorkletNode, so it would be based on that.

jbphet commented 3 years ago

I've got something working and committed, and it's hooked up to the "LongSoundTest" in the tambo demo code as a combination test harness and demonstration. It's got a fairly significant problem at this point, which is that it assumes that the asynchronous loading of the module that comprises the AudioWorklet processor will be completed by the time the AudioWorklet node is instantiated. This works in my demo and some other testing, but it is not necessarily reliable and shouldn't be trusted long term.

I can think of two ways to handle this. One is to have a static factory sort of thing with a method that hooks up the peak detector once it is sure that the processor module has been loaded. There would be some sort of method like attachPeakDetector and the code would instantiate and connect the AudioWorklet node once the AudioWorklet processor was successfully loaded. I don't like this idea very much, because it doesn't allow clients to use the peak detector just like any other audio node, e.g. a gain node.

Another possibility is to use the same process that we current use for images and sounds, and pause the instantiation of the sim until all resources are loaded. The AudioWorklet module would be such a resource.

I'm going to mark this for the next developer meeting so the dev team can kick it around a bit. They may have other ideas too.

jbphet commented 3 years ago

In addition to the issue with loading the processor module, there are some improvements that could be made, but I probably won't get to them for a while, so I thought I'd list them here as possible future improvements.

jbphet commented 3 years ago

The peak detector in the demo was causing a CT failure, so I've removed it. Also, based on this table, I think loading of worklet modules may not be supported in Safari, so I should probably test that and build in protection.

There is another issue that could be discussed at the developer meeting: How would this work in built mode? Right now it's referencing a script that defines the processor portion of the worklet, but it's doing so with a relative path. This won't work in built mode. This isn't a big deal, at least not yet, since we don't have any plan to use this in production builds, but it would be nice to be able to have it in a build for a while and not cause CT failures.

jbphet commented 3 years ago

Un-marking for dev meeting. I'd like to give this a little more thought and potentially do some additional work on it before discussing it with the rest of the developers.

jbphet commented 3 years ago

@pixelzoom just tried to use the peak detector to verify some changes in Fourier, and it didn't go well, see https://github.com/phetsims/fourier-making-waves/issues/45. He and I met on Slack, and it looks like the peak detector in its current form doesn't work on Mac+Chrome. I also just tried it on Firefox on my Win10 machine and it doesn't work there either, though the failure mode is different.

This isn't entirely surprising. Audio worklets are a pretty new feature in Web Audio, and it appears as though there is significant cross-platform variation in their behavior. This is probably not a high enough priority for me to take the time to make it work on all of our supported platforms, so for now it will have to remain in the "experimental" state, and I'll use it for very specific tests and then take it back out.

jbphet commented 3 years ago

As I noted above, spending the time necessary to make this work on all of our supported platforms is not justified at this time. I'm going to mark the issue as deferred for now.