From 71210f3fa23ca5a761dcb5994fdeb0bb38d4ce5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Sep 2026 14:52:51 -0600 Subject: [PATCH] GE recorder: Don't let a second request hijack a recording RecordNextFrame now refuses while a recording is active and during frame dump playback (which asserted on the next replay). The callback handoff to the CPU thread is locked, and a failed file open no longer crashes. Co-Authored-By: Claude Opus 5.5 (1M context) --- GPU/Debugger/Record.cpp | 44 +++++++++++++++++++++++++++++------------ GPU/Debugger/Record.h | 5 ++++- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/GPU/Debugger/Record.cpp b/GPU/Debugger/Record.cpp index 38330805c3..5a542e22cf 100644 --- a/GPU/Debugger/Record.cpp +++ b/GPU/Debugger/Record.cpp @@ -126,13 +126,16 @@ void Recorder::DirtyDrawnVRAM() { } bool Recorder::BeginRecording() { + std::unique_lock guard(callbackLock_); + nextFrame = false; if (PSP_CoreParameter().fileType == IdentifiedFileType::PPSSPP_GE_DUMP) { - // Can't record a GE dump. + // Can't record a GE dump. RecordNextFrame refuses this too. + writeCallback = nullptr; return false; } active = true; - nextFrame = false; + guard.unlock(); lastTextures.clear(); lastRenderTargets.clear(); flipLastAction = gpuStats.totals.numFlips; @@ -180,6 +183,10 @@ Path Recorder::WriteRecording() { NOTICE_LOG(Log::G3D, "Recording filename: %s", filename.c_str()); FILE *fp = File::OpenCFile(filename, "wb"); + if (!fp) { + ERROR_LOG(Log::G3D, "Failed to open '%s' for writing the recording", filename.c_str()); + return Path(); + } Header header{}; memcpy(header.magic, HEADER_MAGIC, sizeof(header.magic)); header.version = VERSION; @@ -574,14 +581,19 @@ void Recorder::EmitBezierSpline(u32 op) { } bool Recorder::RecordNextFrame(const std::function callback) { - if (!nextFrame) { - flipLastAction = gpuStats.totals.numFlips; - flipFinishAt = -1; - writeCallback = callback; - nextFrame = true; - return true; + if (PSP_CoreParameter().fileType == IdentifiedFileType::PPSSPP_GE_DUMP) { + return false; } - return false; + std::lock_guard guard(callbackLock_); + // Don't take over a recording in progress, it would get the wrong callback and end point. + if (nextFrame || active) { + return false; + } + flipLastAction = gpuStats.totals.numFlips; + flipFinishAt = -1; + writeCallback = callback; + nextFrame = true; + return true; } void Recorder::FinishRecording() { @@ -596,15 +608,21 @@ void Recorder::FinishRecording() { lastVRAM.clear(); NOTICE_LOG(Log::System, "Recording finished"); - active = false; flipLastAction = gpuStats.totals.numFlips; flipFinishAt = -1; lastEdramTrans = 0x400; - if (writeCallback) { - writeCallback(filename); + std::function callback; + { + std::lock_guard guard(callbackLock_); + callback = std::move(writeCallback); + writeCallback = nullptr; + active = false; + } + + if (callback && !filename.empty()) { + callback(filename); } - writeCallback = nullptr; } void Recorder::CheckEdramTrans() { diff --git a/GPU/Debugger/Record.h b/GPU/Debugger/Record.h index 514ca1c04c..3c291057af 100644 --- a/GPU/Debugger/Record.h +++ b/GPU/Debugger/Record.h @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -50,7 +51,7 @@ public: } bool RecordNextFrame(const std::function callback); void ClearCallback() { - // Not super thread safe.. + std::lock_guard guard(callbackLock_); writeCallback = nullptr; } @@ -95,6 +96,8 @@ private: int flipFinishAt = -1; uint32_t lastEdramTrans = 0x400; std::function writeCallback; + // RecordNextFrame is called from other threads. Guards writeCallback, nextFrame and the writes to active. + std::mutex callbackLock_; std::vector pushbuf; std::vector commands;