Merge pull request #3965 from lioncash/fifo

FifoPlayer: Cull some explicit delete and new instances

Source/Core/Core/FifoPlayer/FifoAnalyzer.cpp

@@ -27,30 +27,30 @@ void Init()
   VertexLoader_Normal::Init();
 }
 
-u8 ReadFifo8(u8*& data)
+u8 ReadFifo8(const u8*& data)
 {
   u8 value = data[0];
   data += 1;
   return value;
 }
 
-u16 ReadFifo16(u8*& data)
+u16 ReadFifo16(const u8*& data)
 {
   u16 value = Common::swap16(data);
   data += 2;
   return value;
 }
 
-u32 ReadFifo32(u8*& data)
+u32 ReadFifo32(const u8*& data)
 {
   u32 value = Common::swap32(data);
   data += 4;
   return value;
 }
 
-u32 AnalyzeCommand(u8* data, DecodeMode mode)
+u32 AnalyzeCommand(const u8* data, DecodeMode mode)
 {
-  u8* dataStart = data;
+  const u8* dataStart = data;
 
   int cmd = ReadFifo8(data);
 

Source/Core/Core/FifoPlayer/FifoAnalyzer.h

@@ -13,9 +13,9 @@ namespace FifoAnalyzer
 {
 void Init();
 
-u8 ReadFifo8(u8*& data);
+u8 ReadFifo8(const u8*& data);
-u16 ReadFifo16(u8*& data);
+u16 ReadFifo16(const u8*& data);
-u32 ReadFifo32(u8*& data);
+u32 ReadFifo32(const u8*& data);
 
 enum DecodeMode
 {
@@ -23,7 +23,7 @@ enum DecodeMode
   DECODE_PLAYBACK,
 };
 
-u32 AnalyzeCommand(u8* data, DecodeMode mode);
+u32 AnalyzeCommand(const u8* data, DecodeMode mode);
 
 struct CPMemory
 {

Source/Core/Core/FifoPlayer/FifoDataFile.cpp

@@ -18,13 +18,6 @@ FifoDataFile::FifoDataFile() : m_Flags(0)
 
 FifoDataFile::~FifoDataFile()
 {
-  for (auto& frame : m_Frames)
-  {
-    for (auto& update : frame.memoryUpdates)
-      delete[] update.data;
-
-    delete[] frame.fifoData;
-  }
 }
 
 bool FifoDataFile::HasBrokenEFBCopies() const
@@ -106,17 +99,17 @@ bool FifoDataFile::Save(const std::string& filename)
     // Write FIFO data
     file.Seek(0, SEEK_END);
     u64 dataOffset = file.Tell();
-    file.WriteBytes(srcFrame.fifoData, srcFrame.fifoDataSize);
+    file.WriteBytes(srcFrame.fifoData.data(), srcFrame.fifoData.size());
 
     u64 memoryUpdatesOffset = WriteMemoryUpdates(srcFrame.memoryUpdates, file);
 
     FileFrameInfo dstFrame;
-    dstFrame.fifoDataSize = srcFrame.fifoDataSize;
+    dstFrame.fifoDataSize = static_cast<u32>(srcFrame.fifoData.size());
     dstFrame.fifoDataOffset = dataOffset;
     dstFrame.fifoStart = srcFrame.fifoStart;
     dstFrame.fifoEnd = srcFrame.fifoEnd;
     dstFrame.memoryUpdatesOffset = memoryUpdatesOffset;
-    dstFrame.numMemoryUpdates = (u32)srcFrame.memoryUpdates.size();
+    dstFrame.numMemoryUpdates = static_cast<u32>(srcFrame.memoryUpdates.size());
 
     // Write frame info
     u64 frameOffset = frameListOffset + (i * sizeof(FileFrameInfo));
@@ -130,7 +123,7 @@ bool FifoDataFile::Save(const std::string& filename)
   return true;
 }
 
-FifoDataFile* FifoDataFile::Load(const std::string& filename, bool flagsOnly)
+std::unique_ptr<FifoDataFile> FifoDataFile::Load(const std::string& filename, bool flagsOnly)
 {
   File::IOFile file;
   file.Open(filename, "rb");
@@ -146,7 +139,7 @@ FifoDataFile* FifoDataFile::Load(const std::string& filename, bool flagsOnly)
     return nullptr;
   }
 
-  FifoDataFile* dataFile = new FifoDataFile;
+  auto dataFile = std::make_unique<FifoDataFile>();
 
   dataFile->m_Flags = header.flags;
   dataFile->m_Version = header.file_version;
@@ -182,13 +175,12 @@ FifoDataFile* FifoDataFile::Load(const std::string& filename, bool flagsOnly)
     file.ReadBytes(&srcFrame, sizeof(FileFrameInfo));
 
     FifoFrameInfo dstFrame;
-    dstFrame.fifoData = new u8[srcFrame.fifoDataSize];
+    dstFrame.fifoData.resize(srcFrame.fifoDataSize);
-    dstFrame.fifoDataSize = srcFrame.fifoDataSize;
     dstFrame.fifoStart = srcFrame.fifoStart;
     dstFrame.fifoEnd = srcFrame.fifoEnd;
 
     file.Seek(srcFrame.fifoDataOffset, SEEK_SET);
-    file.ReadBytes(dstFrame.fifoData, srcFrame.fifoDataSize);
+    file.ReadBytes(dstFrame.fifoData.data(), srcFrame.fifoDataSize);
 
     ReadMemoryUpdates(srcFrame.memoryUpdatesOffset, srcFrame.numMemoryUpdates,
                       dstFrame.memoryUpdates, file);
@@ -234,12 +226,12 @@ u64 FifoDataFile::WriteMemoryUpdates(const std::vector<MemoryUpdate>& memUpdates
     // Write memory
     file.Seek(0, SEEK_END);
     u64 dataOffset = file.Tell();
-    file.WriteBytes(srcUpdate.data, srcUpdate.size);
+    file.WriteBytes(srcUpdate.data.data(), srcUpdate.data.size());
 
     FileMemoryUpdate dstUpdate;
     dstUpdate.address = srcUpdate.address;
     dstUpdate.dataOffset = dataOffset;
-    dstUpdate.dataSize = srcUpdate.size;
+    dstUpdate.dataSize = static_cast<u32>(srcUpdate.data.size());
     dstUpdate.fifoPosition = srcUpdate.fifoPosition;
     dstUpdate.type = srcUpdate.type;
 
@@ -266,11 +258,10 @@ void FifoDataFile::ReadMemoryUpdates(u64 fileOffset, u32 numUpdates,
     MemoryUpdate& dstUpdate = memUpdates[i];
     dstUpdate.address = srcUpdate.address;
     dstUpdate.fifoPosition = srcUpdate.fifoPosition;
-    dstUpdate.size = srcUpdate.dataSize;
+    dstUpdate.data.resize(srcUpdate.dataSize);
-    dstUpdate.data = new u8[srcUpdate.dataSize];
+    dstUpdate.type = static_cast<MemoryUpdate::Type>(srcUpdate.type);
-    dstUpdate.type = (MemoryUpdate::Type)srcUpdate.type;
 
     file.Seek(srcUpdate.dataOffset, SEEK_SET);
-    file.ReadBytes(dstUpdate.data, srcUpdate.dataSize);
+    file.ReadBytes(dstUpdate.data.data(), srcUpdate.dataSize);
   }
 }

Source/Core/Core/FifoPlayer/FifoDataFile.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -26,15 +27,13 @@ struct MemoryUpdate
 
   u32 fifoPosition;
   u32 address;
-  u32 size;
+  std::vector<u8> data;
-  u8* data;
   Type type;
 };
 
 struct FifoFrameInfo
 {
-  u8* fifoData;
+  std::vector<u8> fifoData;
-  u32 fifoDataSize;
 
   u32 fifoStart;
   u32 fifoEnd;
@@ -70,7 +69,7 @@ public:
   u32 GetFrameCount() const { return static_cast<u32>(m_Frames.size()); }
   bool Save(const std::string& filename);
 
-  static FifoDataFile* Load(const std::string& filename, bool flagsOnly);
+  static std::unique_ptr<FifoDataFile> Load(const std::string& filename, bool flagsOnly);
 
 private:
   enum

Source/Core/Core/FifoPlayer/FifoPlaybackAnalyzer.cpp

@@ -19,7 +19,7 @@ struct CmdData
 {
   u32 size;
   u32 offset;
-  u8* ptr;
+  const u8* ptr;
 };
 
 void FifoPlaybackAnalyzer::AnalyzeFrames(FifoDataFile* file,
@@ -51,10 +51,10 @@ void FifoPlaybackAnalyzer::AnalyzeFrames(FifoDataFile* file,
 
 #if LOG_FIFO_CMDS
     // Debugging
-    vector<CmdData> prevCmds;
+    std::vector<CmdData> prevCmds;
 #endif
 
-    while (cmdStart < frame.fifoDataSize)
+    while (cmdStart < frame.fifoData.size())
     {
       // Add memory updates that have occurred before this point in the frame
       while (nextMemUpdate < frame.memoryUpdates.size() &&

Source/Core/Core/FifoPlayer/FifoPlayer.cpp

@@ -28,7 +28,6 @@ bool IsPlayingBackFifologWithBrokenEFBCopies = false;
 
 FifoPlayer::~FifoPlayer()
 {
-  delete m_File;
 }
 
 bool FifoPlayer::Open(const std::string& filename)
@@ -40,7 +39,7 @@ bool FifoPlayer::Open(const std::string& filename)
   if (m_File)
   {
     FifoAnalyzer::Init();
-    FifoPlaybackAnalyzer::AnalyzeFrames(m_File, m_FrameInfo);
+    FifoPlaybackAnalyzer::AnalyzeFrames(m_File.get(), m_FrameInfo);
 
     m_FrameRangeEnd = m_File->GetFrameCount();
   }
@@ -53,8 +52,7 @@ bool FifoPlayer::Open(const std::string& filename)
 
 void FifoPlayer::Close()
 {
-  delete m_File;
+  m_File.reset();
-  m_File = nullptr;
 
   m_FrameRangeStart = 0;
   m_FrameRangeEnd = 0;
@@ -209,7 +207,7 @@ void FifoPlayer::WriteFrame(const FifoFrameInfo& frame, const AnalyzedFrameInfo&
   // Core timing information
   m_CyclesPerFrame = SystemTimers::GetTicksPerSecond() / VideoInterface::GetTargetRefreshRate();
   m_ElapsedCycles = 0;
-  m_FrameFifoSize = frame.fifoDataSize;
+  m_FrameFifoSize = static_cast<u32>(frame.fifoData.size());
 
   // Determine start and end objects
   u32 numObjects = (u32)(info.objectStarts.size());
@@ -258,7 +256,7 @@ void FifoPlayer::WriteFrame(const FifoFrameInfo& frame, const AnalyzedFrameInfo&
   }
 
   // Write data after the last object
-  WriteFramePart(position, frame.fifoDataSize, memoryUpdate, frame, info);
+  WriteFramePart(position, static_cast<u32>(frame.fifoData.size()), memoryUpdate, frame, info);
 
   FlushWGP();
 
@@ -273,7 +271,7 @@ void FifoPlayer::WriteFrame(const FifoFrameInfo& frame, const AnalyzedFrameInfo&
 void FifoPlayer::WriteFramePart(u32 dataStart, u32 dataEnd, u32& nextMemUpdate,
                                 const FifoFrameInfo& frame, const AnalyzedFrameInfo& info)
 {
-  u8* data = frame.fifoData;
+  const u8* const data = frame.fifoData.data();
 
   while (nextMemUpdate < frame.memoryUpdates.size() && dataStart < dataEnd)
   {
@@ -325,10 +323,10 @@ void FifoPlayer::WriteMemory(const MemoryUpdate& memUpdate)
   else
     mem = &Memory::m_pRAM[memUpdate.address & Memory::RAM_MASK];
 
-  memcpy(mem, memUpdate.data, memUpdate.size);
+  std::copy(memUpdate.data.begin(), memUpdate.data.end(), mem);
 }
 
-void FifoPlayer::WriteFifo(u8* data, u32 start, u32 end)
+void FifoPlayer::WriteFifo(const u8* data, u32 start, u32 end)
 {
   u32 written = start;
   u32 lastBurstEnd = end - 1;

Source/Core/Core/FifoPlayer/FifoPlayer.h

@@ -64,7 +64,7 @@ public:
   // PowerPC state.
   std::unique_ptr<CPUCoreBase> GetCPUCore();
 
-  FifoDataFile* GetFile() { return m_File; }
+  FifoDataFile* GetFile() { return m_File.get(); }
   u32 GetFrameObjectCount();
   u32 GetCurrentFrameNum() const { return m_CurrentFrame; }
   const AnalyzedFrameInfo& GetAnalyzedFrameInfo(u32 frame) const { return m_FrameInfo[frame]; }
@@ -104,7 +104,7 @@ private:
 
   // writes a range of data to the fifo
   // start and end must be relative to frame's fifo data so elapsed cycles are figured correctly
-  void WriteFifo(u8* data, u32 start, u32 end);
+  void WriteFifo(const u8* data, u32 start, u32 end);
 
   void SetupFifo();
 
@@ -143,7 +143,7 @@ private:
   CallbackFunc m_FileLoadedCb;
   CallbackFunc m_FrameWrittenCb;
 
-  FifoDataFile* m_File;
+  std::unique_ptr<FifoDataFile> m_File;
 
   std::vector<AnalyzedFrameInfo> m_FrameInfo;
 };

Source/Core/Core/FifoPlayer/FifoRecordAnalyzer.cpp

@@ -38,7 +38,7 @@ void FifoRecordAnalyzer::ProcessLoadIndexedXf(u32 val, int array)
   FifoRecorder::GetInstance().UseMemory(address, size * 4, MemoryUpdate::XF_DATA);
 }
 
-void FifoRecordAnalyzer::WriteVertexArray(int arrayIndex, u8* vertexData, int vertexSize,
+void FifoRecordAnalyzer::WriteVertexArray(int arrayIndex, const u8* vertexData, int vertexSize,
                                           int numVertices)
 {
   // Skip if not indexed array

Source/Core/Core/FifoPlayer/FifoRecordAnalyzer.h

@@ -16,5 +16,5 @@ namespace FifoRecordAnalyzer
 void Initialize(u32* cpMem);
 
 void ProcessLoadIndexedXf(u32 val, int array);
-void WriteVertexArray(int arrayIndex, u8* vertexData, int vertexSize, int numVertices);
+void WriteVertexArray(int arrayIndex, const u8* vertexData, int vertexSize, int numVertices);
 };

Source/Core/Core/FifoPlayer/FifoRecorder.cpp

@@ -78,10 +78,7 @@ void FifoRecorder::WriteGPCommand(u8* data, u32 size)
 
   if (m_FrameEnded && m_FifoData.size() > 0)
   {
-    size_t dataSize = m_FifoData.size();
+    m_CurrentFrame.fifoData = m_FifoData;
-    m_CurrentFrame.fifoDataSize = (u32)dataSize;
-    m_CurrentFrame.fifoData = new u8[dataSize];
-    memcpy(m_CurrentFrame.fifoData, m_FifoData.data(), dataSize);
 
     {
       std::lock_guard<std::recursive_mutex> lk(sMutex);
@@ -126,12 +123,11 @@ void FifoRecorder::UseMemory(u32 address, u32 size, MemoryUpdate::Type type, boo
     MemoryUpdate memUpdate;
     memUpdate.address = address;
     memUpdate.fifoPosition = (u32)(m_FifoData.size());
-    memUpdate.size = size;
     memUpdate.type = type;
-    memUpdate.data = new u8[size];
+    memUpdate.data.resize(size);
-    memcpy(memUpdate.data, newData, size);
+    std::copy(newData, newData + size, memUpdate.data.begin());
 
-    m_CurrentFrame.memoryUpdates.push_back(memUpdate);
+    m_CurrentFrame.memoryUpdates.push_back(std::move(memUpdate));
   }
   else if (dynamicUpdate)
   {

Source/Core/DolphinWX/FifoPlayerDlg.cpp

@@ -928,7 +928,7 @@ wxString FifoPlayerDlg::CreateRecordingFifoSizeLabel() const
   {
     size_t fifoBytes = 0;
     for (size_t i = 0; i < file->GetFrameCount(); ++i)
-      fifoBytes += file->GetFrame(i).fifoDataSize;
+      fifoBytes += file->GetFrame(i).fifoData.size();
 
     return wxString::Format(_("%zu FIFO bytes"), fifoBytes);
   }
@@ -946,8 +946,8 @@ wxString FifoPlayerDlg::CreateRecordingMemSizeLabel() const
     for (size_t frameNum = 0; frameNum < file->GetFrameCount(); ++frameNum)
     {
       const std::vector<MemoryUpdate>& memUpdates = file->GetFrame(frameNum).memoryUpdates;
-      for (auto& memUpdate : memUpdates)
+      for (const auto& memUpdate : memUpdates)
-        memBytes += memUpdate.size;
+        memBytes += memUpdate.data.size();
     }
 
     return wxString::Format(_("%zu memory bytes"), memBytes);