Shiru wrote:Song data parsing and playing happens in the main thread. Once a row is parsed, player sets the sync counter to the speed value, counted in sample rate units. Then it just waits until the interrupt handler will decrement it down to zero, and next row gets processed. One major weird issue I had was related to this part. Initially I waited for the counter to be zero with just while(counter);. This caused major tempo fluctuations, with both speed increase and decrease. Once I changed it to while(counter) delay(1); it started to work, although the 1 ms delay may change the tempo a bit. Still not figured why the original approach didn't work as expected.
Hi, I have downloaded and examined your Phaser1 engine for Arduino where there is the same issue you described:
// delay 1 is important, by some reason sng tempo starts to jump a lot without it
while(parser_sync>0)
_delay_ms(1);
The reason why this doesn't work is because reading the parser_sync variable in a while loop might look something like:
loop lds r24,0x0114
lds r25,0x0115
or r24,r25
brne loop
The problem is because when the interrupt happens in between the two lds instructions and since the loop is very fast it happens quite often. For example if parser_sync is 0x0100 and the interrupt happens after the first lds instruction r24 is going to be 0x00. Inside the interrupt routine the value stored in locations 0x0114 (low byte) and 0x0115 (high byte) will change to 0x00ff and upon returning from the interrupt routine the second lds will fetch the value 0x00 effectively reading the variable as 0x0000 instead as 0x00ff. Similarly the variable might 'jump' from 0x0b00 to 0x0a00 instead to 0x0aff etc. and because during the while loop there are many interrupts the error quickly accumulates. When you added the 1 ms delay you considerably slowed down the while loop - lowered the frequency of the lds instructions (because now the loop spends most of the time in the delay loop) so the error happens only sporadically. It is similar to as when you are drawing the sprite into the Spectrum's video RAM and the electron beam 'runs' over the location where you are changing the bytes :-)
Similarly, the line:
parser_sync=MUSIC_FRAME*tag; //wait for given number of tempo units before playing next row
might look something like:
ldd r24,y+1
mov r11,r24
lsl r11
lsl r11
lsl r11
clr r10
sts 0x0115,r11
sts 0x0114,r10
where everything goes wrong if the interrupt happens after the first sts. In that case the high byte of the variable would inside interrupt routine be decreased by one (because the lower byte is still zero), the lower byte would become 0xff but it would be overwritten immediately upon returning from the interrupt by the second sts. This is in fact the actual code generated by the compiler, high byte is stored the first which is worse.
The solution is - you must not allow the interrupt to happen in between load or store instructions which are accesing or writing the variable which takes more than one byte. Here is one way how that could be done:
unsigned int p_sync;
do {
cli();
p_sync = parser_sync;
sei();
} while (p_sync);
and
cli();
parser_sync=MUSIC_FRAME*tag; // wait for given number of tempo units before playing next row
sei();
Despite parser_sync variable is volatile, that isn't enough. Declaring the variable volatile only prevents the compiler to store that variable in registers because in that case the interrupt routine would modify the registers after push instructions and at the end of the interrupt routine the pop instructions would destroy all the changes.
The other solution (which I used more often than cli/sei one) is to use one byte flag to signal from the main program to the interrupt routine and/or from the interrupt routine to the main program when the data is ready to read (or when the while loop should end). In that case you might declare a flag:
which would send a signal from the interrupt routine when parser_sync counter is zero - and then you can use:
Similarly you can in the main program load the new counter value in some temporary variable and then use another flag to signal the interrupt routine to read the value and to update the real counter.
I have BTW modified your code to run on ATmega328 and on ATmega128 because I don't have an Arduino. I can see you are looking for the best way to debug the C code by stepping through assembler instructions generated by the compiler. I will describe you in one of my next posts what I think is the best approach. In short - AVR Studio 4.18 (not Atmel Studio!! and not the latest AVR Studio) + $8 ATmega128 PCB with JTAG connector + $7 USB JTAG ICE. That way you can debug the code both in the real hardware (using hardware breakpoints) almost for free - at least compared to newer and 'better' options :-) and in the simulator where you can count cycles and time. Since you didn't use any C++ code it took me just 10 minutes to modify Arduino code for using with the AVR Studio meaning you can, after debugging with ATmega128 board, change the code to work on Arduino in minutes.
And one more thing, I did a 2 channel 1-bit sound routine (using PWM and waveform tables) with portamento and decay envelope which runs on ATmega8 with 8 MHz internal oscillator and the results are quite good, some of the sounds could be compared with low end keyboards. Not to say what could be done with 8 pin ATtiny85 which has 64 MHz (yes, megahertz :-) ) PLL clock and can generate 250 kHz PWM output.
Welcome to the 1-bit forum, and thanks for all the info.