6502 MIDI file player

[lordbubsy]

Well that took me some effort! First I replaced the SC26C92 with the SC28L92. In order to make use of the 16 bit FIFO, I had to enable it by setting the MR0A[3] bit to ‘1’. This made no difference in playback performance.

Then I implemented a circular 256 byte software buffer together with interrupt based writing to the UART. That was indeed a bit fiddly because of the distinction between the UART and TIMER interrupts. Eventually I got it sort of working. The MIDI file is playing exactly as before, including the lag at those specific areas, but it also misses several MIDI events which of course is even less desirable. I suspect my mistake is that I’m not clearing the interrupt status register bit that indicates the UART interrupt because I don’t know if or how I have to disable it. (I don’t think I have to clear it, because it’s cleared automatically when reading the interrupt status register?)


I’m using the title music of the game Duke Nukem 3D, which is a standard MIDI file from the 90’s, specifically arranged to be played on a Roland Sound Canvas. On a PC, playing that MIDI file over a conventional MIDI interface with a 5-pin MIDI cable, it plays without any lag. It even plays perfectly after setting the tempo to 250 BPM. That should prove that the MIDI file isn’t hogging a physical PC MIDI interface nor the MIDI synthesizer itself.

I’ve analyzed the MIDI file with an event viewer and the moment my player starts to lag is when various MIDI events are to be played at the same time, i.e. when their delta times are 0. After that, when events are streamed with delta times other than 0, it resumes playing at normal speed.

What I will do next is to 1) change the delta times in those areas to 1, eliminating the fact that they have to be send in one interrupt cycle. 2) Replace frequently used subroutines with macros to speed up the code. 3) Perform some code optimizations suggested previously.

[Chromatix]

Perhaps we should take a look at your code.

[lordbubsy]

OK, this is the interrupt code:

I’ve also appended the whole source, it’s too long to put it in the post.

Code: Select all

interrupt
		pha
		phx
		phy

		lda	isr
		and	#%00001000		;check if interrupt is caused by C/T
		beq	HandleUart		;no? goto uart irq
HandleTimer
		lda	tc			;is binvlq = tc?
		cmp	binvlq
		bne	IncTc			;no, increment tc and end irq
		lda	tc+1
		cmp	binvlq+1
		bne	IncTc
		lda	tc+2
		cmp	binvlq+2
		bne	IncTc
		lda	tc+3
		cmp	binvlq+3
		bne	IncTc

ExEv		jsr	DecodeEvent		;yes, decode a track event (midi/sysex/meta)
		jsr	GetVarLength		;get variable length quantity

		lda	binvlq			;is binvlq zero?
		ora	binvlq+1		;no, reset tc and end irq
		ora	binvlq+2
		ora	binvlq+3
		bne	ResetTickCounter
		bra	ExEv

IncTc
		inc	tc			;increment tick counter tc
		bne	+
		inc	tc+1
+		bne	+
		inc	tc+2
+		bne	+
		inc	tc+3
+
		bra	EndTimerIrq

ResetTickCounter
		stz	tc			;Reset Tick Counter tc  and end irq
		stz	tc+1
		stz	tc+2
		stz	tc+3

EndTimerIrq
		jsr	buf_dif			;are there bytes to send?
		beq	ClearCountReady		;no, exit
		lda	imr
		ora	#%00010000		;enable transmitter TxEMT interrupt according to FIFO level in mr0b
		sta	imr
ClearCountReady
		lda	rop12			;clear counter ready interrupt status bit
		ply
		plx
		pla
		rti

HandleUart
		lda	isr
		and	#%00010000		;check if interrupt is caused by transmitter buffer empty
		beq	EndIrq			;no? exit irq

-		jsr	buf_dif			;are there bytes to send?
		beq	+			;no, disable TxEMT interrupt, exit
		jsr	rd_buf			;read next character to send
		sta	txfifob			;put character to port
		lda	srb			;get status reg B
		and	#%00000010		;is transmit buffer full?
		bne	EndIrq			;yes, exit
		bra	-
+
		lda	imr
		and	#%11101111		;disable transmitter TxEMT interrupt according to FIFO level in mr0b
		sta	imr
EndIrq
		ply
		plx
		pla
		rti

Player27 cleanup.asm

(21.23 KiB) Downloaded 145 times

[Chromatix]

I can see one potential problem:

Code: Select all

                lda     imr
                ora     #%00010000              ;enable transmitter TxEMT interrupt according to FIFO level in mr0b
                sta     imr

The interrupt mask register on your UART is not a read-modify-write register, but write-only; it shares its address with the interrupt status register, which is read-only. So when this code is executed, the timer interrupt will be masked unless it has already fired and not been reset. In practice you are only resetting that interrupt after this code, but it's still bad practice. You should be resetting the timer interrupt very soon after entering the timer ISR, and certainly before unmasking the CPU interrupt (for the "better timer").

To solve this properly, you need to keep a shadow copy of the IMR somewhere. One possibility is to use the "uncommitted" UART register (ureg) at offset $C, which effectively acts as a byte of RAM. The above would then become:

Code: Select all

                lda     ureg
                ora     #%00010000              ;enable transmitter TxEMT interrupt according to FIFO level in mr0b
                sta     ureg
                sta     imr

…and similarly for disabling the interrupt later.

That MIDI events are lost indicates that your send buffer is overrunning itself. To guard against that, you need to put a check in your midiout routine for the tail pointer becoming equal to the head pointer. To begin with you could make that an abort condition (busy-loop) so that you become aware of the first time it occurs. With the "better timer" routines I suggested, you would be filling the buffer concurrently with the UART handler draining it, so you could simply busy-wait until space appears in the buffer. I would therefore suggest modifying your buffer routines as follows:

Code: Select all

init_buf:       stz     rd_ptr                  ;setting wr_ptr equal to rd_ptr initializes
                stz     wr_ptr                  ;the buffer, showing it to be empty.
                rts
 ;-------------
midiout:        ldx     wr_ptr                  ;start with a containing the byte to put in the buffer.
                sta     buffer,x                ;get the pointer value and store the data where it says,
                inx
-               cpx     rd_ptr                  ;wait for space in the buffer,
                beq -
                stx     wr_ptr                  ;then increment the pointer for the next write.
                rts
 ;-------------
rd_buf:         ldx     rd_ptr                  ;ends with a containing the byte just read from buffer.
                lda     buffer,x                ;get the pointer value and read the data it points to.
                inc     rd_ptr                  ;then increment the pointer for the next read.
                rts
 ;-------------
buf_empty:        lda     wr_ptr
                cmp     rd_ptr                  ;ends with Z set if the buffer is empty.
                rts
 ;-------------
rd_ptr  !byte   0                               ;reserve one byte of ram for the read pointer
wr_ptr  !byte   0                               ;and one for the write pointer.

[BigDumbDinosaur]

Turning the TxD IRQ on and off during run time is not a recommended procedure. Instead you should disable the transmitter when there is nothing to transmit and re-enable it when data to transmit becomes available. Disabling the transmitter will automatically cancel a pending TxD IRQ, and is accomplished with a single write to the channel command register—also the case when re-enabling the transmitter. I've given you working code in the past, Marco, on how to correctly handle this stuff—you should refer to it to avoid having to re-invent the wheel.

As for the timer, reading the ISR to determine if the timer is responsible for an IRQ only tells if that is the case. It doesn't cancel the IRQ. A timer IRQ is cleared by issuing a "stop timer" command. If the timer is in free-run mode it continues to run despite the "stop timer" command.

Lastly, subroutines in interrupt service handlers are not advisable. Unless there is a really compelling reason to use a subroutine, all ISR code should be linear to avoid the stack activity required with subroutines.

[Chromatix]

I suspect he's trying to learn by doing, rather than by following a recipe. In context, I don't think subroutines or the precise method by which interrupts are turned off are that big a deal.

But yes, he needs to move the timer IRQ reset much earlier in the routine. That in itself should give him some more margin.

[BigDumbDinosaur]

Chromatix wrote:

In context, I don't think subroutines or the precise method by which interrupts are turned off are that big a deal.

They matter in code that has to meet processing deadlines in order to function in a satisfactory fashion—MIDI playback, I opine, qualifies as such a case. In the 28L92 (and other NXP UARTs of that type), manipulating the interrupt mask register (IMR) is somewhat cumbersome, since it's a write-only register, necessitating the overhead of maintaining a shadow somewhere. Stopping a transmitter involves a single write that requires no shadow, since a separate write command is used to start the transmitter.

As for subroutine usage, when the interrupt rate starts getting up there, the additional cycles represented by a JSR/RTS pair become significant. In my 65C816 interrupt processing article, I noted the following:

• The use of subroutines in an interrupt service routine can substantially hurt performance, as each JSR – RTS pair will consume 12 clock cycles, or 14 cycles if using JSL – RTL. If your interrupt handler includes three calls to the same subroutine and is processing a 100 Hz jiffy interrupt, 3600 clock cycles will be consumed per second just in executing JSR and RTS instructions. A lot of foreground processing can be completed in 3600 cycles! Only use subroutines if you have to squeeze every last byte out of the available address space.

So yes, it can be a big deal.

[Chromatix]

Can be, I'll grant you. But I don't think @lordbubsy is at the level of worrying about that just yet; those are optimisations he can make in due course.

[lordbubsy]

@Chromatix

Quote:

I suspect he's trying to learn by doing, rather than by following a recipe.

Indeed I am. I just began writing this MIDI player by reading the MIDI implementation. It worked out very quickly and very well, but I must admit that I have a hard time to make sense of what happens when the code in the interrupt routine takes more time than it’s given for one interrupt period. Especially when there is more than one interrupt involved.

Quote:

The interrupt mask register on your UART is not a read-modify-write register...

OK, that’s what BDD also said about the MRxx registers, I just have forgotten to implement it. However, that made no difference. It still loses events and stalls.

Quote:

You should be resetting the timer interrupt very soon after entering the timer ISR

Done that.

Quote:

That MIDI events are lost indicates that your send buffer is overrunning itself.

I tried your suggestion but it made no difference, events still got lost. So I tried this to stop the program if the buffer would get full.

Code: Select all

midiout:        ldx     wr_ptr                  ;start with a containing the byte to put in the buffer.
                sta     buffer,x                ;get the pointer value and store the data where it says,
                inx
                cpx     rd_ptr                  ;wait for space in the buffer,
                beq +
                stx     wr_ptr                  ;then increment the pointer for the next write.
                rts
+               stp

...but it doesn’t stop, so the buffer isn’t overrun???

I suspect that my UART interrupt routine is not functioning well in conjunction with the timer routine.

I think I’ll revisit the "better timer" routines you suggested, because I haven’t implemented or even understood them.

@BDD
I replaced the TxD IRQ on and off with disabling / enabling the transmitter with:

Code: Select all

		lda	#%00000101		;enable Tx enable Rx
		sta	crb

		lda	#%00001010		;disable Tx disable Rx
		sta	crb

Unfortunately no change.

Quote:

I've given you working code in the past, Marco, on how to correctly handle this stuff—you should refer to it

Yes you did, but back then I got it working on MARC-2 by copy and paste. A few months back I tried to get it working on MARC-4 but I failed. Perhaps I should give it another try. I’m rather new to the interrupt thing, I got several things working on a VIA handshake interrupt, the VDP scanline interrupt and the DUART timer interrupt. But I’ve still got to figure out the UART interrupt a bit more.

Quote:

A timer IRQ is cleared by issuing a "stop timer" command.

As far as I know I’m doing that by reading register “duart_base + $f”, right?

Quote:

subroutines in interrupt service handlers are not advisable

That’s definitely an easy thing to do next, but I don’t think it will save the day.

Here is how I initialize the timer and UART interrupt:
Initialize the MIDI UART channel B:

Code: Select all

acia_b_init
		lda	#%10110000		;set MR pointer to 0
		sta	crb
		lda	#%00000000		;MR0 Normal mode
		sta	mr0b
		lda	#%00010011		;No parity, 8 bits per char.
		sta	mr1b
		lda	#%00001111		;Stop bit length 2.000
		sta	mr2b
		lda	#%11101110		;receiver clock select IP5-16X transmitter clock select IP5-16X
		sta	csrb
		lda	#%00000101		;enable Tx enable Rx
		sta	crb
		rts

Setup the interrupts:

Code: Select all

SetupDUARTInterrupt
                sei                             ;disable interrupts
                lda     #<interrupt             ;set interrupt vector
                sta     $fffe                   ;RAM on MARC-4
                lda     #>interrupt
                sta     $ffff
                lda     #>$1000                 ;set c/t for 192Hz or 5208us
                sta     ctpu
                lda     #<$1000                 ;deviser = 3.6864MHz / (2*192) = 9600 = $2580
                sta     ctpl
                lda     #%01100000              ;enable timer (square wave) X1 mode
                sta     acr                     ;clear bit 4 and set bits 6 & 5
                lda     #%10110000              ;set MRB pointer to 0
                sta     crb
                lda     #%00000000              ;MR0[5] MR0[4] Transmitter FIFO Interrupt Fill Level
                sta     mr0b                    ;8 bytes empty TxEMPTY
                lda     #%00011000              ;enable counter ready interrupt and transmitter interrupt
                sta     imr
                lda     sop12                   ;start timer by reading register 14
                cli                             ;enable interrupts
                rts 

The interrupt routine itself:

Code: Select all

interrupt
                pha
                phx
                phy

                lda     isr
                and     #%00010000              ;check if interrupt is caused by C/T
                beq     HandleTimer             ;no? goto uart irq
                jmp     HandleUart
HandleTimer
                lda     rop12                   ;clear counter ready interrupt status bit
                lda     tc                      ;is binvlq = tc?
                cmp     binvlq
                bne     IncTc                   ;no, increment tc and end irq
                lda     tc+1
                cmp     binvlq+1
                bne     IncTc
                lda     tc+2
                cmp     binvlq+2
                bne     IncTc
                lda     tc+3
                cmp     binvlq+3
                bne     IncTc

ExEv            jsr     DecodeEvent             ;yes, decode a track event (midi/sysex/meta)
                jsr     GetVarLength            ;get variable length quantity

                lda     binvlq                  ;is binvlq zero?
                ora     binvlq+1                ;no, reset tc and end irq
                ora     binvlq+2
                ora     binvlq+3
                bne     ResetTickCounter
                bra     ExEv

IncTc
                inc     tc                      ;increment tick counter tc
                bne     +
                inc     tc+1
+               bne     +
                inc     tc+2
+               bne     +
                inc     tc+3
+
                bra     EndTimerIrq

ResetTickCounter
                stz     tc                      ;Reset Tick Counter tc  and end irq
                stz     tc+1
                stz     tc+2
                stz     tc+3

EndTimerIrq
                jsr     buf_dif                 ;are there bytes to send?
                beq     ClearCountReady         ;no, exit
                lda     #%00000101              ;enable Tx enable Rx
                sta     crb
ClearCountReady
                ply
                plx
                pla
                rti

HandleUart
                lda     isr
                and     #%00010000              ;check if interrupt is caused by transmitter buffer empty
                beq     EndIrq                  ;no? exit irq

-               jsr     buf_dif                 ;are there bytes to send?
                beq     +                       ;no, disable TxEMT interrupt, exit
                jsr     rd_buf                  ;read next character to send
                sta     txfifob                 ;put character to port
                lda     srb                     ;get status reg B
                and     #%00000010              ;is transmit buffer full?
                bne     EndIrq                  ;yes, exit
                bra     -
+
                lda     #%00001010              ;disable Tx enable Rx
                sta     crb
EndIrq
                ply
                plx
                pla
                rti 

Thanks for all the patience, I hope it isn’t getting too boring.

[dp11]

If x or y is not needed on the interrupt handler then there is no need to stack them.

The HandleUart code can be before the interrupt handler then you wouldn't need the Jmp.

[Arlet]

Instead of AND for checking bits, you can use BIT. This doesn't destroy contents of A.

[Chromatix]

Code: Select all

                lda     srb                     ;get status reg B
                and     #%00000010              ;is transmit buffer full?
                bne     EndIrq                  ;yes, exit

I think I see a bug here. The bit tested is the one for Receive FIFO Full, but you want the one for Transmit FIFO Ready. The following should fix it:

Code: Select all

                lda     srb                     ;get status reg B
                bit     #%00000100              ;is transmit buffer ready?
                beq     EndIrq                  ;no, exit

[lordbubsy]

@Chromatix
Thank you! It’s working now, that is, the MIDI file is playing back as previously. All events are being send, but there where’re multiple events with delta time 0, it starts stalling. i.e. the player behaves as if there were no software buffer or transmitter interrupt.
I’m not sure if I said this before, but the tempo restores to normal when the MIDI stream is less busy. What I didn’t notice until now is that there is a difference in playback speed during those stalls while playing them back with 8MHz or 1MHz.

Could it be that a PC MIDI interface is better in handling those dense MIDI streams?
There is also something called intelligent mode, which I haven’t implemented.

@BDD
I’m trying to figure out the SC26C92 routines you gave me years ago. It’s going slowly...

[Chromatix]

In all honesty, this is good practice for me as well. It might even give me an excuse to get interrupts and some devices working in my emulator. Perhaps you could upload the code as it presently stands, as its misbehaviour would be a good test case for emulation.

I recall previously you said the tempo didn't vary with CPU speed, so I think we're making progress, in that the bottleneck has moved. That means we've solved one problem and uncovered another. Previously the UART was blocking progress, now we're simply taking too long to process each flood of MIDI events in software. The outcome is similar; we miss timer interrupts, so the beat slows.

The solution will probably be that "better timer" I described previously. Implementing that correctly will require a relatively deep understanding of how interrupts really work on the 6502, but it will then count the number of missed timer interrupts, and allow the MIDI routine to catch up once it's finished processing the current tick.

Optimising your code is also likely to help, but only on the quantitative level (like increasing the CPU speed), not qualitative (avoiding perceptible slowdowns entirely). The PC benefits here from having a vastly more powerful CPU, but that is overkill for the simple task of decoding a MIDI file, and much of the extra power is absorbed by increasingly inefficient layers of "modern" software. There are, however, one or two hints at reasonable optimisations in the pseudocode I posted earlier, such as counting down from the delta value to zero, instead of counting up to it from zero. This means you can perform the test against reaching the deadline more efficiently. This is a standard transformation that you can often apply in low-level coding, and which compilers often apply behind the scenes.

As it happens, while counting down a single-byte variable to zero is easy and fast on the 6502, for multi-byte variables it's more efficient to bitwise-invert the value and then count up to zero. Consider this variant on your main loop:

Code: Select all

HandleTimer
               ; insert mutex test etc here

IncTc
                inc     tc                      ; increment tick counter tc and test against zero
                bne     EndTimerIrq
                inc     tc+1
                bne     EndTimerIrq
                inc     tc+2
                bne     EndTimerIrq
                inc     tc+3
                bne     EndTimerIrq

ExEv
                jsr     DecodeEvent             ; decode a track event (midi/sysex/meta)
                jsr     GetVarLength            ; get variable length quantity

                lda     binvlq                  ; is binvlq (delta ticks) zero?
                ora     binvlq+1
                ora     binvlq+2
                ora     binvlq+3
                beq     ExEv

                lda     binvlq                  ; transform binvlq into count-up-to-zero form
                eor     #$FF
                sta     tc
                lda     binvlq+1
                eor     #$FF
                sta     tc+1
                lda     binvlq+2
                eor     #$FF
                sta     tc+2
                lda     binvlq+3
                eor     #$FF
                sta     tc+3
                bra    IncTc                      ; complete the negation by incrementing it

[BigDumbDinosaur]

lordbubsy wrote:

@BDD
I’m trying to figure out the SC26C92 routines you gave me years ago. It’s going slowly...

Is your MIDI machine running on a 65C02 or 65C816?