diff --git a/docs/drive-access.md b/docs/drive-access.md index 5d7f585..be9a9b2 100644 --- a/docs/drive-access.md +++ b/docs/drive-access.md @@ -26,8 +26,8 @@ platform driver. The drive is ready for `wait_ready()` and `init()`. | `wait_ready()` | Wait for disc insertion (30s timeout, TUR polling) | | `init()` | Firmware upload + unlock + speed calibration | | `probe_disc()` | Probe disc surface for optimal speeds | -| `read(lba, count, buf)` | Read sectors with built-in error recovery | -| `reset()` | Close/reopen device, TUR, escalate if needed | +| `read(lba, count, buf, recovery)` | Read sectors. Single-shot — no inline retries or reset. | +| `reset()` | Eject-cycle escape hatch. Caller-invoked only; not on the read path. | | `lock_tray()` | Prevent tray ejection during rip | | `unlock_tray()` | Allow tray ejection (also runs on Drop) | | `eject()` | Eject disc tray | @@ -44,22 +44,33 @@ platform driver. The drive is ready for `wait_ready()` and `init()`. 3. Speed calibration after unlock 4. Max 3 attempts before giving up -### read() with Recovery +### read() — single-shot -`Drive::read(lba, count, buf, recovery)` is the single read method. The -`recovery` parameter controls whether to attempt multi-phase recovery on -failure or return immediately (used by DiscStream's binary search for -single-sector probes). +`Drive::read(lba, count, buf, recovery)` is the single read method. It issues +exactly one READ(10) CDB and returns the result. The `recovery` parameter only +selects the per-CDB timeout: -On error with `recovery = true`: +| `recovery` | Timeout | Used by | +|------------|----------|------------------------------------------| +| `false` | 1.5 s | `Disc::copy` fast skip-forward sweep, `DiscStream::fill_extents` | +| `true` | 30 s | `Disc::patch` multi-pass over the mapfile | -1. **Phase 1 — gentle retry (5 attempts):** set min speed, sleep 30s, retry. - Each retry has a hard wall-clock timeout via async SG_IO. -2. **Phase 2 — fresh start:** close transport, reset device, reopen, reinit. -3. **Phase 3 — gentle retry on fresh connection (5 attempts).** -4. If all fail: return `Err(DiscRead)`. DiscStream handles it (binary search, - skip, zero-fill). -5. Stay at min speed for 500 MB after any recovery (recovery window). +On any SCSI failure or timeout, `read` returns `Err(DiscRead)` immediately. +There are no inline retries, no SCSI reset, no Phase 1/2/3 escalation. + +Recovery is layered above `Drive::read`: + +- **Layer 1 — `Disc::patch`** loops over the ddrescue mapfile and re-issues + `read(.., recovery=true)` against each non-`+` range. +- **Layer 3 — `DiscStream::fill_extents`** halves the request size on + failure, retries at the same LBA, and probes back up on a clean-read + streak. + +Inline recovery (5× gentle retry → close + reset + reopen → 5× more) was +removed in 0.13.6. See `freemkv-private/postmortems/2026-04-25-stop-wedge-and-zero-kbs.md` +for rationale: the inline reset wedged drive firmware on the LG BU40N (Initio +USB-SATA bridge) without ever recovering a sector. See +[`rip-recovery.md`](rip-recovery.md) for the full three-layer model. --- @@ -99,6 +110,17 @@ cannot block us. Opens with `O_RDWR | O_NONBLOCK`. On non-zero SCSI status, the transport parses sense key from the sense buffer and returns `Error::ScsiError`. +`SgIoTransport::reset` (Linux) does pure userspace state cleanup: an open + +close pair to make the kernel cancel any SG_IO commands queued against a +previous fd, a 2 s sleep to let the kernel finish that cancellation, then a +fresh fd to send ALLOW MEDIUM REMOVAL to clear any stale tray lock. It does +NOT issue `SG_SCSI_RESET` or escalate via STOP+START UNIT. Both were tried +in 0.13.0–0.13.5 against the LG BU40N (Initio USB-SATA bridge); both failed +to recover wedged drives and made the wedge worse. The macOS reset (which +had been a no-op) was removed entirely in 0.13.6, and the top-level +`scsi::reset()` / `reset_with_timeout()` / `reset_blocking()` wrappers were +removed at the same time (no callers). + ### CDB Builders The `scsi` module provides platform-agnostic CDB constructors: diff --git a/docs/rip-recovery.md b/docs/rip-recovery.md index a2152d5..2554593 100644 --- a/docs/rip-recovery.md +++ b/docs/rip-recovery.md @@ -1,12 +1,27 @@ -# Rip recovery — multi-pass architecture +# Rip recovery — three-layer architecture -`libfreemkv` supports a two-stage rip model for damaged or protection-bearing -discs: a fast forward sweep that tolerates read failures, followed by targeted +`libfreemkv` supports a multi-stage rip model for damaged or protection-bearing +discs: a fast forward sweep that tolerates read failures, in-loop request-size +adaptation that survives transient drive trouble without bailing, and targeted retry passes against a persistent bad-range map. The stream pipeline (`DiscStream` + `input`/`output`) operates against the resulting ISO image, so the mux stage never touches the drive. -Three primitives compose the flow: +Recovery is layered cleanly. Each layer has one responsibility and does not +reach into the others. + +| Layer | Where it lives | What it does | +|-------|---------------|--------------| +| 1 — Bad-range retry | `Disc::patch` (multi-pass over the mapfile) | Re-reads non-`+` ranges with the long timeout. Idempotent; call N times. | +| 2 — Single-shot primitive | `Drive::read` in `src/drive/mod.rs` | One CDB, one timeout, one result. No inline retries, no SCSI reset. | +| 3 — In-loop request adaptation | `DiscStream::fill_extents` adaptive batch sizer | Halves the batch on failure, retries at the same LBA, walks back up on a clean-read streak. | + +The caller orchestrates layer 1. Autorip's `rip_disc` loops `copy` then +N × `patch` per the `MAX_RETRIES` config, then hands the ISO off to the +existing mux pipeline. Layer 3 runs inside any consumer of `DiscStream` +(direct PES pipeline, ISO playback, etc.) without caller involvement. + +Three primitives compose the disc-side flow: | Primitive | What it does | |---------------------------|-----------------------------------------------------------------------| @@ -14,9 +29,6 @@ Three primitives compose the flow: | `Disc::patch` | Re-reads bad ranges from the drive. Idempotent; call N times. | | `DiscStream` (ISO source) | Reads sectors from the ISO, feeds decrypt → demux → codec → mux. | -The caller orchestrates. Autorip's `rip_disc` loops `copy` then N × `patch` per -the `MAX_RETRIES` config, then hands the ISO off to the existing mux pipeline. - ## Data model ### Mapfile @@ -26,7 +38,7 @@ plain text, greppable, tool-interoperable. Flushed to disk on every `record()` so a crashed rip loses at most one block. ``` -# Rescue Logfile. Created by libfreemkv v0.11.22 +# Rescue Logfile. Created by libfreemkv v0.13.6 # Current pos / status / pass / pass_time 0x000000000 ? 1 0 # pos size status @@ -50,8 +62,8 @@ Position and size are hex byte offsets into the ISO. ### `CopyOptions` and `PatchOptions` -Defaults preserve pre-`0.11.21` behavior — full drive recovery on every read, -abort on the first unreadable sector. Opt in to the recovery-friendly path: +Defaults preserve pre-`0.11.21` behavior — abort on the first unreadable +sector. Opt in to the recovery-friendly path: ```rust CopyOptions { @@ -65,9 +77,10 @@ CopyOptions { ## Algorithm -### Pass 1 — fast sweep +### Pass 1 — fast sweep (`Disc::copy`) -1. Read 64 KB (32 sectors, one BD ECC block) at the current LBA. +1. Read 64 KB (32 sectors, one BD ECC block) at the current LBA via + `Drive::read(.., recovery=false)` — short 1.5 s timeout, single shot. 2. On success: mark the range `+`, advance by one block. 3. On failure (with `skip_on_error`): zero-fill the block in the ISO, mark it `*`, advance. @@ -80,12 +93,13 @@ CopyOptions { Pass 1 completes when every byte has terminal status (`+`, `-`, or the caller bails via the halt flag). -### Pass 2+ — patch +### Pass 2+ — patch (`Disc::patch`) `Disc::patch` reads the mapfile and iterates every non-`+` range. For each: -1. Issue a drive read with full recovery enabled (SCSI-level retries, - ECC recovery, the lot). +1. Issue a drive read via `Drive::read(.., recovery=true)` — long 30 s + timeout, still single shot. Drive firmware does its own ECC and retries + inside that window; userspace does not pile on additional retries here. 2. On success: write the good bytes into the ISO at the exact byte offset, mark `+`. 3. On failure: mark `-`. @@ -95,21 +109,61 @@ Idempotent. Call `patch` N times for N retry attempts; typically the caller stops early if a pass recovers zero bytes (structure-protected sectors will never yield). +### In-stream — adaptive batch halving (`DiscStream::fill_extents`) + +When a consumer reads a `DiscStream` directly (no ISO intermediate), +`fill_extents` runs an adaptive sizer in front of `Drive::read`: + +1. Try the current preferred batch size (e.g. 32 sectors, one BD ECC block). +2. On failure: halve the batch and retry at the same LBA. Emit + `EventKind::BatchSizeChanged { reason: Shrunk }`. +3. On a clean-read streak: probe back up toward the preferred size. Emit + `EventKind::BatchSizeChanged { reason: Probed }`. +4. If a single-sector read fails: skip (zero-fill, emit + `EventKind::SectorSkipped`) when `skip_errors` is set, otherwise return + `Err(DiscRead)`. + +This is layer 3. It exists so a transient single-sector glitch in a 32-sector +batch can be isolated and read individually without the caller needing to +implement retry logic. + ## Design choices +**`Drive::read` is single-shot.** No inline retry phases, no SCSI reset, +no eject cycle. The `recovery` flag controls only the per-CDB timeout +(1.5 s vs. 30 s); on any failure it returns `Err(DiscRead)` immediately. +Inline recovery (5× gentle retry → close + SCSI reset + reopen → 5× more) +was removed in 0.13.6. See `freemkv-private/postmortems/2026-04-25-stop-wedge-and-zero-kbs.md` +for rationale: the inline reset on the LG BU40N (Initio USB-SATA bridge) +wedged drive firmware below the bridge without ever recovering a sector, +and the gentle-retry phase produced long stretches of 0 KB/s with no +recoveries to show for it. Recovery responsibility is now layered: layer 1 +handles ranges, layer 3 handles request size, neither touches the +wedge-prone reset path. + **No `MODE SELECT` to disable drive retries.** Research showed neither ddrescue nor MakeMKV does this. Drive firmware has access to raw analog signal, laser power control, and drive-specific ECC tuning that userspace can't replicate — disabling it throws away recovery headroom on marginal sectors. We fail fast -via short SG_IO timeouts instead, and we avoid per-sector probing on first -contact by using large blocks + skip-forward. +via short SG_IO timeouts in pass 1 and let the firmware work the long timeout +in pass 2 / patch. + +**No SCSI reset from any retry path.** `SgIoTransport::reset` (Linux) is +trimmed to a kernel SG_IO state flush plus ALLOW MEDIUM REMOVAL — the +`SG_SCSI_RESET` ioctl and STOP/START UNIT escalation were removed in 0.13.6. +The macOS reset (which had been a no-op) was removed entirely. The top-level +`scsi::reset()` / `reset_with_timeout()` / `reset_blocking()` wrappers were +also removed (no callers). The remaining `Drive::reset()` is only invoked +explicitly by callers that need an eject-cycle escape hatch — it is never +reached from a read path. **ISO intermediate, even for single-pass.** Pass 1 always writes an ISO. The mux stage reads the ISO via `IsoSectorReader`. For single-pass (no retries), this adds ~2-3 min (local disk mux) but gains resumability across crashes, re-muxability without re-ripping, and a persistent forensic artifact. Callers who need pure speed can bypass and use `DiscStream::new(Box::new(drive), …)` -directly — the lib doesn't forbid it. +directly — the lib doesn't forbid it, and layer 3 (adaptive batch halving) +still applies there. **Mapfile in ddrescue format.** Plain text so users can `less` it, `diff` it, or feed it to ddrescue's own tooling. Crash-safe (flush-per-record). Entries @@ -117,11 +171,11 @@ coalesce on adjacent same-status ranges so files stay small. **Patches target `-`, `*`, `/`, and `?` alike.** The status state machine is ddrescue's but `patch` collapses the distinction — it just tries every -non-finished range with full recovery. Future work can specialize (trim vs. +non-finished range with the long timeout. Future work can specialize (trim vs. scrape vs. retry with direction reversal) if there's measured benefit. ## References - [ddrescue manual, Algorithm chapter](https://www.gnu.org/software/ddrescue/manual/ddrescue_manual.html) - [ddrescue optical media notes](https://www.electric-spoon.com/doc/gddrescue/html/Optical-media.html) -- Source: [`src/disc/mapfile.rs`](../src/disc/mapfile.rs), [`src/disc/mod.rs`](../src/disc/mod.rs) (`Disc::copy`, `Disc::patch`) +- Source: [`src/disc/mapfile.rs`](../src/disc/mapfile.rs), [`src/disc/mod.rs`](../src/disc/mod.rs) (`Disc::copy`, `Disc::patch`), [`src/drive/mod.rs`](../src/drive/mod.rs) (`Drive::read`), [`src/mux/disc.rs`](../src/mux/disc.rs) (`DiscStream::fill_extents`). diff --git a/src/mux/disc.rs b/src/mux/disc.rs index bb91e8d..4556f26 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -157,8 +157,7 @@ impl DiscStream { content_format: crate::disc::ContentFormat, ) -> Self { let extents = title.extents.clone(); - let bytes_total_extents: u64 = - extents.iter().map(|e| e.sector_count as u64 * 2048).sum(); + let bytes_total_extents: u64 = extents.iter().map(|e| e.sector_count as u64 * 2048).sum(); let mut pids = Vec::new(); let mut parsers = Vec::new(); diff --git a/tests/integration_progress_and_halt.rs b/tests/integration_progress_and_halt.rs index 56ffb97..494c7b5 100644 --- a/tests/integration_progress_and_halt.rs +++ b/tests/integration_progress_and_halt.rs @@ -199,7 +199,10 @@ fn test_disc_copy_progress_callback_fires() { let n = calls.load(Ordering::Relaxed); let last = last_bytes.load(Ordering::Relaxed); assert!(n > 0, "on_progress should fire at least once, got {n}"); - assert!(last > 0, "final progress bytes should be non-zero, got {last}"); + assert!( + last > 0, + "final progress bytes should be non-zero, got {last}" + ); } // ── 3. Halt aborts disc copy promptly ─────────────────────────────────────