ffAudio / ff_meters

Plug and play component to display LED meters for JUCE audio buffers
BSD 3-Clause "New" or "Revised" License
114 stars 31 forks source link

Race conditions in ChannelData #21

Open Rincewind34 opened 2 years ago

Rincewind34 commented 2 years ago

Hi again about multi threading

ChannelData also raises a bit suspicion. The rmsHistory should really be a vector of atomics, getAvgRMS (from UI thread) clashes with pushRMS (from AudioThread). You did it right with the rmsSum field.

Then there is also a race condition inside pushNextRMS because it might be called by the UI thread by decayIfNeeded. I would highly suggest removing that from LevelMeterSource and by that use a const reference to the LevelMeterSource in LevelMeter. You could do the time check inside the LevelMeter class and just "render zeros" no matter whats inside the LevelMeterSource. If thats not possible, I would make rmsPtr atomic too and use "fetch_add" in pushNextRMS to read and increment the ptr in one atomic action. You'd calculate the module operation every time so the ptr just increases until the max value of size_t and then overflow to start over at zero (perfectly acceptable behaviour).

Rincewind34 commented 2 years ago

After a quick second thought, I don't think anyone really expects the check anyway. It only appears if either one doesn't know how to use the source properly or the audio block lags. The second one isn't really something I would pay any interest in as at that point everything is going down hill anyway.