void GainMeterAudioProcessorEditor::paint (Graphics& g){
g.fillAll(Colours::black);
g.setImageResamplingQuality(Graphics::lowResamplingQuality);
processor.gainMeter.draw(g, getLocalBounds().toFloat());
}
void GainMeterAudioProcessorEditor::timerCallback() {
if(processor.gainMeter.shouldRepaint())
repaint();
}
void GainMeterAudioProcessor::processBlock (AudioBuffer<float>& buffer, MidiBuffer& midiMessages){
ScopedNoDenormals noDenormals;
auto totalNumInputChannels = getTotalNumInputChannels();
auto totalNumOutputChannels = getTotalNumOutputChannels();
for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
buffer.clear (i, 0, buffer.getNumSamples());
gainMeter.setBuffer(buffer);
}
the line that takes the sample data from the audio buffer is in my method setBuffer() the first line with m_magnitude.
i just pass the magnitude value to my gui basically, along with a subtle envelope follower adding a bit of release
-> in this part of my video you can see how it glitches out on cubase’ mixer visuals even though the performance is shown to be very ok (the link goes to a specific time). interestingly you can also see that my actual visuals are fluid. it’s just that they block my daw from working correctly:
Ok, the m_imageIdx is accessed from both threads and should therefore be wrapped into an atomic:
std::atomic<int> m_imageIdx { 0 };
Then your compiler should complain about shouldRepaint(): there is a path that won’t return a value. That is undefined behaviour.
It should probably look like:
bool shouldRepaint() {
const auto current = m_imageIdx.load();
if (! m_isReady || m_imageIdxCache == current)
return false;
m_imageIdxCache = current;
m_imageIdx.store (current < m_images.size() ? current : m_imagesMax);
return true;
}
I can’t tell if either of those fixes solve your problem, but it’s something.
ok so. your code change suggestions totally make sense and it seems like it glitches slightly less now on 60fps and 700x700px. i was also able to remove the need to limit the index in shouldRepaint by improving the setBuffer-method a bit. my actual problem is not solved yet ofc but i have another question about this. i tried to write:
m_imageIdx.store(int(m_gain * m_imagesMax));
instead of
m_imageIdx = int(m_gain * m_imagesMax);
and it seems to do the same thing. are these different operations?
You are right, it is the same. The = operator for atomic has an overload that calls store(). The first version is just a bit more verbous as it makes it obvious that it is storing atomically, but there is no technical necessity to write that. It would be, if you need different memory_orders, which you can supply as second argument to store(), but for your case AFAIK that is not necessary.
Reference: std::atomic<T>::store - cppreference.com
// set buffer from processBlock().
void setBuffer(AudioBuffer<float>& buffer) {
auto m_magnitude = buffer.getMagnitude(0, buffer.getNumSamples());
if (m_gain < m_magnitude)
m_gain = m_magnitude;
else
m_gain = m_gain + m_release * (m_magnitude - m_gain);
if (m_gain > 1.f)
m_gain = 1.f;
m_imageIdx.store(int(m_gain * m_imagesMax), std::memory_order(1));
}
// called in timerCallback to check if repaint is needed
bool shouldRepaint() {
const auto curIdx = m_imageIdx.load();
if (!m_isReady || m_imageIdxCache == curIdx)
return false;
m_imageIdxCache = curIdx;
m_imageIdx.store(curIdx, std::memory_order(0));
return true;
}
that’s exciting. i just watched a video on atomics and learned that they are meant to store variables in a way that they can’t be accessed from 2 threads at the same time. i haven’t learned a lot about threads yet, but i guess they split up the cpu work into different parallel processes or so, right? so now i added std::memory_order(0) and (1) to the lines where i use store, as you can see. am i right to assume that this would now set the priority of which process to be applied first to the line in bool shouldRepaint() ?
Since it is the first entry in the enum, it is most likely 0, but it is also much more readable to use the name of that option.
Second, for an int variable, on most architectures the int is an atomic type, so there will no locking going on anyway. But the atomic serves a second purpose: it stops the optimiser from false assumptions, like “this variable cannot change during this block, so a certain statement is a NOP and can be left out”.
Best video on that topic is Fabian (former JUCE maintainer) and Dave (Tracktion) on the 2019 ADC:
Watch both parts, I learned more in that 90 minutes than usually in a year
btw back to the actual gain meter: do you think it would make sense to not just get the magnitude at the end of a block but also in smaller steps to reduce the “visual latency”? theoretically that would make sense, wouldn’t it? because now the gui thread can get the index whenever it wants to
I don’t think it makes sense. Even with your ridiculous 60 FPS (sorry, but IMHO that makes sense for games, but not for an information display, the eye cannot capture that information), each frame is shown for 16ms, while a 512 samples block at 48kHz lasts 10ms, so a frame will cover almost two blocks already.
I would go down to 30FPS.
You will actually face the opposite problem (ok, maybe not with your traffic light visuals), that your indicator moves jumpy and by just taking a quick look you cannot get the right information.
Classic VU meters have a defined timing, like rising faster and releasing slower. If you want a fancy display, this might not be important, but if you want to give information to professionals, you want to check and implement the standards.
Also I usually prefer to see both values, RMS and Peak, so my meters have a max line, that holds for 300ms and a rms bar, that has averaging over a longer block (accumulating a longer period) to have smooth movements.
always feel free to critisize my plan, even if it’s offtopic. very appreciated i also think 60 fps is too much anyway. 24 or 30 sounds reasonable. i was just wondering why it created this weird behaviour because a gain meter alone is not much in a plugin and if this thing already breaks everything i can’t even think of adding anything else to it, can i? so regarding this it should possibly work at 60fps without any other functions
edit: i now added the thing i mentioned in this comment and it looks like this:
// this is called in the sample/channel-loop in processBlock. *sample = buffer.getReadPointer();
void setSample(float sample) {
if (++m_sampleIdx < m_samplesCount) {
auto absSample = std::abs(sample);
if (m_magnitude < absSample)
m_magnitude = absSample;
}
else {
if (m_gain < m_magnitude)
m_gain = m_magnitude;
else
m_gain = m_gain + m_release * (m_magnitude - m_gain);
if (m_gain > 1.f)
m_gain = 1.f;
m_sampleIdx = 0;
m_magnitude = 0.f;
m_imageIdx.store(int(m_gain * m_imagesMax));
}
}
m_sampleIdx just iterates up until it reaches m_samplesCount, which was calculated by sampleRate / fps and it’s the amount of samples it should do its thing before calculating the index value for the image, so it only calculates them in the same rate as stuff is being drawn. visually nothing seemed to change but i guess it works if my logic has no flaws
i just went to bed though. i’ll let you know what will have changed tommorow. sleep well