From 2c5a23b696ae5fa78799ddc8ae457c7601e88e54 Mon Sep 17 00:00:00 2001 From: Filjo Abraham Date: Sun, 5 Jul 2020 10:13:29 -0500 Subject: [PATCH] CDVD: ensure thread/state is ok before proceeding --- pcsx2/CDVD/CDVDaccess.cpp | 21 +++++----- pcsx2/CDVD/CDVDaccess.h | 16 ++------ pcsx2/CDVD/CDVDdiscReader.cpp | 70 ++++++++++++++-------------------- pcsx2/CDVD/CDVDdiscThread.cpp | 3 +- pcsx2/CDVD/CDVDisoReader.cpp | 5 +-- pcsx2/System/SysCoreThread.cpp | 9 ++++- pcsx2/gui/AppConfig.cpp | 1 - pcsx2/gui/AppCoreThread.cpp | 2 - pcsx2/gui/MainMenuClicks.cpp | 4 -- 9 files changed, 54 insertions(+), 77 deletions(-) diff --git a/pcsx2/CDVD/CDVDaccess.cpp b/pcsx2/CDVD/CDVDaccess.cpp index c4ea6066c3..281e5acfde 100644 --- a/pcsx2/CDVD/CDVDaccess.cpp +++ b/pcsx2/CDVD/CDVDaccess.cpp @@ -251,7 +251,7 @@ static void DetectDiskType() int baseMediaType = CDVD->getDiskType(); int mType = -1; - + // Paranoid mode: do not trust the plugin's detection system to work correctly. // (.. and there's no reason plugins should be doing their own detection anyway). @@ -324,6 +324,9 @@ CDVD_SourceType CDVDsys_GetSourceType() void CDVDsys_ChangeSource( CDVD_SourceType type ) { + if(CDVD != NULL) + DoCDVDclose(); + switch( m_CurrentSourceType = type ) { case CDVD_SourceType::Iso: @@ -466,8 +469,9 @@ s32 DoCDVDreadTrack(u32 lsn, int mode) { CheckNullCDVD(); - //TODO_CDVD I believe ISO and Disc use the new CDVDgetBuffer style // TEMP: until all the plugins use the new CDVDgetBuffer style + // TODO: The CDVD api only uses the new getBuffer style. Why is this temp? + // lastReadSize is needed for block dumps switch (mode) { case CDVD_MODE_2352: @@ -492,7 +496,7 @@ s32 DoCDVDreadTrack(u32 lsn, int mode) s32 DoCDVDgetBuffer(u8* buffer) { CheckNullCDVD(); - int ret = CDVD->getBuffer2(buffer); + const int ret = CDVD->getBuffer(buffer); if (ret == 0 && blockDumpFile.IsOpened()) { @@ -549,10 +553,9 @@ s32 CALLBACK NODISCreadTrack(u32 lsn, int mode) return -1; } -// return can be NULL (for async modes) -u8* CALLBACK NODISCgetBuffer() +s32 CALLBACK NODISCgetBuffer(u8* buffer) { - return NULL; + return -1; } s32 CALLBACK NODISCreadSubQ(u32 lsn, cdvdSubQ* subq) @@ -599,11 +602,6 @@ s32 CALLBACK NODISCreadSector(u8* tempbuffer, u32 lsn, int mode) return -1; } -s32 CALLBACK NODISCgetBuffer2(u8* buffer) -{ - return -1; -} - s32 CALLBACK NODISCgetDualInfo(s32* dualType, u32* _layer1start) { return -1; @@ -627,6 +625,5 @@ CDVD_API CDVDapi_NoDisc = NODISCnewDiskCB, NODISCreadSector, - NODISCgetBuffer2, NODISCgetDualInfo, }; diff --git a/pcsx2/CDVD/CDVDaccess.h b/pcsx2/CDVD/CDVDaccess.h index 739cba7a3c..79c1015c56 100644 --- a/pcsx2/CDVD/CDVDaccess.h +++ b/pcsx2/CDVD/CDVDaccess.h @@ -88,15 +88,11 @@ typedef s32(CALLBACK* _CDVDopen)(const char* pTitleFilename); // Returns 0 on success. typedef s32(CALLBACK* _CDVDreadTrack)(u32 lsn, int mode); -// *OBSOLETE* returns a pointer to the buffer, or NULL if data hasn't finished -// loading yet. -typedef u8* (CALLBACK* _CDVDgetBuffer)(); - // Copies loaded data to the target buffer. // Returns -2 if the asynchronous read is still pending. // Returns -1 if the asyncronous read failed. // Returns 0 on success. -typedef s32(CALLBACK* _CDVDgetBuffer2)(u8* buffer); +typedef s32(CALLBACK* _CDVDgetBuffer)(u8* buffer); typedef s32(CALLBACK* _CDVDreadSubQ)(u32 lsn, cdvdSubQ* subq); typedef s32(CALLBACK* _CDVDgetTN)(cdvdTN* Buffer); @@ -143,19 +139,15 @@ struct CDVD_API // special functions, not in external interface yet _CDVDreadSector readSector; - _CDVDgetBuffer2 getBuffer2; _CDVDgetDualInfo getDualInfo; }; // ---------------------------------------------------------------------------- -// Multiple interface system for CDVD, used to provide internal CDVDiso and NoDisc, -// and external plugin interfaces. Do* functions are meant as replacements for -// direct CDVD plugin invocation, and add universal block dumping features. +// Multiple interface system for CDVD. Do* functions are meant as replacements +// for direct CDVD plugin invocation, and add universal block dumping features. // ---------------------------------------------------------------------------- -//TODO_CDVD update comment ^ - -extern CDVD_API* CDVD; // currently active CDVD access mode api (either Iso, NoDisc, or Plugin) +extern CDVD_API* CDVD; // currently active CDVD access mode api (either Iso, NoDisc, or Disc) extern CDVD_API CDVDapi_Iso; extern CDVD_API CDVDapi_Disc; diff --git a/pcsx2/CDVD/CDVDdiscReader.cpp b/pcsx2/CDVD/CDVDdiscReader.cpp index 42c081fffb..fdbc20609a 100644 --- a/pcsx2/CDVD/CDVDdiscReader.cpp +++ b/pcsx2/CDVD/CDVDdiscReader.cpp @@ -221,15 +221,38 @@ s32 CALLBACK DISCreadTrack(u32 lsn, int mode) return 0; } -// return can be NULL (for async modes) -u8* CALLBACK DISCgetBuffer() +s32 CALLBACK DISCgetBuffer(u8* dest) { - if (lastReadInNewDiskCB) { - lastReadInNewDiskCB = 0; - return directReadSectorBuffer; + // Do nothing for out of bounds disc sector reads. It prevents some games + // from hanging (All-Star Baseball 2005, Hello Kitty: Roller Rescue, + // Hot Wheels: Beat That! (NTSC), Ratchet & Clank 3 (PAL), + // Test Drive: Eve of Destruction, etc.). + if (csector >= src->GetSectorCount()) + return 0; + + int csize = 2352; + switch (cmode) { + case CDVD_MODE_2048: + csize = 2048; + break; + case CDVD_MODE_2328: + csize = 2328; + break; + case CDVD_MODE_2340: + csize = 2340; + break; } - return cdvdGetSector(csector, cmode); + if (lastReadInNewDiskCB) { + lastReadInNewDiskCB = 0; + + memcpy(dest, directReadSectorBuffer, csize); + return 0; + } + + memcpy(dest, cdvdGetSector(csector, cmode), csize); + + return 0; } s32 CALLBACK DISCreadSubQ(u32 lsn, cdvdSubQ* subq) @@ -443,40 +466,6 @@ s32 CALLBACK DISCreadSector(u8* buffer, u32 lsn, int mode) return cdvdDirectReadSector(lsn, mode, buffer); } -s32 CALLBACK DISCgetBuffer2(u8* dest) -{ - // Do nothing for out of bounds disc sector reads. It prevents some games - // from hanging (All-Star Baseball 2005, Hello Kitty: Roller Rescue, - // Hot Wheels: Beat That! (NTSC), Ratchet & Clank 3 (PAL), - // Test Drive: Eve of Destruction, etc.). - if (csector >= src->GetSectorCount()) - return 0; - - int csize = 2352; - switch (cmode) { - case CDVD_MODE_2048: - csize = 2048; - break; - case CDVD_MODE_2328: - csize = 2328; - break; - case CDVD_MODE_2340: - csize = 2340; - break; - } - - if (lastReadInNewDiskCB) { - lastReadInNewDiskCB = 0; - - memcpy(dest, directReadSectorBuffer, csize); - return 0; - } - - memcpy(dest, cdvdGetSector(csector, cmode), csize); - - return 0; -} - s32 CALLBACK DISCgetDualInfo(s32* dualType, u32* _layer1start) { switch (src->GetMediaType()) { @@ -514,6 +503,5 @@ CDVD_API CDVDapi_Disc = DISCnewDiskCB, DISCreadSector, - DISCgetBuffer2, DISCgetDualInfo, }; \ No newline at end of file diff --git a/pcsx2/CDVD/CDVDdiscThread.cpp b/pcsx2/CDVD/CDVDdiscThread.cpp index ecf1c66ab0..96b8de9d65 100644 --- a/pcsx2/CDVD/CDVDdiscThread.cpp +++ b/pcsx2/CDVD/CDVDdiscThread.cpp @@ -261,7 +261,8 @@ void cdvdStopThread() { cdvd_is_open = false; s_notify_cv.notify_one(); - s_thread.join(); + if(s_thread.joinable()) + s_thread.join(); } void cdvdRequestSector(u32 sector, s32 mode) diff --git a/pcsx2/CDVD/CDVDisoReader.cpp b/pcsx2/CDVD/CDVDisoReader.cpp index 59cbb1206a..3cba193658 100644 --- a/pcsx2/CDVD/CDVDisoReader.cpp +++ b/pcsx2/CDVD/CDVDisoReader.cpp @@ -370,7 +370,7 @@ s32 CALLBACK ISOreadTrack(u32 lsn, int mode) return 0; } -s32 CALLBACK ISOgetBuffer2(u8* buffer) +s32 CALLBACK ISOgetBuffer(u8* buffer) { return iso.FinishRead3(buffer, pmode); } @@ -410,7 +410,7 @@ CDVD_API CDVDapi_Iso = ISOopen, ISOreadTrack, - NULL, //ISOgetBuffer, // emu shouldn't use this one. + ISOgetBuffer, ISOreadSubQ, ISOgetTN, ISOgetTD, @@ -423,6 +423,5 @@ CDVD_API CDVDapi_Iso = ISOnewDiskCB, ISOreadSector, - ISOgetBuffer2, ISOgetDualInfo, }; diff --git a/pcsx2/System/SysCoreThread.cpp b/pcsx2/System/SysCoreThread.cpp index 8b36beac60..64185ae27c 100644 --- a/pcsx2/System/SysCoreThread.cpp +++ b/pcsx2/System/SysCoreThread.cpp @@ -285,7 +285,13 @@ void SysCoreThread::OnSuspendInThread() void SysCoreThread::OnResumeInThread( bool isSuspended ) { GetCorePlugins().Open(); - DoCDVDopen(); + + // When applying settings thread isn't suspended + // so any components that are still loaded don't need to be opened again + if (isSuspended) + { + DoCDVDopen(); + } } @@ -300,6 +306,7 @@ void SysCoreThread::OnCleanupInThread() R3000A::ioman::reset(); // FIXME: temporary workaround for deadlock on exit, which actually should be a crash vu1Thread.WaitVU(); + DoCDVDclose(); GetCorePlugins().Close(); GetCorePlugins().Shutdown(); diff --git a/pcsx2/gui/AppConfig.cpp b/pcsx2/gui/AppConfig.cpp index 2076e0dc70..be6bfcf594 100644 --- a/pcsx2/gui/AppConfig.cpp +++ b/pcsx2/gui/AppConfig.cpp @@ -1307,7 +1307,6 @@ static void SaveUiSettings() } sApp.GetRecentIsoManager().Add( g_Conf->CurrentIso ); - //TODO_CDVD Should this be done for CurrentDisc? AppIniSaver saver; g_Conf->LoadSave( saver ); diff --git a/pcsx2/gui/AppCoreThread.cpp b/pcsx2/gui/AppCoreThread.cpp index 45faeb86b7..0c2a292c4c 100644 --- a/pcsx2/gui/AppCoreThread.cpp +++ b/pcsx2/gui/AppCoreThread.cpp @@ -178,7 +178,6 @@ void AppCoreThread::ChangeCdvdSource() // Fast change of the CDVD source only -- a Pause will suffice. ScopedCoreThreadPause paused_core; - //TODO_CDVD close CDVDdiscReader here? CDVDsys_ChangeSource( cdvdsrc ); paused_core.AllowResume(); @@ -541,7 +540,6 @@ void AppCoreThread::OnResumeInThread( bool isSuspended ) { if( m_resetCdvd ) { - //TODO_CDVD close CDVDdiscReader here? CDVDsys_ChangeSource( g_Conf->CdvdSource ); cdvdCtrlTrayOpen(); m_resetCdvd = false; diff --git a/pcsx2/gui/MainMenuClicks.cpp b/pcsx2/gui/MainMenuClicks.cpp index 88bc364fe6..cc8829bce6 100644 --- a/pcsx2/gui/MainMenuClicks.cpp +++ b/pcsx2/gui/MainMenuClicks.cpp @@ -174,8 +174,6 @@ wxWindowID SwapOrReset_Iso( wxWindow* owner, IScopedCoreThread& core_control, co core_control.AllowResume(); } - //TODO_CDVD enable/disable disc selector menu item - return result; } @@ -272,8 +270,6 @@ wxWindowID SwapOrReset_CdvdSrc( wxWindow* owner, CDVD_SourceType newsrc ) sApp.SysExecute( g_Conf->CdvdSource ); } - //TODO_CDVD enable/disable disc selector menu item - return result; }