synthVoice - Why is override needed? Note stealing problem

Audio Plugins
1

I am a good way into developing a synthesizer using synthVoice, but am running into a problem that stopNote() is called on every voice still playing, when repeatedly hitting same key.

I looked into solutions presented here https://forum.juce.com/t/voice-stealing-when-the-same-note-is-pressed/30344/2, especially as @DaveH posted “Just as a catch-up, all I had to was override ‘isVoiceActive’ in the voice itself, and return true if the ADSR is still running.”, but it did not solve my problem.

// Including or omitting override seems to make no difference
bool isVoiceActive () const override 
	{
		return adsrRunning;
	}

Then I looked briefly at the “Tutorial: Build a MIDI synthesiser” here https://docs.juce.com/master/tutorial_synth_using_midi_input.html and was wondering why startNote, renderNextBlock, and stopNote have to be overridden? It is not something I had done before, and adding made no detectable difference.

Anyways back to the problem causing me to ask whether override is really needed.

In my stopNote I got;

// Including or omitting override seems to make no difference
void stopNote (float velocity, bool allowTailOff) override
	{
		ignoreUnused (allowTailOff);

		adsrVoice.release (); // Starts release stage of my custom ADSR

        // I got the below test variable for all my 16 voices, each synthVoice with its 
        // own unique voicenumber identifier. When playing notes, all 16 test values
        // is displayed in the plugin so I can see what is going on.
		testValue[voiceNumber]++;
	}

So if I repeatedly hit the same note, that is hit the note (that calls startNote) which leads to my ADSR start its attack stage, then release the key (which calls my stopNote) before sustain stage is reached, and with a decent amount of release time, therefore before the release stage is done (which if it had would make my code call clearCurrentNote), then hit the same key (note) again, every single voice still going through its release stage is forced an unexplainable stopNote. In other words if for example voice 0 through 5 is still sounding, going through its release stage, when I hit the same note again, voices 0 through 5 have its stopNote called, even though startNote correctly is invoked in a free synthVoice.

I should mention that I off course considered that my problem could easily be because my custom ADSR is the cause, however if I instead of repeatedly and in fast succession hits the same note, hit different notes, say quickly go up or down the scale, there is no problem!

2

“Override” is a keyword that makes no difference for how the code behaves at run time. It’s only for detecting the problem during compile time where the developer thought a virtual method was being overridden, but actually wasn’t, because there was an error in the method name/signature.

Example : if you subclass Component and misspell the method name :

class MyComponent : public Component
{
public::
  void MouseDown(const MouseEvent& ev)
  {
    // are we ever getting called by Juce? no, because of the tiny typo in the method name. but the code would compile just fine.
  }
}

“Fix” with “override” :

class MyComponent : public Component
{
public::
  void MouseDown(const MouseEvent& ev) override
  {
    // Now the compiler will complain there is no overridable method MouseDown.
    // And hopefully you will soon notice you needed to name the method mouseDown, 
    // not MouseDown
  }
}

Sorry, I have no idea about your actual problem in your synth at the moment.

3

Ok thanks for that explanation.

4

Trying to track down my note stealing problem, which as I just amended in my original post, only happens if I hit the same note repeatedly.

In my startNote I currently and unefficiently simply copy my ADSR class to the local synthVoice like so;

adsrVoice = myADSR;

That way if I edit any parameter in the ADSR, duration, level, etc, whenever I hit a note, it is updated in synthVoice. However as I mentioned that is off course inefficient, as it really only needs to update if there is a change and not every time I hit a note. Sure I could have an “if” to test if there was a change, but as I also had other local parameters update/copied every time I hit a note, I realized it would be better to move all of that to an updateParameters function.

So in synthVoice I added this function, just starting really simple with doing only one thing, updating the local ADSR;

void updateLocalParameters ()
{
    if (isVoiceActive () == false)
	{
	  adsrVoice = myADSR;
    }
}

Then in pluginProcessor I do this after I have added voices, and initialized my ADSR;

for (int i = 0; i < maxVoices; i++)
			myVoice[i].updateLocalParameters ();

I do something similar in pluginEditor, when I change an ADSR parameter, but either way whenever a call is made crash! A crash actually happens whenever I try to update any local synthVoice variable. How can I overcome this?

5

Ok I figured out how to update my local synthVoice parameters from pluginProcessor by adding a function there, and that function I can also call from pluginEditor whenever I update my ADSR;

void MutineerAudioProcessor::updateVoiceParameters ()
{
  for (int i = 0; i < mutineer.getNumVoices(); i++)
	{
		if (auto voice = dynamic_cast<SynthVoice*>(mutineer.getVoice (i))) 
        {
          voice->updateLocalParameters ();
        }
	}
}

It makes my startNote more efficient, but it does not solve my voice stealing problem.

6

If you replace your custom ADSR with the Juce ADSR, do you get the same problem?

7

wha…?

so instead of each voice having its own ADSR object, you only have one global ADSR object and pass it to each voice every time they turn on…?

or are you attempting to pass the state of the parameters of an ADSR object? Because if that’s your intent, it’s likely that calling your updateLocalParameters() function is updating your adsr parameters but also setting every voice’s ADSR state to the state of whatever “myADSR” is – so if “myADSR” is off, then calling updateLocalParameters() will probably turn off all your voice’s ADSRs. If “myADSR” is in its release phase, then calling updateLocalParameters() will jump every voice’s asdr to its release phase, regardless of where its adsr “should” be.

This is why the juce ADSR class separates its parameters into the ADSR::Parameters struct. I would suggest making some kind of “parameters” struct for your custom ADSR class, and create a setParameters method for your ADSR class. Inside updateLocalParameters(), you should be calling some kind of setParameters() on the voice’s adsr object, not actually replacing or copying or moving or reassigning the ADSR object itself!

8

Thanks for taking the time to chime in. No I have not. But for now I have made a “hack” that works, but you’re right and I thought of trying your suggestion too. Anyways for my hack see my reply in your next answer.

9

I could certainly have made a mistake somewhere, however each of my voice has its own ADSR class. And to try and solve my problem, I even split my ADSR class into two similar to what you suggested, one for the parameters, and for returning current level.

Also as I was suspecting something was happening due to me copying global adsr to voice adsr, I no longer due that.

Above changes did not solve the issue, so for now I have made the following “hack” that solved my weird problem;

void stopNote (float velocity, bool allowTailOff)
	{
		ignoreUnused (allowTailOff);

        // Release, unless already released - Sounds strange but this solved my problem
		if (adsrVoice.getStage () < 3) adsrVoice.release ();
	}

Still you’re right, I may have messed up somewhere, and will eventually investigate some more.

10

The allowTailOff tells you if the note was stolen or is releasing regularily.

I have in my synth this (with the stock juce adsr class):

void FoleysSynth::FoleysVoice::stopNote ([[maybe_unused]]float velocity,
                                         bool allowTailOff)
{
    if (allowTailOff)
    {
        adsr.noteOff();
    }
    else
    {
        adsr.reset();
        clearCurrentNote();
    }
}

I guess if you never call clearCurrentNote() you will end up with all notes being stolen.

You also need to call clearCurrentNote() in your renderNextBlock() once your custom adsr reached the end.

11

I do call clearCurrentNote() in my renderNextBlock(), like so;

// If I detect release is done
{
    adsrRunning = false;
    clearCurrentNote ();
}

But I do not call it like your example in stopNote(). I'll look into that, thanks.
12

Well I did a quick test and it did not solve my problem. I admit I had been simplifying my posted code example, as actually my synth is kind of semi-modular with 8 tone generators, 8 envelopes, 8 effects, 8 LFO’s, and 8 filters, all running in a synthVoice per voice(note), so off course it complicates things a bit. This is what I tried, with my actual normal 8 envelopes;

void stopNote (float velocity, bool allowTailOff)
{
    // I am testing for false (stolen I assume) first since I need to test 8 envelopes
	if (allowTailOff == false)
	{
		adsrRunning = false;
		clearCurrentNote ();
	}
	else
	{
		for (int module = 0; module < maxModules; module++)
		{
			adsrVoice[module].release ();
		}
    }
}

What I am seeing that the allowTailOff == false part seems to only be done during the creation of the synthVoices via PluginProcessor. Strangely it is done exactly 96 times, and since my synth has 16 voices that number 96 equals exactly 16 x my 8 tone generators. To clarify when rapidly hitting the same note, it never enters that part. However it does go to the (else) allowTailOff == true part.