CreateJS / SoundJS

A Javascript library for working with Audio. It provides a consistent API for loading and playing audio on different browsers and devices. Currently supports WebAudio, HTML5 Audio, Cordova / PhoneGap, and a Flash fallback.
http://createjs.com/
MIT License
4.42k stars 838 forks source link

how can release or destory sounds? #259

Open SeminYun opened 7 years ago

SeminYun commented 7 years ago

hi.

thanks for developing SoundJS.

We are developing mmorpg with sound.js. I know how can we release loaded mp3 by calling instance.destroy()

but i gave error.. i don't know correct reason yet..

i wanna know how release sounds after registerSound, when every using done..

thx

lannymcnie commented 7 years ago

The destroy method just clears out an instance. A reference to each instance is maintained by the Sound class to enable things like global mute and volume, so that method is available if you are sure you no longer need an instance.

To remove sounds that have been registered by SoundJS, use the static removeSound, or the similar removeSounds and removeAllSounds methods. This clears out all references to sounds, including web audio buffers.

Sound.removeAllSounds();

http://createjs.com/docs/soundjs/classes/Sound.html#method_removeAllSounds

SeminYun commented 7 years ago

@lannymcnie thx a lot..

and can i remove by instance?

like instance.removeSound or instance.destroy?

SeminYun commented 7 years ago

i used removeSound function createjs.Sound.removeSound(prop.src, prop.basePath);

and it gave me like following error soundjs-0.6.2.combined.js:6746 Uncaught TypeError: Cannot read property 'duration' of null at WebAudioSoundInstance.p._setDurationFromSource (soundjs-0.6.2.combined.js:6746) at WebAudioSoundInstance.p.setPlaybackResource (soundjs-0.6.2.combined.js:5893) at WebAudioSoundInstance.p.destroy (soundjs-0.6.2.combined.js:5654) at WebAudioSoundInstance.p.destroy (soundjs-0.6.2.combined.js:6717) at WebAudioPlugin.p.removeSound (soundjs-0.6.2.combined.js:6382) at Function.s.removeSound (soundjs-0.6.2.combined.js:4524) at SoundManager.clearAllSounds (soundmanager.js:259) at Jandi.Map.destroy (map.js:98) at Jandi.MapManager.load (mapmanager.js:48) at Game.Jandi.Game.Game.procPacket (game_procpacket.js:124)

thx

lannymcnie commented 7 years ago

We can check this out. Thanks for the report!

SeminYun commented 7 years ago

@lannymcnie thx..

i hope it will be fixed asap

thx..

a4jp-com commented 7 years ago

I was having the same problem. Glad I found this.

SeminYun commented 6 years ago

is this resolved?

a4jp-com commented 6 years ago

Not that I know of.

anagrath commented 6 years ago

So hey guys, this is a fairly serious issue because your documentation says that we should be calling createjs.Sound.play. I register a sound for a button click, now each time I click a button and play the button click I have a new WebAudioSoundInstance that does not disappear until I deregister the the sound. Why would I deregister the sound? I'm tracking several megabytes of retained memory within the createjs.Sound possession with no easy way to remove besides deregistering the sound and re-registering it.

I get that you need a global instance for certain reasons, but an intelligent solution would be not to keep creating new instances, especially when we are registering by id and playing by id. Why not just play the instance that is already in existence... At least then it is not leaking like a sieve when you use the library as documented.

screen shot 2018-02-17 at 8 27 52 pm

As a temporary solution I have created a global sound cache to replicate ._soundInstances but by ID and a new global play function as a wrapper for Sound.play --> what is the reason for storing multiple WebAudioSoundInstances? Is it so that you can play more than one at once? Would be nice if there were an option to fetch out of the ._soundInstances to reuse rather than create, like an extra parameter on play so that we may use the library as documented but have some control over memory usage.

gSkinner-Blair commented 6 years ago

Hey, @anagrath, thanks for the message!

There's a few reasons why SoundJS works the way it does, but the main reason we don't clean up SoundInstances automatically is that we have no way of knowing whether a user is done with them or not. A given user might have a reference to an instance, expecting to be able to use it later - and if we clean up our sounds as soon as they finish playing, those references are suddenly invalid, and will cause runtime errors when accessed.

A second issue with instance re-use is some of the awkward situations that arise surrounding user intention - Imagine someone plays a sound once, and immediately sets the volume of that instance to 0.5. If we reused instances, when that person played the sound (using createjs.Sound.play) again five minutes later, it would still be at only half volume "without them having said so" - not a good or intuitive experience.

There is still that potential for a memory leak though, as you've pointed out, and I agree with you that that's not great. You're also right that the docs suggest using createjs.Sound.play. The intended way to use this function, if you want to keep replaying the same identical sound, is to keep a reference to the sound instance it returns yourself, and call play on that instead - that's how you tell the library that you actually want an identical sound, so to speak. Though this still is not exactly great, as it means that using just the static API out of the box is still leaky.

But, I'm happy to tell you that we're working on a rearchitect of this system as part of SoundJS 2.0 that should solve this issue, and more! So stay tuned for 2.0!

anagrath commented 6 years ago

Thank you for your reply. All I’m suggesting is give us a function so that we can tell you we are done with the sound and then there would not be an issue with memory...

gSkinner-Blair commented 6 years ago

If you are done with an individual sound instance, you can use SoundInstance.destroy() to clean it up. Alternately, if you're done with an entire sound file, use Sound.removeSound(src), which will also destroy all instances that point to that source.

I hope that solves your use case! Let me know if you have any further questions or concerns.

reinierf commented 5 years ago

The suggested solution only partly solves the problem. SoundInstance.destroy() does remove references to objects within the sound instance, but not the reference to the SoundInstance itself, which is kept in AbstractPlugin._soundInstances. When you have an event sound which is played often, for instance associated with a button, references keep piling up. Sound.removeSound(src) does remove references to SoundInstance instances, but removing/unloading the source sound is often not desired, as is the case with the button sound.

A bit of a hacky way (because it accesses the 'private' property _soundInstances of the plugin) to remove the references is:

var idx = Sound.activePlugin._soundInstances[soundInstance.src].indexOf(soundInstance);
Sound.activePlugin._soundInstances[soundInstance.src].splice(idx,1);

(You still also have to call soundInstance.destroy() though.)