From c2d24f436cef9e6232c3d436f81dfdcef45d0e7f Mon Sep 17 00:00:00 2001 From: Stenzek Date: Sat, 27 Jul 2024 14:24:21 +1000 Subject: [PATCH] CDROM: Re-enable error-on-seeking-pause behaviour See the comments in the diff. This **will** cause lag in Final Fantasy VII during preload areas, but that has also been confirmed on console. Duke Nukem - Total Meltdown does this silly Read -> Pause command chain, except it sets its data/INT1 callback on the read, but never clears it after the pause. Therefore, if it doesn't receive at least one sector, the callback never gets cleared, and when the next read happens, it stores the "current" callback in the "backup" variable, which should be null, but now has the callback from the dud read. The result is any INT1 during XA playback running the dud callback, which says "hey, I'm not expecting any data, so pause, and stops the background music playback. Making sure at least one sector from that silly chain is delivered ensures the callback is cleared, and this does not happen. Since the pause first mentioned above will now error out until the first sector is delievered, the game spams pause until it eventually does succeed after the INT1. This behaviour has also been verified on hardware, thanks to rama for the xStation logs. --- src/core/cdrom.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/core/cdrom.cpp b/src/core/cdrom.cpp index 06cd873cb..4e287946d 100644 --- a/src/core/cdrom.cpp +++ b/src/core/cdrom.cpp @@ -1916,15 +1916,17 @@ void CDROM::ExecuteCommand(void*, TickCount ticks, TickCount ticks_late) ClearCommandSecondResponse(); SendACKAndStat(); - if (s_drive_state == DriveState::SeekingLogical || s_drive_state == DriveState::SeekingPhysical) + // This behaviour has been verified with hardware tests! The mech will reject pause commands if the game + // just started a read/seek, and it hasn't processed the first sector yet. This makes some games go bananas + // and spam pause commands until eventually it succeeds, but it is correct behaviour. + if (s_drive_state == DriveState::SeekingLogical || s_drive_state == DriveState::SeekingPhysical || + ((s_drive_state == DriveState::Reading || s_drive_state == DriveState::Playing) && + s_secondary_status.seeking)) { - // TODO: On console, this returns an error. But perhaps only during the coarse/fine seek part? Needs more - // hardware tests. - WARNING_LOG("CDROM Pause command while seeking from {} to {} - jumping to seek target", s_seek_start_lba, - s_seek_end_lba); - s_read_after_seek = false; - s_play_after_seek = false; - CompleteSeek(); + WARNING_LOG("CDROM Pause command while seeking - sending error response"); + SendErrorResponse(STAT_ERROR, ERROR_REASON_NOT_READY); + EndCommand(); + return; } else { @@ -2491,6 +2493,11 @@ void CDROM::BeginReading(TickCount ticks_late /* = 0 */, bool after_seek /* = fa ClearSectorBuffers(); ResetAudioDecoder(); + // Even though this isn't "officially" a seek, we still need to jump back to the target sector unless we're + // immediately following a seek from Play/Read. The seeking bit will get cleared after the first sector is processed. + if (!after_seek) + s_secondary_status.SetSeeking(); + s_drive_state = DriveState::Reading; s_drive_event.SetInterval(ticks); s_drive_event.Schedule(first_sector_ticks);