faiface / beep

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

calling Seek rapidly causes out of range crash #168

Closed vojislav closed 1 year ago

vojislav commented 1 year ago

Calling this function repeatedly causes a crash

// seek from the current position by step seconds
func seek(step int) {
    seekTo := currentTrack.stream.Position() + step*sr.N(time.Second)
    if seekTo < 0 {
        seekTo = 0
    } else if seekTo > currentTrack.stream.Len() {
        nextTrack()
    }

    currentTrack.stream.Seek(seekTo)
}

I'm pretty sure this had something to do with seeking to a part of the stream before it's buffered, but I'm not sure how to avoid it. I'm guessing there's a check for it or something, but I can't find it. Thanks in advance.

Error message:

panic: runtime error: slice bounds out of range [5860:4608] [recovered]
        panic: runtime error: slice bounds out of range [5860:4608]

goroutine 1 [running]:
github.com/rivo/tview.(*Application).Run.func1()
        /usr/local/bin/go/pkg/mod/github.com/rivo/tview@v0.0.0-20231007183732-6c844bdc5f7a/application.go:251 +0x4d
panic({0xa56860, 0xc00002a168})
        /usr/local/go/src/runtime/panic.go:884 +0x213
github.com/hajimehoshi/go-mp3.(*Decoder).Seek(0xc00009e660, 0x0?, 0x1?)
        /usr/local/bin/go/pkg/mod/github.com/hajimehoshi/go-mp3@v0.3.4/decode.go:111 +0x30d
github.com/faiface/beep/mp3.(*decoder).Seek(0xc00007c140, 0x1df9b9)
        /usr/local/bin/go/pkg/mod/github.com/faiface/beep@v1.1.0/mp3/decode.go:90 +0x105
main.seek(0x5)
        /home/vojislav/Documents/music-player-go/audioplayback.go:171 +0xaf
main.trackInputHandler(0x0?)
        /home/vojislav/Documents/music-player-go/tview.go:133 +0x1e5
main.appInputHandler(0xc0000e4020)
        /home/vojislav/Documents/music-player-go/tview.go:445 +0x167
github.com/rivo/tview.(*Application).Run(0xc000290000)
        /usr/local/bin/go/pkg/mod/github.com/rivo/tview@v0.0.0-20231007183732-6c844bdc5f7a/application.go:327 +0x42b
main.main()
        /home/vojislav/Documents/music-player-go/main.go:84 +0x20a
exit status 2
MarkKremer commented 1 year ago

Hey Vojislav,

Would you be able to provide a stand-alone snippet to reproduce the problem? That would help immensely with debugging. Looking at the source of go-mp3 it looks like it starts reading new mp3 frames when seeking. So it should have the desired position buffered. If the problem lies there, I'm expecting it to be something subtle.

Also have a look at gopxl/beep, the new maintained fork of Beep! If you use the main branch you'll get some other fixes and version upgrades for free. I'll create a tagged version at the beginning of next month. That said, the go-mp3 package that Beep uses is archived but we can try to figure something out to fix this problem.

vojislav commented 1 year ago

just to comment, if anyone is having this issue, i managed to fix it by pausing it before calling Seek() like this

func seek(step int) {
    seekTo := currentTrack.stream.Position() + step*sr.N(time.Second)
    if seekTo < 0 {
        seekTo = 0
    } else if seekTo >= currentTrack.stream.Len() {
        nextTrack()
        return
    }

    wasPaused := playerCtrl.Paused

    playerCtrl.Paused = true
    currentTrack.stream.Seek(seekTo)
    playerCtrl.Paused = wasPaused
}

this is more of a hack, so i'll try with using the gopxl/beep fork and report back. thanks for the suggestion.

MarkKremer commented 1 year ago

This makes me think that this is a concurrency issue. If you're using speaker, you'll need to Lock() it before making changes to the streamers it's pulling from (and then Unlock() after). See the wiki: https://github.com/faiface/beep/wiki/Composing-and-controlling

Good luck with your project!

vojislav commented 1 year ago

yeah i tried this and it seems to do the trick as well. thanks!