Chorus

[stephenhensley]

Just a really brief check over this so far, but the DEL_LEN macro should be changed to a private constexpr (i.e. static constexpr int32_t kDelayLength = 48000;)

Also, requiring a full second of delay per line seems overkill for flanger/chorus duties. IIRC, chorus usually have delays in the 2-8ms range while flangers are in the 0.2-1ms range.

We'll want to keep this as small as it can be while still doing its job effectively so that it can be easily used with other effects and still be in the sram.

Figured I'd chime in with these notes since we'd discussed a few other changes previously as well.

[stephenhensley]

Alright review time:

the examples

First off, I really liked the seed example. It caught me off guard, because it's not a typical use for chorus, but it definitely had a cool effect on the drums. I pushed an update that boosts the amplitude and adds a synth bass behind the drums so the chorus is a bit more obvious. We may want to tweak it to be a bit louder, or at least be a bit clearer in the amplitudes (not sure why but with out any multiplier, the Analog kick was reallyyyy quiet. Maybe just some settings. not sure.

The Petal example is a bit too timid. It can get close to typical chorus effects, but can't really get to some of the more extreme settings that other pedals/effects tend to have (We have a moog analog chorus pedal here I was using to compare).

the DSP

So I see why you templated it, but I'm not sure I can think of places where more/less delay lines would be useful. I almost think having it be a bit simpler, and non-templated would be better here. Unless there's a compelling reason to support multiple variable amounts of delay lines. I noticed that it's set to 2 in the seed example and 4 in the petal example, but wasn't able to hear much of difference when comparing them (though I was using the petal example with a guitar in mono).

Most chorus effects I've seen have a feedback control, which obviously could be done manually, but doesn't seem to have a built in way to do. It would be nice to support that.

The other thing is the way the stereo output is handled. So far we don't really have a true paradigm for this. So I don't want to be overly picky, but of the few modules I can think of that have multiple outputs (Svf, and ReverbSc, and now this). all of them have different ways of handling the outputs.

I almost think, especially in the case of the mono->stereo process function that we do something like Svf except that it can return the mono output simultaneously. This would satisfy the core single channel principles that the other modules use while still allowing you to use it for multiple channels. We'll still have to consider the correct methodologies for actual stereo modules. Since this wouldn't really apply there.

/** Processes mono->Stereo, but returns the mono output for the mono input */
float Process(float in);

/** Getter for channel 1*/
float GetLeft();

/** Getter for channel 2 */
float GetRight();

/** then in practice you could:*/
float sig = chrs.Process(input);

/** OR */
float left, right;
chrs.Process(input);
left = chrs.GetLeft();
right = chrs.GetRight();