esonderegger / web-audio-peak-meter

Customizable peak meters, using the web audio API.
https://esonderegger.github.io/web-audio-peak-meter/
MIT License
125 stars 33 forks source link

Add disconnectSource method #46

Closed evoyy closed 1 year ago

evoyy commented 1 year ago

Relates to issue https://github.com/esonderegger/web-audio-peak-meter/issues/43

esonderegger commented 1 year ago

Thank you for working on this! It had been my intention to build a tear-down method, that does everything the constructor does in reverse. Unfortunately, I never got around to it.

I think a method that does this (do you have any preferences on name, for example cleanup(), destroy(), or teardown()?), should do the following things:

  1. disconnects the node from the destination
  2. (Maybe) disconnects the srcNode from this node? (I'll have to look into this further)
  3. (Maybe) terminate the worker / disconnect the port's onmessage handler (also needs more investigation)
  4. If there is a DOM node, removes the click event listener for clearing held peaks
  5. If there is a DOM node, cancels the requestAnimationFrame loop
  6. (Maybe) remove the module from the web audio context if after this node is removed there will be no more nodes using the module (I'm torn on this one - on the one hand, it seems like a good thing to clean up after oneself, on the other it would be inefficient if we ever want to toggle the meter back on again)

I think if we do this, then https://github.com/esonderegger/web-audio-peak-meter/pull/45 won't be necessary. A user could create a new WebAudioPeakMeter when they want it to be active, and call this method when they don't want it to be active

evoyy commented 1 year ago

Whatever you call it is fine with me. My main concern was disconnecting the audio node, because that's the only thing that doesn't get cleaned up implicitly and garbage collected (I remove my meter from the DOM using container.replaceChildren() which takes care of all that).

https://github.com/esonderegger/web-audio-peak-meter/pull/45 is up to you, but when I pause playback in my app it's nice to not have an unnecessary animation loop running.

esonderegger commented 1 year ago

Thank you for raising this issue and for suggesting a fix! I incorporated this into https://github.com/esonderegger/web-audio-peak-meter/pull/50 and have published it as version 3.1.0.