Merge pull request #3795 from EmptyChaos/fix-diskreader

DriveReader: Fix View > Show Drives

8 files changed, 310 insertions, 123 deletions

diff --git a/Source/Core/DiscIO/Blob.cpp b/Source/Core/DiscIO/Blob.cpp
index 760669bc1a..19bd9faf7d 100644
--- a/Source/Core/DiscIO/Blob.cpp
+++ b/Source/Core/DiscIO/Blob.cpp
@@ -2,8 +2,8 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <cstddef>
-#include <cstring>
 #include <limits>
 #include <memory>
 #include <string>
@@ -22,87 +22,156 @@
 namespace DiscIO
 {
 
-// Provides caching and split-operation-to-block-operations facilities.
-// Used for compressed blob reading and direct drive reading.
-
 void SectorReader::SetSectorSize(int blocksize)
 {
+	m_block_size = std::max(blocksize, 0);
 	for (auto& cache_entry : m_cache)
-		cache_entry.resize(blocksize);
+	{
+		cache_entry.Reset();
+		cache_entry.data.resize(m_chunk_blocks * m_block_size);
+	}
+}
 
-	m_cache_tags.fill(std::numeric_limits<u64>::max());
-	m_blocksize = blocksize;
+void SectorReader::SetChunkSize(int block_cnt)
+{
+	m_chunk_blocks = std::max(block_cnt, 1);
+	// Clear cache and resize the data arrays
+	SetSectorSize(m_block_size);
 }
 
 SectorReader::~SectorReader()
 {
 }
 
-const std::vector<u8>& SectorReader::GetBlockData(u64 block_num)
+const SectorReader::Cache* SectorReader::FindCacheLine(u64 block_num)
 {
-	// TODO : Expand usage of the cache to more than one block :P
-	if (m_cache_tags[0] == block_num)
-		return m_cache[0];
+	auto itr = std::find_if(m_cache.begin(), m_cache.end(), [&](const Cache& entry)
+	{
+		return entry.Contains(block_num);
+	});
+	if (itr == m_cache.end())
+		return nullptr;
 
-	GetBlock(block_num, m_cache[0].data());
-	m_cache_tags[0] = block_num;
-	return m_cache[0];
+	itr->MarkUsed();
+	return &*itr;
+}
+
+SectorReader::Cache* SectorReader::GetEmptyCacheLine()
+{
+	Cache* oldest = &m_cache[0];
+	// Find the Least Recently Used cache line to replace.
+	for (auto& cache_entry : m_cache)
+	{
+		if (cache_entry.IsLessRecentlyUsedThan(*oldest))
+			oldest = &cache_entry;
+		cache_entry.ShiftLRU();
+	}
+	oldest->Reset();
+	return oldest;
+}
+
+const SectorReader::Cache* SectorReader::GetCacheLine(u64 block_num)
+{
+	if (auto entry = FindCacheLine(block_num))
+		return entry;
+
+	// Cache miss. Fault in the missing entry.
+	Cache* cache = GetEmptyCacheLine();
+	// We only read aligned chunks, this avoids duplicate overlapping entries.
+	u64 chunk_idx = block_num / m_chunk_blocks;
+	u32 blocks_read = ReadChunk(cache->data.data(), chunk_idx);
+	if (!blocks_read)
+		return nullptr;
+	cache->Fill(chunk_idx * m_chunk_blocks, blocks_read);
+
+	// Secondary check for out-of-bounds read.
+	// If we got less than m_chunk_blocks, we may still have missed.
+	// We do this after the cache fill since the cache line itself is
+	// fine, the problem is being asked to read past the end of the disk.
+	return cache->Contains(block_num) ? cache : nullptr;
 }
 
 bool SectorReader::Read(u64 offset, u64 size, u8* out_ptr)
 {
-	u64 startingBlock = offset / m_blocksize;
 	u64 remain = size;
-
-	int positionInBlock = (int)(offset % m_blocksize);
-	u64 block = startingBlock;
+	u64 block  = 0;
+	u32 position_in_block = static_cast<u32>(offset % m_block_size);
 
 	while (remain > 0)
 	{
-		// Check if we are ready to do a large block read. > instead of >= so we don't bother if remain is only one block.
-		if (positionInBlock == 0 && remain > (u64)m_blocksize)
-		{
-			u64 num_blocks = remain / m_blocksize;
-			ReadMultipleAlignedBlocks(block, num_blocks, out_ptr);
-			block += num_blocks;
-			out_ptr += num_blocks * m_blocksize;
-			remain -= num_blocks * m_blocksize;
-			continue;
-		}
+		block = offset / m_block_size;
 
-		const std::vector<u8>& data = GetBlockData(block);
+		const Cache* cache = GetCacheLine(block);
+		if (!cache)
+			return false;
 
-		u32 to_copy = m_blocksize - positionInBlock;
-		if (to_copy >= remain)
-		{
-			// Yay, we are done!
-			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + remain, out_ptr);
-			return true;
-		}
-		else
-		{
-			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + to_copy, out_ptr);
-			out_ptr += to_copy;
-			remain -= to_copy;
-			positionInBlock = 0;
-			block++;
-		}
-	}
+		// Cache entries are aligned chunks, we may not want to read from the start
+		u32 read_offset = static_cast<u32>(block - cache->block_idx) * m_block_size + position_in_block;
+		u32 can_read = m_block_size * cache->num_blocks - read_offset;
+		u32 was_read = static_cast<u32>(std::min<u64>(can_read, remain));
+
+		std::copy(cache->data.begin() + read_offset,
+		          cache->data.begin() + read_offset + was_read,
+		          out_ptr);
 
+		offset  += was_read;
+		out_ptr += was_read;
+		remain  -= was_read;
+		position_in_block = 0;
+	}
 	return true;
 }
 
-bool SectorReader::ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr)
+// Crap default implementation if not overridden.
+bool SectorReader::ReadMultipleAlignedBlocks(u64 block_num, u64 cnt_blocks, u8* out_ptr)
 {
-	for (u64 i = 0; i < num_blocks; i++)
+	for (u64 i = 0; i < cnt_blocks; ++i)
 	{
-		const std::vector<u8>& data = GetBlockData(block_num + i);
-		const u64 offset = i * m_blocksize;
+		if (!GetBlock(block_num + i, out_ptr))
+			return false;
+		out_ptr += m_block_size;
+	}
+	return true;
+}
 
-		std::copy(data.begin(), data.end(), out_ptr + offset);
+u32 SectorReader::ReadChunk(u8* buffer, u64 chunk_num)
+{
+	u64 block_num  = chunk_num * m_chunk_blocks;
+	u32 cnt_blocks = m_chunk_blocks;
+
+	// If we are reading the end of a disk, there may not be enough blocks to
+	// read a whole chunk. We need to clamp down in that case.
+	u64 end_block = GetDataSize() / m_block_size;
+	if (end_block)
+		cnt_blocks = static_cast<u32>(std::min<u64>(m_chunk_blocks, end_block - block_num));
+
+	if (ReadMultipleAlignedBlocks(block_num, cnt_blocks, buffer))
+	{
+		if (cnt_blocks < m_chunk_blocks)
+		{
+			std::fill(buffer + cnt_blocks * m_block_size,
+			          buffer + m_chunk_blocks * m_block_size,
+			          0u);
+		}
+		return cnt_blocks;
 	}
 
-	return true;
+	// end_block may be zero on real disks if we fail to get the media size.
+	// We have to fallback to probing the disk instead.
+	if (!end_block)
+	{
+		for (u32 i = 0; i < cnt_blocks; ++i)
+		{
+			if (!GetBlock(block_num + i, buffer))
+			{
+				std::fill(buffer, buffer + (cnt_blocks - i) * m_block_size, 0u);
+				return i;
+			}
+			buffer += m_block_size;
+		}
+		return cnt_blocks;
+	}
+	return 0;
 }
 
 std::unique_ptr<IBlobReader> CreateBlobReader(const std::string& filename)
diff --git a/Source/Core/DiscIO/Blob.h b/Source/Core/DiscIO/Blob.h
index 229b2fb3bd..bc79effba7 100644
--- a/Source/Core/DiscIO/Blob.h
+++ b/Source/Core/DiscIO/Blob.h
@@ -50,32 +50,112 @@ protected:
 };
 
 
-// Provides caching and split-operation-to-block-operations facilities.
-// Used for compressed blob reading and direct drive reading.
-// Currently only uses a single entry cache.
-// Multi-block reads are not cached.
+// Provides caching and byte-operation-to-block-operations facilities.
+// Used for compressed blob and direct drive reading.
+// NOTE: GetDataSize() is expected to be evenly divisible by the sector size.
 class SectorReader : public IBlobReader
 {
 public:
-	virtual ~SectorReader();
+	virtual ~SectorReader() = 0;
 
-	bool Read(u64 offset, u64 size, u8 *out_ptr) override;
-	friend class DriveReader;
+	bool Read(u64 offset, u64 size, u8* out_ptr) override;
 
 protected:
 	void SetSectorSize(int blocksize);
-	virtual void GetBlock(u64 block_num, u8 *out) = 0;
-	// This one is uncached. The default implementation is to simply call GetBlockData multiple times and memcpy.
-	virtual bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8 *out_ptr);
+	int GetSectorSize() const
+	{
+		return m_block_size;
+	}
 
-private:
-	// A reference returned by GetBlockData is invalidated as soon as GetBlockData, Read, or ReadMultipleAlignedBlocks is called again.
-	const std::vector<u8>& GetBlockData(u64 block_num);
+	// Set the chunk size -> the number of blocks to read at a time.
+	// Default value is 1 but that is too low for physical devices
+	// like CDROMs. Setting this to a higher value helps reduce seeking
+	// and IO overhead by batching reads. Do not set it too high either
+	// as large reads are slow and will take too long to resolve.
+	void SetChunkSize(int blocks);
+	int GetChunkSize() const
+	{
+		return m_chunk_blocks;
+	}
 
-	enum { CACHE_SIZE = 32 };
-	int m_blocksize;
-	std::array<std::vector<u8>, CACHE_SIZE> m_cache;
-	std::array<u64, CACHE_SIZE> m_cache_tags;
+	// Read a single block/sector.
+	virtual bool GetBlock(u64 block_num, u8* out) = 0;
+
+	// Read multiple contiguous blocks.
+	// Default implementation just calls GetBlock in a loop, it should be
+	// overridden in derived classes where possible.
+	virtual bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr);
+
+private:
+	struct Cache
+	{
+		std::vector<u8> data;
+		u64 block_idx  = 0;
+		u32 num_blocks = 0;
+
+		// [Pseudo-] Least Recently Used Shift Register
+		// When an empty cache line is needed, the line with the lowest value
+		// is taken and reset; the LRU register is then shifted down 1 place
+		// on all lines (low bit discarded). When a line is used, the high bit
+		// is set marking it as most recently used.
+		u32 lru_sreg = 0;
+
+		void Reset()
+		{
+			block_idx  = 0;
+			num_blocks = 0;
+			lru_sreg   = 0;
+		}
+		void Fill(u64 block, u32 count)
+		{
+			block_idx  = block;
+			num_blocks = count;
+			// NOTE: Setting only the high bit means the newest line will
+			//   be selected for eviction if every line in the cache was
+			//   touched. This gives MRU behavior which is probably
+			//   desirable in that case.
+			MarkUsed();
+		}
+		bool Contains(u64 block) const
+		{
+			return block >= block_idx && block - block_idx < num_blocks;
+		}
+		void MarkUsed()
+		{
+			lru_sreg |= 0x80000000;
+		}
+		void ShiftLRU()
+		{
+			lru_sreg >>= 1;
+		}
+		bool IsLessRecentlyUsedThan(const Cache& other) const
+		{
+			return lru_sreg < other.lru_sreg;
+		}
+	};
+
+	// Gets the cache line that contains the given block, or nullptr.
+	// NOTE: The cache record only lasts until it expires (next GetEmptyCacheLine)
+	const Cache* FindCacheLine(u64 block_num);
+
+	// Finds the least recently used cache line, resets and returns it.
+	Cache* GetEmptyCacheLine();
+
+	// Combines FindCacheLine with GetEmptyCacheLine and ReadChunk.
+	// Always returns a valid cache line (loading the data if needed).
+	// May return nullptr only if the cache missed and the read failed.
+	const Cache* GetCacheLine(u64 block_num);
+
+	// Read all bytes from a chunk of blocks into a buffer.
+	// Returns the number of blocks read (may be less than m_chunk_blocks
+	// if chunk_num is the last chunk on the disk and the disk size is not
+	// evenly divisible into chunks). Returns zero if it fails.
+	u32 ReadChunk(u8* buffer, u64 chunk_num);
+
+	static constexpr int CACHE_LINES = 32;
+	u32 m_block_size   = 0;  // Bytes in a sector/block
+	u32 m_chunk_blocks = 1;  // Number of sectors/blocks in a chunk
+	std::array<Cache, CACHE_LINES> m_cache;
 };
 
 class CBlobBigEndianReader
diff --git a/Source/Core/DiscIO/CompressedBlob.cpp b/Source/Core/DiscIO/CompressedBlob.cpp
index e41642d483..3e99890c81 100644
--- a/Source/Core/DiscIO/CompressedBlob.cpp
+++ b/Source/Core/DiscIO/CompressedBlob.cpp
@@ -79,7 +79,7 @@ u64 CompressedBlobReader::GetBlockCompressedSize(u64 block_num) const
 	return 0;
 }
 
-void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
+bool CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 {
 	bool uncompressed = false;
 	u32 comp_block_size = (u32)GetBlockCompressedSize(block_num);
@@ -97,7 +97,13 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 	memset(&m_zlib_buffer[comp_block_size], 0, m_zlib_buffer.size() - comp_block_size);
 
 	m_file.Seek(offset, SEEK_SET);
-	m_file.ReadBytes(m_zlib_buffer.data(), comp_block_size);
+	if (!m_file.ReadBytes(m_zlib_buffer.data(), comp_block_size))
+	{
+		PanicAlertT("The disc image \"%s\" is truncated, some of the data is missing.",
+		            m_file_name.c_str());
+		m_file.Clear();
+		return false;
+	}
 
 	// First, check hash.
 	u32 block_hash = HashAdler32(m_zlib_buffer.data(), comp_block_size);
@@ -133,8 +139,12 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 		}
 		inflateEnd(&z);
 		if (uncomp_size != m_header.block_size)
+		{
 			PanicAlert("Wrong block size");
+			return false;
+		}
 	}
+	return true;
 }
 
 bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u32 sub_type,
diff --git a/Source/Core/DiscIO/CompressedBlob.h b/Source/Core/DiscIO/CompressedBlob.h
index 7b73664b30..baca0e24c0 100644
--- a/Source/Core/DiscIO/CompressedBlob.h
+++ b/Source/Core/DiscIO/CompressedBlob.h
@@ -55,7 +55,7 @@ public:
 	u64 GetDataSize() const override { return m_header.data_size; }
 	u64 GetRawSize() const override { return m_file_size; }
 	u64 GetBlockCompressedSize(u64 block_num) const;
-	void GetBlock(u64 block_num, u8* out_ptr) override;
+	bool GetBlock(u64 block_num, u8* out_ptr) override;
 private:
 	CompressedBlobReader(const std::string& filename);
 
diff --git a/Source/Core/DiscIO/DriveBlob.cpp b/Source/Core/DiscIO/DriveBlob.cpp
index 695b694f47..7aaefc402d 100644
--- a/Source/Core/DiscIO/DriveBlob.cpp
+++ b/Source/Core/DiscIO/DriveBlob.cpp
@@ -18,6 +18,16 @@
 
 #ifdef _WIN32
 #include "Common/StringUtil.h"
+#else
+#include <sys/ioctl.h>
+#include <stdio.h>      // fileno
+#if defined __linux__
+#include <linux/fs.h>   // BLKGETSIZE64
+#elif defined __FreeBSD__
+#include <sys/disk.h>   // DIOCGMEDIASIZE
+#elif defined __APPLE__
+#include <sys/disk.h>   // DKIOCGETBLOCKCOUNT / DKIOCGETBLOCKSIZE
+#endif
 #endif
 
 namespace DiscIO
@@ -25,24 +35,39 @@ namespace DiscIO
 
 DriveReader::DriveReader(const std::string& drive)
 {
+	// 32 sectors is roughly the optimal amount a CD Drive can read in
+	// a single IO cycle. Larger values yield no performance improvement
+	// and just cause IO stalls from the read delay. Smaller values allow
+	// the OS IO and seeking overhead to ourstrip the time actually spent
+	// transferring bytes from the media.
+	SetChunkSize(32);    // 32*2048 = 64KiB
+	SetSectorSize(2048);
 #ifdef _WIN32
-	SectorReader::SetSectorSize(2048);
 	auto const path = UTF8ToTStr(std::string("\\\\.\\") + drive);
 	m_disc_handle = CreateFile(path.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE,
 	                           nullptr, OPEN_EXISTING, FILE_FLAG_RANDOM_ACCESS, nullptr);
-	if (m_disc_handle != INVALID_HANDLE_VALUE)
+	if (IsOK())
 	{
 		// Do a test read to make sure everything is OK, since it seems you can get
 		// handles to empty drives.
 		DWORD not_used;
-		std::vector<u8> buffer(m_blocksize);
-		if (!ReadFile(m_disc_handle, buffer.data(), m_blocksize, &not_used, nullptr))
+		std::vector<u8> buffer(GetSectorSize());
+		if (!ReadFile(m_disc_handle, buffer.data(), GetSectorSize(), &not_used, nullptr))
 		{
 			// OK, something is wrong.
 			CloseHandle(m_disc_handle);
 			m_disc_handle = INVALID_HANDLE_VALUE;
-			return;
 		}
+	}
+	if (IsOK())
+	{
+		// Initialize m_size by querying the volume capacity.
+		STORAGE_READ_CAPACITY storage_size;
+		storage_size.Version = sizeof(storage_size);
+		DWORD bytes = 0;
+		DeviceIoControl(m_disc_handle, IOCTL_STORAGE_READ_CAPACITY, nullptr, 0,
+		                &storage_size, sizeof(storage_size), &bytes, nullptr);
+		m_size = bytes ? storage_size.DiskLength.QuadPart : 0;
 
 	#ifdef _LOCKDRIVE // Do we want to lock the drive?
 		// Lock the compact disc in the CD-ROM drive to prevent accidental
@@ -53,10 +78,24 @@ DriveReader::DriveReader(const std::string& drive)
 		                0, &dwNotUsed, nullptr);
 	#endif
 #else
-	SectorReader::SetSectorSize(2048);
 	m_file.Open(drive, "rb");
 	if (m_file)
 	{
+		int fd = fileno(m_file.GetHandle());
+#if defined __linux__
+		// NOTE: Doesn't matter if it fails, m_size was initialized to zero
+		ioctl(fd, BLKGETSIZE64, &m_size); // u64*
+#elif defined __FreeBSD__
+		off_t size = 0;
+		ioctl(fd, DIOCGMEDIASIZE, &size); // off_t*
+		m_size = size;
+#elif defined __APPLE__
+		u64 count = 0;
+		u32 block_size = 0;
+		ioctl(fd, DKIOCGETBLOCKCOUNT, &count); // u64*
+		ioctl(fd, DKIOCGETBLOCKSIZE,  &block_size); // u32*
+		m_size = count * block_size;
+#endif
 #endif
 	}
 	else
@@ -95,45 +134,32 @@ std::unique_ptr<DriveReader> DriveReader::Create(const std::string& drive)
 	return reader;
 }
 
-void DriveReader::GetBlock(u64 block_num, u8* out_ptr)
+bool DriveReader::GetBlock(u64 block_num, u8* out_ptr)
 {
-	std::vector<u8> sector(m_blocksize);
-
-#ifdef _WIN32
-	u64 offset = m_blocksize * block_num;
-	LONG off_low = (LONG)offset & 0xFFFFFFFF;
-	LONG off_high = (LONG)(offset >> 32);
-	DWORD not_used;
-	SetFilePointer(m_disc_handle, off_low, &off_high, FILE_BEGIN);
-	if (!ReadFile(m_disc_handle, sector.data(), m_blocksize, &not_used, nullptr))
-		PanicAlertT("Disc Read Error");
-#else
-	m_file.Seek(m_blocksize * block_num, SEEK_SET);
-	m_file.ReadBytes(sector.data(), m_blocksize);
-#endif
-
-	std::copy(sector.begin(), sector.end(), out_ptr);
+	return DriveReader::ReadMultipleAlignedBlocks(block_num, 1, out_ptr);
 }
 
 bool DriveReader::ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr)
 {
 #ifdef _WIN32
-	u64 offset = m_blocksize * block_num;
-	LONG off_low = (LONG)offset & 0xFFFFFFFF;
-	LONG off_high = (LONG)(offset >> 32);
-	DWORD not_used;
-	SetFilePointer(m_disc_handle, off_low, &off_high, FILE_BEGIN);
-	if (!ReadFile(m_disc_handle, out_ptr, (DWORD)(m_blocksize * num_blocks), &not_used, nullptr))
+	LARGE_INTEGER offset;
+	offset.QuadPart = GetSectorSize() * block_num;
+	SetFilePointerEx(m_disc_handle, offset, nullptr, FILE_BEGIN);
+	DWORD bytes_read;
+	if (!ReadFile(m_disc_handle, out_ptr, static_cast<DWORD>(GetSectorSize() * num_blocks),
+	              &bytes_read, nullptr))
 	{
 		PanicAlertT("Disc Read Error");
 		return false;
 	}
+	return bytes_read == GetSectorSize() * num_blocks;
 #else
-	fseeko(m_file.GetHandle(), (m_blocksize * block_num), SEEK_SET);
-	if (fread(out_ptr, 1, (m_blocksize * num_blocks), m_file.GetHandle()) != (m_blocksize * num_blocks))
-		return false;
+	m_file.Seek(GetSectorSize() * block_num, SEEK_SET);
+	if (m_file.ReadBytes(out_ptr, num_blocks * GetSectorSize()))
+		return true;
+	m_file.Clear();
+	return false;
 #endif
-	return true;
 }
 
 }  // namespace
diff --git a/Source/Core/DiscIO/DriveBlob.h b/Source/Core/DiscIO/DriveBlob.h
index 8e70d57f99..fd9126fa65 100644
--- a/Source/Core/DiscIO/DriveBlob.h
+++ b/Source/Core/DiscIO/DriveBlob.h
@@ -30,18 +30,18 @@ public:
 
 private:
 	DriveReader(const std::string& drive);
-	void GetBlock(u64 block_num, u8 *out_ptr) override;
+	bool GetBlock(u64 block_num, u8 *out_ptr) override;
 	bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr) override;
 
 #ifdef _WIN32
-	HANDLE m_disc_handle;
+	HANDLE m_disc_handle = INVALID_HANDLE_VALUE;
 	PREVENT_MEDIA_REMOVAL m_lock_cdrom;
-	bool IsOK() { return m_disc_handle != INVALID_HANDLE_VALUE; }
+	bool IsOK() const { return m_disc_handle != INVALID_HANDLE_VALUE; }
 #else
 	File::IOFile m_file;
-	bool IsOK() { return m_file != nullptr; }
+	bool IsOK() const { return m_file.IsOpen() && m_file.IsGood(); }
 #endif
-	s64 m_size;
+	u64 m_size = 0;
 };
 
 }  // namespace
diff --git a/Source/Core/DolphinWX/GameListCtrl.cpp b/Source/Core/DolphinWX/GameListCtrl.cpp
index d994a5b5e2..0533ef8e8e 100644
--- a/Source/Core/DolphinWX/GameListCtrl.cpp
+++ b/Source/Core/DolphinWX/GameListCtrl.cpp
@@ -4,6 +4,7 @@
 
 #include <algorithm>
 #include <cinttypes>
+#include <cmath>
 #include <cstddef>
 #include <cstdio>
 #include <cstring>
@@ -345,24 +346,22 @@ void CGameListCtrl::Update()
 	SetFocus();
 }
 
-static wxString NiceSizeFormat(u64 _size)
+static wxString NiceSizeFormat(u64 size)
 {
 	// Return a pretty filesize string from byte count.
 	// e.g. 1134278 -> "1.08 MiB"
 
-	const char* const unit_symbols[] = {"B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB", "YiB"};
+	const char* const unit_symbols[] = { "B", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB" };
 
-	// Find largest power of 2 less than _size.
-	// div 10 to get largest named unit less than _size
+	// Find largest power of 2 less than size.
+	// div 10 to get largest named unit less than size
 	// 10 == log2(1024) (number of B in a KiB, KiB in a MiB, etc)
-	const u64 unit = IntLog2(std::max<u64>(_size, 1)) / 10;
-	const u64 unit_size = (1ull << (unit * 10));
-
-	// mul 1000 for 3 decimal places, add 5 to round up, div 10 for 2 decimal places
-	std::string value = std::to_string((_size * 1000 / unit_size + 5) / 10);
-	// Insert decimal point.
-	value.insert(value.size() - 2, ".");
-	return StrToWxStr(StringFromFormat("%s %s", value.c_str(), unit_symbols[unit]));
+	// Max value is 63 / 10 = 6
+	const int unit = IntLog2(std::max<u64>(size, 1)) / 10;
+
+	// Don't need exact values, only 5 most significant digits
+	double unit_size = std::pow(2, unit * 10);
+	return wxString::Format("%.2f %s", size / unit_size, unit_symbols[unit]);
 }
 
 // Update the column content of the item at _Index
diff --git a/Source/Core/DolphinWX/ISOProperties.cpp b/Source/Core/DolphinWX/ISOProperties.cpp
index 84f03534c3..d7606c9dad 100644
--- a/Source/Core/DolphinWX/ISOProperties.cpp
+++ b/Source/Core/DolphinWX/ISOProperties.cpp
@@ -1501,8 +1501,11 @@ void CISOProperties::ChangeBannerDetails(DiscIO::IVolume::ELanguage language)
 	m_Comment->SetValue(comment);
 	m_Maker->SetValue(maker);//dev too
 
-	std::string filename, extension;
-	SplitPath(OpenGameListItem.GetFileName(), nullptr, &filename, &extension);
+	std::string path, filename, extension;
+	SplitPath(OpenGameListItem.GetFileName(), &path, &filename, &extension);
+	// Real disk drives don't have filenames on Windows
+	if (filename.empty() && extension.empty())
+		filename = path + ' ';
 	// Also sets the window's title
 	SetTitle(StrToWxStr(StringFromFormat("%s%s: %s - ", filename.c_str(),
 		extension.c_str(), OpenGameListItem.GetUniqueID().c_str())) + name);