From fae4f7c8b45d2e15f5777552cc3f5991ec317a4f Mon Sep 17 00:00:00 2001 From: Stenzek Date: Mon, 25 Sep 2023 23:12:15 +1000 Subject: [PATCH] ChdFileReader: Rewrite CHD opening Almost copied verbatim from DuckStation. - Doesn't have race conditions between checking header and opening the file for reading. - Handles both MD5/SHA1 hashes. - Caches CHD headers when searching for parents. - Doesn't break with unicode filenames on Windows. --- pcsx2/CDVD/ChdFileReader.cpp | 220 +++++++++++++++++++++-------------- pcsx2/CDVD/ChdFileReader.h | 3 +- 2 files changed, 131 insertions(+), 92 deletions(-) diff --git a/pcsx2/CDVD/ChdFileReader.cpp b/pcsx2/CDVD/ChdFileReader.cpp index 7fd66071cae63..257259b36adb9 100644 --- a/pcsx2/CDVD/ChdFileReader.cpp +++ b/pcsx2/CDVD/ChdFileReader.cpp @@ -1,5 +1,5 @@ /* PCSX2 - PS2 Emulator for PCs - * Copyright (C) 2002-2021 PCSX2 Dev Team + * Copyright (C) 2002-2023 PCSX2 Dev Team * * PCSX2 is free software: you can redistribute it and/or modify it under the terms * of the GNU Lesser General Public License as published by the Free Software Found- @@ -17,18 +17,18 @@ #include "ChdFileReader.h" #include "common/Assertions.h" +#include "common/Error.h" #include "common/FileSystem.h" #include "common/Path.h" #include "common/StringUtil.h" -#ifdef __clang__ -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wunused-function" -#endif #include "libchdr/chd.h" -#ifdef __clang__ -#pragma clang diagnostic pop -#endif + +#include "xxhash.h" + +static constexpr u32 MAX_PARENTS = 32; // Surely someone wouldn't be insane enough to go beyond this... +static std::vector> s_chd_hash_cache; // +static std::recursive_mutex s_chd_hash_cache_mutex; ChdFileReader::ChdFileReader() { @@ -39,9 +39,6 @@ ChdFileReader::ChdFileReader() ChdFileReader::~ChdFileReader() { Close(); - - for (std::FILE* fp : m_files) - std::fclose(fp); } bool ChdFileReader::CanHandle(const std::string& fileName, const std::string& displayName) @@ -52,111 +49,154 @@ bool ChdFileReader::CanHandle(const std::string& fileName, const std::string& di return true; } -static chd_error chd_open_wrapper(const char* filename, std::FILE** fp, int mode, chd_file* parent, chd_file** chd) +static chd_file* OpenCHD(const std::string& filename, FileSystem::ManagedCFilePtr fp, Error* error, u32 recursion_level) { - *fp = FileSystem::OpenCFile(filename, "rb"); - if (!*fp) - return CHDERR_FILE_NOT_FOUND; - - const chd_error err = chd_open_file(*fp, mode, parent, chd); + chd_file* chd; + chd_error err = chd_open_file(fp.get(), CHD_OPEN_READ | CHD_OPEN_TRANSFER_FILE, nullptr, &chd); if (err == CHDERR_NONE) - return err; - - std::fclose(*fp); - *fp = nullptr; - return err; -} - -bool ChdFileReader::Open2(std::string fileName) -{ - Close2(); + { + // fp is now managed by libchdr + fp.release(); + return chd; + } + else if (err != CHDERR_REQUIRES_PARENT) + { + Console.Error(fmt::format("Failed to open CHD '{}': {}", filename, chd_error_string(err))); + Error::SetString(error, chd_error_string(err)); + return nullptr; + } - m_filename = std::move(fileName); + if (recursion_level >= MAX_PARENTS) + { + Console.Error(fmt::format("Failed to open CHD '{}': Too many parent files", filename)); + Error::SetString(error, "Too many parent files"); + return nullptr; + } - chd_file* child = nullptr; - chd_file* parent = nullptr; - std::FILE* fp = nullptr; + // Need to get the sha1 to look for. chd_header header; - chd_header parent_header; - - std::string chds[8]; - chds[0] = m_filename; - int chd_depth = 0; - chd_error error; + err = chd_read_header_file(fp.get(), &header); + if (err != CHDERR_NONE) + { + Console.Error(fmt::format("Failed to read CHD header '{}': {}", filename, chd_error_string(err))); + Error::SetString(error, chd_error_string(err)); + return nullptr; + } - std::string dirname; - FileSystem::FindResultsArray results; + // Find a chd with a matching sha1 in the same directory. + // Have to do *.* and filter on the extension manually because Linux is case sensitive. + chd_file* parent_chd = nullptr; + const std::string parent_dir(Path::GetDirectory(filename)); + const std::unique_lock hash_cache_lock(s_chd_hash_cache_mutex); - while (CHDERR_REQUIRES_PARENT == (error = chd_open_wrapper(chds[chd_depth].c_str(), &fp, CHD_OPEN_READ, nullptr, &child))) + // Memoize which hashes came from what files, to avoid reading them repeatedly. + for (auto it = s_chd_hash_cache.begin(); it != s_chd_hash_cache.end(); ++it) { - if (chd_depth >= static_cast(std::size(chds) - 1)) - { - Console.Error("CDVD: chd_open hit recursion limit searching for parents"); - return false; - } + if (!StringUtil::compareNoCase(parent_dir, Path::GetDirectory(it->first))) + continue; - // TODO: This is still broken on Windows. Needs to be fixed in libchdr. - if (chd_read_header(chds[chd_depth].c_str(), &header) != CHDERR_NONE) - { - Console.Error("CDVD: chd_open chd_read_header error: %s: %s", chd_error_string(error), chds[chd_depth].c_str()); - return false; - } + if (!chd_is_matching_parent(&header, &it->second)) + continue; - bool found_parent = false; - dirname = Path::GetDirectory(chds[chd_depth]); - if (FileSystem::FindFiles(dirname.c_str(), "*.*", FILESYSTEM_FIND_FILES | FILESYSTEM_FIND_HIDDEN_FILES, &results)) + // Re-check the header, it might have changed since we last opened. + chd_header parent_header; + auto parent_fp = FileSystem::OpenManagedSharedCFile(it->first.c_str(), "rb", FileSystem::FileShareMode::DenyWrite); + if (parent_fp && chd_read_header_file(parent_fp.get(), &parent_header) == CHDERR_NONE && + chd_is_matching_parent(&header, &parent_header)) { - for (const FILESYSTEM_FIND_DATA& fd : results) + // Need to take a copy of the string, because the parent might add to the list and invalidate the iterator. + const std::string filename_to_open = it->first; + + // Match! Open this one. + parent_chd = OpenCHD(filename_to_open, std::move(parent_fp), error, recursion_level + 1); + if (parent_chd) { - const std::string_view extension(Path::GetExtension(fd.FileName)); - if (extension.empty() || StringUtil::Strncasecmp(extension.data(), "chd", 3) != 0) - continue; - - if (chd_read_header(fd.FileName.c_str(), &parent_header) == CHDERR_NONE && - memcmp(parent_header.sha1, header.parentsha1, sizeof(parent_header.sha1)) == 0) - { - found_parent = true; - chds[++chd_depth] = std::move(fd.FileName); - break; - } + Console.WriteLn( + fmt::format("Using parent CHD '{}' from cache for '{}'.", Path::GetFileName(filename_to_open), Path::GetFileName(filename))); } } - if (!found_parent) + // No point checking any others. Since we recursively call OpenCHD(), the iterator is invalidated anyway. + break; + } + if (!parent_chd) + { + // Look for files in the same directory as the chd. + FileSystem::FindResultsArray parent_files; + FileSystem::FindFiles( + parent_dir.c_str(), "*.*", FILESYSTEM_FIND_FILES | FILESYSTEM_FIND_HIDDEN_FILES | FILESYSTEM_FIND_KEEP_ARRAY, &parent_files); + for (FILESYSTEM_FIND_DATA& fd : parent_files) { - Console.Error("CDVD: chd_open no parent for: %s", chds[chd_depth].c_str()); - break; + if (StringUtil::EndsWithNoCase(Path::GetExtension(fd.FileName), ".chd")) + continue; + + // Re-check the header, it might have changed since we last opened. + chd_header parent_header; + auto parent_fp = FileSystem::OpenManagedSharedCFile(fd.FileName.c_str(), "rb", FileSystem::FileShareMode::DenyWrite); + if (!parent_fp || chd_read_header_file(parent_fp.get(), &parent_header) != CHDERR_NONE) + continue; + + // Don't duplicate in the cache. But update it, in case the file changed. + auto cache_it = std::find_if(s_chd_hash_cache.begin(), s_chd_hash_cache.end(), [&fd](const auto& it) { return it.first == fd.FileName; }); + if (cache_it != s_chd_hash_cache.end()) + std::memcpy(&cache_it->second, &parent_header, sizeof(parent_header)); + else + s_chd_hash_cache.emplace_back(fd.FileName, parent_header); + + if (!chd_is_matching_parent(&header, &parent_header)) + continue; + + // Match! Open this one. + parent_chd = OpenCHD(fd.FileName, std::move(parent_fp), error, recursion_level + 1); + if (parent_chd) + { + Console.WriteLn(fmt::format("Using parent CHD '{}' for '{}'.", Path::GetFileName(fd.FileName), Path::GetFileName(filename))); + break; + } } } - - if (error != CHDERR_NONE) + if (!parent_chd) { - Console.Error("CDVD: chd_open return error: %s", chd_error_string(error)); - return false; + Console.Error(fmt::format("Failed to open CHD '{}': Failed to find parent CHD, it must be in the same directory.", filename)); + Error::SetString(error, "Failed to find parent CHD, it must be in the same directory."); + return nullptr; } - if (child) + // Now try re-opening with the parent. + err = chd_open_file(fp.get(), CHD_OPEN_READ | CHD_OPEN_TRANSFER_FILE, parent_chd, &chd); + if (err != CHDERR_NONE) { - pxAssert(fp != nullptr); - m_files.push_back(fp); + Console.Error(fmt::format("Failed to open CHD '{}': {}", filename, chd_error_string(err))); + Error::SetString(error, chd_error_string(err)); + return nullptr; } - for (int d = chd_depth - 1; d >= 0; d--) + // fp now owned by libchdr + fp.release(); + return chd; +} + +bool ChdFileReader::Open2(std::string fileName) +{ + Close2(); + + m_filename = std::move(fileName); + + auto fp = FileSystem::OpenManagedSharedCFile(m_filename.c_str(), "rb", FileSystem::FileShareMode::DenyWrite); + if (!fp) { - parent = child; - child = nullptr; - error = chd_open_wrapper(chds[d].c_str(), &fp, CHD_OPEN_READ, parent, &child); - if (error != CHDERR_NONE) - { - Console.Error("CDVD: chd_open return error: %s", chd_error_string(error)); - if (parent) - chd_close(parent); - return false; - } + Console.Error(fmt::format("Failed to open CHD '{}': errno {}", m_filename, errno)); + return false; + } - m_files.push_back(fp); + // TODO: Propagate error back to caller. + Error error; + ChdFile = OpenCHD(m_filename, std::move(fp), &error, 0); + if (!ChdFile) + { + Console.Error(fmt::format("Failed to open CHD '{}': {}", m_filename, error.GetDescription())); + return false; } - ChdFile = child; const chd_header* chd_header = chd_get_header(ChdFile); hunk_size = chd_header->hunkbytes; @@ -245,7 +285,7 @@ bool ChdFileReader::ParseTOC(u64* out_frame_count) if (err == CHDERR_NONE) { if (std::sscanf(metadata_str, CDROM_TRACK_METADATA2_FORMAT, &track_num, type_str, subtype_str, &frames, - &pregap_frames, pgtype_str, pgsub_str, &postgap_frames) != 8) + &pregap_frames, pgtype_str, pgsub_str, &postgap_frames) != 8) { Console.Error(fmt::format("Invalid track v2 metadata: '{}'", metadata_str)); return false; diff --git a/pcsx2/CDVD/ChdFileReader.h b/pcsx2/CDVD/ChdFileReader.h index 8d8a85848b837..89ed7bc6b5de8 100644 --- a/pcsx2/CDVD/ChdFileReader.h +++ b/pcsx2/CDVD/ChdFileReader.h @@ -1,5 +1,5 @@ /* PCSX2 - PS2 Emulator for PCs - * Copyright (C) 2002-2021 PCSX2 Dev Team + * Copyright (C) 2002-2023 PCSX2 Dev Team * * PCSX2 is free software: you can redistribute it and/or modify it under the terms * of the GNU Lesser General Public License as published by the Free Software Found- @@ -43,5 +43,4 @@ class ChdFileReader : public ThreadedFileReader chd_file* ChdFile; u64 file_size; u32 hunk_size; - std::vector m_files; };