lgblgblgb / xemu

Emulations (running on Linux/Unix/Windows/macOS, utilizing SDL2) of some - mainly - 8 bit machines, including the Commodore LCD, Commodore 65, and the MEGA65 as well.
https://github.com/lgblgblgb/xemu/wiki
GNU General Public License v2.0
207 stars 32 forks source link

MEGA65: Audio DMA does not stop playback at TADDR when bit depth is 16 and TADDR is odd #398

Closed dansanderson closed 8 months ago

dansanderson commented 8 months ago

Describe the bug Audio DMA does not stop playback at TADDR when TADDR is odd and the bit depth is 16. This affects both non-looping and looping playback: in both cases, it continues playback beyond the top address, and does not loop.

Used version of the project Xemu/MEGA65 20240228183812, macOS

To Reproduce

10 bload "sample",p($0050000)
20 poke $d720,0   : rem disable play while loading
30 poke $d711,$80 : rem enable audio DMA
40 poke $d71c,155 : rem ch0rvol
50 poke $d729,155 : rem ch0volume
60 wpoke $d721,0:poke $d723,$05 : rem ch0baddr
70 wpoke $d72a,0:poke $d72c,$05 : rem ch0curaddr
80 wpoke $d724,3314:poke $d726,0 : rem ch0freq = 3314 (8 khz of 40 mhz)
90 wpoke $d727,65535:rem low 16 bits of end addr; my sample is too big!
100 poke $d720,128+3:rem ch0en,ch0sbits=16
110 print chr$(147);"ch0curaddr = ";hex$(peek($d72c));hex$(wpeek($d72a)):goto 110

You can either provide an 8 kHz 16-bit signed sound sample in the "sample" file, or just remove line 10 and play whatever is in memory. If it's zeroes it won't sound; if it's garbage you might want to turn down the volume. :)

Expected behavior On real hardware, when $d727-8 is 65535, ch0curaddr stops at $60000. With the looping flag on, it loops at that point.

Computer/Device (please complete the following information):

lgblgblgb commented 8 months ago

I suppose, the problem can be somewhere here in targets/mega65/audio65.c:

if (XEMU_UNLIKELY((addr & 0xFFFF) == limit)) {
    // if top address is reached: either stop, or loop (on looped samples only!)
    [...]

I guess, changing == to >= may help. The problem here, that I haven't thought on this too much, and the code was mostly tested on 8 bit samples, when this is not a problem.

Also it would worth to test on other edge cases:

Surely I need to test this, if it's really the case, and this simple fix is enough or not.

@dansanderson Thanks for the report!

lgblgblgb commented 8 months ago

Proposal (I have to test this, I had no time for it yet ...):

if (XEMU_UNLIKELY(
    ((addr & 0xFFFFU) == limit)                     // limit reached
        || (                                        // OR ...
            (chio[0] & 3) == 3 &&                   // ... 16 bit sample
            ((addr - 1) & 0xFFFFU) == limit         // ... and the limit was the previous byte (as with 16 bit samples we would miss the limit if not aligned!)
        )
)) {
[...]

But, as far as I see so far, there is another problem here, that even if playback is not stopped correctly, it shouldn't "break free" to infinity. That's the question that audio DMA is "wrapped" within a 64K range always or not. Ie, if so, in theory some can set up a sample from - let's say - $48000 and setting the limit (that's a 64K setting) to $1000, thus then playback should go from $48000 to $48FFF and then continue at $40000 till hitting $41000. Well, if that's true it is wrapped. If not, it will continue to $50000 and stopping at $51000, I guess. But actually I can easily test this by looking the output of your test program (after modifying the limit for that test) on a real MEGA65 for sure.

lgblgblgb commented 8 months ago

The proposed fix has been pushed to next. I've tested this on real MEGA65 too and with Xemu, looks like identical now in behaviour. I've also tested the wrapping (or warping? I am never sure ...) around, and indeed MEGA65 doesn't do that either (ie, it continues on the next 64K "bank").

So, I guess, the only real problem here was the "alignment" of start/limit addresses in case of 16 bit samples. Which has been - hopefully - cured by this commit above.

lgblgblgb commented 8 months ago

I'm about to close this issue now, please free to re-open if there is some problem with this fix, or the problem remained in certain circumstances.