ffAudio / ff_meters

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

Fix a few crashes I had #4

Closed christofmuc closed 2 years ago

christofmuc commented 4 years ago

I might be using the component the wrong way, but I am able to trigger these functions being called with a channel ID that no longer is within the valid channel range, and this will crash the application within the at() operation.

I think this happens when the draw loop/audio loop is running, and some reconfiguration takes place? I wasn't really able to reproduce the crash yet.

ffAudio commented 4 years ago

Seems like the easiest fix. However, this method is called very often, so I would want to avoid to check every call. Could you please instead of checking add an assert like:

// instead of:
if (channel < levels.size())
// do this:
jassert (channel < levels.size());

When this happens again, your callstack will tell, how we got into that problem. So we can fix the problem at a place, where its not so expensive at runtime.

christofmuc commented 4 years ago

Thank you! Will do!

I am trying to understand the design (why not checking could be legal at all) - there are only two write operations to the levels member, both of which could invalidate the channel count another component has stored. One in resize() (which has a comment saying FIXME: don't call this when measureBlock is processing) and one in measureBlock(). The first I never call, the second I call from within the audio thread.

The crashes happen because the LevelMeter::paint calls drawMeterBars which in turn calls getRMSLevel() amongst others, with a channel num previously extracted from levels.size().

But the paint is on a different thread than the measureBlock(), so I suspect a thread problem when the number of channels changes and measureBlock() is called with less channels than before (which happens in my application).

To do this without the checks above I'd need the audio thread to stop the timer of the LevelMeter triggering the repaint, call measureBlock() with the reduced channelCount which will cause the levels.resize(), and then restart the timer.

The Timer is private in LevelMeter (and it should be), and also this feels backwards.

ffAudio commented 2 years ago

Since the resize in measureBlock() is removed now this fix is no longer needed.