I've been trying to work out the intended threading model is when using drowaudio AudioFilePlayer / AudioFilePlayerExt?
From what I've been able to work out:
drowaudio is in various ways not threadsafe to use in a way that one is creating or calling functions (such as setPosition) on AudioFilePlayer / AudioFilePlayerExt objects outside the critical audio thread (i.e. in a different thread than the one that calls getNextAudioBlock). I've found multiple ways in which it is not threadsafe (causing asserts+crashes-etc) for such a use-case, and making it threadsafe is not possible 'from the outside' as it were.
A MessageManager lock is required in order to create AudioFilePlayer objects (when AudioFilePlayer::commonInitialise() calls audioTransportSource.addChangeListener(this)) - this means that one is forced to acquire a MessageManager lock in this critical audio thread - obviously not a desirable thing to be doing (and also currently causing a deadlock for me on shutdown when it tries to stop the audio if it's in the process of starting some file playback inside the audio thread, tho I'll likely find some way to work around that).
Note, I haven't upgraded to juce 4 yet - I'd be curious whether there's any improvements in juce 4 to any such audio / threading-related areas? i.e. one potential fix for the above and something that would be generally preferable, is that changeListeners could be made internally threadsafe and thus not requiring any global MessageManager lock for them at least, thus no global lock would be required to add a changeListener to a ChangeBroadcaster.
I'll be honest it's been a long time since I've used those classes and even longer since I wrote them but they did used to work. They follow the same threading guidelines as other juce audio sources as they mostly just pass methods on to juce classes.
1. Looking into setPosition, you're really talking about if juce::AudioTransportSource is thread safe. This should be fine as long as you don't change the source concurrently with setting the position or similar.
2. You shouldn't be creating AudioFilePlayer objects on anything other than the message thread. In general if you find yourself using MessageManager locks you're probably doing something wrong.
Basically, I'd stick to the message thread for anything apart from the getNextAudioBlock method.
What exactly are you using these for? If you've got a complex threading model you may need some more specialised classes.
I had been doing this creation of AudioFilePlayer on a separate thread (not the messaging thread and not the getNextAudioBlock() thread), as I didn't want it blocking/being-blocked-by what was going on in the 'message' thread .... but it was then having to get the MessageManager lock where juce was asserting if it didn't (when constructing AudioFilePlayer objects), so that plan was anyway not being entirely achieved .......
And then due to the threading issues that I saw, I moved it to the thread that calls getNextAudioBlock(), which is definitely not my preference / ideal, but as that seemed like the only way to solve the threading issues I was seeing ...... my recollection is that they didn't appear to be issues that would be solved by moving creation+control of the AudioFilePlayer objects to the 'message' thread, but to be sure I would really have to try.
So I will try that (moving to message thread), and if I still see+reproduce the threading problems I will post them specifically here, since I at least understand now from you that this is the expected way that it should work and be threadsafe. Note I'm not doing anything complicated with these AudioFilePlayer objects, not switching their source or anything.
Constructing these objects should be relatively trivial, I don't think they do a huge amount of work. Unless you're seeing a problem I would stick to allocating on the message thread. (I appreciate this isn't always possible e.g. if you load projects on a background thread, but then you have to do all sorts of synchronisation with the main thread anyway).
The best method I've found to solve the above is some kind of blocking message posting that will safely bail out and return if the calling thread is requested to stop. It's non-trivial to get right though so should only be used if you can't avoid it.
But the bigger issue, is simply struggling to deal with the fact that it appears that, due to the way its designed threading-wise and as you suggested, the user has no choice but to control the audio from the main juce/UI thread, or at least while holding MessageManagerLock, which either way means that it has to thus be blocked by things like any graphics drawing (exacerbated by the fact that, in my project, doing some very simple graphics stuff on iOS using juce’s graphics functions for example easily leads to it blocking 200ms+ per frame every frame [not to speak of when something more significant happens], which is for some projects’ needs rather unacceptable for then wanting to control audio on the same thread).
Note - I’ve worked on major realtime audio projects in the past, and this locking model that has been taken into drowaudio for some reason is simply not how I’ve experienced things being done / done things. There appears to be no strong unavoidable reason to tie such audio stuff in at all with this global lock, and I’ve worked on major audio products in the past that were certainly not doing anything at all close to this level of global locking, thus allowing everything audio-related to be totally never blocked by UI work or other things … so my suggestion (since I can’t see many other options) would be really to think about reworking the threading-model as used in the drowaudio stuff at least? Also keen to hear other suggestions …?
Firstly, thanks for the bug fixes. I had stumbled in to the infinite loop in SoundTouch before but couldn’t make sense of the code so was unable to fix it. I know there’s been updates to SoundTouch since though, perhaps they have a fix?
As I’ve mentioned, you shouldn’t need to be holding MessageManagerLocks. If you’ve got to this point you’re probably using the classes in a way they weren’t intended for. These classes are very old and really just wrap up a chain of juce::AudioSources. If you really need custom threading behaviour it shouldn’t take too long to duplicate the functionality in your own class? Apart from creating these objects, what methods are you trying to call in the audio callback?
It’s also worth noting that these classes were designed to simply and quickly load a file and play it back. They’re not really intended for large scale applications. Whenever I’ve written a serious app I’ve always needed more specific control over audio playback (with sample accurate sync etc.) which these classes don’t offer.
They’re really designed for new users to quickly load and playback an audio file and get familiar with the JUCE classes. If you’re at the point when you’re dealing with scheduling threads etc. your app has probably outgrown their usefulness.
I’d love to go back and re-write all these classes for proper, large scale applications and make them completely thread safe, sample accurate and efficient etc. but I simply don’t have the time these days. That’s the reason I made the project MIT licensed, as I can’t offer a paid level of support and I was hoping others would fork and continue development, in turn helping other beginners.
Hi - thanks for the response - so you’re saying I shouldn’t need to be holding MessageManagerLocks … so, that means, I could control the audio from my own background thread without holding MessageManagerLocks while starting playback of a file, changing gain, etc, in theory?
The thing us - that that would be more ideal … however, I have actually run through all the potential ‘threading models’ - not coz I’m trying to do anything too crazy, but simply guessing at what drowaudio was expecting, given the problems I kept encountering … So I tried controlling from within the realtime audio thread (and hit issue which made clear that it wasn’t designed with this usage in mind), controlling from own background thread with no MessageManagerLock, and controlling from main UI thread (or from own background thread while holding MessageManagerLocks, which I understand to be basically the same thing here). And with all three I hit various issues …
Although I’m a little fuzzy right now though on exactly what the issues were when running in my own background thread and not holding MessageManagerLocks - it might have been partly various audio issues that I suspected were caused by it not handling this usage, but they may well have been the ones that anyway didn’t go away (and/or I fixed with some of those commits). Although, one thing also to note is that, at least without one hack I made - https://github.com/mbevin/drowaudio/commit/829ed9602951fd7c52c91dd566a56e100341ff86 - there’s at least that one place where it does acquire/require a MessageManagerLock (as I mentioned in my second post in this thread) …
And so also: you say I’ve probably outgrown drowaudio - would you/anyone recommend something else, like YSE - http://www.attr-x.net/yse/ - instead, for example (which says its based on juce)? (if you happen to know about YSE) I had come across it, but kind of assumed in saying ‘based on juce’, that it was based on drowaudio too, and thus unlikely to be solving the problems I was seeing within drowaudio and thus only to be potentially adding more … but maybe that was the wrong assumption …?
