alnitak / flutter_soloud

Flutter low-level audio plugin using SoLoud C++ library and FFI
MIT License
209 stars 21 forks source link

fix: handles become invalid at unknown points #83

Closed LeeMatthewHiggins closed 4 months ago

LeeMatthewHiggins commented 4 months ago

If I play a sound and hold on to the handle to set value later, if the sound is looping everything is fine. If not when I try and set volume using that handle I get a crash. "The player is not initialized (on the C++ side)."

Steps To Reproduce

  1. Play a non looping short sound, hold on to the handle
  2. Call setVolume using the handle later after the sound has finsihed playing
  3. Crash (The player is not initialized (on the C++ side).) line 1236 in soloud.dart

Expected Behavior

The system should not crash, handles should remain valid OR We need a system to tell us when handles are invalid

Additional Context

iOS, flutter_soloud: ^2.0.1

alnitak commented 4 months ago

Hi, thanks for your feedback.

Are you sure you got a crash and not an exception? The setVolume() can throw SoLoudNotInitializedException and for your use-case, it would be better to put it inside a try-catch. Also, you are right, the error message is wrong, I'll fix it soon. It should print out something related to the invalid handle.

To know when a handle is no longer valid, you can listen to the AudioSource you got from loadFile. It has soundEvents stream to listen to which has the event.handleIsNoMoreValid event you are looking for.

LeeMatthewHiggins commented 4 months ago

Yes sorry an exception not a crash (crashed due to unhandled exception)! Is there a reason for handles to become invalid? It means that users of the api need do a bit of work to maintain them. Would be nice if they had a complete state or something.

alnitak commented 4 months ago

The sound handles are the representation of the loaded sound and are used internally to control them.

For example, you can play an unlimited number of the same instance of a sound and every instance has its unique handle ID. By control them I mean that you can play, pause, get info, or set a filter on that playing handle independently from each other.

So a sound (the AudioSource) can have multiple active handles. They are active when playing or even when paused, but when they reach the end or when they are stopped, the handle becomes invalid and its unique ID is removed from the internal handles list.

The setVolume() you are using acts on a specific instance (handle) of a sound. Maybe could be better for your use-case to use setGlobalVolume() instead?

LeeMatthewHiggins commented 4 months ago

I have 2 main volumes, SFX and music. I need to set the volume of the SFX sounds independently, so this is why I do this. Also the sounds are entities placed in the level and the designer can set their volume. For now I can just catch and ignore the exception. However, i would ask what would you do with that exception accept ignore it? You can´t really know when the handles become invalid, unless you listen to the events and record that fact yourself, which seems like a lot of work and is quite hidden. Maybe a developer experience inprovement could be to fix that. Maybe another abstraction that includes the play state, and that never becomes invalid?, or maybe simply don´t throw an exception?

alnitak commented 4 months ago

Throwing an exception calling a method that needs a handle that may not exist anymore is mandatory I think. But marking it as invalid when it reaches the end maybe can be done. I mean, mark it as invalid only when it's manually stopped. Something like an autoDisposeHandle boolean parameter in the play() method.

I have to look for this, but I don't know yet if it is supported by the SoLoud lib.

alnitak commented 4 months ago

It's not possible to control how the engine disposes handles. But how does it sound, for your use case, if is there a way to set the volume for all current or future handles of the loaded file (AudioSource)? This should not throw an exception even if AudioSource does not have yet any active handles.

LeeMatthewHiggins commented 4 months ago

hmm in my case no, I might want to have the same source have a different volume (in fact the level designer uses this now in some levels). I think for now it seems i have my usecase working, i just ignore the exception. However I would say that it would be better if handles were not invalid at an unknown point, the same issue as the volume applies to any of the controls, play pause etc. Maybe there needs to be a higher level abstraction that hides the handle detail.

alnitak commented 4 months ago

I close this issue since there is no way for a handle to stay valid after it is stopped/ended. Feel free to reopen if you think something other can be done.

A side note: you can check handle validity before calling some methods that need it with getIsValidVoiceHandle

LeeMatthewHiggins commented 4 months ago

I close this issue since there is no way for a handle to stay valid after it is stopped/ended. Feel free to reopen if you think something other can be done.

A side note: you can check handle validity before calling some methods that need it with getIsValidVoiceHandle

Perfect, I will use that to check rather than ignore the exception