Two fixes from round 5.
The Linux and Windows backends truncated a CDB longer than 16 bytes
(`cdb.len().min(16)`) where macOS returned InvalidCdbLength. Under SPC-4 a
command's length is fixed by its opcode group code, so a shortened CDB is
not a shorter form of the same command — it is a DIFFERENT command, and
the drive executes it and answers GOOD with data for a request nobody
made. A silently wrong result on the layer everything else sits on.
Rather than mirror the guard a third time it now lives in scsi::mod as
checked_cdb_len, with all three backends routed through it, so it cannot
drift per platform again. That also makes it testable everywhere: each
platform module is cfg-gated to its own host, so a guard inlined into
linux.rs and windows.rs would have had no test coverage on any single
machine. The shared helper is the only place the behaviour can be
asserted on every platform's CI.
The two existing macOS tests were tautological — they replicated the
guard's logic inline instead of calling it, so they would have passed with
the guard deleted. They now call the real helper.
Separately, the H.264 keyframe parameter-set re-assert grew a
few-hundred-byte prefix buffer to the full access-unit size, copied the
whole frame into it, and dropped the presized buffer: one extra
whole-frame allocation and copy per keyframe. A UHD title is ~200,000
frames of 150-400 KB with a keyframe every second or two, so that is
thousands of avoidable multi-hundred-KB copies per title, each large
enough to go through mmap. It now splices into the reserved headroom in
place. This mirrors the identical fix already made in hevc.rs, which the
H.264 path had drifted from.
Byte-for-byte equivalence is pinned by a test whose expected literals
were captured from the pre-change implementation, and which I confirmed
still passes when the old build-and-copy code is restored. The
no-reallocation claim is measured rather than argued: a counter over 30
bare keyframes, which reports 30 of 30 against the old path and 0 with
the splice.
The reallocation test initially passed even with PARAM_REASSERT_HEADROOM
set to zero, because a small parameter set fits in the presize's
incidental slack — it proved the fixture did not reallocate, not that the
headroom prevented it. Its SPS is now large enough that the constant is
load-bearing, so zeroing it fails the test.
Not verified: no runtime behaviour on Linux or Windows: no drive, no
ioctl. Both files were confirmed to compile for their own targets.
Audited every #[repr(C)] struct and IOCTL/flag constant in scsi/windows.rs
against the authoritative Windows SDK headers (ntddscsi.h, winioctl.h,
devioctl.h, winnt.h, fileapi.h). All correct except the already-reverted
ScsiPassThroughDirect packing. Add the missing regression guards:
- StoragePropertyQuery layout (STORAGE_PROPERTY_QUERY: 0/4/8, size 12).
- IOCTL/flag constants, with IOCTLs asserted against an independent CTL_CODE
re-derivation (not a tautological literal) so a mistyped code is caught.
Validated compiling via cargo xwin check --target x86_64-pc-windows-msvc.
rc.4 added #[repr(C, packed(4))] to ScsiPassThroughDirect on the false premise
that ntddscsi.h wraps SCSI_PASS_THROUGH_DIRECT in #pragma pack(push, 4). It does
NOT — verified against the Windows SDK ntddscsi.h: the struct has no pragma pack
and uses natural alignment. On 64-bit Windows (LLP64) that puts DataBuffer at
offset 24 and the struct at 56 bytes, which bare #[repr(C)] produces and which
DeviceIoControl expects.
packed(4) instead imposed offset 20 / 48 bytes — the layout of the SDK's
SEPARATE 32-bit thunk struct SCSI_PASS_THROUGH_DIRECT32 (VOID* POINTER_32). Using
that on a 64-bit host malformed every IOCTL_SCSI_PASS_THROUGH_DIRECT, so the
INQUIRY in drive enumeration failed and autorip/CLI reported zero drives
('RC4 no longer detects my drive'). rc.3.1 (bare repr(C)) worked for the same
users; this restores that layout.
Replace the tautological packed-layout test (which asserted the same wrong
offsets the struct produced) with one cross-checked against the SDK header:
DataBuffer@24, SenseInfoOffset@32, Cdb@36, size 56. Verified compiling via
cargo xwin check for x86_64-pc-windows-msvc.
ntddscsi.h wraps SCSI_PASS_THROUGH_DIRECT in #pragma pack(push, 4),
forcing the PVOID DataBuffer field to 4-byte alignment even on 64-bit
hosts. The Rust struct used bare #[repr(C)], so the compiler applied
natural 8-byte pointer alignment and inserted 4 padding bytes after
TimeOutValue. That shifted DataBuffer to offset 24 (SDK: 20),
SenseInfoOffset to 32 (28), and Cdb to 36 (32), and grew the struct to
56 bytes (48). DeviceIoControl reads at the SDK offsets, so every SPTI
ioctl on 64-bit Windows either got rejected or interpreted garbage as
the CDB and DataBuffer pointer.
Add #[repr(C, packed(4))] to ScsiPassThroughDirect and the companion
SptwbDirect (so offset_of!(SptwbDirect, sense) stays correct for
SenseInfoOffset), plus a layout regression test asserting DataBuffer at
offset 20 and a 48-byte struct size.
STORAGE_ADAPTER_DESCRIPTOR.BusType is STORAGE_BUS_TYPE, an int-sized
(4-byte) enum, but the repr(C) struct declared it as u8. The total
size stayed 32 bytes by coincidence of alignment padding, and the two
fields actually read today (MaximumTransferLength at offset 8 and
AlignmentMask at offset 16) sit ahead of BusType, so there was no
runtime impact. But BusMajorVersion and BusMinorVersion landed at
offsets 26 and 28 instead of the SDK's 28 and 30, so any future reader
of those fields would have gotten wrong values.
Widen BusType to u32 so BusMajorVersion/BusMinorVersion fall at the
correct offsets, and add a layout regression test asserting every
field offset and the 32-byte total against the winioctl.h layout.
SptiTransport::reset() unconditionally slept 2 seconds after sending
IOCTL_STORAGE_RESET_DEVICE, even when the IOCTL failed (e.g.
ERROR_INVALID_FUNCTION on a driver that does not support the reset).
On failure no reset occurred, so there is nothing to settle and the
2-second penalty was pure waste. Gate the settle sleep on the IOCTL
return so it only fires when the drive was actually reset.
SptiTransport::reset() discarded the DeviceIoControl return value, so a
wrong or unsupported reset IOCTL would fail with ERROR_INVALID_FUNCTION
and silently no-op while the unconditional 2s settle sleep made it look
like a reset happened. That is exactly the regression class the doc block
records for the two earlier (incorrect) code values.
Bind the result and warn (with GetLastError) when the reset fails, debug
on success. Lift IOCTL_STORAGE_RESET_DEVICE to module scope and add a
test recomputing it from the CTL_CODE formula so a wrong value can't slip
back in unnoticed.
The dual-model Windows audit (Sonnet) caught that the prior 'fix' (0x002DD000)
was also wrong: that decodes to the OBSOLETE RESET_BUS code (function 0x400,
R|W access) which class drivers reject. Canonical ntddstor.h:
IOCTL_STORAGE_RESET_DEVICE = CTL_CODE(0x2D, 0x0401, METHOD_BUFFERED, FILE_READ_ACCESS)
= (0x2D<<16)|(1<<14)|(0x401<<2)|0 = 0x002D5004.
Only affects the best-effort drive-reset recovery path (return value is
discarded); the normal read/unlock/rip path never calls it, so this does not
change normal Windows operation. Verified correct-by-construction; the
AlignmentMask finding is deferred to rc4 (benign on USB optical bridges,
needs a real Windows SCSI-HBA rig to validate).
- patch (Pass N) now aborts immediately on transport failure (status=0xFF),
symmetric with the sweep and single-pass mux. Previously a USB-bridge crash
was treated as an ordinary bad sector and the pass hammered the crashed
device sector-by-sector until the per-range watchdog expired. (medium)
- patch AACS recovery reads are now unit-aligned: a mid-unit single-sector
read on an AACS disc was rejected by the decrypting reader (DecryptFailed)
and the sector abandoned without asking the drive. The read is now widened
to the enclosing whole 3-sector unit and the requested window copied out,
leaving all recovery accounting (pos/block_bytes/cursor) untouched so it
cannot desync. Only affected CLI decrypt-to-ISO --multipass re-runs. (low)
- IOCTL_STORAGE_RESET_DEVICE corrected 0x002D1004 -> 0x002DD000 (the old value
decoded to function 0x401 with the access bits cleared, so DeviceIoControl
would fail ERROR_INVALID_FUNCTION instead of resetting). Windows-only. (low)
Adds a transport-failure classification regression test.
Three fixes for a Windows ASUS Blu-ray drive that failed/spammed errors:
- resolve.rs: accept disk:// as an alias for disc:// (identical behavior;
empty = auto-detect, path = device). Windows users commonly type
disk://i: after the drive-letter convention.
- drive::find_drive: prefer a drive that reports media present. Enumerate
all optical drives, query Drive::drive_status() (GET EVENT STATUS, works
regardless of firmware), and return the first reporting DiscPresent;
fall back to the first enumerated drive when none report a disc so
single-drive / quirky setups don't regress. Selection policy split into
select_drive_with_media() for unit testing.
- READ chunking: add ScsiTransport::max_transfer_bytes() (default 1 MiB).
Windows SPTI overrides it with the adapter MaximumTransferLength queried
via IOCTL_STORAGE_QUERY_PROPERTY / StorageAdapterProperty, clamped to a
64 KiB floor (fallback on query failure). Drive::read now caps each
READ(10) to that limit: small reads take the unchanged single-CDB path,
larger reads loop over read_one() chunks, reporting the failing chunk's
LBA on error. This stops the 16 MiB single read that exceeded the
adapter limit, made DeviceIoControl fail, and spammed transport-failure
warnings with slow tiny-read fallbacks.
Tests added for the disk:// alias, media-preference selection, and READ
chunk decomposition / per-chunk error LBA.
Library-wide review-and-fix pass: tightened AACS keydb/handshake/variant
handling and trailing-partial-unit policy, corrected MPLS mark offset and
added UDF allocation bounds, hardened the mux/codec framing and M2TS paths,
guarded SCSI READ CAPACITY short transfers and unified error mapping, added
overflow guards on untrusted disc input, and made prefetch shutdown
deterministic. Release profile now builds with thin LTO + single codegen unit.
Through the entire 0.13.x line, every CHECK CONDITION reply from the
drive (the standard way SCSI tells you why a sector failed) was being
collapsed into a synthetic status=0xFF, sense_key=0 transport-wedge
sentinel and the actual sense data was thrown away. Confirmed live on
the BU40N reading Dune 2 on 2026-04-27: drive returned host_status=0,
driver_status=8, status=2, exec_elapsed_ms=1416 on every bad sector
— a clean CHECK CONDITION carrying full sense data — and Disc::copy
was bailing on it as if the bridge had wedged.
Root cause: scsi/linux.rs's wedge check was
`host_status != 0 || driver_status != 0`
SG's DRIVER_SENSE bit (0x08) is set on every CHECK CONDITION reply
just to flag "sense buffer is populated" — it's not a transport
failure on its own. Pre-fix we conflated the two and silently lost
every drive-reported error reason. macOS and Windows backends had
the same shape: they extracted sense_key only, dropping ASC/ASCQ.
API restructure (clean separation):
Error::ScsiError {
opcode: u8,
status: u8, // 0xFF = synthetic transport-failure
sense: Option<ScsiSense>, // None ⇔ no sense delivered
}
pub struct ScsiSense { sense_key: u8, asc: u8, ascq: u8 }
impl ScsiSense {
pub fn is_marginal(&self) -> bool // keys 0/1/3/B
pub fn is_medium_error(&self) -> bool
pub fn is_hardware_error(&self) -> bool
pub fn is_unit_attention(&self) -> bool
pub fn is_data_protect(&self) -> bool
pub fn is_not_ready(&self) -> bool
pub fn is_illegal_request(&self) -> bool
pub fn is_aborted_command(&self) -> bool
}
impl Error {
pub fn scsi_sense(&self) -> Option<&ScsiSense>
pub fn is_scsi_transport_failure(&self) -> bool
pub fn is_marginal_read(&self) -> bool
}
SCSI protocol constants (SCSI_STATUS_*, SENSE_KEY_*) moved from
error.rs to scsi/mod.rs where they belong alongside SCSI_INQUIRY,
SCSI_READ_10, etc. parse_sense replaces parse_sense_key (returns the
full triple, not just the key); inline tests now exercise ASC/ASCQ
extraction at the right offsets for both descriptor (0x72/0x73) and
fixed (0x70/0x71) sense formats.
Disc::copy + Disc::patch sense-aware dispatch:
- marginal sense (MEDIUM ERROR / ABORTED COMMAND / RECOVERED ERROR
/ NO SENSE) → engage hysteresis (Block→Single, bpt=1)
- non-marginal sense (HARDWARE / DATA PROTECT / UNIT ATTENTION /
NOT READY / ILLEGAL REQUEST / transport failure / kernel
IoError) → bail with full sense info preserved; caller (autorip)
surfaces "physical replug" / "drive failing" / "media changed"
Pre-fix: every CHECK CONDITION → 0xFF synthetic → Disc::copy bailed
→ bytes_good froze at the bad zone. The hysteresis from v0.13.22
was correct but never got to run. This release unblocks it.
Disc::patch's wedged_threshold (50 consecutive failures) stays as
defense-in-depth for chains of marginal failures; a single
non-marginal sense now short-circuits it.
New phase=bail trace event records the bail reason with the sense
triple. phase=transport_err remains for genuine bridge wedges /
kernel timeouts; phase=scsi_err carries the parsed sense_key, asc,
ascq for drive-reported errors.
All 350 tests pass. Clippy clean across all targets.
- scsi/linux.rs: full rewrite from async write/poll/read+1.5s timeout+
close-on-timeout to one synchronous ioctl(fd, SG_IO, &hdr). Kernel
honors hdr.timeout and runs its own ABORT/RESET escalation. Errors
check host_status and driver_status (both 0xFF-synthesised) plus
status. Sense-key parser handles descriptor (0x72/0x73) + fixed
(0x70/0x71) formats. Deleted fd_recovery, bg close+open thread, fd
swap dance. -331/+155 lines.
- scsi/macos.rs: try_recover() removed (userspace handle-recovery on
task failure was the same anti-pattern stripped from Linux). bsd_name
field deleted. Errors bubble up directly.
- scsi/windows.rs: try_recover() removed, wide_path field deleted,
INVALID_HANDLE guard removed.
- scsi/mod.rs: parse_sense_key() helper extracted (used by all three
platforms now — single canonical sense-key parse rather than three
inlined copies). +10 unit tests covering descriptor format, fixed
format, truncated buffers, unknown response codes.
- drive/mod.rs: Drive::reset() deleted (escalating eject + STOP/START +
reinit recovery — per audit, kernel handles its own escalation;
userspace shouldn't).
pub fn find_drives() -> Vec<Drive> deleted (opened N drives just to
throw most away). find_drive() now uses discover_drives() directly.
wait_ready() simplified — drops the reset path on sense_key=5,
just keeps polling TUR for 60 iterations.
- lib.rs: find_drives re-export removed.
- benches/sgio_read.rs: switched to find_drive() (no longer iterates a
drive list).
Net: 9 files changed, 226 insertions(+), 473 deletions(-). 329 tests
pass, clippy -D warnings clean. No consumer breakage (CLI, autorip,
bdemu compile + test green).
Architecture decision documented in
(internal)/docs/audits/2026-04-26-scsi-architecture-research.md
(primary-source survey of MakeMKV, sg_dd, ddrescue, and the kernel
mid-layer's own scsi_eh.rst escalation ladder).
Fix 1: delete stall guard from Disc::copy. Pass 1 must sweep end-to-end
per ddrescue model (RIP_DESIGN.md §2.1, §3, §9). The v0.13.9 guard at
disc/mod.rs broke Pass 1 at 30% on Dune 2 with 56 GB still NonTried.
Removed stall_secs field, narrative comment in scsi/linux.rs, and the
broken regression test. Replaced with test_disc_copy_completes_full_disc_
with_failing_reader and test_disc_copy_halts_promptly_on_failing_reader.
Fix 2: async SCSI transport recovery. Added Arc<AtomicI32> fd_recovery
on SgIoTransport. On poll timeout: spawn close + spawn open in
background, return Err immediately. Top of execute() swaps fd from
recovery atomic. Main thread never blocked beyond ~1.5s poll budget
(was up to ~60s per timeout because kernel serialized main-thread
open() against in-flight close()). Drop drains pending recovery fd.
§15.1 cross-platform parity: Windows + macOS now have the same
observable recovery contract. SptiTransport gets try_recover()
(synchronous CloseHandle + CreateFileW; Windows close is fast, no
in-flight CDB drain like Linux). MacScsiTransport gets try_recover()
(release IOKit interface + reacquire via new acquire_device_iface()
helper); stores bsd_name for re-resolution. Drop guards null'd-out
interfaces. Stripped English error strings ("try as root" / "run as
administrator") on Linux + Windows. Fixed Windows TimeOutValue
ms→s ceiling so 1500ms gets 2s (was 1s; broke Drive::read fast path).
Fix 4: instrument Disc::patch arms. PatchResult exposes
blocks_attempted, blocks_read_ok, blocks_read_failed so the v0.13.11
mystery (Dune 2 Pass 2 recovered 0 bytes in 100 min) is diagnosable
from the live device log without re-instrumenting from outside.
Cleanup: honor PatchOptions::full_recovery (was read into _ and
ignored; now routed to read_sectors recovery arg). Updated
CopyOptions::batch_sectors doc to describe the actual production
path (sysfs detect_max_batch_sectors, typically 60 sectors / ~120 KB
on BU40N) rather than the test-only 32-sector internal default.
All four crates clippy-clean and tests green on the host targets
(macOS native + cargo check on Linux). Cross-platform CI watches
Linux + Windows + macOS builds + tests.
USB/SCSI recovery escalation in drive_has_disc (0.13.1-0.13.3) tested
on LG BU40N USB BD-RE: USBDEVFS_RESET, authorized toggle, driver
unbind/rebind, SCSI host rescan — all succeed at the USB transport
layer but the drive firmware below the bridge stays locked. Only
physical unplug-replug clears it. Rolled back so consumers can
surface the real failure to the user.
New: list_drives falls back to sysfs-cached vendor/model/rev from
/sys/class/scsi_generic/sgN/device/ when live INQUIRY returns empty,
so wedged drives still show their identity in UIs.
Removed: scsi::usb_reset, usb_reset_with_timeout, per-platform
usb_reset methods, recover_then_probe, is_wedge_signature. Breadcrumb
comment in scsi/linux.rs::drive_has_disc points at v0.13.3 tag for
the full implementation if future hardware needs it back.
Linux/macOS/Windows pass-through symmetric; 233 tests passing.
Architectural cleanup. autorip + freemkv CLI were reimplementing drive
discovery (sysfs walking, type-5 filtering, sg-path construction) and
calling SCSI reset primitives directly. All of that hardware-aware code
moves into libfreemkv with two cheap public probes:
- DriveInfo + list_drives() — multi-OS enumeration (Linux/macOS/Windows)
with peripheral-type-5 filtering and INQUIRY identity. Cheap.
- drive_has_disc(path) — single TUR with internal wedge recovery
escalation (SCSI reset → USB reset → retry) hidden from callers.
USB-layer reset (USBDEVFS_RESET / IOUSBDeviceInterface::ResetDevice /
storport's combined reset) wired across all three platforms.
Visibility tightening — scsi::reset, scsi::usb_reset, and the timeout
constants are now pub(crate). Compile-time guarantee that no consumer
crate can issue SCSI commands directly.
233 lib tests pass; clippy clean.
Matthew Jackson