From a47ce0d62564608ee3c0aee37c076fc27e5510d1 Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 14 Sep 2017 11:54:05 -0700 Subject: [PATCH 1/2] FifoRecorder: remove use of volatile --- Source/Core/Core/FifoPlayer/FifoRecorder.cpp | 20 ++++++-------------- Source/Core/Core/FifoPlayer/FifoRecorder.h | 19 ++++++++++--------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp index 3473b4deb4..f4465e273f 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp @@ -6,7 +6,6 @@ #include #include -#include #include "Common/MsgHandler.h" #include "Common/Thread.h" @@ -16,24 +15,16 @@ #include "Core/HW/Memmap.h" static FifoRecorder instance; -static std::recursive_mutex sMutex; FifoRecorder::FifoRecorder() = default; -FifoRecorder::~FifoRecorder() -{ - m_IsRecording = false; -} - void FifoRecorder::StartRecording(s32 numFrames, CallbackFunc finishedCb) { - std::lock_guard lk(sMutex); - - delete m_File; + std::lock_guard lk(m_mutex); FifoAnalyzer::Init(); - m_File = new FifoDataFile; + m_File = std::make_unique(); // TODO: This, ideally, would be deallocated when done recording. // However, care needs to be taken since global state @@ -68,6 +59,7 @@ void FifoRecorder::StartRecording(s32 numFrames, CallbackFunc finishedCb) void FifoRecorder::StopRecording() { + std::lock_guard lk(m_mutex); m_RequestedRecordingEnd = true; } @@ -95,7 +87,7 @@ void FifoRecorder::WriteGPCommand(const u8* data, u32 size) m_CurrentFrame.fifoData = m_FifoData; { - std::lock_guard lk(sMutex); + std::lock_guard lk(m_mutex); // Copy frame to file // The file will be responsible for freeing the memory allocated for each frame's fifoData @@ -153,7 +145,7 @@ void FifoRecorder::UseMemory(u32 address, u32 size, MemoryUpdate::Type type, boo void FifoRecorder::EndFrame(u32 fifoStart, u32 fifoEnd) { // m_IsRecording is assumed to be true at this point, otherwise this function would not be called - std::lock_guard lk(sMutex); + std::lock_guard lk(m_mutex); m_FrameEnded = true; @@ -196,7 +188,7 @@ void FifoRecorder::EndFrame(u32 fifoStart, u32 fifoEnd) void FifoRecorder::SetVideoMemory(const u32* bpMem, const u32* cpMem, const u32* xfMem, const u32* xfRegs, u32 xfRegsSize, const u8* texMem) { - std::lock_guard lk(sMutex); + std::lock_guard lk(m_mutex); if (m_File) { diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.h b/Source/Core/Core/FifoPlayer/FifoRecorder.h index ba0ab38fa6..3f819f1b65 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.h +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.h @@ -4,6 +4,8 @@ #pragma once +#include +#include #include #include "Core/FifoPlayer/FifoDataFile.h" @@ -14,12 +16,11 @@ public: typedef void (*CallbackFunc)(void); FifoRecorder(); - ~FifoRecorder(); void StartRecording(s32 numFrames, CallbackFunc finishedCb); void StopRecording(); - FifoDataFile* GetRecordedFile() const { return m_File; } + FifoDataFile* GetRecordedFile() const { return m_File.get(); } // Called from video thread // Must write one full GP command at a time @@ -46,15 +47,15 @@ public: private: // Accessed from both GUI and video threads + std::recursive_mutex m_mutex; // True if video thread should send data - volatile bool m_IsRecording = false; + bool m_IsRecording = false; // True if m_IsRecording was true during last frame - volatile bool m_WasRecording = false; - volatile bool m_RequestedRecordingEnd = false; - volatile s32 m_RecordFramesRemaining = 0; - volatile CallbackFunc m_FinishedCb = nullptr; - - FifoDataFile* volatile m_File = nullptr; + bool m_WasRecording = false; + bool m_RequestedRecordingEnd = false; + s32 m_RecordFramesRemaining = 0; + CallbackFunc m_FinishedCb = nullptr; + std::unique_ptr m_File; // Accessed only from video thread From 738acb6c076af584ee53fd1ccce464aa86794820 Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 14 Sep 2017 11:56:09 -0700 Subject: [PATCH 2/2] FifoRecorder: move function definitions out of header --- Source/Core/Core/FifoPlayer/FifoRecorder.cpp | 10 ++++++++++ Source/Core/Core/FifoPlayer/FifoRecorder.h | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp index f4465e273f..274b4c1cd0 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.cpp +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.cpp @@ -63,6 +63,11 @@ void FifoRecorder::StopRecording() m_RequestedRecordingEnd = true; } +FifoDataFile* FifoRecorder::GetRecordedFile() const +{ + return m_File.get(); +} + void FifoRecorder::WriteGPCommand(const u8* data, u32 size) { if (!m_SkipNextData) @@ -205,6 +210,11 @@ void FifoRecorder::SetVideoMemory(const u32* bpMem, const u32* cpMem, const u32* FifoRecordAnalyzer::Initialize(cpMem); } +bool FifoRecorder::IsRecording() const +{ + return m_IsRecording; +} + FifoRecorder& FifoRecorder::GetInstance() { return instance; diff --git a/Source/Core/Core/FifoPlayer/FifoRecorder.h b/Source/Core/Core/FifoPlayer/FifoRecorder.h index 3f819f1b65..7389d3014d 100644 --- a/Source/Core/Core/FifoPlayer/FifoRecorder.h +++ b/Source/Core/Core/FifoPlayer/FifoRecorder.h @@ -20,7 +20,7 @@ public: void StartRecording(s32 numFrames, CallbackFunc finishedCb); void StopRecording(); - FifoDataFile* GetRecordedFile() const { return m_File.get(); } + FifoDataFile* GetRecordedFile() const; // Called from video thread // Must write one full GP command at a time @@ -41,7 +41,7 @@ public: u32 xfRegsSize, const u8* texMem); // Checked once per frame prior to callng EndFrame() - bool IsRecording() const { return m_IsRecording; } + bool IsRecording() const; static FifoRecorder& GetInstance(); private: