CDROM: Revamp/improve preload image procedure

src/core/cdrom.cpp

@@ -1,9 +1,11 @@
 #include "cdrom.h"
 #include "common/align.h"
+#include "common/file_system.h"
 #include "common/log.h"
 #include "common/platform.h"
 #include "dma.h"
 #include "host.h"
+#include "host_interface_progress_callback.h"
 #include "imgui.h"
 #include "interrupt_controller.h"
 #include "settings.h"
@@ -398,6 +400,30 @@ std::unique_ptr<CDImage> CDROM::RemoveMedia(bool force /* = false */)
   return image;
 }
 
+bool CDROM::PrecacheMedia()
+{
+  if (!m_reader.HasMedia())
+    return false;
+
+  if (m_reader.GetMedia()->HasSubImages() && m_reader.GetMedia()->GetSubImageCount() > 1)
+  {
+    Host::AddFormattedOSDMessage(
+      15.0f, Host::TranslateString("OSDMessage", "CD image preloading not available for multi-disc image '%s'"),
+      FileSystem::GetDisplayNameFromPath(m_reader.GetMedia()->GetFileName()).c_str());
+    return false;
+  }
+
+  HostInterfaceProgressCallback callback;
+  if (!m_reader.Precache(&callback))
+  {
+    Host::AddOSDMessage(Host::TranslateStdString("OSDMessage", "Precaching CD image failed, it may be unreliable."),
+                        15.0f);
+    return false;
+  }
+
+  return true;
+}
+
 void CDROM::SetReadaheadSectors(u32 readahead_sectors)
 {
   const bool want_thread = (readahead_sectors > 0);

src/core/cdrom.h

@@ -12,6 +12,8 @@
 #include <tuple>
 #include <vector>
 
+class ProgressCallback;
+
 class StateWrapper;
 class TimingEvent;
 
@@ -35,6 +37,7 @@ public:
 
   void InsertMedia(std::unique_ptr<CDImage> media);
   std::unique_ptr<CDImage> RemoveMedia(bool force = false);
+  bool PrecacheMedia();
 
   void CPUClockChanged();
 

src/core/cdrom_async_reader.cpp

@@ -57,6 +57,45 @@ std::unique_ptr<CDImage> CDROMAsyncReader::RemoveMedia()
   return std::move(m_media);
 }
 
+bool CDROMAsyncReader::Precache(ProgressCallback* callback)
+{
+  WaitForIdle();
+
+  std::unique_lock lock(m_mutex);
+  if (!m_media)
+    return false;
+  else if (m_media->IsPrecached())
+    return true;
+
+  EmptyBuffers();
+
+  const CDImage::PrecacheResult res = m_media->Precache(callback);
+  if (res == CDImage::PrecacheResult::Unsupported)
+  {
+    // fall back to copy precaching
+    std::unique_ptr<CDImage> memory_image = CDImage::CreateMemoryImage(m_media.get(), callback);
+    if (memory_image)
+    {
+      const CDImage::LBA lba = m_media->GetPositionOnDisc();
+      if (!memory_image->Seek(lba))
+      {
+        Log_ErrorPrintf("Failed to seek to LBA %u in memory image", lba);
+        return false;
+      }
+
+      m_media.reset();
+      m_media = std::move(memory_image);
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+
+  return (res == CDImage::PrecacheResult::Success);
+}
+
 void CDROMAsyncReader::QueueReadSector(CDImage::LBA lba)
 {
   if (!IsUsingThread())

src/core/cdrom_async_reader.h

@@ -6,6 +6,8 @@
 #include <condition_variable>
 #include <thread>
 
+class ProgressCallback;
+
 class CDROMAsyncReader
 {
 public:
@@ -40,6 +42,9 @@ public:
   void SetMedia(std::unique_ptr<CDImage> media);
   std::unique_ptr<CDImage> RemoveMedia();
 
+  /// Precaches image, either to memory, or using the underlying image precache.
+  bool Precache(ProgressCallback* callback);
+
   void QueueReadSector(CDImage::LBA lba);
 
   bool WaitForReadToComplete();

src/core/system.cpp

@@ -89,8 +89,7 @@ static bool LoadEXE(const char* filename);
 static std::string GetExecutableNameForImage(ISOReader& iso, bool strip_subdirectories);
 
 /// Opens CD image, preloading if needed.
-static std::unique_ptr<CDImage> OpenCDImage(const char* path, Common::Error* error, bool force_preload,
+static std::unique_ptr<CDImage> OpenCDImage(const char* path, Common::Error* error, bool check_for_patches);
-                                            bool check_for_patches);
 static bool ReadExecutableFromImage(ISOReader& iso, std::string* out_executable_name,
                                     std::vector<u8>* out_executable_data);
 static bool ShouldCheckForImagePatches();
@@ -756,42 +755,12 @@ bool System::RecreateGPU(GPURenderer renderer, bool force_recreate_display, bool
   return true;
 }
 
-std::unique_ptr<CDImage> System::OpenCDImage(const char* path, Common::Error* error, bool force_preload,
+std::unique_ptr<CDImage> System::OpenCDImage(const char* path, Common::Error* error, bool check_for_patches)
-                                             bool check_for_patches)
 {
   std::unique_ptr<CDImage> media = CDImage::Open(path, error);
   if (!media)
     return {};
 
-  if (force_preload || g_settings.cdrom_load_image_to_ram)
-  {
-    if (media->HasSubImages() && media->GetSubImageCount() > 1)
-    {
-      Host::AddFormattedOSDMessage(
-        15.0f, Host::TranslateString("OSDMessage", "CD image preloading not available for multi-disc image '%s'"),
-        FileSystem::GetDisplayNameFromPath(media->GetFileName()).c_str());
-    }
-    else
-    {
-      HostInterfaceProgressCallback callback;
-      const CDImage::PrecacheResult res = media->Precache(&callback);
-      if (res == CDImage::PrecacheResult::Unsupported)
-      {
-        // fall back to copy precaching
-        std::unique_ptr<CDImage> memory_image = CDImage::CreateMemoryImage(media.get(), &callback);
-        if (memory_image)
-          media = std::move(memory_image);
-        else
-          Log_WarningPrintf("Failed to preload image '%s' to RAM", path);
-      }
-      else if (res != CDImage::PrecacheResult::Success)
-      {
-        Host::AddOSDMessage(Host::TranslateStdString("OSDMessage", "Precaching CD image failed, it may be unreliable."),
-                            15.0f);
-      }
-    }
-  }
-
   if (check_for_patches)
   {
     const std::string ppf_filename(
@@ -804,7 +773,7 @@ std::unique_ptr<CDImage> System::OpenCDImage(const char* path, Common::Error* er
         Host::AddFormattedOSDMessage(
           30.0f, Host::TranslateString("OSDMessage", "Failed to apply ppf patch from '%s', using unpatched image."),
           ppf_filename.c_str());
-        return OpenCDImage(path, error, force_preload, false);
+        return OpenCDImage(path, error, false);
       }
     }
   }
@@ -1041,9 +1010,9 @@ bool System::SaveState(const char* filename, bool backup_existing_save)
 
 bool System::BootSystem(SystemBootParameters parameters)
 {
-  if (parameters.filename.empty() && !parameters.save_state.empty())
+  if (!parameters.save_state.empty())
   {
-    // loading a state without media, so pull the media path from the save state to avoid a double change
+    // loading a state, so pull the media path from the save state to avoid a double change
     parameters.filename = GetMediaPathFromSaveState(parameters.save_state.c_str());
   }
 
@@ -1080,8 +1049,7 @@ bool System::BootSystem(SystemBootParameters parameters)
     else
     {
       Log_InfoPrintf("Loading CD image '%s'...", parameters.filename.c_str());
-      media =
+      media = OpenCDImage(parameters.filename.c_str(), &error, ShouldCheckForImagePatches());
-        OpenCDImage(parameters.filename.c_str(), &error, parameters.load_image_to_ram, ShouldCheckForImagePatches());
       if (!media)
       {
         Host::ReportErrorAsync("Error", fmt::format("Failed to load CD image '{}': {}",
@@ -1243,6 +1211,9 @@ bool System::BootSystem(SystemBootParameters parameters)
     }
   }
 
+  if (parameters.load_image_to_ram || g_settings.cdrom_load_image_to_ram)
+    g_cdrom.PrecacheMedia();
+
   ResetPerformanceCounters();
   return true;
 }
@@ -1703,6 +1674,8 @@ std::string System::GetMediaPathFromSaveState(const char* path)
 
 bool System::DoLoadState(ByteStream* state, bool force_software_renderer, bool update_display)
 {
+  Assert(IsValid());
+
   SAVE_STATE_HEADER header;
   if (!state->Read2(&header, sizeof(header)))
     return false;
@@ -1748,7 +1721,7 @@ bool System::DoLoadState(ByteStream* state, bool force_software_renderer, bool u
     }
     else
     {
-      media = OpenCDImage(media_filename.c_str(), &error, false, ShouldCheckForImagePatches());
+      media = OpenCDImage(media_filename.c_str(), &error, ShouldCheckForImagePatches());
       if (!media)
       {
         if (old_media)
@@ -1794,32 +1767,22 @@ bool System::DoLoadState(ByteStream* state, bool force_software_renderer, bool u
 
   ClearMemorySaveStates();
 
-  if (s_state == State::Starting)
+  g_cdrom.Reset();
+  if (media)
   {
-    if (!Initialize(force_software_renderer))
+    g_cdrom.InsertMedia(std::move(media));
-      return false;
+    if (g_settings.cdrom_load_image_to_ram)
-
+      g_cdrom.PrecacheMedia();
-    if (media)
-      g_cdrom.InsertMedia(std::move(media));
-
-    UpdateControllers();
-    UpdateMemoryCardTypes();
-    UpdateMultitaps();
-    InternalReset();
   }
   else
   {
-    g_cdrom.Reset();
+    g_cdrom.RemoveMedia();
-    if (media)
-      g_cdrom.InsertMedia(std::move(media));
-    else
-      g_cdrom.RemoveMedia();
-
-    // ensure the correct card is loaded
-    if (g_settings.HasAnyPerGameMemoryCards())
-      UpdatePerGameMemoryCards();
   }
 
+  // ensure the correct card is loaded
+  if (g_settings.HasAnyPerGameMemoryCards())
+    UpdatePerGameMemoryCards();
+
   if (header.data_compression_type != 0)
   {
     Host::ReportFormattedErrorAsync("Error", "Unknown save state compression type %u", header.data_compression_type);
@@ -2810,7 +2773,7 @@ std::string System::GetMediaFileName()
 bool System::InsertMedia(const char* path)
 {
   Common::Error error;
-  std::unique_ptr<CDImage> image = OpenCDImage(path, &error, false, ShouldCheckForImagePatches());
+  std::unique_ptr<CDImage> image = OpenCDImage(path, &error, ShouldCheckForImagePatches());
   if (!image)
   {
     Host::AddFormattedOSDMessage(10.0f, Host::TranslateString("OSDMessage", "Failed to open disc image '%s': %s."),
@@ -2822,6 +2785,9 @@ bool System::InsertMedia(const char* path)
   g_cdrom.InsertMedia(std::move(image));
   Log_InfoPrintf("Inserted media from %s (%s, %s)", s_running_game_path.c_str(), s_running_game_code.c_str(),
                  s_running_game_title.c_str());
+  if (g_settings.cdrom_load_image_to_ram)
+    g_cdrom.PrecacheMedia();
+
   Host::AddFormattedOSDMessage(10.0f, Host::TranslateString("OSDMessage", "Inserted disc '%s' (%s)."),
                                s_running_game_title.c_str(), s_running_game_code.c_str());
 

src/util/cd_image.cpp

@@ -330,6 +330,11 @@ CDImage::PrecacheResult CDImage::Precache(ProgressCallback* progress /*= Progres
   return PrecacheResult::Unsupported;
 }
 
+bool CDImage::IsPrecached() const
+{
+  return false;
+}
+
 void CDImage::ClearTOC()
 {
   m_lba_count = 0;

src/util/cd_image.h

@@ -303,6 +303,7 @@ public:
 
   // Returns true if the source supports precaching, which may be more optimal than an in-memory copy.
   virtual PrecacheResult Precache(ProgressCallback* progress = ProgressCallback::NullProgressCallback);
+  virtual bool IsPrecached() const;
 
 protected:
   void ClearTOC();

src/util/cd_image_chd.cpp

@@ -10,6 +10,7 @@
 #include "common/file_system.h"
 #include "common/log.h"
 #include "common/platform.h"
+#include "fmt/format.h"
 #include "libchdr/chd.h"
 #include <algorithm>
 #include <cerrno>
@@ -53,6 +54,7 @@ public:
   bool ReadSubChannelQ(SubChannelQ* subq, const Index& index, LBA lba_in_index) override;
   bool HasNonStandardSubchannel() const override;
   PrecacheResult Precache(ProgressCallback* progress) override;
+  bool IsPrecached() const override;
 
 protected:
   bool ReadSectorFromIndex(void* buffer, const Index& index, LBA lba_in_index) override;
@@ -73,6 +75,7 @@ private:
 
   std::vector<u8> m_hunk_buffer;
   u32 m_current_hunk_index = static_cast<u32>(-1);
+  bool m_precached = false;
 
   CDSubChannelReplacement m_sbi;
 };
@@ -303,16 +306,27 @@ bool CDImageCHD::HasNonStandardSubchannel() const
 
 CDImage::PrecacheResult CDImageCHD::Precache(ProgressCallback* progress)
 {
-  const std::string_view title(FileSystem::GetDisplayNameFromPath(m_filename));
+  if (m_precached)
-  progress->SetFormattedStatusText("Precaching %.*s...", static_cast<int>(title.size()), title.data());
+    return CDImage::PrecacheResult::Success;
+
+  progress->SetStatusText(fmt::format("Precaching {}...", FileSystem::GetDisplayNameFromPath(m_filename)).c_str());
   progress->SetProgressRange(100);
 
   auto callback = [](size_t pos, size_t total, void* param) {
     const u32 percent = static_cast<u32>((pos * 100) / total);
     static_cast<ProgressCallback*>(param)->SetProgressValue(std::min<u32>(percent, 100));
   };
-  return (chd_precache_progress(m_chd, callback, progress) == CHDERR_NONE) ? CDImage::PrecacheResult::Success :
+
-                                                                             CDImage::PrecacheResult::ReadError;
+  if (chd_precache_progress(m_chd, callback, progress) != CHDERR_NONE)
+    return CDImage::PrecacheResult::ReadError;
+
+  m_precached = true;
+  return CDImage::PrecacheResult::Success;
+}
+
+bool CDImageCHD::IsPrecached() const
+{
+  return m_precached;
 }
 
 // There's probably a more efficient way of doing this with vectorization...

src/util/cd_image_memory.cpp

@@ -19,6 +19,8 @@ public:
   bool ReadSubChannelQ(SubChannelQ* subq, const Index& index, LBA lba_in_index) override;
   bool HasNonStandardSubchannel() const override;
 
+  bool IsPrecached() const override;
+
 protected:
   bool ReadSectorFromIndex(void* buffer, const Index& index, LBA lba_in_index) override;
 
@@ -128,6 +130,11 @@ bool CDImageMemory::HasNonStandardSubchannel() const
   return (m_sbi.GetReplacementSectorCount() > 0);
 }
 
+bool CDImageMemory::IsPrecached() const
+{
+  return true;
+}
+
 bool CDImageMemory::ReadSectorFromIndex(void* buffer, const Index& index, LBA lba_in_index)
 {
   DebugAssert(index.file_index == 0);