Mount reliability, logging, FTP and MDS fixes

addon/discimage/mdsfile.cpp

@@ -290,11 +290,23 @@ bool CMDSFileDevice::Init() {
     if (m_nTotalFrames == 0) {
         // No track lengths recorded. Fall back to the old behaviour rather
         // than presenting an empty disc.
+        m_bFlatOffsets = true;
         m_nTotalFrames = (u32)(f_size(m_pFile) / 2352);
         LOGWARN("No track lengths in MDS; deriving %u frames from the MDF size",
                 m_nTotalFrames);
     }
 
+    // Alcohol drops the pregap, so most multi-track images have a hole. Not asked
+    // when the count came from the file size: nothing would count as stored.
+    m_bHasUnstoredGaps = false;
+    if (!m_bFlatOffsets) {
+        const u32 covered = CountStoredFrames();
+        m_bHasUnstoredGaps = (covered != m_nTotalFrames);
+        if (m_bHasUnstoredGaps) {
+            LOGNOTE("=== MDF is sparse: %u of %u frames stored ===", covered, m_nTotalFrames);
+        }
+    }
+
     LOGNOTE("=== Image has subchannel data: %s ===",
             m_hasSubchannels ? "YES (SafeDisc compatible)" : "NO");
     LOGNOTE("=== Disc length: %u frames ===", m_nTotalFrames);
@@ -325,7 +337,7 @@ CMDSFileDevice::~CMDSFileDevice(void) {
 }
 
 bool CMDSFileDevice::TouchesUnstoredGap(u32 firstLBA, size_t nSectors) const {
-    if (!m_parser) {
+    if (!m_parser || !m_bHasUnstoredGaps) {
         return false;
     }
     for (size_t i = 0; i < nSectors; i++) {
@@ -343,17 +355,20 @@ bool CMDSFileDevice::TouchesUnstoredGap(u32 firstLBA, size_t nSectors) const {
 
 int CMDSFileDevice::ReadAcrossGaps(void *pBuffer, size_t nSize) {
     u8* dest = (u8*)pBuffer;
-    const size_t sectors = nSize / 2352;
     size_t total_read = 0;
 
-    for (size_t i = 0; i < sectors; i++) {
+    while (total_read < nSize) {
+        // A short read would look like an I/O error rather than a hole.
+        const size_t remaining = nSize - total_read;
+        const size_t chunk = remaining < 2352 ? remaining : 2352;
+
         int session, trackIdx;
         MDS_TrackBlock* track = FindTrackForLBA(m_nCurrentLBA, &session, &trackIdx);
 
         if (!track) {
             // Unstored pregap. Zeros are what the pregap of a data track holds
             // anyway, and they keep the transfer whole instead of failing it.
-            memset(dest, 0, 2352);
+            memset(dest, 0, chunk);
         } else {
             // Seek per frame rather than trusting the file pointer: a gap
             // consumed no file position, so it is stale after one.
@@ -365,17 +380,20 @@ int CMDSFileDevice::ReadAcrossGaps(void *pBuffer, size_t nSize) {
                 return total_read > 0 ? (int)total_read : -1;
             }
             UINT bytes_read = 0;
-            FRESULT result = f_read(m_pFile, dest, 2352, &bytes_read);
-            if (result != FR_OK || bytes_read != 2352) {
+            FRESULT result = f_read(m_pFile, dest, chunk, &bytes_read);
+            if (result != FR_OK || bytes_read != chunk) {
                 LOGERR("Gap-aware read: LBA %u returned %u bytes (err %d)",
                        m_nCurrentLBA, bytes_read, result);
                 return total_read > 0 ? (int)total_read : -1;
             }
         }
 
-        dest += 2352;
-        total_read += 2352;
-        m_nCurrentLBA++;
+        dest += chunk;
+        total_read += chunk;
+        // A partial tail leaves the position inside the frame it stopped in.
+        if (chunk == 2352) {
+            m_nCurrentLBA++;
+        }
     }
 
     return (int)total_read;
@@ -390,8 +408,10 @@ int CMDSFileDevice::Read(void *pBuffer, size_t nSize) {
     // A transfer that crosses a pregap the MDF does not store cannot be one
     // f_read, because part of it has no bytes behind it. That is rare enough
     // to be worth detecting rather than paying for frame-by-frame reads on
-    // every transfer, so the paths below are left as they were.
-    if (nSize >= 2352 && TouchesUnstoredGap(m_nCurrentLBA, nSize / 2352)) {
+    // every transfer, so the paths below are unchanged. Sub-frame transfers are
+    // still checked; gating on nSize let them return stale bytes.
+    const size_t framesTouched = (nSize + 2351) / 2352;
+    if (framesTouched > 0 && TouchesUnstoredGap(m_nCurrentLBA, framesTouched)) {
         return ReadAcrossGaps(pBuffer, nSize);
     }
 
@@ -452,6 +472,11 @@ int CMDSFileDevice::Read(void *pBuffer, size_t nSize) {
         LOGERR("Failed to read %d bytes into memory, err %d", nSize, result);
         return -1;
     }
+
+    // Advance by what was consumed, as the two paths above do, or a caller
+    // reading on judges every later frame against the first one's address.
+    m_nCurrentLBA += nBytesRead / 2352;
+
     return nBytesRead;
 }
 
@@ -475,18 +500,29 @@ u64 CMDSFileDevice::Seek(u64 nOffset) {
         return static_cast<u64>(-1);
     }
 
-    // Don't seek if we're already there
-    if (Tell() == nOffset)
-        return nOffset;
-
     // Calculate which LBA is being requested
     u32 lba = nOffset / 2352;  // Assuming 2352 bytes per sector
     u32 offset_in_sector = nOffset % 2352;
-
+
+    // Before any early exit can skip it: Read() keys its gap detection off this.
+    m_nCurrentLBA = lba;
+
+    // No track table to map through, so the MDF is a flat run of frames from LBA 0.
+    // Otherwise the branch below leaves the file position untouched.
+    if (m_bFlatOffsets) {
+        FRESULT flat = f_lseek(m_pFile, nOffset);
+        if (flat != FR_OK) {
+            LOGERR("Seek to flat offset %llu failed, err %d",
+                   (unsigned long long)nOffset, flat);
+            return static_cast<u64>(-1);
+        }
+        return nOffset;
+    }
+
     // Find which track contains this LBA
     int session, trackIdx;
     MDS_TrackBlock* track = FindTrackForLBA(lba, &session, &trackIdx);
-    
+
     if (!track) {
         // An LBA inside the disc but outside every track is a pregap the
         // imaging tool chose not to store - Alcohol omits them by default, so
@@ -495,7 +531,6 @@ u64 CMDSFileDevice::Seek(u64 nOffset) {
         // track lands here legitimately, and a real drive answers rather than
         // failing. There is no file position to take up; Read() serves zeros.
         if (lba < m_nTotalFrames) {
-            m_nCurrentLBA = lba;
             return nOffset;
         }
         LOGERR("Seek: LBA %u not found in any track", lba);
@@ -504,23 +539,25 @@ u64 CMDSFileDevice::Seek(u64 nOffset) {
 
     // Calculate offset into MDF file
     u32 sectors_from_track_start = lba - track->start_sector;
-    u64 actual_file_offset = track->start_offset + 
-                             (sectors_from_track_start * track->sector_size) + 
+    u64 actual_file_offset = track->start_offset +
+                             ((u64)sectors_from_track_start * track->sector_size) +
                              offset_in_sector;
-    
-    // LOGDBG("Seek: LBA %u (offset %llu) -> track %d, file offset %llu", 
+
+    // LOGDBG("Seek: LBA %u (offset %llu) -> track %d, file offset %llu",
     //        lba, nOffset, track->point, actual_file_offset);
-
+
+    // Compare the FILE offset just computed, not the disc address nOffset, which
+    // coincides often enough to skip a seek that was needed.
+    if (Tell() == actual_file_offset) {
+        return nOffset;
+    }
+
     FRESULT result = f_lseek(m_pFile, actual_file_offset);
     if (result != FR_OK) {
         LOGERR("Seek to file offset %llu failed, err %d", actual_file_offset, result);
         return static_cast<u64>(-1);
     }
 
-    // Remember which frame this was: Read() cannot recover it from the file
-    // position once subchannel data makes the physical stride 2448 bytes.
-    m_nCurrentLBA = lba;
-
     // Return the logical offset that was requested (not the physical file offset)
     return nOffset;
 }
@@ -618,6 +655,73 @@ bool CMDSFileDevice::IsAudioTrack(int track) const {
     return false;
 }
 
+// Distinct frames stored. Summing counts an overlap twice, and an overlap the
+// size of a real hole then adds up to a full disc, hiding the hole.
+u32 CMDSFileDevice::CountStoredFrames() const {
+    struct Range {
+        u32 start;
+        u32 end;
+    };
+
+    // 99 tracks is the Red Book limit, so this only has to be big enough not to
+    // truncate a legitimate image.
+    static const size_t kMaxRanges = 128;
+    Range Ranges[kMaxRanges];
+    size_t nRanges = 0;
+
+    for (int i = 0; i < m_parser->getNumSessions(); i++) {
+        MDS_SessionBlock* session = m_parser->getSession(i);
+        for (int j = 0; j < session->num_all_blocks; j++) {
+            MDS_TrackBlock* track = m_parser->getTrack(i, j);
+            if (track->point == 0 || track->point >= 0xA0) {
+                continue;
+            }
+            MDS_TrackExtraBlock* extra = m_parser->getTrackExtra(i, j);
+            const u32 length = extra ? extra->length : 0;
+            if (length == 0) {
+                continue;
+            }
+            if (nRanges == kMaxRanges) {
+                // Walking gaps needlessly is slow; missing one serves wrong bytes.
+                LOGWARN("More than %u stored ranges; assuming the MDF is sparse",
+                        (unsigned)kMaxRanges);
+                return 0;
+            }
+            Ranges[nRanges].start = track->start_sector;
+            Ranges[nRanges].end = track->start_sector + length;
+            nRanges++;
+        }
+    }
+
+    // Insertion sort: a disc has few tracks and they arrive nearly ordered.
+    for (size_t i = 1; i < nRanges; i++) {
+        const Range Key = Ranges[i];
+        size_t j = i;
+        while (j > 0 && Ranges[j - 1].start > Key.start) {
+            Ranges[j] = Ranges[j - 1];
+            j--;
+        }
+        Ranges[j] = Key;
+    }
+
+    u32 covered = 0;
+    size_t i = 0;
+    while (i < nRanges) {
+        u32 end = Ranges[i].end;
+        const u32 start = Ranges[i].start;
+        while (i + 1 < nRanges && Ranges[i + 1].start <= end) {
+            if (Ranges[i + 1].end > end) {
+                end = Ranges[i + 1].end;
+            }
+            i++;
+        }
+        covered += end - start;
+        i++;
+    }
+
+    return covered;
+}
+
 MDS_TrackBlock* CMDSFileDevice::FindTrackForLBA(u32 lba, int* sessionOut, int* trackOut) const {
     if (!m_parser) return nullptr;
 
@@ -651,20 +755,25 @@ int CMDSFileDevice::ReadSubchannel(u32 lba, u8* subchannel) {
 
     int session, trackIdx;
     MDS_TrackBlock* track = FindTrackForLBA(lba, &session, &trackIdx);
-    
+
     if (!track) {
+        // An unstored pregap, which Seek() and Read() answer with zeros.
+        if (lba < m_nTotalFrames) {
+            memset(subchannel, 0, 96);
+            return 96;
+        }
         LOGERR("LBA %u not found in any track", lba);
         return -1;
     }
-    
+
     // Check if this track has subchannel data
     if (track->subchannel == 0) {
         return -1;
     }
-    
+
     // Calculate offset into the MDF file
     u32 sectors_from_track_start = lba - track->start_sector;
-    u64 sector_offset = track->start_offset + (sectors_from_track_start * track->sector_size);
+    u64 sector_offset = track->start_offset + ((u64)sectors_from_track_start * track->sector_size);
 
     // Subchannel data is stored in the last 96 bytes of each raw sector
     // Raw sector format: 2352 bytes user data + 96 bytes subchannel

addon/discimage/mdsfile.h

@@ -82,11 +82,22 @@ class CMDSFileDevice : public IMDSDevice {
     /// bytes, so its length divided by 2352 over-reports the disc.
     u32 m_nTotalFrames = 0;
 
-    /// The LBA Seek() last resolved. Read() needs it because Tell() reports
-    /// a PHYSICAL offset in the MDF, which on a 2448-byte-per-sector track
-    /// is not lba * 2352.
+    /// The reader's position: set by Seek(), advanced by Read(). Not recoverable
+    /// from Tell(), which reports a physical MDF offset.
     u32 m_nCurrentLBA = 0;
 
+    /// True if the MDF omits at least one frame. Cached because the alternative
+    /// is an O(frames x tracks) lookup per read, and most images have no hole.
+    bool m_bHasUnstoredGaps = false;
+
+    /// No track lengths recorded, so the count came from the MDF size and the
+    /// file is read as a flat run of frames from LBA 0.
+    bool m_bFlatOffsets = false;
+
+    /// Distinct frames in the MDF, overlapping ranges merged. 0 if there are too
+    /// many to merge, which reads as sparse and costs speed, not correctness.
+    u32 CountStoredFrames() const;
+
     // Helper to find track containing an LBA
     MDS_TrackBlock* FindTrackForLBA(u32 lba, int* sessionOut, int* trackOut) const;