Alternative PhaseShifter implementation | Algorithm Design | Forum

Avatar

Please consider registering
guest

sp_LogInOut Log In sp_Registration Register

Register | Lost password?
Advanced Search

— Forum Scope —




— Match —





— Forum Options —





Minimum search word length is 3 characters - maximum search word length is 84 characters

sp_Feed Topic RSS sp_TopicIcon
Alternative PhaseShifter implementation
Avatar
Anglia
Member
Members
September 9, 2021 - 1:38 pm
Member Since: June 2, 2014
Forum Posts: 46
sp_UserOfflineSmall Offline

Hi all,

I've refactored the implementation of the PhaseShifter's processAudioSample() method, as the lists of variables, which admittedly correspond precisely with the variables covered in the PhaseShifter design section, were crying out to be refactored.

The below implementation might seem a little daunting for beginner programmers, but I'm quite sure it's solid, and more robust than the original, specifically with regards to extensibility. With the below implementation, the number of APFs can be extended (or reduced) simply be changing the value of the constant PHASER_STAGES.

There are a couple of helper methods I've used, which aren't necessarily part of this refactor, but I've included them to aid readability. These simply act as factory methods, or 'getters' (for internal use), to retrieve the desired constants for the APF cutoff values and mix coefficients. I intend at some point in the future to extend this plugin to allow the users to select from a choice of these values, but for now at least which values I'm using has been abstracted away via these factory methods. Obviously if PHASER_STAGES is changed, then the array returned by Phaser::getPhaserApfParameters() will need to extended, to ensure it also contains PHASER_STAGES number of elements.

If someone fancies doing a code review of this alternative implementation, that would be greatly appreciated!

struct PhaserAPFParameters {
	const double minF;
	const double maxF;
};
 
struct PhaserMixCoeffs
{
	const double dry;
	const double wet;
};
// --- these are the exact values from the National Semiconductor Phaser design
const PhaserAPFParameters nsPhaserParams[PHASER_STAGES] = 
    {{32.0, 1500.0}, {68.0, 3400.0}, {96.0, 4800.0}, {212.0, 10000.0}, {320.0, 16000.0}, {636.0, 20480.0}};
 
// dry/wet mix coefficients
const PhaserMixCoeffs idealPhaserMixCoeffs = {0.125, 1.25};

const PhaserAPFParameters* Phaser::getPhaserApfParameters()
{
	return nsPhaserParams;
}
 
PhaserMixCoeffs Phaser::getPhaserMixCoeffs()
{
	return idealPhaserMixCoeffs;
}
double Phaser::processAudioSample(double xn)
{
	const SignalGenData lfoData = lfo.renderAudioOutput();
 
	// --- create the bipolar modulator value
	double lfoValue = lfoData.normalOutput;
	if (parameters.quadPhaseLFO)
		lfoValue = lfoData.quadPhaseOutput_pos;
 
	const double depth = parameters.lfoDepth_Pct / 100.0;
	const double modulatorValue = lfoValue * depth;
 
	const PhaserAPFParameters* apfParams = getPhaserApfParameters();
	double gammas[PHASER_STAGES];
	double gamma = 1;
	for (uint32_t i = 0; i < PHASER_STAGES; i++)
	{
		// --- calculate modulated values for each APF; note they have different ranges
		AudioFilterParameters params = apfs[i].getParameters();
		params.fc = doBipolarModulation(modulatorValue, apfParams[i].minF, apfParams[i].maxF);
		apfs[i].setParameters(params);
 
		// --- calculate gamma values
		gamma = apfs[PHASER_STAGES - (i + 1)].getG_value() * gamma;
		gammas[i] = gamma;
	}
 
	// --- create combined feedback
	double Sn = 0;
	for (uint32_t i = 0; i < PHASER_STAGES; i++)
	{
		Sn += i < PHASER_STAGES - 1 ? gammas[PHASER_STAGES - (i+2)] * apfs[i].getS_value() : apfs[i].getS_value();
	}
	
	// --- set the alpha0 value
	const double K = parameters.intensity_Pct / 100.0;
	const double alpha0 = 1.0 / (1.0 + K * gamma);
 
	// --- form input to first APF
	double apfsOutput = alpha0 * (xn + K * Sn);
 
	// --- cascade of APFs
	for (auto& apf : apfs)
	{
		apfsOutput = apf.processAudioSample(apfsOutput);
	}
 
	// Mix dry & wet signal, based on chosen coefficients
	const PhaserMixCoeffs mixCoeffs = getPhaserMixCoeffs();
	const double output = mixCoeffs.dry * xn + mixCoeffs.wet * apfsOutput;
 
	return output;
}
Avatar
Admin
October 7, 2021 - 11:28 am
Member Since: January 1, 2020
Forum Posts: 107
sp_UserOfflineSmall Offline

Hi Steve,

In processAudioSample() I think you can move your combined feedback line for the Sn variable into the for loop above it, rather than looping over twice.

Also there are a couple of places where a multiply of .01 would save the more costly division of 100.

 

Excellent stuff otherwise, thanks for sharing

Avatar
Anglia
Member
Members
October 12, 2021 - 4:36 pm
Member Since: June 2, 2014
Forum Posts: 46
sp_UserOfflineSmall Offline

jim said
Hi Steve,

In processAudioSample() I think you can move your combined feedback line for the Sn variable into the for loop above it, rather than looping over twice.

Also there are a couple of places where a multiply of .01 would save the more costly division of 100.

 

Excellent stuff otherwise, thanks for sharing  

Hi Jim,

First of all, congrats on your promotion to site admin!

With regards to division operations being computationally more expensive than multiplication, I must agree, and I've updated my implementation accordingly (can't however edit the above post).

Regarding the incrementation of Sn however, I thought long & hard over this when I was refactoring Will's original algorithm, and I don't believe it can be moved into the previous for loop.

If this was done, the following scenario would occur:

Let's assume for the sake of argument, that PHASER_STAGES has the value of 6 (the default as delivered in fxobjects.h).

Then, in the first iteration of the first for loop, i == 0;

gamma is then calculated, multiplying by 1 (gamma's initial value, the first time around), then

gammas[0] is assigned the value of gamma.

If we were to move the line incrementing Sn to directly under this assignment statement (i.e. inside the first for loop), we'd have the following:

Sn += gammas[PHASER_STAGES - (i+2)] * apfs[i].getS_value();  // Since i (being 0) < PHASER_STAGES - 1, the ternary operator executes the 'if' branch

which, with PHASER_STAGES == 6 and i == 0, evaluates to

Sn += gammas[4] * apfs[0].getS_value();

But, since gammas[4] has not yet been assigned a value, the result of this assignment statement would be indeterminate, and therefore erroneous.

Hence, the first for loop has to complete before Sn can be calculated.

Avatar
Admin
October 13, 2021 - 3:25 am
Member Since: January 1, 2020
Forum Posts: 107
sp_UserOfflineSmall Offline

Oh I didn't notice at first that this was a ZDF implementation. Extremely cool.
But you're right in that it does complicate the loop somewhat and we must calculate all of the modulated APF coefficients first to determine the gamma values and must calculate all of them before determining Sn.

You could try something like this though;

for( uint32_t i = 0; i < N ; i++){
   AudioFilterParameters params = apfs[i].getParameters();
   params.fc = doBipolarModulation(modulatorValue, apfParams[i].minF, apfParams[i].maxF);
   apfs[i].setParameters(params);
}

// --- calculate gamma values
double gammas[N] = { 1. }; ///sets all elements to 1. (we can use gammas[0] in place of gamma)

for( uint32_t i = 0; i < (N ); i++){

   gammas[i + 1] = apfs[(N - 1) - i].getG_value() * gammas[ i ];
}

// --- create combined feedback
double Sn = 0.;

for(uint32_t i = 0; i < N; i++){

   Sn = gammas[(N - 1) - i] * apfs[i].getS_value() + Sn ;
}

// --- set the alpha0 value
const double K = parameters.intensity_Pct * .01;

const double alpha0 = 1.0 + K * gamma[N] ;

// --- form input to first APF
double apfsOutput = ( xn + K * Sn ) / alpha0;

 

Slightly tighter and removes the conditional. Maybe less readable, also untested.

Note - You can recombine the gammas and apf params loop as you previously had it or not. That introduces an old vs current position error for some of the LFO'd parameters, same reasons you stated above for the Sn variable. Not a big deal either way though.

Cheers

Avatar
Anglia
Member
Members
November 8, 2021 - 2:16 pm
Member Since: June 2, 2014
Forum Posts: 46
sp_UserOfflineSmall Offline

Hi Jim,

Now it's my turn to apologise for the tardy reply! I'm studying other courses beside Will's book, as well as working full time, so I've not found time to revisit this yet.

However, I believe the following line contains a bug:

// --- calculate gamma values
double gammas[N] = { 1. }; ///sets all elements to 1. (we can use gammas[0] in place of gamma)

for( uint32_t i = 0; i < (N ); i++){
   gammas[i + 1] = apfs[(N - 1) - i].getG_value() * gammas[ i ];
}

Namely, assuming N == PHASER_STAGES == 6, for the sake of argument, then in this loop, when i==5 (the very last iteration), then i+1==6. You're addressing element [i+1], i.e. 6, of the array gamma, which has been declared to contain N, i.e. 6, elements, so gammas[6] is out of bounds.

Don't worry too much about this, since I'm quite happy with my implementation, and am now moving onto Chapter 14, which I hope to finish before I have to get back to my other studies.

Avatar
Admin
November 9, 2021 - 8:44 am
Member Since: January 1, 2020
Forum Posts: 107
sp_UserOfflineSmall Offline

Hi Steve, you're quite right. Big compiler error right there. Also apfs[-1] !!! Thanks for pointing it out.

Think I'd meant to write, i < (N-1) in that for loop.

The logic, is to take advantage of the fact that gamma[0] always has a known value of 1.

We can loop over just the other N-1 gammas since that one value was already set.

The main advantage to that approach, is it avoids a branch condition in the for loop. Those can be painfully slow as a result of the cache mispredictions. More of an issue though if you wanted N to approach an order of several hundreds.

Have good fun with chptr 14, delays totally rule.

Also @ Will. If you dont mind and it's not copyright to AES. Could you please provide us a link to your paper on Resolving Delay-Free Loops in filters using the Modified Härmä Method?

Avatar
Admin
November 9, 2021 - 9:49 am
Member Since: January 29, 2017
Forum Posts: 698
sp_UserOfflineSmall Offline

Hey guys - the paper is copyrighted and I still want to publish more at AES so I can't give a link to it : (

However, it is all explained and derived in the 2nd edition synth plugin book, with examples.

Also, if you have a friend in the AES, they can download it and then do whatever they want with it... : )

Will 

Avatar
Anglia
Member
Members
November 9, 2021 - 5:17 pm
Member Since: June 2, 2014
Forum Posts: 46
sp_UserOfflineSmall Offline

W Pirkle said

However, it is all explained and derived in the 2nd edition synth plugin book, with examples.
 

I have the 2nd edition synth book, and am looking forward to starting that once I complete the FX bookSmile

Forum Timezone: America/New_York

Most Users Ever Online: 152

Currently Online:
8 Guest(s)

Currently Browsing this Page:
1 Guest(s)

Top Posters:

Chaes: 56

Skyler: 48

StevieD: 46

Derek: 46

Frodson: 45

Peter: 43

TheSmile: 43

Nickolai: 43

clau_ste: 39

jeanlecode: 37

Member Stats:

Guest Posters: 2

Members: 786

Moderators: 1

Admins: 6

Forum Stats:

Groups: 13

Forums: 42

Topics: 853

Posts: 3385

Moderators: W Pirkle: 698