A small question about a data race

[dariop]

Hi, I’m in the process of making a sampler,
I have one structure, the “Sample”, that most of my audio classes (and some gui classes) point to that contains the sample data. When the user drag and drops a new file to load, the message thread calls loadFile() and some new memory gets allocated and the file gets written to the buffer in “Sample”. However, while the message thread is doing this the processBlock might be processing some samples, and accessing data from the sample buffer, which might have just been invalidiated by the allocations that ive just described on the message thread.

I’m just looking to see how some people on here would solve this problem.

I want to serialise the two threads but without using locks and without introducing any other data races.

I see a few solutions:

1. when the message thread calls loadFile() it instead sets a flag “setHasFileChanged(const String& newFile);”, then in process block check "has file changed? → loadFile(newFile).
2. save the state of the pluginprocessor as an enum (still introduces a data race in processBlock):

enum ProcessState {
  idle,
  processing,
  loading,
}

processBlock()
{
  if(state==loading) return;
  if(state==idle) state=processing;
  ... process audio ...
  state = idle;
}

void loadFile()
{
while (state == processing);
state = loading;
...load file...
state = idle;
}

3. Use locks (not very experienced with locks).

I hope anyone can give me their opinion on this issue!

[HowardAntares]

How about not putting it directly into the Sample object, but into a separate buffer instead, then setting an atomic flag that new data is in that separate buffer? Then, at the top of processBlock, get the state of that flag and reset it (using .exchange(false)), and if the flag had been set, then copying the data into Sample at that point in time, before processing the current block? That’s how we handle updating arrays of data that might change while processing.

[dariop]

thanks for your reply! .
I’m trying to picture how this would look in code:

void loadFile(String& file)
{
...
//allocate new data and set parameters
newSample = Sample();
newSample.data = HeapBlock(len);
newSample.sampleLength = len;
...
loadFileFlag.store(true);
}

void processBlock()
{
  if(loadFileFlag.load()){
    currentSample = newSample;
    newSample = Sample();
    setDataPointers();
  }

  ...process...
}

does something like this look alright? apologies for the crude pseudocode.

[HowardAntares]

Not quite. You’d want to use loadFileFlag.store(false) at the start of loadFile(), to prevent the swap from happening at the same time, and you’d want to use if (loadFileFlag.exchange(false)) in processBlock, to atomically reset the flag while checking it.

Otherwise, yeah, something like that.

You never want to try reading from a file in processBlock(), by the way. Pretty much guaranteed to stall the process thread doing that.

[zsliu98]

I use locks in my filters. It works, but I am not sure whether it is the most efficient way.

First, I declare a private class member, e.g.

juce::ReadWriteLock paraUpdateLock;

Then when I want to update the coeffs, I do something like this

// calculate coeffs
const juce::ScopedWriteLock scopedLock(paraUpdateLock);
// update coeffs

And the process function will need a ReadLock:

auto block = juce::dsp::AudioBlock<FloatType>(buffer);
auto context = juce::dsp::ProcessContextReplacing<FloatType>(block);
const juce::ScopedReadLock scopedLock(paraUpdateLock);
for (auto &f: filters) {
    f.process(context);
}

[Rincewind]

@HowardAntares

How about not putting it directly into the Sample object, but into a separate buffer instead, then setting an atomic flag that new data is in that separate buffer? Then, at the top of processBlock, get the state of that flag and reset it (using .exchange(false)), and if the flag had been set, then copying the data into Sample at that point in time, before processing the current block? That’s how we handle updating arrays of data that might change while processing.

I truly can’t believe that this is an always working and save solution. What happens in the following:

• Audio thread sets value to false (knowing something updated)
• Starts reading the data half way
• UI thread pushes new changes and sets the boolean back to true
• Audio thread continues reading and reads potential garbage.

@dariop please read this thread, especially my post. The creator of the library I references also has a talk on JUCEs YouTube channel. Please watch that for a deeper explanation of what the code does.

Smarter Atomic Smart Pointers

I just watched again that CPPCON talk: [Smarter Cpp Atomic Smart Pointers - Efficient Concurrent Memory Management - Daniel Anderson CppCon] I’m curious to know if somebody in that forum already considered/tried it?

Now to your exact use-case. There is this number one rule in concurrent programming: don’t implement the algorithms yourself. Use what is out there. It is 1) for sure better tested, 2) for sure better maintained and 3) developed by someone who has more experience on that topic than you do.

With that being said, here is what I would do:

1. First of all, make sure your sampler voices use an own self-sustaining pointer to you sample. Which means, don’t keep a reference to the sample living in the actual sampler, but give them their own “shared” (if you so will) pointer. That ensures that, what ever happens, you are not screwing with the voice already playing.
2. The backend structure (might be the so called UI code in your case) has somewhere the SnapshotSource<AudioBuffer<float>> currentSample object. When the user changes the sample, the UI loads the file and flushed the data into currentSample. Ever time a new voice is started, the sampler pushes Snapshot<AudioBuffer> into the voice to be played.
3. Start a time thread, that regularly deletes the outdated objects from the currentSample. (regular juce::Timer is enough, probably ever second is fine). EDIT: actually, I think you could cue up the timer thread when the user changed something as this is the only way you could built up stale objects. But that sounds a bit like premature optimisation to me, but might be a fun task to think about.

Thats it, you are done. It is actually a very easy task to accomplish if you know that there is a certain type of concurrent memory management that should be going on (which you did, since you asked the question) and you know the library that has the abilities to solve the problem. You should work with that as passing around std::shared_ptrs but the library assures you, you are doing it in a thread safe way.

This is of course a very basic and simple implementation. For example you always have the sample completely loaded in RAM. If you are building something small, this is not a problem. If you have a bigger sampler, you should overthink this part.

[dariop]

thanks for the reply! lots to dig into there.

@Rincewind

• Audio thread sets value to false (knowing something updated)
• Starts reading the data half way
• UI thread pushes new changes and sets the boolean back to true
• Audio thread continues reading and reads potential garbage.

I think you misunderstand Howard’s approach.

• Audio thread sets value to false - (has just updated the sample data).
• Starts processing, reading data etc. (no writing)
• UI thread pushes new changes to a “new sample” object so the data that the process block is reading from isn’t affected.
• UI thread sets boolean back to true.
• At the start of the next process block it reads that there is fresh data in the “new sample” object so copy over the contents into the sample the actual Sample structure, then flush the previous contents of the sample.

Unless I am missing some hidden data race then I believe this is indeed a safe solution.

@Rincewind

First of all, make sure your sampler voices use an own self-sustaining pointer to you sample

I’m not quite sure what you mean. Do you simply mean to use std::shared_ptr? or maybe a weak_ptr? since the voices don’t really own the sample, they simply read from it.
Currently in all my voices and MPESynthesiser class I have a reference to a Sample structure contained in the Processor:

struct Sample {
//buffers need to be stored as 8 bit integers ;)
//theres 2 so I can ping pong any modficications I make to the buffer.
    HeapBlock<int8_t> buffers[2];
    int8_t* data = &buffers[0]; //stores the current sample data to read from
    int sampleLength;
    double sampleRate;
    //etc.
};

class MySampler : juce::MPESynthesiser {
private:
Sample* sample;
}

class MyVoice : juce::MPESynthesiserVocie {
private:
Sample* sample
}

These sample pointers are updated through a call to “setDataPointer” in the Processor. Do you mean instead that each voice or perhaps the MPESynthesiser object should hold in memory the actual data? instead of it being passed from the Processor?

[HowardAntares]

@Rincewind

• Audio thread sets value to false (knowing something updated)
• Starts reading the data half way
• UI thread pushes new changes and sets the boolean back to true
• Audio thread continues reading and reads potential garbage.

Um, the UI thread doesn’t write to the “live” data at all. It writes to an alternate buffer, which the processBlock code reads into the live code before processing that block. In our code, we don’t even have the process code copy the data; it just swaps pointers between the “live” data and the data that can be modified in one atomic operation. In either case, the UI can be made to wait on the process thread to do its work before writing to the “alternate” array, when the process code is swapping/copying the data. That won’t block the process thread, only the UI thread.