Pops when using pot input for momentary switch

[Ant]

Hi all,
I'm having some trouble getting a smooth transition when using a momentary switch connected to a pot input to control pitch transposition.
The pitch transpose algorithm is just a modification of the familiar one and works fine (with a constant shift defined before the loop). The switching also works well but there is often an audible pop or click when it is engaged or disengaged. The switch pulls the pot high (with a pull down resistor for when it is off) and it is debounced with a standard RC circuit (with a resistor also in place to discharge the cap). I've played around with exaggerated debounce time constants and it hasn't improved the situation so I suspect it's an issue in the code.
I initially tried it with the switch reading code before the delay writes but then moved it to after them in case it was an issue with trying to perform the pitch processing on unpopulated delay lines, but it didn't make any noticeable difference.
Any help in identifying and remedying the problem would be much appreciated!
Ant

Code: Select all

LOOP:

rdax ADCL,	1.0		;read left input
wra 	ldel, 	0.0		;write to delay --start
rdax ADCR,	1.0		;read right input 
wra 	rdel, 	0.0		;write to delay start

ldax 	pot0				;load pot0 to acc
sof	1,-0.5			;scale to -0.5 -> 0.5
skp	NEG,SWOFF		;if acc -ve (switch is off) skip to SWOFF, else continue with pitch loop
clr

; We use the base of the sample memory block as the
; address since we are using a positive only ramp 
; that ranges 0 to 1.0 (511 in this case)
;do left chan:

cho 	rda,	rmp0,reg|compc,ldel	; (1-k)*sample[addr]
cho 	rda,	rmp0,0,ldel+1			; k*sample[addr+1] + ACC
wra 	dtemp,	0				; Save it off to memory and clear ACC
cho 	rda,	rmp0,rptr2|compc, ldel	; (1-k)*sample[addr+ half ramp]
cho 	rda,	rmp0,rptr2,ldel+1		; k*sample[addr+ half ramp + 1] + ACC
cho 	sof, 	rmp0,na|compc,0		; Result in ACC, multiply it by (1-XFADE) coefficient
cho 	rda,	rmp0,na,dtemp			; Add in earlier value saved in memory, multiply saved value by XFADE coefficient
wrax 	dacl,	0 				; Write it to DACL and clear ACC

;now do right chan:
cho 	rda,	rmp0,compc,rdel		; (1-k)*sample[addr]
cho 	rda,	rmp0,0,rdel+1			; k*sample[addr+1] + ACC
wra 	dtemp,	0				; Save it off to memory and clear ACC
cho 	rda,	rmp0,rptr2|compc, rdel	; (1-k)*sample[addr+ half ramp]
cho 	rda,	rmp0,rptr2,rdel+1		; k*sample[addr+ half ramp + 1] + ACC
cho 	sof, 	rmp0,na|compc,0		; Result in ACC, multiply it by (1-XFADE) coefficient
cho 	rda,	rmp0,na,dtemp			; Add in earlier value saved in memory, multiply saved value by XFADE coefficient
wrax 	dacr,	0 			; Write it to DACL and clear ACC

skp 	ZRO,END					;unconditional skip to END

SWOFF:				;switch is off so pass signal through
ldax	ADCL			;overwrite acc with left input
wrax	DACL,0			;write to left output then clear acc
rdax	ADCR,1			;load right input into acc
wrax	DACR,0			;write to right output then clear acc

END:

[frank]

It appears you are switching between the dry and the shifted signal, as a result there is no guarantee that the signals will be in the same place in their waveforms so they may click. For example the dry may be at +1.0 but the shifted signal is at -1.0 so switching between them means the output suddenly jumps from one extreme to another.

You would need to cross fade from one to the other anytime they switch to eliminate the click. That is basically what the pitch shifter is doing with XFADE as it goes from the end of the buffer to the beginning.

[Ant]

Hi Frank,
Thanks very much for your help, I figured it might be something along those lines. I'll have a play with it and let you know how I get on,
Ant

[Ant]

Ok so I'm running out of instructions the way I was trying to do it, which leads me to believe there must be an easier way...

The key elements I identified to query here were as follows:

current pedal state
previous pedal state
crossfade state (is there already a previous crossfade happening?)
ramp coefficient (if it reaches 1, then the fade has finished)

The first three of which can be treated as respective flags (as you kindly helped me with in my other forum post) giving the following possible conditions (the MSB is current pedal state, LSB is crossfade state):

000 - switch not pressed, no transition, no crossfade -> output dry signal
001 - switch not pressed, no transition, still processing crossfade from the last transition -> output crossfaded signal (to dry)
010 - switch not pressed, transition, no previous crossfade -> begin ramp to fade to dry but output wet (previous state) for this sample
011 - this shouldn't happen... A new transition with a fade already underway. This should be filtered out by the hardware switch debounce.
100 - switch pressed, transition, no previous crossfade -> begin ramp to fade to wet but output dry (previous state) for this sample
101 - another transition with fade underway (a fade reversal) that should be prevented by the debounce
110 - switch pressed, no transition, no crossfade -> output wet signal
111 - switch pressed, no transition, still processing crossfade from the last transition -> output crossfaded signal (to wet)


In lieu of a switch statement, is the following pseudocode reasonable? The processing for the various flags and skip statements takes up almost all of the instruction memory...
The actual effect processing will be performed beforehand, this code is just to crossfade when the pedal is pressed or released. I've written it out as nested IF statements to demonstrate a similar flow to the ASM code. I've included the cases when a transition happens mid-crossfade but the hardware switch debounce should rule this out so they don't necessarily need to be implemented in the actual code.

Code: Select all

If (pedal_pressed = 1)                //is the pedal pressed now?
{
     set pedal_flag
     if (last_pedal_state = 1)       //was the pedal pressed immediately before this sample?
     {
          if (fade_flag = 1)    //is there already a crossfade underway?
          {
               if (fade_ramp = 1)      //has the coefficient of the ramp used to crossfade reached the end yet? (is the previous crossfade finished?)
               {
                    clear fade_flag     //if so, set the fade flag to 0 ready for the next sample
                    output wet          //output 100% wet signal. In ASM these output instructions skip to their respective functions near the end
               }
               else                           //previous crossfade hasn't finished
               {
                    output fade_to_wet    //calculate the crossfade to effect from clean using ramp coefficient and its complement
               }
          else                                //fade_underway = 0, no crossfade is currently happening
          {
               output wet                //both the current and previous pedal states were 1, no fade is underway, so output 100% wet signal
          }
     }
     else                                     //last pedal state was 0, but current is 1, so we have a transition to the effect from clean
     {
          if (fade_flag = 1)
          {
               skip to end                //This is a transition when a fade is already underway (a fade reversal). Switch debounce should prevent this...
          }
          else                                //This is a transition from off to on but with no crossfade yet underway, as expected
          {
               set fade_flag
               restart ramp              //jam the ramp
               output dry                 //as ramp coefficient will be zero anyway we can just output 100% dry for this sample
          }
     }
}
else                                          //current pedal state is 0
{
     clear pedal_flag
     if (last_pedal_state = 1)       //is this a transition from on to off?
     {
          if (fade_flag = 1)
          {
               skip to end                 //another transition mid-crossfade. Hopefully the switch debounce will prevent this...
          }
          else                                //a transition from on to off with no prior fade underway, as expected
          {
               set fade_flag
               restart ramp
               output wet                //again, ramp has only just started so we can just output wet for this sample
          }
     }
     else                                      //both current and last pedal states were 0
     {
          if (fade_flag = 1)             //is the previous fade still underway?
          {
               if (fade_ramp = 1)      //has the coefficient of the ramp used to crossfade reached the end yet? (is the previous crossfade finished?)
               {
                    clear fade_flag     //if so, set the fade flag to 0 ready for the next sample
                    output dry            //output 100% dry signal
               }
               else                           //crossfade hasn't finished
               {
                    output fade_to_dry   //calculate the crossfade to dry from wet using ramp coefficient and its complement
               }
          }
          else                                 //fade_flag = 0, so just a steady output of 100% dry signal
          {
               output dry
          }
     }
}
last_pedal_state <- pedal_state  //update the last pedal state, ready for the next sample

[frank]

I might just make it 4 states:

00 : not xfading, output wet
01: not xfading, output dry
10 : xfading wet to dry, output xfaded signal
11 : xfading dry to wet, output xfaded signal

Only check the switch in states 00 and 01, once you start an xfade complete it no matter the switch state. Unless the user modulating the switch is part of the effect ignore it once xfade starts.

[Ant]

I'm not sure I quite get it. I still need at least three flags in order to detect the six possible states (including the transitions) and thus perform the six possible behaviours:

1. no fade, output dry
2. no fade, output wet
3. fading to dry
4. fading to wet
5. starting a fade to dry, output wet
6. starting a fade to wet, output dry

Do you know if there is any pre-existing code out there for people to take a look at? It seems like an incredibly common function (that anything switching on/off a pitch-based effect would need to avoid pops) but it seems disproportionately difficult to implement, at least within the memory constraints of the chip. I can't help but feel like I must be missing something obvious. Is this something that other developers usually implement in hardware somehow?

[frank]

Why do you need states 5 and 6? While you are cross fading just output the result of the cross fade. It does not matter if it is the first or last step in the fade, just calculate it and output it.

If knowing it is the start of the fade is important that is easy to determine as the old fade state and the new one are different and you shouldn't need it for more than 1 sample period so making it a persistent flag is a waste.

Basically:
if (!xfade_flag) {
if (button) { ; only check button if we are not already doing an xfade
xfade_coeff = 0;
xfade_flag = true;
wet/dry flag = !wet/dry flag; I am assuming each push inverts the flag to go dry->wet or wet-> dry
; we know this is the first time in xfade and which way we are going so if you need to do something the first time do it here
} else {
output = wet or dry based on wet/dry flag
}
; check xfade flag in case it was just set
if (xfade_flag) {
do xfade between wet/dry based on flag
output = xfade result
if (xfade is done) xfade_flag = false;
}

I am making some assumptions about your project so this may not be the best implementation but try to only save values or flags that must persist across sample periods.

[Ant]

Ah that makes sense, thanks. I was going about it completely the wrong way around, testing each flag individually and working from the current and last pedal states rather than using each possible scenario as an individual case.
I'll rewrite it tomorrow and give it another shot.

In terms of setting the frequency for the crossfade ramp, is that done using the same formula for frequency that features in the WLDS interpolation example in the AN-0001 document? Or is there another way of calculating the frequency for WLDR?

[frank]

Should be the same but I would just use a register and increment it each period while in fade, I had assumed that as that is why I have the xfade_coeff = 0; but it may save a few instructions to use the ramp, you will need to watch for the ram rolling over to a lower value to catch the peak as it may jump over the max value depending on what the coefficient is. I.e. a coeff of 0.3 makes it go 0, 0.3, 0.6, 0.9, 0.2 so a rollover happened but we never saw 0.99...

[Ant]

Using the register certainly sounds safer and simpler.
Let's say I decrement a counter for 2048 samples (62.5ms xfade on a 32768Hz clock, deliberately long), would this be a reasonable approach?

Code: Select all

FADE_TO_DRY:
rdax		XFADE_CTR,$0008		;load XFADE_CTR and divide by 2048 (S1.14) to obtain xfade coefficient, acc is 0 after skip so no offset
wrax		XFADE_COEF,1		;save crossfade coefficient
mulx		WET_L			;multiply left pitch-shifted signal by coefficient
wrax		WET_L			;overwrite WET_L with this scaled value
sof		0,0.5			;clear acc and add 0.5
sof		2,0			;multiply by two, acc now contains 1
rdax		XFADE_COEF,-1		;one minus xfade coeficient
mulx		ADCL			;multiply this by clean signal
rdax		WET_L,1			;add this to scaled wet signal
wrax		DACL			;write to left output

I suppose I could save an instruction or two by saving "1" into a register during set up instead of the dual sofs but is the general concept here about right?

[frank]

Yeah that is on the right path, without knowing the full code I would suggest the following (which may not work with your code depending on what else is going on):

xfade_ctr and xfade_coef can probably be the same thing.

Load 0.999... into ACC via an OR instruction assuming the ACC is 0 already rather than the SOF

So you load xfade_coef into ACC (was set to 0x7fffff by an OR in the button detection part)
sof 1.0, -1/2048 to subtract 1/2048
on the 2049th loop the result will be negative so you can easily detect the end of the process

Also since we are calculating xfade_result = a*xfade_coef + b(1-xfade_coef) where a and b are wet or dry depending on which way you are going you can expand the equation to:

xfade_result = a*xfade_coef + b - b*xfade_coef
which is:
xfade_result = xfade_coef*(a-b) + b

So now it is a subtraction, one multiply and an addition and you no longer need to calculate "1-xfade_coef"

[Ant]

That was very helpful thanks. I've knocked out an implementation based on your advice but I'm still getting the pops on the transitions from both wet-to-dry and dry-to-wet as before. Can you spot anything glaring in my code that I might have overlooked?

Code: Select all

FL00	EQU	%00000000_00000000_00000000     ;possible values of flags
FL01	EQU	%00000000_00000000_00000001
FL10	EQU	%00000000_00000000_00000010
FL11	EQU	%00000000_00000000_00000011

COEF_DEC   EQU %0_00_0000_0001  ;decrement xfade coefficient by 1/1024 each sample while fading (1024 samples is 31.25ms)

XFADE_COEF	EQU	60		;use register 60 for crossfade coefficient
FLAGS		EQU	61		;use register 61 for flags
WET_L		EQU	62		;use register 62 for wet signal (left)
WET_R		EQU	63		;use register 63 for wet signal (right)

;declarations for effect go here

; Initialization, only run on first execution of code
; Skip the following two instructions if NOT the first time

skp		RUN, LOOP

;set up for effect goes here

clr						;clear acc (set to zero)
wrax		FLAGS, 0			;write zero to FLAGS

LOOP:

;effect code goes in here and saves to WET_L and WET_R

ldax		FLAGS
xor		FL01
skp		ZRO,FADE_TO_WET		;if 01 transition, skip to FADE_TO_WET
ldax		FLAGS
xor		FL10
skp		ZRO,FADE_TO_DRY		;else if 10 transition, skip to FADE_TO_DRY
ldax		FLAGS
xor		FL11
skp		ZRO,FLAGS11		  	;else if flags are 11, skip to that branch

;FLAGS00
ldax		POT0				;else FLAGS=00 so read switch input
sof		1,-0.5
skp		NEG,OUT_DRY			;if switch is off skip to OUT_DRY
clr
or		FL01
wrax		FLAGS,0				;else set flags to 01 (start fade to wet) and clear acc
or		$7fffff                		;set XFADE_COEF to 0.999...
wrax		XFADE_COEF,0
skp		ZRO,OUT_DRY			;u/c skip to OUT_DRY

FLAGS11:
ldax   	POT0
sof     	1,-0.5
skp     	GEZ,OUT_WET            	;if switch is on, skip to OUT_WET
clr
or      	FL10
wrax    	FLAGS,0                 		;else set flags to 10 (start fade to dry), clear acc
or		$7fffff                 		;set XFADE_COEF to 0.999...
wrax		XFADE_COEF,0
skp     	ZRO,OUT_WET             	;u/c skip to OUT_WET


FADE_TO_DRY:
ldax		XFADE_COEF
sof     	1,-COEF_DEC          		;XFADE_COEF = XFADE_COEF - (1/2048)
skp		NEG,DRY_FADE_CMPLT	;if XFADE_COEF - COEF_DEC is -ve, skip to DRY_FADE_COMPLT
wrax    	XFADE_COEF,1         		 ;save decremented coefficient
ldax		WET_L			     	;load left wet signal into acc
rdax		ADCL,-1		        	;add acc to -1*ADCL, this gives us "wet-dry" in the acc
mulx		XFADE_COEF	         	;multiply acc by coefficient, this gives us "xfade_coef*(wet-dry)"
rdax		ADCL,1		         	;add dry signal again, giving us "xfade_coef*(wet-dry) + dry"
wrax		DACL,1

ldax		WET_R			     	;load right wet signal into acc
rdax		ADCR,-1		         	;add acc to -1*ADCL, this gives us "wet-dry" in the acc
mulx		XFADE_COEF	         	;multiply acc by coefficient, this gives us "xfade_coef*(wet-dry)"
rdax		ADCR,1			     	;add dry signal again, giving us "xfade_coef*(wet-dry) + dry"
wrax		DACR,0
skp     	ZRO,END


FADE_TO_WET:
ldax		XFADE_COEF
sof     	1,-COEF_DEC          		;XFADE_COEF = XFADE_COEF - (1/2048)
skp		NEG,WET_FADE_CMPLT   	;if XFADE_COEF is -ve, skip to WET_FADE_COMPLT
wrax   	XFADE_COEF,1         		;save decremented coefficient
ldax		ADCL			     	;load left dry signal into acc
rdax		WET_L,-1		     		;add acc to -1*ADCL, this gives us "dry-wet" in the acc
mulx		XFADE_COEF	         	;multiply acc by coefficient, this gives us "xfade_coef*(dry-wet)"
rdax		WET_L,1			     	;add wet signal again, giving us "xfade_coef*(dry-wet) + wet"
wrax		DACL,1

ldax		ADCR				;load right dry signal into acc
rdax		WET_R,-1				;add acc to -1*ADCL, this gives us "dry-wet" in the acc
mulx		XFADE_COEF	   		;multiply acc by coefficient, this gives us "xfade_coef*(dry-wet)"
rdax		WET_R,1				;add wet signal again, giving us "xfade_coef*(dry-wet) + wet"
wrax		DACR,0
skp     	ZRO,END


DRY_FADE_CMPLT:
clr
wrax		FLAGS,0		    	;set FLAGS to 00
OUT_DRY:
ldax    	ADCL
wrax    	DACL,0
ldax    	ADCR
wrax    	DACR,0
skp     	ZRO,END         		;u/c skip to end


WET_FADE_CMPLT:
clr
or      	FL11
wrax		FLAGS,0		    	;set FLAGS to 11
OUT_WET:
ldax    	WET_L
wrax    	DACL,0
ldax    	WET_R
wrax    	DACR,0


END:

[frank]

I think it is an assembler issue with how it is handling the number, if we look at a line like:

sof 1,-COEF_DEC

it assembles to:
4000800D

C = 0x4000 = 1.0
D = 0x400 = -1.0 which is not correct, we want 0x7ff here so you are really jumping from one to the other in a single sample period so it goes POP

In fact in the output window for the assembler we can see it thinks COEF_DEC is 1 not 0.00097656250 (1/1024)

Change the EQU statement to
COEF_DEC EQU 1/1024

which should assemble to
4000FFED

If something seems wrong look at the output window to see what the assembler thinks an equate works out to and look at the assembled code to see what the instruction assembled to and rip it apart to look at the values.

[Ant]

Ah that's the badger. Muchas gracias.
I didn't realise that you could use the "Assemble to EEPROM" button to build a single program and get those details in the output window (and that doing that first then makes the "View Machine Code Listing" button work).

Am I right in thinking then that there's no way to symbolically use a particular hex or binary number using EQU?
I had thought that if the operand had to be in S.10 then %0_00_0000_0001 would be interpreted as 1/1024.

[frank]

Am I right in thinking then that there's no way to symbolically use a particular hex or binary number using EQU?
I had thought that if the operand had to be in S.10 then %0_00_0000_0001 would be interpreted as 1/1024.

It really depends on the instruction, some will map that properly others will not. In the case of SOF the instruction sheet says it takes a real for "D" so it must be like 1/1024 and not a hex or binary.

Part of the problem with things like the assembler is we try to let the user define values in different ways according to their program but that means we are also at times guessing what they are attempting to do. Most assemblers for things like a microprocessor only allow integers so you can write 10 or 0xA or %1010 but they all equate to 10 decimal. You usually have to use a higher level language compiler like C to have mixed types and then they are usually strongly typed and will toss an error if you pass the wrong type (or just return wrong results).