ricky0123 / vad

Voice activity detector (VAD) for the browser with a simple API
https://www.vad.ricky0123.com
Other
809 stars 129 forks source link

AudioWorkletNode cleanup issue #19

Closed pciporin25 closed 10 months ago

pciporin25 commented 1 year ago

Hi Ricky,

Thank you for your continued hard work on this repo. I am having an issue cleaning up the MicVAD when unmounting my hook in React. Currently I am running the following cleanup logic on unmount:

      // pause processing
      if (vad?.listening) {
        vad.pause();
      }

      // release audio tracks
      vad?.audioContext.close();
      vad?.stream.getTracks().forEach((track) => {
        track.stop();
      });

However, in some situations it seems like the AudioWorkletNode is not being shut down. I believe this is due to the fact that in the Processor class (worklet.ts), the process function is implemented with a return value of true. Based on the documentation here, it sounds like this would cause the node to remain active “even if the user agent’s internal logic would otherwise decide that it’s safe to shut down the node.” Is the cleanup occurring elsewhere? Or if I'm missing something, I am wondering if you have any advice for how to handle cleanup properly?

Thanks!

ricky0123 commented 1 year ago

Hi Peter, thanks for bringing this to my attention. I haven't really thought through this requirement. I suppose we could add a destroy method to MicVAD that gets called in the unmount, maybe added to the cleanUp function here along with the other code you are using to close the audiocontext and stuff. The destroy method could use the AudioWorkletProcessor port message api to send a message to the processor and tell it to return false from the process method. I'm open to other suggestions as well.

pciporin25 commented 1 year ago

Hi Ricky, your suggestion here sounds great to me and sounds like it should address the issue I’m having. Let me know if you’d like help implementing this fix or if you plan to do it yourself. Really appreciate the prompt response!

ricky0123 commented 1 year ago

Would definitely be grateful for the help in implementing this. Otherwise, I'm happy to do it, but probably won't have time this week. Could hopefully get to it next week.

ricky0123 commented 1 year ago

If you do decide to give it a go, you can try out the following development flow I created. The "test-site" directory in the top level of this repo is a site that uses the vad-react package. You can modify that site to do whatever interactive testing/exploration you want to do. Whenever you make a change to vad-react and want to see it reflected in the test-site, you can build vad-react (with npm run build) and then rebuild and run the test-site with npm run build-test-site and npm run serve-test-site. Another thing to look out for is that when you run npm run build from the top level of the repo it may build vad-web after vad-react and so you may need to run it again to see changes to vad-web reflected in vad-react.

Anyway, this is a work in progress and not very sophisticated, but that's what it is for now.

pciporin25 commented 1 year ago

Thanks Ricky, sounds good. I may have some time later this week (or early next) but not entirely sure yet. If you do end up getting to it before me I can definitely help review, but I'll let you know if I'm able to get to it sooner.

v3ss0n commented 1 year ago

Any progress on this? Any pointers? we could work on it.

GalDayan commented 1 year ago

Any progress? :) @ricky0123

v3ss0n commented 1 year ago

we had done it here and we are using our own fork since @ricky0123 is busy .

Please have a look https://github.com/K4UNG/vad

ricky0123 commented 10 months ago

Thanks all, it's fixed now. If you run npm run dev and go to the "React destroy" page you can play with it by clicking a button to repeatedly mount and unmount the VAD.