snes9xgit / snes9x

Snes9x - Portable Super Nintendo Entertainment System (TM) emulator
http://www.snes9x.com
Other
2.65k stars 457 forks source link

More MSU-1 issues #209

Closed qwertymodo closed 7 years ago

qwertymodo commented 7 years ago

Creating a new issue to avoid cluttering up #195

Still tracking down the exact issue, but I've found several other issues with the MSU-1 code that are probably causing problems of their own, like S9xMSU1Samples returning a uint16, when it should be at least a uint32, or more likely a size_t. Also, the partial_samples counter could very easily overflow, so I suggest increasing its size, as well as removing as many trailing zeroes in the increment/decrement values as possible (i.e. 3204, 4410). Still getting the audio jump on the first track, but some of these changes seem to be helping with other random crackling/popping.

qwertymodo commented 7 years ago

Here is a recording demonstrating the issue, along with the original audio track for comparison. The first "tick" is delayed, then cut short. It's like it's lagging, then jumping to catch up https://dl.qwertymodo.com/snes9x_msu1_playback.zip

roflcopter777 commented 7 years ago

Oh man ><

OV2 commented 7 years ago

Can you post your audio settings?

One issue I've found is that S9xMSU1SetOutput is called at the end of S9xFinalizeSamples, but S9xMSU1Generate might already need the buffer at the beginning of the function. However that would only miss a few samples the first time and not explain your skipping. I'd just move S9xMSU1SetOutput directly before S9xMSU1Generate.

roflcopter777 commented 7 years ago

In the 1.54.1 version?

API/Sound driver: XAudio2 Playback rate - 32 kHz Buffer length: 80 ms Input Rate: 31900 Hz (I forgot why I had it set to this, I can change it back to 32 kHz) Sync to sound core 16-bit stero OS: Windows 7 Pro 64-bit Audio: Realtek HD Audio 5.1

I hope this helps. If not, my apologies. Is there something that you need me to test?

qwertymodo commented 7 years ago

Sweet, I think I got it. Basically, S9xMSU1Generate needs to properly handle the sample buffer when the MSU-1 isn't playing, i.e. write 0's to the output buffer. Need to do a little more testing and clean up some of the other spitballing fix attempts I made, but I think this is it.

qwertymodo commented 7 years ago

Ok, give 32f70fa38e96b90469baa470dac10ffcf186f073 a try.

roflcopter777 commented 7 years ago

I hope that wasn't directed to me, I can't compile emulators worth crap. Someone else like @bearoso or @OV2 would be more likely candidates. Sorry.

qwertymodo commented 7 years ago

Here's a Windows build: https://dl.qwertymodo.com/snes9x-32f70fa.zip 32f70fa.exe should be the fixed one, and the other is a previous, buggy build for comparison.

roflcopter777 commented 7 years ago

So that download link is the fixed one then?

qwertymodo commented 7 years ago

It's both. The exe that matches the number on the zip is (hopefully) fixed. The other one is an older build included for comparison.

roflcopter777 commented 7 years ago

Okay,. yeah, I went to sleep shortly after I downloaded it,

The music seems to start up properly from what I tested, it's a lot smoother when it begins, using the 32f70fa app.

Tested the following ROMs: F-Zero Mega Man X Mega Man X2 Mega Man X3 A Link to the Past Secret of Mana

All of them work 100% flawlessly, even when loading and saving savestates over and over ad infinitum.

OV2 commented 7 years ago

Sounds fine here. I didn't get the skipping with msu1 chrono trigger, but all popping disappeared with this change. The size of the S9xMSU1Generate parameter shouldn't be an issue since any useful buffer sizes are < 150ms.

qwertymodo commented 7 years ago

Ok, so the parameter sizes are probably superfluous changes for the most part (aside from the S9xMSU1Samples count, uint16 was definitely too small there), the real fix was generating the null samples. But there's no harm in using a couple extra bytes of RAM for larger counters. If anything, if you wanted to increase the partial_samples increment/decrement counts back up to 32040/44100 for consistency, that's probably fine. Otherwise, I think I'll go ahead and close this.