TASEmulators / desmume

DeSmuME is a Nintendo DS emulator
http://desmume.org
GNU General Public License v2.0
2.93k stars 544 forks source link

[Regression] Random chance of Audio cracking when "Advanced SPU Logic" is on. #589

Open yontekh opened 2 years ago

yontekh commented 2 years ago

Somewhere between 0.9.9a and 0.9.11 an issue emerged where when "Advanced SPU Logic" is enabled, there is a seemingly random chance that background music will crack alot.

This only happens when Advanced SPU Logic is on. I cannot reproduce this issue on 0.9.9a. But still occurs on the latest nightly of 0.9.13.

It happens on multiple games, also in the BIOS boot sound randomly.

Here is a sample of Phantom Hourglass in the first cave: DeSmuME_0.9.13_x64.webm

The first 2 times I am in the cave in this sample the audio is cracking for as long as you stay in the cave, when i re-enter the cave it will randomly either not crack or crack.

Also my settings: desmume.ini.txt

zeromus commented 2 years ago

@Aikku93 0dc75ca08944433051a138e2d750ac8694633df6 ? UPDATE: err.. that was just before 0.9.13, not 0.9.11. I misread. SO this could be almost anything

zeromus commented 2 years ago

We also have someone saying savestates are no longer reliable for some reason due to the SPU, I can't remember the details right now. maybe related.

yontekh commented 2 years ago

DeSmuME.webm here is a sample of the bios sound doing the same. the 5,8,9,10th time the sound plays correctly, but all other times it is distorted.

I also tested this on my laptop which has completely different hardware, and windows 11 instead of 10, and the same thing happens. So I dont think its my hardware.

yontekh commented 2 years ago

"Enable advanced bus-level timing" was off, I turned it on and this seems to have solved the issue. Good enough for me.

zeromus commented 2 years ago

not good enough for me

Aikku93 commented 2 years ago

I've had this mentioned to me, as well. Even after attempting to track it down, and all the changes in the SPU re-write branch, it still happens; all I managed to do was reduce how often it happens, which seems to suggest it might be a synchronization issue on the host hardware. I'll see if I can track it down, anyway.

zeromus commented 2 years ago

this type of stuff is expected to happen in the case of "dual spu", although not in the case of looping samples but rather samples being cut to steal voices in complex sequences, or possibly in streams. It's because the spu and the cpu do run asynchronously with chaotic timing and sometimes the cpu cuts things early. Now it should be said that "advanced spu logic" enables the capturing effects which are processed by the CPU on this system, so if this is only being heard with advanced spu logic enabled then it's possible it's the aforementioned problem but just smoked out by the reverb processing. But I didn't expect the bios boot sound to be doing that..... but INDEED, even though I cant remember how to get the bios sound now, I see that it's doing a capture process even with the ticking clock there.

@yontekh can you post a savestate (on latest git version) or save file as close as possible to that cave music? the cave music is actually quite good for debugging this, I would think.

Aikku93 commented 2 years ago

I've replicated the problem with the BIOS startup sound. The issue is that the BIOS (and some games) are really terribly synchronized to the audio hardware, and when they're processing captured data, they write far too close to the data that is actually being played back. Because we aren't able to get perfect timings compared with real hardware, we sometimes get issues where games will cause buffer overrun (the CPU is "running too quickly"; this is why it's especially obvious with advanced bus-level timing turned off) and process data that isn't ready yet, causing clicks/pops/garbage until the next SPU update.

I'll try to find a proper solution, but a quick fix was reducing the capture FIFO size to 4 samples. I'm not entirely sure why that works, though.

Aikku93 commented 2 years ago

Found the true culprit: When SPUFifo is being filled with the first 16 samples, it /should/ be outputting 0s (or at the very least advancing cap.runtime.curdad), but it isn't. Because of that, the capture output channel will become desynchronized with the capture unit by 16 samples, which is apparently a large enough window for overrun to creep into.

This is already fixed in the SPU re-write (I used a more straightforward FIFO implementation that avoided the issue altogether), but fixing in master is fairly easy. Should I create an interim PR for this, or just leave it for a time when the re-write is merged?

zeromus commented 2 years ago

Works either way for me, do what you want

Aikku93 commented 2 years ago

I've created a PR for this issue (based on master), just to make sure it's in the commit history. This will cause a merge conflict in the new-spu-logic branch, but it can be ignored since the code is completely different anyway.