Upstream fix from Audaspace with simplified PulseAudio code.
Details
Diff Detail
- Repository
- rB Blender
- Branch
- T86851_fix (branched from master)
- Build Status
Buildable 13784 Build 13784: arc lint + arc unit
Event Timeline
| extern/audaspace/include/devices/ThreadedDevice.h | ||
|---|---|---|
| 92–93 | You might want to document that these functions are only allowed to be called from within runMixingThread, otherwise it's not obvious this is correct. | |
| 93 | I think that technically this should have a mutex lock, to ensure both values are set atomically. | |
| extern/audaspace/src/devices/ThreadedDevice.cpp | ||
| 46–49 | I think this will fail:
I think then the thread will stop without being restarted, even though it should be playing. I would expect shouldStop() and doStop() to be a single atomic function call somehow, and playing() to also use a mutex lock? | |
I can add some documentation that the functions are only to be called when the device is locked.
What you're missing here is that the device is in fact always locked whenever playing, doStop, and shouldStop are called.
That's actually the reason why I had to rewrite this in the first place, because the lock that is used by the threaded mainloop of Pulse is deadlocking with the lock of the SoftwareDevice.
Ok, that makes sense. It's your code so up to you how you want to document it, but I think it would help to document the implicit assumptions in ThreadedDevice.
Playback works, but I do have one question about the code.
| extern/audaspace/plugins/pulseaudio/PulseAudioDevice.cpp | ||
|---|---|---|
| 86–87 | I don't know anything about "corking" in this context, so I just look at it from the perspective of my old physics teacher telling the yappy student to go to the chemistry lab, get the biggest cork he could find, bring it back, and put it in his mouth so he'd finally shut up (true story this, it was hilarious). Is it correct that the stream should be corked if it already has a cork in it? Or does passing 0, nullptr, nullptr, contrary to the function name, actuall un-cork it? | |
| extern/audaspace/plugins/pulseaudio/PulseAudioDevice.cpp | ||
|---|---|---|
| 86–87 | Yes, the 0 is basically the (C) boolean that states whether it should be corked (1) or uncorked (0). | |