phetsims / wave-interference

"Wave Interference" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
18 stars 5 forks source link

Update speaker membrane sound immediately on changes and on reset #497

Closed samreid closed 4 years ago

samreid commented 4 years ago

From https://github.com/phetsims/wave-interference/issues/473 we would like the speaker membrane sounds to update immediately on changes or on reset all rather than waiting for the next cycle. Thanks for the recommendation @jbphet !

samreid commented 4 years ago

@jbphet mentioned this may be easier with a continuous loop and low frequency oscillator. He may add better support for that, since it is coming up in Proportion Playground as well. But for now, we should keep track of the existing SoundClip and update it accordingly.

samreid commented 4 years ago

After the commit, the frequency, amplitude, pause and reset all update the sound right away. The sound still waits for the next cycle when creating a new sound, but perhaps that is not essential for the first release. @jbphet can you please review?

samreid commented 4 years ago

I also noticed https://github.com/phetsims/tambo/issues/106 is now implemented. Can you please describe how we would set it up to emulate the speaker membrane sound?

jbphet commented 4 years ago

Can you please describe how we would set it up to emulate the speaker membrane sound?

Sure. I feel like it would simplify the existing code, but there are some tradeoffs (not the least of which is that you have something working which may suffice), so it will be up to you @samreid as to whether this is worthwhile.

To produce the speaker membrane sound using an amplitude modulator, you'd create a subclass of SoundGenerator called something like SpeakerMembraneSoundGenerator that takes three properties, one for whether the speaker was on, one for the frequency, and one for the amplitude. You'd have a single continuous tone in the form of a SoundClip that would serve as the basic, unmodulated tone, and its playback rate would be a function of the speaker frequency, similar to what is done now. The output level of the sound clip would be a function of the amplitude property. The sound clip would be hooked to an AmplitudeModulator instance, and the frequency at which the amplitude is being modulated would also be a function of the input frequency. And, of course, the sound clip would not be playing at all if the speaker was off.

AmplitudeModulatorDemo is somewhat similar in that it is a sound generator that connects sounds clips to an amplitude modulator, so it could serve as a bit of a reference design for this effort.

One tradeoff here will be that it may be harder to have the sort of precise coordination between the speaker membrane position and the sound, the way you do now. For this, the sound generator could potentially watch for zero crossing of the oscillator value the way it does now to trigger a start of the sound clip and LFO, and it would behave similarly to what it does now, meaning that it still wouldn't produce sound right away if paused and then unpaused at a peak. @samreid - back to you to decide whether to pursue this. I haven't reviewed the current code, since it may be replaced, so please assign this back to me whatever you decide to do and I'll review at that point.

jbphet commented 4 years ago

Unassigning myself until @samreid has a chance to determine the next steps.

samreid commented 4 years ago

Thanks for describing how this could be done, the existing implementation seems appropriate for the upcoming release. In the future if more work is warranted in this area or changes are necessary, we can try the proposal above. @jbphet anything else for this issue before dev or RC testing?

jbphet commented 4 years ago

@jbphet anything else for this issue before dev or RC testing?

Yes. If you're going with the current implementation, I still need to review that, as noted in my comment above.

jbphet commented 4 years ago

Reviewed while working on https://github.com/phetsims/wave-interference/issues/426. @samreid - you can probably close this issue and just address the comments in #426 if you like.

samreid commented 4 years ago

Excellent, thanks! Closing.