gopxl / beep

A little package that brings sound to any Go application. Suitable for playback and audio-processing.
MIT License
288 stars 14 forks source link

index out of range panic #197

Open uzudil opened 5 days ago

uzudil commented 5 days ago

First, thanks for maintaining this great library! I use it for my game enalim.

I intermittently see the following error:

panic: runtime error: index out of range [-1]

goroutine 9339 [running]: github.com/gopxl/beep/v2.(Mixer).Stream(0xc000eb26a0, {0xc07bf9c000, 0x200, 0x200}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/mixer.go:66 +0x371 github.com/gopxl/beep/v2.(Mixer).Stream(0xc000eb2680, {0xc07bf86000, 0x200, 0x200}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/mixer.go:54 +0x1bd github.com/gopxl/beep/v2/effects.(Volume).Stream(0xc032a79770, {0xc07bf86000, 0x200, 0x200}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/effects/volume.go:31 +0x36 github.com/gopxl/beep/v2.(Mixer).Stream(0x10a6e3620, {0xc03aa86000, 0x89d, 0x89d}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/mixer.go:54 +0x1bd github.com/gopxl/beep/v2/speaker.(sampleReader).stream(0xc000052008?, {0xc03aa86000?, 0x10955d9a5?, 0xc0a75a4180?}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/speaker/speaker.go:207 +0xba github.com/gopxl/beep/v2/speaker.(sampleReader).Read(0xc032a79da0, {0xc037aa5900, 0x2274, 0xc000134d28?}) /Users/gtorok/go/pkg/mod/github.com/gopxl/beep/v2@v2.1.0/speaker/speaker.go:176 +0xc5 github.com/ebitengine/oto/v3/internal/mux.(playerImpl).read(0xc06feb5e58?, {0xc037aa5900?, 0x109a77ea0?, 0x109e6d6e0?}) /Users/gtorok/go/pkg/mod/github.com/ebitengine/oto/v3@v3.2.0/internal/mux/mux.go:268 +0xc2 github.com/ebitengine/oto/v3/internal/mux.(playerImpl).readSourceToBuffer(0xc000361710) /Users/gtorok/go/pkg/mod/github.com/ebitengine/oto/v3@v3.2.0/internal/mux/mux.go:529 +0x13a github.com/ebitengine/oto/v3/internal/mux.(*Mux).loop(0xc000ed55c0) /Users/gtorok/go/pkg/mod/github.com/ebitengine/oto/v3@v3.2.0/internal/mux/mux.go:108 +0x1f1 created by github.com/ebitengine/oto/v3/internal/mux.New in goroutine 1 /Users/gtorok/go/pkg/mod/github.com/ebitengine/oto/v3@v3.2.0/internal/mux/mux.go:69 +0xf8

Thanks!

MarkKremer commented 4 days ago

Love to see Beep being used! Your game looks cool!

From only the stack trace I can only make some guesses to what's happening:

  1. Some concurrency error. Make sure that when you make changes to the Mixer (Add or Clear) that you Lock the speaker. See the tutorial.
  2. I see that one Mixer is used by another. Make sure the Mixer isn't consuming itself.

If that doesn't help, I would appreciate if you could share some code so we can further debug the problem.

uzudil commented 2 days ago

Thanks for the quick reply! The problem was due to locking. For some reason (I can no longer remember) I was using my own mutex instead of speaker.Lock(). Switching to locking/unlocking via the speaker fixed the problem. Thanks again!

uzudil commented 2 days ago

One last point: this issue is still happening but far less frequently now. Specifically, it seems to be related to calling mixer.Clear() from inside a Callback. According to the source, speaker is locked during a callback so I'm not calling speaker.Lock() here. (Calling it caused a deadlock, which makes sense.)

uzudil commented 2 days ago

I think maybe the issue is that I try to clear() the mixer after a sequence has finished playing. In faiface/beep I had to do this but maybe finished streamers are automatically removed in this version?

MarkKremer commented 21 hours ago

I've made a PR to check whether the mixer has been cleared during streaming to prevent the panic.

Both faiface's implementation and ours remove streamers which are done playing but our version is a bit more aggressive about it and removes them when they return less than the requested samples instead of waiting for the !ok signal that has to follow it. This doesn't entirely explain the panic but it's possible that you don't need the clear for your use case.

uzudil commented 18 hours ago

Yes my "fix" was to not clear the mixer after the sequence is done. This eliminated the crashes, I was just curious if it will lead to a memory leak. Thanks for looking into this!

Ps.: I can also post the audio code in a github gist but I didn't want to saddle you with debugging my code. If you want to see it though, let me know.

MarkKremer commented 18 hours ago

I think we tackled the problem so it's just out of curiosity. If it isn't a hassle, please share :)

Also, blind debugging is more difficult than with a small reproduction snippet.

uzudil commented 17 hours ago

Here is the audio code for enalim. Please don't judge, I'm a total n00b in audio coding... If you have suggestions for improvement, please let me know!

Enalim plays:

Most of the game is implemented in script, which makes calls to (for example) audio.Play(). Because it's an opengl game, all of this is single-threaded.

https://gist.github.com/uzudil/5091f7d4ef55cd6ae5bc5b78bba44cf7

Also, if you want, I can give you access to the full game source.

uzudil commented 17 hours ago

I forgot to add: the previous version (where the crash happened, used to call audio.Music.mixer.Clear() from the Callback on line 439.

uzudil commented 17 hours ago

One more comment: audio.updateMusic() is called from the main game loop to handle fading between various musics. (eg. if combat breaks out, we fade to the combat music.)