PCSX2 / pcsx2

PCSX2 - The Playstation 2 Emulator
https://pcsx2.net
Other
10.63k stars 1.55k forks source link

[BUG]: Spyro Enter the Dragonfly cutscene audio is delayed and cut #8286

Open CRASHARKI opened 1 year ago

CRASHARKI commented 1 year ago

Describe the Bug

In Spyro Enter the Dragonfly all the cutscenes (Intros, midgame and endings) have the audio delayed and the cutscenes end before the audio does. This might affect other sounds in the game, but it's most noticeable in the cutscenes since they requiere sync. This issue seems to be present since the game was made playable in PCSX2.

Reproduction Steps

Launch the game and start a new game to watch the intro cutscene with audio desynced and cut.

These are a couple of videos showcasing the issue, they are old, but the issue is still the same. https://www.youtube.com/watch?v=q2ndlQNMWoc https://www.youtube.com/watch?v=sdJWeTqsolw

Expected Behavior

Audio playing at the correct time.

Here's an old video recorded from an actual PS2 showing the audio playing correctly: https://www.youtube.com/watch?v=-xJCAQMnhhs

PCSX2 Revision

v1.7.4156

Operating System

Windows 10 (64bit)

If Linux - Specify Distro

No response

CPU

i7-6700K

GPU

NVIDIA GeForce GTX 960

GS Settings

It happens in both hardware and software.

Emulation Settings

All settings are default.

GS Window Screenshots

No response

Logs & Dumps

No response

Buzzardsoul commented 1 year ago

Likely due to the fact that this game is still using a 9 year old patch.

CRASHARKI commented 1 year ago

The patch was to prevent a crash/black screen on startup, but I don't know if it was related to audio or file loading.

Buzzardsoul commented 1 year ago

@CRASHARKI It's been known that the patch is very janky for a long time but was never replaced due to the issue being extremely inconsequential.

Mrlinkwii commented 1 year ago

@Buzzardsoul sorry what

Buzzardsoul commented 1 year ago

@Buzzardsoul sorry what

To put it simply, this issue is entirely caused by the Startup patch that the game still needs in order to work.

It's not a fault of the SPU2 nor the GS emulation.

RedDevilus commented 1 year ago

The game can already crash with or without patches when you try to play/skip FMVs. Where is your source that it isn't the fault of any emulation or the game itself? Making random assumptions is what happened in the past and can lead to wrong conclusions, not saying it isn't true what you say, but it does look odd out of nowhere that you comment on random issues.

CRASHARKI commented 1 year ago

@RedDevilus But this game has no FMVs, unless you mean the in-game cutscenes.

Mrlinkwii commented 1 year ago

@CRASHARKI The patch fixes the crash that the game encounters when you get to the title screen

RedDevilus commented 1 year ago

@RedDevilus But this game has no FMVs, unless you mean the in-game cutscenes.

Yeah, meant the cutscenes. Force of habit to say FMV even if difference between pre-rendered and real-time, my bad.

Buzzardsoul commented 1 year ago

I really don't feel like arguing, therefore I will just post my proof that it's quite literally a GIF issue that's being ignored by the patch itself.

This is the offset the game gets stuck on (PAL)

untitled Edit: Nope, my mistake.

CRASHARKI commented 1 year ago

Update: I made a few tests with PCSX2 1.6.0 (Since that one allows to disable Automatic Gamefixes, I don't think nightly builds do, and the issue still occurs in 1.6.0), with Automatic Gamefixes off the main menu sound loads at the right time instead of delayed but the screen stays black, as soon as you activate Automatic Gamefixes with the game still on, the screen will show up like normal (Although the sound won't sync obviously). If you now disable them and start a new game, the game will play as normal and the audio will sync just fine with no more black screens. I don't know if this could be automatically applied through patches though, but at leats now we know more about how the patch is messing with the audio. Could there be a way to disable the code in the patch after it's been applied once maybe?

Goatman13 commented 1 year ago

I really don't feel like arguing, therefore I will just post my proof that it's quite literally a GIF issue that's being ignored by the patch itself.

This is the offset the game gets stuck on (PAL)

untitled

Game just wait there for Vblank, and continue when irq happen.

Update: I made a few tests with PCSX2 1.6.0 (Since that one allows to disable Automatic Gamefixes, I don't think nightly builds do, and the issue still occurs in 1.6.0), with Automatic Gamefixes off the main menu sound loads at the right time instead of delayed but the screen stays black, as soon as you activate Automatic Gamefixes with the game still on, the screen will show up like normal (Although the sound won't sync obviously). If you now disable them and start a new game, the game will play as normal and the audio will sync just fine with no more black screens. I don't know if this could be automatically applied through patches though, but at leats now we know more about how the patch is messing with the audio. Could there be a way to disable the code in the patch after it's been applied once maybe?

I can't reproduce your test result. Code patch applied once stays there even if you disable it in menu. Unless game overwrite that memory area, which isn't the case here (I tested this).

Current patch return true in SpyroWorld::CinematicsMusicStreamReady() [0x1E7070]. Which normally fail because JukeBox::GetMusicStreamStatus() [0x27EA50] return false. JukeBox::GetMusicStreamStatus() return val_from_iop & 0x200. On IOP side there seems to be only one place where that can be set, function IsChannelPlaying [0x8D1BC] should return 1 (under the hood its u16 result = sceSdGetParam(0x502), which in turn just read 0x1F90001A), result is then shifted left by 9 and should end up as 0x200 eventually. This get later ored to current status, and send by sif to EE which expect that mentioned 0x200 to be true. But function IsChannelPlaying return 0x4000 for some reason, which shifted by 9 is 0x800000. So obviously JukeBox::GetMusicStreamStatus() return 0 at this point. Anyway. Forcing return true at any stage of that chain have same effect as current patch.

Issue could be anything, but it seems that game want that reg to be set to 1. I tried to read SPU2 code to figure out little bit more, but its kinda hard part of pcsx2 code to read. This seems to be reading ADSR.Value for core 0 voice 1. But only place that can set this value to non 0/1 seems to be WriteRegPS1? Maybe there is a way to set it directly from DMA or something? For people that want to investigate this. 0x921C0 is where game call IsChannelPlaying on IOP side. Given memory offsets are for SLUS-20315 version of game.

Sorry for wall of text and little off-topic, since my post explain why patch is there and not why there is desync between a/v.

stenzek commented 1 year ago

Reading ADSR volume to know which channels are busy was very common in PS1 games, wouldn't be surprised if the idea carried over. I suppose the question here would be why the voice isn't busy in the first place.

Edit: Keying off a voice will reduce the volume to zero over time.

CRASHARKI commented 1 year ago

I can't reproduce your test result. Code patch applied once stays there even if you disable it in menu. Unless game overwrite that memory area, which isn't the case here (I tested this).

I forgot to mention I'm using PAL version, and I can reproduce it very consistently. Although when I activate and deactivate the patch too fast or at certain points in the loading the delay happens too, I don't really know why.

Buzzardsoul commented 1 year ago

Ok, after doing some hardware tests on the PAL version, it seems I may have found out why the patch is needed just for curiosity's sake. The offset "0x0038A290" Is a read/write offset in this game. On a real ps2 during the "Main menu" its value will be "0x007FFE00" But on PCSX2, its value is for some reason "0x00800000" and thus, the executable offset of "0x0027EF80" will end up setting the incorrect value of "0x00800000" instead of the correct value of "0x007FFE00" as the v0 register's value

Edit: But it's likely not related to this issue in any way because even after implementing it, the cutscene audio still isn't in sync

patches: 0EF1D4BA: content: |- comment=My patch that emulates the correct behavior patch=1,EE,2027EF80,word,0803FFD4 patch=1,EE,200FFF58,word,8C420000 patch=1,EE,200FFF5C,word,3C150080 patch=1,EE,200FFF60,word,16A20006 patch=1,EE,200FFF68,word,2442FE00 patch=1,EE,200FFF84,word,24150000 patch=1,EE,200FFFAC,word,0809FBE1

Goatman13 commented 1 year ago

With hardware test result from Buzzardsoul, i figured out that "ADSR Value" on real hardware is 0x3FFF, while on pcsx2 it is 0x4000. Because value is left shifted by 9 before sending to EE. 0x4000 << 9 = 800000, but 0x3FFF << 9 give exactly 0x7FFE00. So it's a matter of 1. Remember that game do stupid check and test this value with 0x200 to continue or not.

On pcsx2 i narrowed down issue to be V_ADSR::Calculate() case2 // decay issue. Currently when we enter that state in Spyro game, Value is 0x7FFFFFFF, DecayRate is 0, calculated "off" is 0xC, which in turn mean we are going to use PsxRates[0x90] to decrease ADSR Value. PsxRates[0x90] = 0x3FFFFFFF set by PsxRates[i] = (int)std::min(rate, (s64)0x3FFFFFFFLL);. So we end up with 0x40000000 Value.

Since nothing else seems to happen there. My guess is that PsxRates values should be allowed up to 0x40000000 instead of 0x3FFFFFFF. I tested this, and patch is not longer required to go in-game. Although this still don't fix delay which is most likely EE or IOP timing issue.

This seems to be correct, because there is no other way to end up with 0x3FFFFFFF while leaving case 2. But i don't feel that I want to PR this. Too many things can break if i'm wrong. :P

refractionpcsx2 commented 1 year ago

values on the SPU are generally -0x4000 to 0x3fff, so I'm not sure 0x4000 makes much sense :P

Goatman13 commented 1 year ago

Then i don't know how real PS2 end up with 0x3FFF "ADSR Value". Maybe 0x4000 should be allowed there for situations where it is used to decrease volume? So its like doing Value + -0x4000 or something.

refractionpcsx2 commented 1 year ago

Yeah I dunno, not even sure where this rate table came from, maybe a hangover from PCSX :P ADSR tables and handling are not my forté, but yeah I'm not sure what the values should be personally...

Ziemas commented 1 year ago

I think it might indeed be off by one, if the decay rate is 0 I think the decrease should be 0x4000 per step. so 0x7fff (ADSR max) - 0x4000 is 0x3fff.

With the decay target set to 0x6800 ((0xc + 1) << 11) anything below that would cause the ADSR to switch phase, so they're being very silly by expecting a value lower than that.

Note that unlike pcsx2 the spu uses 16bit math for ADSR, and the rate table was from before the algorithm was figured out. (although the table was at least improved since the pcsx days)

refractionpcsx2 commented 1 year ago

So I guess this line in the Init function is the culprit? PsxRates[i] = (int)std::min(rate, (s64)0x3fffffffLL);

Buzzardsoul commented 1 year ago

So I guess this line in the Init function is the culprit? PsxRates[i] = (int)std::min(rate, (s64)0x3fffffffLL);

Changing it to PsxRates[i] = (int)std::min(rate, (s64)0x40000000LL); has also managed to start the game without the need for the startup fix on my end. I wonder if it truly is the real behavior of the original hardware.

refractionpcsx2 commented 1 year ago

the startup fix? You mean the patch? An audio volume caused a TLB miss? O_o

Buzzardsoul commented 1 year ago

the startup fix? You mean the patch? An audio volume caused a TLB miss? O_o

Seems like it did lmao.

refractionpcsx2 commented 1 year ago

I've seen some strange things in my time, that one is top 10, at least!

RedDevilus commented 1 year ago

Spyro can be quite sensitive, like when I tried skipping a FMV it hardcrashed. No other games seem to do that.

refractionpcsx2 commented 1 year ago

I just tried it, the patch is still required.

refractionpcsx2 commented 1 year ago

I tell a lie, the EU version is just broken, the US version does indeed work without a patch

Buzzardsoul commented 1 year ago

I tell a lie, the EU version is just broken, the US version does indeed work without a patch untitled

Eu version working perfectly here without the patch. Look at the v0 register

refractionpcsx2 commented 1 year ago

yeah but the release build is probably ignoring the TLB misses the EU version does, I'm using a development build, so it exits.

Buzzardsoul commented 1 year ago

Sorry for flooding this issue, I've tested it on both a debug build and a release build and it's working on both without the patch being present in the gameindex file and enable compatibility patches being unchecked with the game's serial being SLES_510.43

refractionpcsx2 commented 1 year ago

I mean, it does this on the EU version

image

but it only happens on the EU one, you can continue, if you ignore them :P

Buzzardsoul commented 1 year ago

Likely another issue that's not related to the black screen.

refractionpcsx2 commented 1 year ago

yeah it is unrelated

refractionpcsx2 commented 1 year ago

yeah it hits that TLB miss code a few times, I haven't traced back the bad value to where it comes from.

(I know you deleted your comment, but I'm still putting it here :P)

Goatman13 commented 1 year ago

Only PAL version TLB miss because problem is in CSpyroWorld::UpdateMenuLanguageSelect(float), NTSC skip that menu.

Right before that game try to read 0 sectors.

cdvdWrite04: NCMD CdReadDVDV (8) (ParamP = b)
DvdRead > startSector=536681, seekTo=535510 nSectors=0, RetryCnt=100, Speed=2x(CLV), ReadMode=3(0) SpindleCtrl=2
Bad Sector Count Error
refractionpcsx2 commented 1 year ago

it does, yep :P I'm not sure if it just does that, or if that's an error and it should be sending a sector count.

Buzzardsoul commented 1 year ago

@Goatman13 I have messed around with the SIF and the IOP quite extensively, I can only guess that the cutscene audio not being in sync is the fault of the SPU2. I mean, think about it, the game having an ADSR check is there for a reason. So, I'll conclude with the fact that I just can't wrap my head around where the issue lies in the SPU2's code but someone should probably check that part of the source code for inaccuracies.

Edit: and yes I'm aware that this is not relevant to the TLB miss and thank you for taking the time to check it out.

Goatman13 commented 1 year ago

@Goatman13 I have messed around with the SIF and the IOP quite extensively, I can only guess that the cutscene audio not being in sync is the fault of the SPU2. I mean, think about it, the game having an ADSR check is there for a reason. So, I'll conclude with the fact that I just can't wrap my head around where the issue lies in the SPU2's code but someone should probably check that part of the source code for inaccuracies.

Edit: and yes I'm aware that this is not relevant to the TLB miss and thank you for taking the time to check it out.

Game launch dialog/sound independently, at the same time EE is supposed to get flag that "cinematic is ready" by that ADSR 0x3fff stuff. Then EE side trigger rendering. You can test it on pcsx2 version without ADSR fix, if you disable patch sound play anyway with black screen. Game just expect EE to get there when it expect to be there. That should be better with +3 cyclerate, but is not. Maybe because its not tight loop to catch it up soon enough. Idk, from SPU2 side it looks correct now.

it does, yep :P I'm not sure if it just does that, or if that's an error and it should be sending a sector count.

Game request 0 sectors from cw_opt0.irx (custom streaming module, total surprise :P ). Seems to be intentional, sceCdRead return 0x6 , and module is happy that is not 0. That where error detection seems to end... Module have cdvd error detection itself expecting CDVD_STATUS_EMERGENCY to be set before going into any error check, setting that doesn't change anything for me. Module itself when in "emergency" perform check only against:

0x14: SCECdErCUD - Command is not valid for current disc 0x30: SCECdErREAD - Error while reading 0x31: SCECdErTRMOPN - Tray has been opened 0xFD: SCECdErREADCF - ?

So really no idea about that one.

Attached save state (slot3) is at point when previous read command is almost sent, next command is problematic one. Called from 0008E4B8 on IOP which = 0x00001888 of cw_opt0.irx, 00027368 is where sceCdRead is at iop dump. SLES-51043 (0EF1D4BA).03.zip

Buzzardsoul commented 1 year ago

Seems that this issue is no longer.

refractionpcsx2 commented 1 year ago

Good to know, the TLB miss is still a problem though

Goatman13 commented 1 year ago

PAL version is still have sound desync for me, even on press start screen. Tested on v1.7.4651 without using savestates.

refractionpcsx2 commented 8 months ago

Just to note the sound sync thing is still a thing, though the TLB misses are completely gone these days on all versions, the bad sector count error is still there but it seems to be non-fatal, it may be that it's suppose to unintentionally be there.

I tried messing around with EE counters/timers, cycle rates etc to try and work out what causes the video to get ahead of the audio, but this was to no avail. I did notice the game uses Timer Gates on Counter 0, but adjusting that doesn't seem to have an impact, from what I could tell (interestingly Colin McRae 2005 also uses this gate and has serious timing issues, but fiddling with it has no effect)

I keep wondering if there's some PAL thing we're missing and still doing NTSC-like. I know it gets affected by HBLANK timing and EE cycle rates etc, but Valkyrie Profile 2 also has the video blasting ahead of the audio (I also achieved it by doing chunky cycle delays on the IPU, but it felt like too much and the "okay" area was a very small amount of wiggle room, I don't believe the game is this sensitive to IPU sync). I'm not sure what it could be that we're not taking in to account, though, if that is the case.

Another case of this is Time Crisis 2, which does seem to be affected by EE cycle rate, again could be a red herring, but the audio gets ahead of the 3D rendering in this, rather than the other way around, then it starts making noises like the SPU overran, but the SPU DMA timings have changed a fair bit recently and it's actually no better or worse than it used to be, so again, another mystery.

Is there some other clock rate we're not dealing with, or something else reliant on vblank/hblank speeds but we're doing it based on CPU cycles some how? I really don't know..

Anyway, sorry for the ramblings, just needed to get this down somewhere.

Goatman13 commented 8 months ago

Not sure if i remember correctly, but I think COP0 performance counter affects sound related timings in Spyro. This can be wrong i don't have access to game now. I mean one of those games you mentioned for sure is affected by that, but i think it was Spyro.

refractionpcsx2 commented 8 months ago

I think I tried dividing those by two :D maybe I didn't check that game, I don't remember!

refractionpcsx2 commented 8 months ago

I just tried it, made no difference :(

Buzzardsoul commented 5 months ago

I thought maybe finding which pr fixed the desync for the US version could be useful for finding a fix for EU version, and it seems that #8657 is the one, because v1.7.4381's sound is perfectly in sync as opposed to v1.7.4380's.

StillPlay2 commented 5 days ago

Spyro - Enter the Dragonfly_SLES-51043_20240630162310.zip

Seems fixed & in-sync as of 1.7.5948

refractionpcsx2 commented 5 days ago

A GS dump isn't going to show audio sync.

Anyway, the SLES version is still massively wrong.

StillPlay2 commented 5 days ago

A GS dump isn't going to show audio sync.

Anyway, the SLES version is still massively wrong.

It's a video of the intro, not a GSDump. I couldn't upload it because of GitHub's 10MB limit so had to zip it