RingBuffer isn't thread safe
rweichler opened this issue · 12 comments
I'm calling AddInterleavedFloatData on one thread and RetrieveFreshAudio on another thread and sometimes I get an EXC_BAD_ACCESS on this line.
After further investigation it seems that it isn't thread safe. After a bit of Googling it seems you should be using the functions in OSAtomic.h. I'm not entirely sure how this could be used for floats. Might need a refactor?
Here's a threadsafe ringbuffer, but it doesn't store floats: https://github.com/michaeltyson/TPCircularBuffer
Actually, an int and a float are represented with 4 bytes on every architecture so maybe I might be able to get away with this
Actually^2, I don't think you need to atomically copy the float data, just the lastwrittenindex and numunreadframes since that would imply what float data is readable and writable.
I just implemented a fix locally and im going to play the music overnight, if I don't get a crash I'll make a pull request
How'd it go? I've used the RingBuffer in multiple threads, including separate OpenGL display threads, and it did work. However, my testing of this has been extremely limited, and am very open to a more general solution. Overall, anything by Michael Tyson in mobile audio DSP is awesome, and would love to fold stuff in.
I'm feeding the audio in using one thread, and I'm getting the audio out using whatever thread Core Audio uses. Apparently you don't have to worry about thread safety if you're feeding the audio in on the main thread, but if you're feeding it in on a custom thread then it could pose some problems.
The crashes are pretty rare too, like after a few hours of playback.
I didn't get a crash, so I'll setup the pull request now. :D
fuck, this didn't fix it. i just got a crash :/
actually, it might be a bug in my code
Status of this? Should I roll back?
I'd say using atomic operations would be the proper way to implement a
thread-safe ringbuffer, and you should keep it.
On Fri, Mar 14, 2014 at 12:09 PM, Alex Wiltschko
notifications@github.comwrote:
Status of this? Should I roll back?
Reply to this email directly or view it on GitHubhttps://github.com//issues/77#issuecomment-37665274
.
Morgan Packard
cell: (720) 891-0122
twitter: @morganpackard
I figured out what the problem was. RingBuffer's ClearBuffer() is bugged when doing it on a non-main thread. So I did a delete and new as a "fix" in my application (which is not thread safe at all). I fixed up ClearBuffer and called that instead and I'm not getting any crashes anymore.
I'll setup another pull request for the ClearBuffer fix.