search

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

Preventing int64 out of range panics on the Resampler #97

Open skuzzymiglet opened 4 years ago

skuzzymiglet commented 4 years ago

In Resamplers, a slice can be indexed with over 9223372036854775807 unchecked, which causes a panic. An error returned is obviously better than panic()

faiface commented 4 years ago

Do you have a code which actually causes this?

skuzzymiglet commented 4 years ago

beep.ResampleRatio, above a certain value. Really high tempos on my program (https://github.com/skuzzymiglet/metro) cause panic. In one case a 0.29s flac sample, resampled to 1/1000s is the minimum for a panic()

faiface commented 4 years ago

Okay, but I don't think returning an error is the right way. I think we just need to fix the bug.

skuzzymiglet commented 4 years ago

My trace:

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

goroutine 1 [running]:
github.com/faiface/beep.(*Resampler).Stream(0xc0001b8000, 0xc0001ba000, 0x200, 0x200, 0xc0001b6000, 0x741a88)
        /home/skuzzymiglet/.local/go/pkg/mod/github.com/faiface/beep@v1.0.2/resample.go:96 +0x3eb
github.com/faiface/beep.(*Buffer).Append(0xc0002b9f28, 0x81a5e0, 0xc0001b8000)
        /home/skuzzymiglet/.local/go/pkg/mod/github.com/faiface/beep@v1.0.2/buffer.go:198 +0x87
main.main()
        /home/skuzzymiglet/code/metro/main.go:135 +0x5c8
skuzzymiglet commented 4 years ago

I don't know about how to fix it. I guess we could use a uint64 rather than int64. But the panic needs to be prevented

faiface commented 4 years ago

It's impossible to have such large slices anyway, they wouldn't fit into memory, it's clearly some kind of a bug. Right now I'm travelling, so I can't take a look at it, but if you get a chance and find the bug, please tell, or submit a PR.

ianling commented 3 years ago

Old issue, and I'm not sure what in particular causes it, but I think it may be related to using a Resampler with the same old and new sample rates, e.g. beep.Resample(4, 48000, 48000, stream). Something about the math involved with the resampling may result in funny values when the sample rates match.

edit: Actually, now I'm getting the same panic I was getting before, even though I am resampling from 44.1KHz to 48KHz. I am able to trigger it by replaying from the same stream over and over and over. Here is a screenshot of a debugger window with relevant values listed in case it helps in determining what's happening: https://i.imgur.com/um4xylV.png