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

Create a peak detector #133

Open
jbphet opened this issue Mar 4, 2021 · 6 comments
Open

Create a peak detector #133

jbphet opened this issue Mar 4, 2021 · 6 comments
Assignees

Comments

@jbphet
Copy link
Contributor

jbphet commented Mar 4, 2021

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
Copy link
Contributor Author

jbphet commented Mar 30, 2021

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
Copy link
Contributor Author

jbphet commented Mar 30, 2021

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.

  • Currently, the update period is fixed at 1 second, and it would be nice to make it variable
  • The communication protocol between the two threads is very basic, and it would be nice to make something more extensible
  • Peaks are only shown as positive values, it would be nice to make this configurable

@jbphet
Copy link
Contributor Author

jbphet commented Mar 31, 2021

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
Copy link
Contributor Author

jbphet commented Apr 1, 2021

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
Copy link
Contributor Author

jbphet commented Jun 14, 2021

@pixelzoom just tried to use the peak detector to verify some changes in Fourier, and it didn't go well, see phetsims/fourier-making-waves#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
Copy link
Contributor Author

jbphet commented Jun 23, 2021

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.

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

2 participants