Why Doesn't This Choke Script Work?

ericchesek

Hi folks!

Can someone check my logic here? I am attempting to fade out any ringing snare samples when a side stick is played, but I am getting an error

Snare Proc:! onNoteOn() - Line 13, column 24: NoteOn with ID3127 wasn't found {U25hcmUgUHJvY3xvbk5vdGVPbigpfDMzOXwxM3wyNA==}

I see 3127 when I check the array data, so I don't understand how that ID is not found. Something to do with artificial events?
Also, this error line repeats when I play the side stick note, even after playing several other snare notes which means eventID isn't updating properly.

Sampler.enableRoundRobin(false);

const eventList = [];
eventList.reserve(32);

inline function RoundRobin(low,hi)
{	
	local RRVal;
	local PrevRRVal;

	while (RRVal == PrevRRVal)
	{
 		RRVal = Math.randInt(low, hi+1);
	}
	PrevRRVal = RRVal;
	return RRVal;
} 
function onNoteOn()
{		
	Message.ignoreEvent(true); 
	Message.makeArtificialOrLocal(); // make note artificial so it can be killed	
	var noteNum = Message.getNoteNumber(); 
	var noteVel = Message.getVelocity();
	var eventID;
	
	switch(noteNum) // get the incoming MIDI note 
	{
		case 37: // side stick
		
			for (eventID in eventList) 
				Synth.addVolumeFade(eventID, 10, -100);
				// fade out previous snare hits when sidestick is played;
				
			eventID = Synth.playNote(RoundRobin(39,47),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1);  
			break;
			
		case 38: // hit
			eventID = Synth.playNote(RoundRobin(0,15),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;
			
		case 39: // flam
			eventID = Synth.playNote(RoundRobin(34,37),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;
			
		case 40: // rimshot
			eventID = Synth.playNote(RoundRobin(17,32),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;		
	}
	
	// Synth.noteOffDelayedByEventId(eventID,1); 
	// this throws "API call with undefinded parameter 0"
	// unless I put this line inside every case statement.
	// Probably because it triggers on every noteOn, 
	// not snare notes only. 

	eventList.push(eventID);
	if (eventList.size > 32){
		// ideally, the event list would clear each eventID as voices finish playing,
		// but I don't know how to do that yet :) 
		eventList.clear();
	}
}



function onNoteOff()
{
	Message.ignoreEvent(true);
}
 function onController()
{
	
}
 function onTimer()
{
	
}
 function onControl(number, value)
{
	
}

d.healey

@ericchesek
Are you doing this in your UI script?

What's with the loop in the RR function?

You should use local variables in the MIDI callbacks, not var

You have declared a variable called eventID, but then you are also using eventID as the loop iterator. Are you sure you want to do that?

When you make the note artificial it will return an event ID, you should capture this so you can use it to turn off the note.

You should probably use a MIDI list instead of an array for storing your event IDs.

ericchesek

@d-healey said in Why Doesn't This Choke Script Work?:

@ericchesek
Are you doing this in your UI script?

What's with the loop in the RR function?

You should use local variables in the MIDI callbacks, not var

You have declared a variable called eventID, but then you are also using eventID as the loop iterator. Are you sure you want to do that?

When you make the note artificial it will return an event ID, you should capture this so you can use it to turn off the note.

You should probably use a MIDI list instead of an array for storing your event IDs.

Nope, this is a MIDI processor inside my snare sampler.

The loop is just so the same RR value never appears twice in a row. I've wondered if this is unsafe to do for some reason, but it seems to work well.

Thanks for the note about local vs. var! I'm still trying to figure out the appropriate uses for these data types. Initially I thought they should be reg's declared in the on init function, but this caused errors from what I remember. I thought it would be bad practice to redeclare a local variable on every Note On.

Using eventID as the loop iterator - no, I'm not sure I want to do that. I misunderstood how the for...in loop works. I thought it was more of a search function, but your use of the word "iterator" clears it up for me. :)

Capturing the artificial eventID - I'll see if I can rework the code keeping this in mind. I thought I was doing this already since I make the note artificial on every NoteOn, then I capture an eventID when a PlayNote function is triggered.

MIDI list instead of an array - I tried this but encountered some problems. I attributed it to the MIDI list storing values from -127 to 128 using the setValue function, but maybe something else was wrong.

d.healey

@ericchesek

@ericchesek said in Why Doesn't This Choke Script Work?:

The loop is just so the same RR value never appears twice in a row.

The loop is not needed - https://youtu.be/8NtZJonqXLo

@ericchesek said in Why Doesn't This Choke Script Work?:

Thanks for the note about local vs. var! I'm still trying to figure out the appropriate uses for these data types.

Use const in on init/namespaces for fixes values, arrays, or objects. Use reg in on init/namespaces for values that will be changed. Use local in the default callbacks and within inline function. Use var within paint routines and anywhere that won't let you use local.

See here - https://docs.hise.audio/scripting/scripting-in-hise/hise-script-coding-standards.html#variables

@ericchesek said in Why Doesn't This Choke Script Work?:

I capture an eventID when a PlayNote function is triggered.

When you play a note you create a brand new note with a brand new ID.

@ericchesek said in Why Doesn't This Choke Script Work?:

I attributed it to the MIDI list storing values from -127 to 128 using the setValue function

A MIDI list is a kind of array with 128 elements. It can only store integers.

ericchesek

@d-healey said in Why Doesn't This Choke Script Work?:

@ericchesek

@ericchesek said in Why Doesn't This Choke Script Work?:

The loop is just so the same RR value never appears twice in a row.

The loop is not needed - https://youtu.be/8NtZJonqXLo

@ericchesek said in Why Doesn't This Choke Script Work?:

Thanks for the note about local vs. var! I'm still trying to figure out the appropriate uses for these data types.

Use const in on init/namespaces for fixes values, arrays, or objects. Use reg in on init/namespaces for values that will be changed. Use local in the default callbacks and within inline function. Use var within paint routines and anywhere that won't let you use local.

See here - https://docs.hise.audio/scripting/scripting-in-hise/hise-script-coding-standards.html#variables

@ericchesek said in Why Doesn't This Choke Script Work?:

I capture an eventID when a PlayNote function is triggered.

When you play a note you create a brand new note with a brand new ID.

@ericchesek said in Why Doesn't This Choke Script Work?:

I attributed it to the MIDI list storing values from -127 to 128 using the setValue function

A MIDI list is a kind of array with 128 elements. It can only store integers.

I love you, David. Seriously, thank you for your willingness to help.
I'll be back with a solution or more problems!

ericchesek

@d-healey
I'm back with more problems.

I tried your RR script, but it results in repeating notes for me. I don't exactly know why.

Also, I reworked my script using a midi list, and I'm still getting errors that the event ID wasn't found inside the midi list despite clearly showing up in the list data AND a Console.print function.

Sampler.enableRoundRobin(false);

const midiList = Engine.createMidiList();
reg midiListCount = 0;
reg counter = 0;
reg lastCount = 0;
const snareNotes = [37,38,39,40];


/* 
inline function RoundRobin(low,hi)
{	
	// My original round robin script
	local RRVal;
	local PrevRRVal;

	while (RRVal == PrevRRVal)
	{
 		RRVal = Math.randInt(low, hi+1);
	}
	PrevRRVal = RRVal;
	return RRVal;
} 
*/

inline function RoundRobin(low,hi)
{
	// this function results in repeating round robins for some reason...
	// as evidenced by Console.print(RRVal)

	local RRVal;
	counter = (lastCount - 1 + Math.randInt(low, hi + 1)) % (hi - low);
	RRVal = counter + low + 1; // adding low because my RR is note-based
	lastCount = counter;
	Console.print(RRVal); 
	return RRVal;	
}

function onNoteOn()
{		
	Message.ignoreEvent(true); 
	Message.makeArtificialOrLocal(); // make note artificial so it can be killed

	local eventID = Message.getEventId();	
	local noteNum = Message.getNoteNumber(); 
	local noteVel = Message.getVelocity();
	
	if (snareNotes.contains(noteNum)){ // if a snare note is played
		midiList.setValue(midiListCount, eventID); // add eventID to midiList
		midiListCount = (midiListCount + 1) % 128; 
		// increment midiList count to 128 and wrap around
	}

	switch(noteNum) // get the incoming MIDI note 
	{
		case 37: // side stick
			for (i = 0; i < midiListCount-1; i++)
				if (midiList[i] > -1)
				{
					Synth.addVolumeFade(midiList[i], 100, -100);	
				}

			
			Synth.playNote(RoundRobin(39,47),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1);
			
			break;
			
		case 38: // hit
			Synth.playNote(RoundRobin(0,15),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;
			
		case 39: // flam
			Synth.playNote(RoundRobin(34,37),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;
			
		case 40: // rimshot
			Synth.playNote(RoundRobin(17,32),noteVel);
			Synth.noteOffDelayedByEventId(eventID,1); 
			break;		
	}
}
function onNoteOff()
{
	Message.ignoreEvent(true);
}
 function onController()
{
	
}
 function onTimer()
{
	
}
 function onControl(number, value)
{
	
}

d.healey

@ericchesek said in Why Doesn't This Choke Script Work?:

Message.makeArtificialOrLocal(); // make note artificial so it can be killed

local eventID = Message.getEventId();

Just use: local eventId = Message.makeArtificialOrLocal();

if (midiList[i] > -1)

A MIDI list is not an array. You can't get the value with [].

ericchesek

@d-healey said in Why Doesn't This Choke Script Work?:

@ericchesek said in Why Doesn't This Choke Script Work?:
Message.makeArtificialOrLocal(); // make note artificial so it can be killed
local eventID = Message.getEventId();	
Just use: local eventId = Message.makeArtificialOrLocal();

if (midiList[i] > -1)

A MIDI list is not an array. You can't get the value with [].

Oh, true. Odd that it worked when I used Console.print(midiList[i]) in the for loop... Anyway, I changed the loop to this:

	for (i = 0; i < midiListCount-1; i++)
				Synth.addVolumeFade(midiList.getValue(i), 100, -100);

And I'm still getting the error. Is it a problem with the artificial event no longer being active?

d.healey

@ericchesek What is midiListCount for?

And I'm still getting the error. Is it a problem with the artificial event no longer being active?

We're getting to that soon, I think

ericchesek

@d-healey

I use that to add each separate eventID to the midiList. Every time a snare note plays, the eventID gets pushed (for lack of a better term) into the midi list. Once all 128 locations are filled, I start overwriting from the first location again.

	if (snareNotes.contains(noteNum)){ // if a snare note is played
		midiList.setValue(midiListCount, eventID); // add eventID to midiList
		midiListCount = (midiListCount + 1) % 128; 
		// increment midiList count to 128 and wrap around
	}

Aaaaand I just realized that this for loop

	for (i = 0; i < midiListCount-1; i++)
				Synth.addVolumeFade(midiList.getValue(i), 100, -100);

won't behave well when midiListCount is back to the small number range if the active events are in the higher indexes of the midiList. I'm more concerned with getting the general behavior to work first, though.

d.healey

@ericchesek A midi list always has 128 elements, no need for the midiListCount. You are not "pushing" into an array, you are setting a specific element to a value. The element index should be the note number. So you always know which note ID belongs to which note.

ericchesek

@d-healey

I get what you mean.

The issue with that is when several voices are overlapped and ringing together (like a 32nd note snare roll for example), so only killing the previously-stored event does not always guarantee the ringing sound of the snare will fade out.

One can argue that a snare can be treated as a monophonic instrument, but then there is the case where a hard note followed by a soft note would sound unnatural since the hard hit ring would be faded too quickly.

I'm aiming for more realism by having this choke script in place, and it will apply to many other drums too.

d.healey

@ericchesek So the problem is you need to have multiple note IDs per note?

By the way, is there a reason you're not using the built in choke script?

ericchesek

@d-healey Yes, I need multiple IDs per note.
I'm not using the built in choke script because I need more control over when and how chokes are triggered. For example, the velocity-based choking thing I mentioned above where a soft note results in a longer choke fade time than two notes with a more similar velocity.

d.healey

@ericchesek I'm not sure you do need multiple IDs per note. When a new note is played, kill the old event that is attached to that note number. Don't fade it out, kill it with Synth.noteOffByEventId(). That will stop the note but the tail will continue to the length of the envelope or the length of the sample, whichever is shortest.

ericchesek

@d-healey

Isn't that already happening in my switch/case statement? I call a synth.playNote(), then immediately afterwards I call a synth.noteOffDelayedByEventId(). That has a time of 1 sample currently for testing purposes, but it works just the same if I use synth.NoteOffByEventId().

Setting the note off is only clearing the eventID, but not stopping the sound. Is my understanding correct?

d.healey

@ericchesek Synth.noteOffByEventId() will kill the note immediately.

ericchesek

@d-healey Yeah that's fine. I switched to that instead of the delayed one and everything is still good.

I found a solution! Still using my round robin loop and an array to store previous event IDs for now.
It seems I do need to check if artificial events are active to make this work properly.
Eventually, I probably need to clear artificial events that are not active so the array doesn't just keep filling up if a sidestick note is never played.

Sampler.enableRoundRobin(false);

const eventList = [];
eventList.reserve(32);

reg prevEvent = 0;

inline function RoundRobin(low,hi)
{	
	local RRVal;
	local PrevRRVal;

	while (RRVal == PrevRRVal)
	{
 		RRVal = Math.randInt(low, hi+1);
	}
	PrevRRVal = RRVal;
	return RRVal;
} 

/*
inline function RoundRobin(low,hi)
{
	// this function results in repeating round robins for some reason...
	// as evidenced by Console.print(RRVal)

	local RRVal;
	counter = (lastCount - 1 + Math.randInt(low, hi + 1)) % (hi - low);
	RRVal = counter + low + 1; // adding low because my RR is note-based
	lastCount = counter;
	Console.print(RRVal); 
	return RRVal;	
}
*/
function onNoteOn()
{		
	Message.ignoreEvent(true); 
	local eventID = Message.makeArtificialOrLocal();
	local noteNum = Message.getNoteNumber(); 
	local noteVel = Message.getVelocity();	

	switch(noteNum)
	{
		case 37: // side stick
			for (i in eventList)
			{
				if (Synth.isArtificialEventActive(i))
					{
						Synth.addVolumeFade(i, 10, -100);
					}
			}
			eventList.clear();

			Synth.playNote(RoundRobin(39,47),noteVel);
			Synth.noteOffByEventId(eventID);
			break;
			
		case 38: // hit
		
			prevEvent = Synth.playNote(RoundRobin(0,15),noteVel);
			eventList.push(prevEvent);
			Synth.noteOffByEventId(eventID);
			break;
			
		case 39: // flam
		
			prevEvent = Synth.playNote(RoundRobin(34,37),noteVel);
			eventList.push(prevEvent);
			Synth.noteOffByEventId(eventID);
			break;
			
		case 40: // rimshot
		
			prevEvent = Synth.playNote(RoundRobin(17,32),noteVel);
			eventList.push(prevEvent);
			Synth.noteOffByEventId(eventID);
			break;		
	}
}
function onNoteOff()
{
	Message.ignoreEvent(true);
}
 function onController()
{
	
}
 function onTimer()
{
	
}
 function onControl(number, value)
{
	
}

d.healey

@ericchesek Clear the event ID in the note off callback

ericchesek

@d-healey Do you mean in place of the separate Synth.noteOffByEventId(eventID);
statements I have in each case statement?

Why Doesn't This Choke Script Work?

17

1.8k

10.2k

89.0k