grumpycoders / pcsx-redux

The PCSX-Redux project is a collection of tools, research, hardware design, and libraries aiming at development and reverse engineering on the PlayStation 1. The core product itself, PCSX-Redux, is yet another fork of the Playstation emulator, PCSX.
https://pcsx-redux.consoledev.net
GNU General Public License v2.0
663 stars 108 forks source link

Stuttering issue in Castlevania Symphony of the Night after " Delaying reads after a seek" commit #643

Open gameblabla opened 3 years ago

gameblabla commented 3 years ago

This commit... : https://github.com/grumpycoders/pcsx-redux/commit/5730e04f0183f37038bc1d133cf9f9092425b90a while it fixed a bunch of games (Ctr's intro music no longer being cutoff, Worms Pinball booting, the freeze after FF8 lunar cry FMV is not there anymore), it regressed in a bunch of other games that play ADPCM tracks it seems. One of the most noticeable one is Castlevania Symphony of the Night during the speech samples but it also happens in other instances (such as the menu music).

See the issue for yourself and listen carefully to Dracula saying "It was not by my hand": https://youtu.be/g6sT0DggNRI

My fix, which you can find https://github.com/gameblabla/pcsx-redux/commit/64d811fbcd4f61202950747063842f51720f2747, simply sets m_locationChanged = false; before playADPCM. This is what it sounds like after my "fix": https://youtu.be/TIbi3nRH6xw

Now, i'm aware that this is a hack of sorts, which is why i opened this issue in the first place and not just a PR.

This is what the game does when it plays a speech sample

CD1 write: 1 (CdlNop)
CD1 write: 1 (CdlNop)
CD1 write: 9 (CdlPause)
CD1 write: e (CdlSetmode) Param[1] = { c8,}
CD1 write: 1 (CdlNop)
CD1 write: d (CdlSetfilter) Param[2] = { b, 6,}
CD1 write: 1 (CdlNop)
CD1 write: 2 (CdlSetloc) Param[3] = { 26, 1, 39,}
CD1 write: 1 (CdlNop)
CD1 write: 6 (CdlReadN)
CD1 write: 1 (CdlNop)
nicolasnoble commented 3 years ago

Yeah this code needs to be refined a bit. Basically there's already seek code in the cdrom logic code, and it needs to be compounded with it. We shouldn't cause a delay if the seek destination is already within a very close ballpark of the current location.

In general, the seeking logic needs to be fully rewritten, with a logarithmic curve indicating the number of sectors covered in one tick.