Why the AudioBuffer class has an atomic clear flag (with cst order!)? It’s hurting too much my performance (due to sub-divisions) for a class that should be single thread.
Could I do some evil hack to use a normal bool flag instead? (or at least an acq-rel combo)
For what it’s worth, I don’t like the flag in there, atomic or not. I think dsp::AudioBlock is a better API overall, and only use AudioBuffer at the boundary to JUCE when it’s required. For example with AudioFormatReader/Writer, where the overhead of the flag is insignificant.
But you cannot call AudioProcessor::processBlock() with a dsp::AudioBlock, it requires an AudioBuffer (I’m sending small 16 samples buffers to hosted plugins)
You could patch the JUCE code and remove the atomic. In my humble opinion the whole isClear logic could be removed from AudioBuffer.
This would make a massive difference! AbstractFIFO has the same problem (and is many times faster if it’s tweaked).
Indeed, since AbstractFIFO is SPSC, a careful usage of release and acquire flag (even some relaxed ones) will increase the performance, but since I’m only using one of them, reading once per processBlock() it’s not a big deal for me.
Anyway, I will change the AudioBuffer::isClear atomic flag to a normal boolean, since I only use it in the audio thread… I hope it doesn’t break nothing.
I don’t think you’ll break anything. I wonder why it was ever made atomic. It used to be a bool.
It was changed April 1st 2019. Interesting date…
“potential” sounds like there was no real problem to fix here and the whole class is not thread safe to begin with.
I’m thinking the potential bug could be when the software invokes the AudioProcessor::prepareToPlay() function, where you resize the buffers. Usually it is done in the audio thread, before the processing starts, but hosts may call this function in another thread, like the audio thread, and THEN, start with the AudioProcessor::processBlock(). Maybe could be some confussion here, but as you said, the rest of the class isn’t thread safe, so you only would save a minimal race.
Anyway I changed it and I’m very happy with the result now. I will blame the host if there is any problem with data race 
Using a wise combination of acquire/release even relaxed would be much better, and this seems a very easy case to do it.
By definition, prepareToPlay and processBlock can never be called at the same time. And whether the buffer is clear or not would be the least of our problems if that happened 
.
Why on earth doesn’t AudioBuffer::clear() actually clear the buffer when I call it? Because of this isClear flag. It’s not in the documentation that the buffer might not actually be cleared if I call that function. This is just dumb. If I call clear(), it should just clear the damn buffer, not check on a hidden, undocumented flag.
Chiming in on the isClear flag debate:
I had some issues with the clear flag aswell and stumbled on to this thread.
So I have a buffer, which is stored in many instances of a processorClass via pointers to this buffer, so the instances work on the buffer (mainly copy from it and transforming it) independently.
Even though everything works fine audio wise, the buffer says it is clear (Even with sound output on my speakers from this exact buffer).
So for example if I try to read the RMS level, it just default returns 0 (because the buffer thinks it’s clear), even though my speakers are clearly saying something else.
So the flag isn’t really tied to the buffer content, it just assumes (based on the functions you called) that it should/could be clear.
Maybe the way I use it isn’t the intended way and there are solutions, but this seems kind of odd to me.
My “hack solution” right now was to just call “getWritePointer(0)” before my usage, since this sets the flag to be false, and then I can access it.
How are you getting in to a state where you have a buffer that the isClear flag is returning true but it has non-zero content?
This would suggest that you are calling getReadPointer and casting away the const-ness?
nope not at all! (wouldn’t even know how to do that to be honest )
No const casting or anything, just using the intended functions ( I think, beginner here so I might be doing nasty stuff without knowing!)
it’s hard to post a code snippet right now since there are many classes involved.
Each of my “child processors” has a pointer to an input/output buffer channel ( I use .getWritePointer(channel) here)
in my main getNextAudioBlock() in pseudo code it looks something like:
myOutputBuffer.clear();
for every child:
process ( copy from input with gain into myOutputBuffer)
myInputBuffer.clear();
copy myOutputBuffer to bufferToFill
the process of a child looks like this :
for(auto samp = 0; samp < numSamples; ++samp)
{
outBuffer[samp] = inBuffer[samp] * gain.getNextValue();
}
where outBuffer and inBuffer are pointers retrieved by .getWritePointer() like mentioned earlier ( Although I don’t really do anything to inBuffer so I guess I could use .getReadPointer here)
If I try to check the outputBuffer after child processing (for example .getMagnitude() ) the flag is true although it is filled and I hear my inputs with gain, everything working fine.
I think it has something to do with the order of steps, clearing it and letting the children work on it via their pointers then?
I might be able to extract the interesting code snippets later on, so an expert can have a look.
I assume that I do something bad, but it’s still weird.
that’s easy to get in such state with something like that:
auto buf = buffer.getWritePointer (0);
buffer.clear();
buf[0] = 1.f;
buffer.isClear() now returns true but it isn’t…
pretty sure that’s what’s happening to me, looking at my code example / child process!
I clear the buffer and then the childs access their pointer and set a sample, so the “buf[0] = 1.0” that @lalala posted.
So is my way of doing it wrong/illegal? Is there a better approach / design so I can avoid this?
Clear first, write samples afterwards.
But that is what I’m doing? I clear the buffer and then I write my samples, like in the example lalala posted?
The only way to avoid it there would be to clear first and THEN get the writePointer (which sets the flag to be false). But I don’t want to get the pointer every time, since I only need to set it once on construction of my objects.
Hmm, I see. I’ve never actually run in to that problem… maybe because I’d probably prefer to clear first as that acts on the whole buffer and then get the channel pointer as close to where I’m using it as possible to minimise scope.
I can see the problem though, I do like APIs to be difficult to misuse or at least assert.
The thing is that AudioBuffer is quite a high level object where the isClear flag can make a real difference in performance, especially when you are doing lots of combining of them in situations where there is likely to be lots of silence.
I guess ideally you’d want some kind of “write handle” class that could assert if you called clear with any active instances. That would add a lot of code and types to the API though.
Did he perhaps mean “clear first, then call getWritePointer()”?
Sorry, I tried to imply that by using the words based on the API provided! Yes, that is entirely what I meant.
I don’t personally see the issue here. You get a concise and obvious set of operations in your code when you lay your code out this way.
I’m just guessing due to not having any example code - to me, by the sounds of it, there’s an over-elaboration of the code that could otherwise be very concise (small static functions? lambdas?) to operate on the audio buffers.
