Noise generator implementation in VICE

[tlr]

Regarding DATAPOP, I haven't understood what are those numbers, should I POKE the values from $10c8-$10f8 into 36877 ? (at a fixed interval)

Yes, that is what DATAPOP does. In this case the interval is $37*256 cycles for every value. I included it for reference in case the effect is related to specific period values.

You can see from the table that I set $7e in between the various sounds. That should be the shortest possible period + gate off which would mean a quick clear of the LFSR in your VHDL implementation. The assumption is then that for example the $fd parts (hi-hat) would be identical for every instance. Might be possible to spot in the waveform.

[groepaz]

Just got this bug report: https://sourceforge.net/p/vice-emu/bugs/1125/

any comments?

[nippur72]

I just replied on the bug tracker.

I've not tested on a real VIC20, but it seems that the two VICE versions sound differently (as well as my FPGA implementation) My mistake was checking with high values of 36877 (252,253,254) they are high pitched and you don't notice the difference in average pitch. But if you e.g. poke 36877,160 you can definitely hear they are one octave apart.

Fixing it is easy:

Code: Select all

//int chspeed = "\4\3\2\1"[j];  // added 4th speed
int chspeed = "\4\3\2\2"[j];  // added 4th speed

But the problem is that it's not consistent with what Lance Ewing has found, he clearly said that there are 4 different speeds. We must ask him if he has any thoughts.

[phvic]

Your noise implementation does not match with what Lance described in the reverse engineering thread. This is what I have used in my own VIC-20 emulator since last April:

Code: Select all

// VIC-20 noise emulation
// by Petri Häkkinen
// based on reverse engineering work by Lance Ewing

struct oscillator
{
	uint8_t		shift_reg;
	uint8_t		freq;		// MSB = oscillator enable, bits 0-6 = frequency value
	uint8_t		freq_counter;
};

struct vic_state
{
	struct oscillator oscillators[4];
	uint16_t	noise_lsfr;
};

	struct oscillator* osc = &vic.oscillators[3];
	if((audio_cycle & ((16 >> 3) - 1)) == 0)  // bass,alto,soprano use clock dividers 1,2,4, noise 8
	{
		osc->freq_counter = (osc->freq_counter + 1) & 127;
		if(osc->freq_counter == 127)
		{
			uint16_t lfsr = vic.noise_lsfr;

			if(lfsr & 1)
			{
				// noise output is '1' -> advance 8-bit shift register
				osc->shift_reg = (osc->shift_reg >> 1) | ((!(osc->shift_reg & 1) << 7) & osc->freq);
			}

			// update 16-bit LFSR
			int feedback = !(((lfsr >> 3) & 1) ^ ((lfsr >> 12) & 1) ^ ((lfsr >> 14) & 1) ^ ((lfsr >> 15) & 1));
			vic.noise_lsfr = (vic.noise_lsfr << 1) | (feedback & (osc->freq >> 7));

			osc->freq_counter = osc->freq & 127;
		}
	}

	output += (osc->shift_reg & 0x80) >> 7;

EDIT: added credits

[groepaz]

[strike]Lance provided the implementation (edit: ehrm no)[/strike], so ... mmh where is the error? =)

[phvic]

I thought you were using code provided by nippur72. Looking at his code in the first page, I can see at least that the frequency counter is not compared against 127 to use as a trigger when to advance the waveform (as described by Lance). This also explains why poking 255 in the frequency channel overflows the 7-bit counter on a real vic, so the highest frequency is actually obtained by 254, not 255 which generates the lowest frequency (even lower than 128).

Afair (it's been a few months) I verified my code by observing the waveform produced by a vic-20 and my emulator using an oscilloscope.

(btw. has nobody really used a scope before on a real vic, because when you do it's pretty obvious that the noise is a square wave with varying frequency? --> https://twitter.com/petrih3/status/1138174204434685952)

[nippur72]

I compared the two pieces of code (our VICE mod and @phvic's) and as far as I can tell they are equivalent.

In VICE, the frequency counter do count backwards, so the check is against 0:

Code: Select all

snd.ch[j].ctr--;
if (snd.ch[j].ctr <= 0) {

the 255-low frequency case is achieved by

Code: Select all

int a = (~snd.ch[j].reg) & 127;
a = a ? a : 128;

(when it's 0 the speed becomes 128, the slowest).

Another difference is that in VICE's code the normal shift register is flipped (MSB<->LSB) respect to @phvic's code, but from what I can see, they are equivalent.

BTW, in @phvic I don't get why there is "& osc->freq" ... shouldn't it be "& (osc->freq >> 7)" ?

Code: Select all

osc->shift_reg = (osc->shift_reg >> 1) | ((!(osc->shift_reg & 1) << 7) & osc->freq);
                                                                           ^

Other than that I have no clue why the implementation in VICE isn't working as expected.

@phvic, the waveform of the noise generator was already known to be a square wave and indeed someone else did use a scope before (check a thread named "DC offset" it's very interesting). What was not known was the way the frequency was made random.

BTW do you have a good recording of a real VIC playing noise? I would like to test the accuracy of our model, seeing when the waveform loops over the same values. I would record it myself, but my CRT display does not have an audio output (earphones or whatever).

P.S. where do I get your emulator?

[phvic]

I double checked the output of a real vic-20 and my vic emulator using an oscilloscope and indeed there is something weird going on... I can confirm that the noise frequency is twice as much as it should be. Since both two independent implementations produces the same (incorrect) result I have to conclude that there is something happening in a real vic20 that we don't know. Weird. Maybe Lance can figure out what happens in the silicon?

In VICE, the frequency counter do count backwards, so the check is against 0:

Code: Select all

snd.ch[j].ctr--;
if (snd.ch[j].ctr <= 0) {

the 255-low frequency case is achieved by

Code: Select all

int a = (~snd.ch[j].reg) & 127;
a = a ? a : 128;

(when it's 0 the speed becomes 128, the slowest).

Ok, that's a weird (and overly compilicated) way of doing it, if I may say, but if it works I guess it's fine. I would still suggest counting upwards because it removes the special case of the overflow and is how the HW actually does it.

BTW, in @phvic I don't get why there is "& osc->freq" ... shouldn't it be "& (osc->freq >> 7)" ?

No, we're masking the oscillator enable bit here and the new bit enters at the MSB.

P.S. where do I get your emulator?

It's not available publicly (yet). I wrote it for making a game (still work in progress) because I got frustrated with VICE. Being able to live update code, and have extended opcodes (assert, print, etc.) is pretty useful I'm also planning to add runtime memory viewer/editor.

[groepaz]

But.... if you just make the frequency counter half as fast, it DOES produce the correct result? If yes, just doing that sounds like a valid fix to me

[phvic]

As far as I can tell it does produce a correctly *sounding* result, but since the theory and implementation does not seem to match, who knows if it really is correct? Proving that it's correct would require recording samples at various frequencies and comparing them to the ground truth. Anyway, as a quick hack fix it sounds good to me. That's what I just did to my emulator.

[nippur72]

@groepaz the fix in VICE is very easy:

Code: Select all

// change this
int chspeed = "\4\3\2\1"[j];  // added 4th speed
// into this
int chspeed = "\4\3\2\2"[j];  // added 4th speed

(btw the code in VICE is ugly because we modified the exising implementation instead of writing a new one).

I wrote on the "die shot" thread, hopefully Lance will comment on the matter. My guess is that out of the 5 signals coming from the clock divider, only three are used, leaving leftmost and rightmost unused (and thus 36876 and 36877 should share the same clock signal).

In the end we'll need to formally verify the correctness of the noise generator against a real VIC. My plan is to sample it playing POKE 36877,255 for long enough and see when it repeats. Then convert the square wave into bits and compare them with the ones generated in our codes. I am not good at math, does anybody know how to calculate the actual length of the cycle before it repeats? I know the LFSR cycles after 65535 values, but since it interacts with the normal shift register I don't know how to calculate it (some brute force would probably be of help).

@phvic completely understand about the need of having a "tweakable" emulator. I have interest in another retrocomputer (VTech Laser 500) and the only option was MAME/MESS. So I ended writing my own emulator which I can modify to all my needs. Being written in JavaScript I can modify it even while it's running.

[groepaz]

ok, that fix is in trunk now. if you guys figure out how it really works - or can confirm the output is correct now anyway - let me know

[nippur72]

thanks! I am going to sample my real VIC for verification.

[nippur72]

as noted by @Barcoboy on the VICE bug tracker, there's also the issue told by @tlr in DATAPOP.

It seems that interrupting and resuming the noise generation leads to a subtle change in the timbre. My suspect is that the 8 bit shift register is involved in some way (listen to the attached wav file).

I managed to sample audio from my VIC-20, so I can now run some tests against the real machine.

[nippur72]

@groepaz this should be the fix that implements the "edge triggering" as found yesterday by Lance. I can't build VICE myself so I have not tried it directly.

Compared to previous code:
- added static variable "noise_LFSR0_old" that keeps track of LFSR bit n. 0 (lagged 1 step) in order to detect the change from "0" to "1".
- mixing of noise voice is put in AND with the enable bit
- noise_LFSR can be initialized to 0x0000 (it doesn't really matter as it quickly fills with "1" at power on).

Code: Select all

static uint16_t noise_LFSR = 0x0000;
static uint8_t noise_LFSR0_old = 0;

void vic_sound_clock(int cycles)
{
    int i, j, enabled;

    if (cycles <= 0) {
        return;
    }

    for (j = 0; j < 4; j++) {
        int chspeed = "\4\3\2\1"[j];

        if (snd.ch[j].ctr > cycles) {
            snd.accum += snd.ch[j].out * cycles;
            snd.ch[j].ctr -= cycles;
        } else {
            for (i = cycles; i; i--) {
                snd.ch[j].ctr--;
                if (snd.ch[j].ctr <= 0) {
                    int a = (~snd.ch[j].reg) & 127;
                    a = a ? a : 128;
                    snd.ch[j].ctr += a << chspeed;
                    enabled = (snd.ch[j].reg & 128) >> 7;

                    int edge_trigger = (noise_LFSR & 1) & !noise_LFSR0_old;
                    
                    if((j != 3) || ((j == 3) && edge_trigger)) {
                        uint8_t shift = snd.ch[j].shift;
                        shift = ((shift << 1) | ((((shift & 128) >> 7)) ^ 1) & enabled);
                        snd.ch[j].shift = shift;
                    }
                    if(j == 3) {
                        int bit3  = (noise_LFSR >> 3) & 1;
                        int bit12 = (noise_LFSR >> 12) & 1;
                        int bit14 = (noise_LFSR >> 14) & 1;
                        int bit15 = (noise_LFSR >> 15) & 1;
                        int gate1 = bit3 ^ bit12;
                        int gate2 = bit14 ^ bit15;
                        int gate3 = (gate1 ^ gate2) ^ 1;
                        int gate4 = (gate3 & enabled) ^ 1;
                        noise_LFSR0_old = noise_LFSR & 1;
                        noise_LFSR = (noise_LFSR << 1) | gate4;
                    }
                    snd.ch[j].out = snd.ch[j].shift & (j==3 ? enabled : 1);                    
                }
                snd.accum += snd.ch[j].out; /* FIXME: doesn't take DC offset into account */
            }
        }
    }

    snd.accum_cycles += cycles;
}