Request for code review - reverb design

Questions Code Review
1

Hey all,

I’ve been working on reverb design for a while now and checking out a lot of the literature, things are starting to go pretty well I think. Recently I was working on a kind of silly idea, using some of the literature on the Hagia Sophia in Istanbul I decided to try to recreate it with an algorithmic reverb in SC (I am aware that this is a terrible idea). I split the signal into 5 bands to fit approximately with the RT60 times detailed in this paper.

The code for this got quite complex and because I’m almost exclusively self-taught I figure I’m probably doing a fair few things either wrong or at least kind of sloppily. The reverb works ok - it’s not the best reverb in the world, but it’s fine as a fun experiment - but I would be very keen for any feedback on the actual coding style/technique (although feedback on the design itself is also very welcome as I still have a lot to learn).

One thing that I was struggling with is that there is a lot of processing which is very similar, but not quite similar enough that I could figure out how to iterate it (eg. I’m using different filters for the different bands).

Code is below, thanks in advance if anyone feels like taking the time.

(
SynthDef(\hs_reverb, {
	|modulation = 0.5|
	var env = Env.asr(0.001, 1, 10).ar(Done.freeSelf, \gate.kr(1));
	var input = In.ar(0, 2);
	var reflections = input.mean;
	var late;
	var early = 0;
	var cd = ControlDur.ir;
	var sig = Array.newClear(2);
	var band = Array.newClear(10);
	var tap = Array.newClear(20);
	var fb = LocalIn.ar(2);
	var earlyDelayTimes = [0.0243, 0.0341, 0.0511, 0.0463, 0.0237, 0.0331, 0.0387, 0.0434];
	var earlyAllpassTimes = [0.0021, 0.0034, 0.00241, 0.00257, 0.00347, 0.00561, 0.00325, 0.00433];
	var allpassTimes = [0.0251, 0.0254263, 0.0451, 0.0456863, 0.0271, 0.0274523, 0.0443, 0.0448759, 0.0365, 0.0369745, 0.0631, 0.0639203, 0.0378, 0.0382914, 0.0623, 0.0631099, 0.0281, 0.0284653, 0.0334, 0.0338342, 0.0294, 0.0297822, 0.0315, 0.0319095, 0.0295, 0.0298835, 0.0314, 0.0318082, 0.0301, 0.0304913, 0.0308, 0.0312004, 0.0421, 0.0426473, 0.0513, 0.0519669, 0.0447, 0.0452811, 0.0506, 0.0512578];
	var allpassDecays = [ 0.9, 12.9, 0.909, 13.029, 0.95, 13.01, 0.9595, 13.1401, 0.9, 11.3, 0.909, 11.413, 0.9, 11.1, 0.909, 11.211, 0.9, 12.1, 0.909, 12.221, 0.85, 12.2, 0.8585, 12.322, 0.95, 10.8, 0.9595, 10.908, 0.89, 11.01, 0.8989, 11.1201, 0.87, 6, 0.8787, 6.06, 0.88, 5.4, 0.8888, 5.454 ]
;
	var delayTimes = [ 0.020081942081451, 0.020566612482071, 0.026000893115997, 0.020242093801498, 0.029465512037277, 0.027917033433914, 0.026404478549957, 0.022379480600357, 0.025247086286545, 0.027848986387253, 0.029049491882324, 0.023493590354919, 0.020490299463272, 0.028556725978851, 0.021322796344757, 0.020029554367065, 0.025117723941803, 0.021801003217697, 0.028470935821533, 0.021489890813828 ]
;
//	var mod = 20.collect { SinOsc.kr(rrand(0.20, 0.33), phase: rrand(0, 2pi)) * rrand(0.001, 0.0015) };
	var mod = 20.collect { LFNoise2.kr(rrand(0.20, 0.33)) * modulation };
	early = 4.collect { |i| AllpassC.ar(OnePole.ar(reflections, 0.9), 1, earlyAllpassTimes[i], 1); };
	late = [early.sum + fb[0], early.sum + fb[1]];

	// bands 0/1, very low frequencies, then split into two channels

	band[0] = LPF.ar(LPF.ar(late[1], 75), 75);
	band[0] = AllpassC.ar(band[0], allpassTimes[0], allpassTimes[0], allpassDecays[0]);
	band[0] = AllpassC.ar(band[0], allpassTimes[1], allpassTimes[1], allpassDecays[1]);
	tap[0] = DelayC.ar(band[0], delayTimes[0] + 1, delayTimes[0] + mod[0] - cd);

	// same again for right channel

	band[1] = LPF.ar(LPF.ar(late[0], 75), 75);
	band[1] = AllpassC.ar(band[1], allpassTimes[2], allpassTimes[2], allpassDecays[2]);
	band[1] = AllpassC.ar(band[1], allpassTimes[3], allpassTimes[3], allpassDecays[3]);
	tap[1] = DelayC.ar(band[1], delayTimes[1] + 1, delayTimes[1] + mod[1] - cd);

	// bands 2/3, 80-180hz

	band[2] = HPF.ar(HPF.ar(late[1], 80), 80);
	band[2] = LPF.ar(LPF.ar(band[2], 180), 180);

	band[2] = AllpassC.ar(band[2], allpassTimes[4], allpassTimes[4], allpassDecays[4]);
	band[2] = AllpassC.ar(band[2], allpassTimes[5], allpassTimes[5], allpassDecays[5]);
	band[2] = BPeakEQ.ar(band[2], 150, 1, -2);
	tap[2] = DelayC.ar(band[2], delayTimes[2] + 1, delayTimes[2] + mod[2] - cd);

	// same for right channel

	band[3] = HPF.ar(HPF.ar(late[0], 80), 80);
	band[3] = LPF.ar(LPF.ar(band[3], 180), 180);

	band[3] = AllpassC.ar(band[3], allpassTimes[6], allpassTimes[6], allpassDecays[6]);
	band[3] = AllpassC.ar(band[3], allpassTimes[7], allpassTimes[7], allpassDecays[7]);
	band[3] = BPeakEQ.ar(band[2], 150, 1, -2);
	tap[3] = DelayC.ar(band[3], delayTimes[3] + 1, delayTimes[3] + mod[3] - cd);

	// bands 4/5 - 180 - 700hz

	band[4] = HPF.ar(HPF.ar(late[1], 180), 180);
	band[4] = LPF.ar(LPF.ar(band[4], 700), 700);

	band[4] = AllpassC.ar(band[4], allpassTimes[8], allpassTimes[8], allpassDecays[8]);
	band[4] = AllpassC.ar(band[4], allpassTimes[9], allpassTimes[9], allpassDecays[9]);
	band[4] = BPeakEQ.ar(band[4], 600, 1, -2);
	tap[4] = DelayC.ar(band[4], delayTimes[4] + 1, delayTimes[4] + mod[4] - cd);

	// same for right channel

	band[5] = HPF.ar(HPF.ar(late[0], 180), 180);
	band[5] = LPF.ar(LPF.ar(band[5], 700), 700);
	band[5] = BPeakEQ.ar(band[5], 600, 1, -2);
	band[5] = AllpassC.ar(band[5], allpassTimes[10], allpassTimes[10], allpassDecays[10]);
	band[5] = AllpassC.ar(band[5], allpassTimes[11], allpassTimes[11], allpassDecays[11]);
	
	tap[5] = DelayC.ar(band[5], delayTimes[5] + 1, delayTimes[5] + mod[5] - cd);

	// bands 6/7 - 700 - 2.5k

	band[6] = HPF.ar(HPF.ar(late[1], 700), 700);
	band[6] = LPF.ar(LPF.ar(band[6], 2500), 2500);

	band[6] = AllpassC.ar(band[6], allpassTimes[12], allpassTimes[12], allpassDecays[12]);
	band[6] = AllpassC.ar(band[6], allpassTimes[13], allpassTimes[13], allpassDecays[13]);
	band[6] = BPeakEQ.ar(band[6], 1000, 1, -3);
	tap[6] = DelayC.ar(band[6], delayTimes[6] + 1, delayTimes[6] + mod[6] - cd);

	// same for right channel

	band[7] = HPF.ar(HPF.ar(late[0], 700), 700);
	band[7] = LPF.ar(LPF.ar(band[7], 2500), 2500);

	band[7] = AllpassC.ar(band[7], allpassTimes[14], allpassTimes[14], allpassDecays[14]);
	band[7] = AllpassC.ar(band[7], allpassTimes[15], allpassTimes[15], allpassDecays[15]);
	band[7] = BPeakEQ.ar(band[7], 1000, 1, -3); 
	tap[7] = DelayC.ar(band[7], delayTimes[7] + 1, delayTimes[7] + mod[7] - cd);

	// bands 8/9 - everything above 2.5k, with bit of extra filtering built in- tweak the hicut later;

	band[8] = HPF.ar(HPF.ar(late[1], 2500), 2500);
	band[8] = BPeakEQ.ar(band[8], 13000, 1, -3);
	band[8] = LPF.ar(band[8], 16000);

	band[8] = AllpassC.ar(band[8], allpassTimes[16], allpassTimes[16], allpassDecays[16]);
	band[8] = AllpassC.ar(band[8], allpassTimes[17], allpassTimes[17], allpassDecays[17]);
	tap[8] = DelayC.ar(band[8], delayTimes[8] + 1, delayTimes[8] + mod[8] - cd);

	// same for right channel

	band[9] = HPF.ar(HPF.ar(late[0], 2500), 2500);
	band[9] = BPeakEQ.ar(band[9], 13000, 1, -3);
	band[9] = LPF.ar(band[9], 16000);

	band[9] = AllpassC.ar(band[9], allpassTimes[18], allpassTimes[18], allpassDecays[18]);
	band[9] = AllpassC.ar(band[9], allpassTimes[19], allpassTimes[19], allpassDecays[19]);
	tap[9] = DelayC.ar(band[9], delayTimes[9] + 1, delayTimes[9] + mod[9] - cd);

	// second round of processing

	band[0] = AllpassC.ar(tap[0], allpassTimes[20], allpassTimes[20], allpassDecays[20]);
	band[0] = AllpassC.ar(band[0], allpassTimes[21], allpassTimes[21], allpassDecays[21]);
	tap[0] = DelayC.ar(band[0], delayTimes[10] + 1, delayTimes[10] + mod[10] - cd);


	band[1] = AllpassC.ar(tap[1], allpassTimes[22], allpassTimes[22], allpassDecays[22]);
	band[1] = AllpassC.ar(band[1], allpassTimes[23], allpassTimes[23], allpassDecays[23]);
	tap[1] = DelayC.ar(band[1], delayTimes[11] + 1, delayTimes[11] + mod[11] - cd);

	band[2] = AllpassC.ar(tap[2], allpassTimes[24], allpassTimes[24], allpassDecays[24]);
	band[2] = AllpassC.ar(band[2], allpassTimes[25], allpassTimes[25], allpassDecays[25]);
	band[2] = BPeakEQ.ar(band[2], 200, 1, -2);
	tap[2] = DelayC.ar(band[2], delayTimes[12] + 1, delayTimes[12] + mod[12] - cd);

	band[3] = AllpassC.ar(tap[3], allpassTimes[26], allpassTimes[26], allpassDecays[26]);
	band[3] = AllpassC.ar(band[3], allpassTimes[27], allpassTimes[27], allpassDecays[27]);
	band[3] = BPeakEQ.ar(band[3], 200, 1, -2);
	tap[3] = DelayC.ar(band[3], delayTimes[13] + 1, delayTimes[13] + mod[13] - cd);

	band[4] = AllpassC.ar(tap[4], allpassTimes[28], allpassTimes[28], allpassDecays[28]);
	band[4] = AllpassC.ar(band[4], allpassTimes[29], allpassTimes[29], allpassDecays[29]);
	band[4] = BPeakEQ.ar(band[4], 500, 1, -2);
	tap[4] = DelayC.ar(band[4], delayTimes[14] + 1, delayTimes[14] + mod[14] - cd);

	band[5] = AllpassC.ar(tap[5], allpassTimes[30], allpassTimes[30], allpassDecays[30]);
	band[5] = AllpassC.ar(band[5], allpassTimes[31], allpassTimes[31], allpassDecays[31]);
	band[5] = BPeakEQ.ar(band[5], 500, 1, -2);
	tap[5] = DelayC.ar(band[5], delayTimes[15] + 1, delayTimes[15] + mod[15] - cd);

	band[6] = AllpassC.ar(tap[6], allpassTimes[32], allpassTimes[32], allpassDecays[32]);
	band[6] = AllpassC.ar(band[6], allpassTimes[33], allpassTimes[33], allpassDecays[33]);
	band[6] = BPeakEQ.ar(band[6], 1100, 1, -3); 
	tap[6] = DelayC.ar(band[6], delayTimes[16] + 1, delayTimes[16] + mod[16] - cd);

	band[7] = AllpassC.ar(tap[7], allpassTimes[34], allpassTimes[34], allpassDecays[34]);
	band[7] = AllpassC.ar(band[7], allpassTimes[35], allpassTimes[35], allpassDecays[35]);
	band[7] = BPeakEQ.ar(band[7], 1100, 1, -3); 
	tap[7] = DelayC.ar(band[7], delayTimes[17] + 1, delayTimes[17] + mod[17] - cd);

	band[8] = AllpassC.ar(tap[8], allpassTimes[36], allpassTimes[36], allpassDecays[36]);
	band[8] = AllpassC.ar(band[8], allpassTimes[37], allpassTimes[37], allpassDecays[37]);
	band[8] = LPF.ar(band[8], 9000);
	tap[8] = DelayC.ar(band[8], delayTimes[18] + 1, delayTimes[18] + mod[18] - cd);

	band[9] = AllpassC.ar(tap[9], allpassTimes[38], allpassTimes[38], allpassDecays[38]);
	band[9] = AllpassC.ar(band[9], allpassTimes[39], allpassTimes[39], allpassDecays[39]);
	band[9] = LPF.ar(band[9], 9000);
	tap[9] = DelayC.ar(band[9], delayTimes[19] + 1, delayTimes[19] + mod[19] - cd);

	// not sure if it's better to have 10 localout channels here or to do it this way to include a bit of crossover between the channels - does this even work?
	// currently only sending the second round of taps to fb to avoid too much actual feedback

	fb = [
		[tap[0].neg + tap[2] + tap[4].neg + tap[6] + tap[8].neg],
		[tap[1] + tap[3].neg + tap[5] + tap[7].neg + tap[9]],
	] * \feedback.kr(-3.dbamp);

	fb = LPF.ar(fb, 6000);
	fb = LeakDC.ar(fb);
	LocalOut.ar(fb);

	early = Splay.ar(early, 0.75);

	sig = [
		[
		tap[0],
		tap[2] * -1,
		tap[4],
		tap[6] * -1,
		tap[8],
		].sum,
		[
		tap[1] * -1,
		tap[3],
		tap[5] * -1,
		tap[7],
		tap[9] * -1,
		].sum
	];
	
	sig = LeakDC.ar(sig);
	sig = LPF.ar(sig, 9000);
	sig = sig + early;
	sig = input + (sig * \wet.kr(-10.dbamp));
	sig = sig * env;
	ReplaceOut.ar(0, sig);
}).add;

SynthDef(\test, {
	var sig = Saw.ar(XLine.kr(ExpRand(1000, 4000), 50, 1)) * Env.perc(0.01, 1).ar(Done.freeSelf);
	sig = LPF.ar(sig, 8000);
	sig = sig ! 2 * \amp.kr(-20.dbamp);
	Out.ar(0, sig);
}).add;
)

Cheers,
Jordan

2

This is pretty personal, so take this with a pinch of salt…

I would stay away from writing large lists of numbers, it get very confusing to see where the value is actually used.

I’d also avoid having mutable variables where possible because it just becomes a nightmare to read and means backtracking to ensure the name describes the use. Your tap variable doesn’t actually act as a tap because you aren’t doing anything with it, this means the first stage of processing and the second can happen all sequentially.

Declaring functions in a synthdef is useful.

I like the function piping syntax (discussed in another thread on here) as it let you chain operations together with out mutable variables, nor an excess of variables.

This means I’d write each stereo pair of frequency ranges separate, having the actual value there, abstracting out similar functions to the top, then combine them later.

{
	var allpass = {|input, delay, decay| AllpassN.ar(input, delay, delay, decay) };
	var delaymod = {|input, del| DelayC.ar(input, del + 1, del + LFNoise2.kr(rrand(0.20, 0.33)) * \modulation.kr(0.5) - ControlDur.ir) };
	
	var input = In.ar(0, 2);
	var early =  OnePole.ar(input.mean, 0.9) |> AllpassC.ar(_, 1, [0.0021, 0.0034, 0.00241, 0.00257], 1);
	var fb = LocalIn.ar(2);
	var late = [early.sum + fb[1], early.sum + fb[0]];
	
	var stereo_lows = late 
	|> LPF.ar(_, 75) |> LPF.ar(_, 75) 
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> delaymod.(_, 0.2) 
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> delaymod.(_, 0.2);
	
	var stereo_low_mid = late
	|> LPF.ar(_, 80) |> LPF.ar(_, 80) |> HPF.ar(_, 180) |> HPF.ar(_, 180) 
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> BPeakEQ.ar(_, 150, 1, -2)
	|> delaymod.(_, 0.2) 
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> delaymod.(_, 0.2);
	
	// etc. for all the frequency ranges.
}

(the actual values above are not correct, but gives an idea).

If you wanted to do a tap after the first delay, I’d use an adverb (tap) on the |> so it would look like this (this was all mentioned with the implementation in that thread).

{
   var number_of_taps = 10;
   var taps = [0, 0]!number_of_taps;

   var stereo_lows = late 
	|> LPF.ar(_, 75) |> LPF.ar(_, 75) 
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> delaymod.(_, 0.2) 
    |>.tap {|input| taps[0] = input } // write to tap.
	|> allpass.(_, delay: [0.2, 0.212], decay: [0.8, 0.74])
	|> allpass.(_, delay: [0.7, 0.312], decay: [1.8, 1.74])
	|> delaymod.(_, 0.2);
}
3

Thanks Jordan!

One thing I should have clarified - I experimented with having taps after the first round of processing, then decided it sounded worse, so took them out. But I think your point is still valid.