v1.0.0-rc.3.1: silent-failure guards (mux empty/zero-frame, CSS crack-vs-unencrypted), Windows keydb path, AlignmentMask, English errors

src/css/mod.rs

@@ -56,6 +56,58 @@ pub fn crack_key(
     crack_key_halt(reader, extents, batch_sectors, None)
 }
 
+/// Outcome of a CSS crack scan that distinguishes the THREE cases the bare
+/// `Option<CssState>` conflated (and which caused a silent-failure bug:
+/// scrambled-but-uncracked content was treated as "unencrypted" and muxed as
+/// plaintext garbage at exit 0):
+///
+/// - [`CrackOutcome::Cracked`] — a scrambled sector yielded a title key.
+/// - [`CrackOutcome::Unencrypted`] — NO scrambled sector was seen across the
+///   scanned extents (`is_scrambled` never true): the content is genuinely
+///   plaintext, so proceeding without a key is correct.
+/// - [`CrackOutcome::ScrambledUncracked`] — scrambled sectors WERE seen but no
+///   key could be recovered (the Stevenson attack found no crackable crib, or
+///   the scrambled region was unreadable). The content is encrypted; muxing it
+///   as plaintext would emit garbage, so callers MUST surface a hard error
+///   ([`crate::error::Error::CssKeyMissing`]) instead of falling through to
+///   "unencrypted".
+#[derive(Debug, Clone)]
+pub enum CrackOutcome {
+    Cracked(CssState),
+    Unencrypted,
+    ScrambledUncracked,
+}
+
+impl CrackOutcome {
+    /// The cracked `CssState`, if any. `None` for `Unencrypted` /
+    /// `ScrambledUncracked`. Lets the `Option`-returning wrappers stay thin.
+    pub fn into_state(self) -> Option<CssState> {
+        match self {
+            CrackOutcome::Cracked(s) => Some(s),
+            _ => None,
+        }
+    }
+
+    /// True when scrambled sectors were seen but no key was recovered — the
+    /// case callers must surface as a hard error instead of "unencrypted".
+    pub fn is_scrambled_uncracked(&self) -> bool {
+        matches!(self, CrackOutcome::ScrambledUncracked)
+    }
+}
+
+/// [`crack_key`] returning the full [`CrackOutcome`] (Cracked / Unencrypted /
+/// ScrambledUncracked) so callers can distinguish "genuinely unencrypted" from
+/// "encrypted but uncrackable" — the latter must become a hard error, never a
+/// silent fall-through to plaintext.
+pub fn crack_key_outcome(
+    reader: &mut dyn SectorSource,
+    extents: &[Extent],
+    batch_sectors: u16,
+    halt: Option<&crate::halt::Halt>,
+) -> CrackOutcome {
+    crack_key_scan(reader, extents, batch_sectors, halt)
+}
+
 /// [`crack_key`] with an optional cooperative-cancellation token.
 ///
 /// "No silent hangs": the crack scans up to 50_000 sectors, which on a live
@@ -70,6 +122,19 @@ pub fn crack_key_halt(
     batch_sectors: u16,
     halt: Option<&crate::halt::Halt>,
 ) -> Option<CssState> {
+    crack_key_scan(reader, extents, batch_sectors, halt).into_state()
+}
+
+/// The crack scan, returning the full [`CrackOutcome`]. Tracks a
+/// `saw_scrambled` flag so a scrambled-but-uncracked disc is distinguished
+/// from a genuinely-unencrypted one (the [`crack_key`] / [`crack_key_halt`]
+/// `Option` wrappers collapse both to `None`).
+fn crack_key_scan(
+    reader: &mut dyn SectorSource,
+    extents: &[Extent],
+    batch_sectors: u16,
+    halt: Option<&crate::halt::Halt>,
+) -> CrackOutcome {
     // Batch the reads: a live optical drive at 1 sector/read is glacial, and the
     // crack only needs to FIND one scrambled sector whose 0x80 plaintext matches
     // a known PES header. `batch_sectors` MUST be sized to the source — a drive
@@ -90,6 +155,12 @@ pub fn crack_key_halt(
     let max_tries = 50_000u32;
     let mut buf = vec![0u8; batch as usize * 2048];
     let mut hb = crate::progress::Heartbeat::new("css_crack");
+    // Track whether ANY scrambled sector was observed. If we exhaust the scan
+    // budget having seen scrambled data but never recovered a key, the content
+    // is encrypted-but-uncrackable — a HARD failure the caller must surface,
+    // NOT silently treat as unencrypted (which would mux scrambled MPEG as
+    // plaintext → garbage at exit 0). See `CrackOutcome::ScrambledUncracked`.
+    let mut saw_scrambled = false;
 
     'outer: for (extent_idx, ext) in extents.iter().enumerate() {
         let mut i = 0u32;
@@ -122,8 +193,9 @@ pub fn crack_key_halt(
                         tried += 1;
                         let sect = &buf[s * 2048..(s + 1) * 2048];
                         if is_scrambled(sect) {
+                            saw_scrambled = true;
                             if let Some(key) = stevenson::crack_title_key(sect) {
-                                return Some(CssState {
+                                return CrackOutcome::Cracked(CssState {
                                     title_key: key,
                                     crack_span,
                                 });
@@ -142,7 +214,16 @@ pub fn crack_key_halt(
         }
     }
 
-    None
+    // Budget exhausted / extents walked with no key recovered. Distinguish the
+    // two indistinguishable-in-`Option` cases: if scrambled sectors were seen
+    // (case b: crack failed; case c: scrambled but the crackable region was
+    // unreadable), this is encrypted-but-uncracked — a hard failure. Only a
+    // scan that NEVER saw a scrambled sector is genuinely unencrypted (case a).
+    if saw_scrambled {
+        CrackOutcome::ScrambledUncracked
+    } else {
+        CrackOutcome::Unencrypted
+    }
 }
 
 /// Descramble a single CSS-encrypted sector in place.
@@ -290,6 +371,68 @@ mod tests {
         );
     }
 
+    // ── CrackOutcome: scrambled-but-uncracked vs genuinely unencrypted (Fix 6) ─
+
+    /// A scan over CLEAR sectors (scramble flag never set) returns
+    /// `Unencrypted` — the content is genuinely plaintext, so proceeding
+    /// without a key is correct.
+    #[test]
+    fn crack_outcome_clear_sectors_is_unencrypted() {
+        let mut src = MockSource::new(0x00); // never scrambled
+        let extents = [Extent {
+            start_lba: 0,
+            sector_count: 100,
+        }];
+        let outcome = crack_key_outcome(&mut src, &extents, 1, None);
+        assert!(
+            matches!(outcome, CrackOutcome::Unencrypted),
+            "no scrambled sector seen → Unencrypted, got {outcome:?}"
+        );
+        // The Option wrapper collapses Unencrypted → None.
+        assert!(crack_key(&mut MockSource::new(0x00), &extents, 1).is_none());
+    }
+
+    /// THE Fix 6 regression: a scan that SEES scrambled sectors (flag set) but
+    /// recovers no key (the mock's zeroed data has no Stevenson crib) must
+    /// return `ScrambledUncracked` — a HARD failure — NOT `Unencrypted`. The
+    /// old code conflated this with "unencrypted" and muxed scrambled MPEG as
+    /// plaintext (garbage at exit 0).
+    #[test]
+    fn crack_outcome_scrambled_uncracked_is_hard_failure() {
+        let mut src = MockSource::new(0x30); // scrambled flag set, no crackable crib
+        let extents = [Extent {
+            start_lba: 0,
+            sector_count: 100,
+        }];
+        let outcome = crack_key_outcome(&mut src, &extents, 1, None);
+        assert!(
+            outcome.is_scrambled_uncracked(),
+            "scrambled sectors seen but no key → ScrambledUncracked, got {outcome:?}"
+        );
+        // The legacy Option wrapper still collapses this to None (the callers
+        // that need the distinction now use crack_key_outcome instead).
+        assert!(crack_key(&mut MockSource::new(0x30), &extents, 1).is_none());
+    }
+
+    /// Even when every read FAILS, a scan that never managed to observe a
+    /// scrambled sector reports `Unencrypted` (we cannot prove encryption from
+    /// unreadable data alone — the AACS/keydb paths and the disc-level
+    /// `css_error` plumbing cover genuinely unreadable encrypted discs).
+    #[test]
+    fn crack_outcome_all_reads_fail_is_unencrypted() {
+        let mut src = MockSource::new(0x30);
+        src.fail_all = true; // no sector is ever inspected
+        let extents = [Extent {
+            start_lba: 0,
+            sector_count: 10,
+        }];
+        let outcome = crack_key_outcome(&mut src, &extents, 1, None);
+        assert!(
+            matches!(outcome, CrackOutcome::Unencrypted),
+            "no readable scrambled sector → Unencrypted, got {outcome:?}"
+        );
+    }
+
     /// The budget spans ALL extents, not per-extent: two extents summing past
     /// the cap must still stop at 50_000 total reads.
     ///

src/disc/mod.rs

@@ -60,6 +60,16 @@ pub struct Disc {
     /// "disc hash not in KEYDB", etc. None when AACS resolution wasn't
     /// attempted (unencrypted disc) or succeeded.
     pub aacs_error: Option<crate::error::Error>,
+    /// CSS crack failure: `Some(Error::CssKeyMissing)` when the scan SAW
+    /// scrambled sectors but could NOT recover a title key (the
+    /// known-plaintext attack found no crackable crib, or the scrambled
+    /// region was unreadable). `css` is `None` in that case — but the disc is
+    /// genuinely encrypted, so callers MUST surface this hard error rather
+    /// than treat `css.is_none()` as "unencrypted" and mux scrambled MPEG as
+    /// plaintext garbage. `None` when no scrambled sector was seen (genuinely
+    /// unencrypted) or a key was recovered (`css.is_some()`). The CSS analogue
+    /// of [`Self::aacs_error`].
+    pub css_error: Option<crate::error::Error>,
     /// Content format (BD transport stream vs DVD program stream)
     pub content_format: ContentFormat,
 }
@@ -1343,7 +1353,7 @@ impl Disc {
                 let crack_batch = detect_max_batch_sectors(session.device_path());
                 tracing::info!(target: "freemkv::scan", crack_batch, "phase: CSS — known-plaintext crack");
                 let crack_t0 = std::time::Instant::now();
-                let crack_result = crate::css::crack_key_halt(
+                let crack_result = crate::css::crack_key_outcome(
                     session,
                     &main_extents,
                     crack_batch,
@@ -1352,15 +1362,27 @@ impl Disc {
                 tracing::info!(
                     target: "freemkv::scan",
                     elapsed_ms = crack_t0.elapsed().as_millis() as u64,
-                    found = crack_result.is_some(),
+                    outcome = ?crack_result,
                     "phase: CSS — crack done"
                 );
-                if let Some(state) = crack_result {
-                    tracing::debug!(target: "freemkv::disc", "dvd css: title key recovered via known-plaintext crack");
-                    disc.css = Some(state);
-                    disc.encrypted = true;
-                } else {
-                    tracing::warn!(target: "freemkv::disc", "dvd css: no crackable scrambled sector (unencrypted or atypical layout)");
+                match crack_result {
+                    crate::css::CrackOutcome::Cracked(state) => {
+                        tracing::debug!(target: "freemkv::disc", "dvd css: title key recovered via known-plaintext crack");
+                        disc.css = Some(state);
+                        disc.encrypted = true;
+                    }
+                    crate::css::CrackOutcome::ScrambledUncracked => {
+                        // Scrambled sectors WERE seen but no key could be
+                        // recovered — the content is encrypted-but-uncrackable.
+                        // Record a hard error so callers fail loudly instead of
+                        // muxing scrambled MPEG as plaintext garbage at exit 0.
+                        tracing::warn!(target: "freemkv::disc", "dvd css: scrambled sectors seen but no title key cracked");
+                        disc.encrypted = true;
+                        disc.css_error = Some(crate::error::Error::CssKeyMissing);
+                    }
+                    crate::css::CrackOutcome::Unencrypted => {
+                        tracing::debug!(target: "freemkv::disc", "dvd css: no scrambled sector seen (genuinely unencrypted)");
+                    }
                 }
             }
         }
@@ -1405,10 +1427,21 @@ impl Disc {
             };
             if !main_extents.is_empty() {
                 // Image reads aren't drive-batch-limited; use a generous batch.
-                if let Some(state) = crate::css::crack_key(reader, &main_extents, 32) {
-                    tracing::info!(target: "freemkv::scan", "image css: title key recovered via known-plaintext crack");
-                    disc.css = Some(state);
-                    disc.encrypted = true;
+                match crate::css::crack_key_outcome(reader, &main_extents, 32, None) {
+                    crate::css::CrackOutcome::Cracked(state) => {
+                        tracing::info!(target: "freemkv::scan", "image css: title key recovered via known-plaintext crack");
+                        disc.css = Some(state);
+                        disc.encrypted = true;
+                    }
+                    crate::css::CrackOutcome::ScrambledUncracked => {
+                        // Scrambled image data with no recoverable key — a hard
+                        // failure, surfaced so the mux path doesn't pass scrambled
+                        // MPEG through as plaintext (garbage at exit 0).
+                        tracing::warn!(target: "freemkv::scan", "image css: scrambled sectors seen but no title key cracked");
+                        disc.encrypted = true;
+                        disc.css_error = Some(crate::error::Error::CssKeyMissing);
+                    }
+                    crate::css::CrackOutcome::Unencrypted => {}
                 }
             }
         }
@@ -1599,6 +1632,9 @@ impl Disc {
             css,
             encrypted,
             aacs_error,
+            // CSS crack runs AFTER scan_with returns (in `scan` / `scan_image`),
+            // which set this when they observe scrambled-but-uncracked content.
+            css_error: None,
             content_format,
         })
     }
@@ -3513,6 +3549,7 @@ mod tests {
             css: None,
             encrypted: false,
             aacs_error: None,
+            css_error: None,
             content_format: ContentFormat::BdTs,
         };
         let gb = disc.capacity_gb();
@@ -3599,6 +3636,7 @@ mod tests {
             css: None,
             encrypted: false,
             aacs_error: None,
+            css_error: None,
             content_format: ContentFormat::BdTs,
         }
     }

src/error.rs

@@ -124,6 +124,11 @@ pub const E_M2TS_PACKET_MALFORMED: u16 = 9021;
 /// connect to (every resolved IP was loopback / private / link-local /
 /// multicast / unspecified). Closes the DNS-rebinding SSRF window.
 pub const E_NETWORK_ADDR_BLOCKED: u16 = 9022;
+/// A muxer's `finish()` was called after zero frames were emitted — the
+/// output would be a header-only container with no media. Surfaced so a
+/// zero-frame mux (undecryptable input, fully-unreadable title, every
+/// frame dropped before the first keyframe) cannot report success.
+pub const E_MUX_EMPTY: u16 = 9023;
 pub const E_EXTENT_NOT_UNIT_ALIGNED: u16 = 9030;
 /// READ CAPACITY returned a short or overflowing transfer.
 pub const E_DISC_CAPACITY_MALFORMED: u16 = 9047;
@@ -368,6 +373,13 @@ pub enum Error {
     NetworkAddrBlocked {
         addr: String,
     },
+    /// A muxer's `finish()` was reached after zero frames were written, so
+    /// the output would be a header-only container with no media. Surfaced
+    /// (instead of writing a valid-but-empty file and reporting success) so
+    /// a zero-frame mux — undecryptable input, a fully-unreadable title, or
+    /// every frame dropped before the first keyframe — fails loudly. The
+    /// `m2ts://` analogue of [`Error::MkvInvalid`]'s zero-frame guard.
+    MuxEmpty,
     PesFrameTooLarge {
         size: usize,
     },
@@ -515,6 +527,7 @@ impl Error {
             Error::StreamUrlMissingPath { .. } => E_STREAM_URL_MISSING_PATH,
             Error::StreamUrlMissingPort { .. } => E_STREAM_URL_MISSING_PORT,
             Error::NetworkAddrBlocked { .. } => E_NETWORK_ADDR_BLOCKED,
+            Error::MuxEmpty => E_MUX_EMPTY,
             Error::PesFrameTooLarge { .. } => E_PES_FRAME_TOO_LARGE,
             Error::PesInvalidMagic => E_PES_INVALID_MAGIC,
             Error::PesTrackTooLarge { .. } => E_PES_TRACK_TOO_LARGE,
@@ -739,6 +752,9 @@ impl From<Error> for std::io::Error {
             // 9022 NetworkAddrBlocked: the output host resolved only to
             // blocked (loopback/private/link-local) addresses — refuse.
             E_NETWORK_ADDR_BLOCKED => std::io::ErrorKind::PermissionDenied,
+            // 9023 MuxEmpty: finish() reached with zero frames — the output
+            // would be a header-only container. Treat as invalid output.
+            E_MUX_EMPTY => std::io::ErrorKind::InvalidData,
             // 9030 ExtentNotUnitAligned: a malformed/non-AACS-aligned
             // extent was handed to the prefetch producer.
             9030 => std::io::ErrorKind::InvalidInput,
@@ -1109,6 +1125,7 @@ mod tests {
             E_STREAM_URL_MISSING_PATH,
             E_STREAM_URL_MISSING_PORT,
             E_NETWORK_ADDR_BLOCKED,
+            E_MUX_EMPTY,
             E_PES_FRAME_TOO_LARGE,
             E_PES_INVALID_MAGIC,
             E_PES_TRACK_TOO_LARGE,
@@ -1195,6 +1212,7 @@ mod tests {
             (Error::SweepConsumerGone, E_SWEEP_CONSUMER_GONE),
             (Error::PipelineConsumerGone, E_PIPELINE_CONSUMER_GONE),
             (Error::DiscCapacityOverflow, E_DISC_CAPACITY_OVERFLOW),
+            (Error::MuxEmpty, E_MUX_EMPTY),
             (Error::M2tsPacketMalformed, E_M2TS_PACKET_MALFORMED),
             (Error::ExtentNotUnitAligned, E_EXTENT_NOT_UNIT_ALIGNED),
             (Error::DiscCapacityMalformed, E_DISC_CAPACITY_MALFORMED),

src/keydb.rs

@@ -43,15 +43,41 @@ fn read_capped_to_string<R: Read>(reader: R) -> Result<String> {
     String::from_utf8(buf).map_err(|_| Error::KeydbParse)
 }
 
-/// Standard keydb storage path.
+/// Standard keydb storage path — the canonical location to write the keydb to.
+///
+/// On Windows this is the idiomatic per-user roaming dir
+/// `%APPDATA%\freemkv\keydb.cfg`, falling back to the legacy
+/// `%USERPROFILE%\.config\freemkv\keydb.cfg` only if `APPDATA` is unset. On
+/// Linux/macOS it stays the long-standing `$HOME/.config/freemkv/keydb.cfg`.
+///
+/// The CLI's read-side search (first existing of several locations) lives in
+/// `freemkv-keysources::keydb_search_paths`; this function is the single
+/// *write* default used by `save`/`update`, kept in lock-step with that crate's
+/// `default_keydb_path` for the same OS.
 pub fn default_path() -> Result<PathBuf> {
-    let home = std::env::var("HOME")
-        .or_else(|_| std::env::var("USERPROFILE"))
-        .map_err(|_| Error::KeydbParse)?;
-    Ok(PathBuf::from(home)
-        .join(".config")
-        .join("freemkv")
-        .join("keydb.cfg"))
+    #[cfg(windows)]
+    {
+        if let Ok(appdata) = std::env::var("APPDATA") {
+            if !appdata.is_empty() {
+                return Ok(PathBuf::from(appdata).join("freemkv").join("keydb.cfg"));
+            }
+        }
+        let profile = std::env::var("USERPROFILE").map_err(|_| Error::KeydbParse)?;
+        Ok(PathBuf::from(profile)
+            .join(".config")
+            .join("freemkv")
+            .join("keydb.cfg"))
+    }
+    #[cfg(not(windows))]
+    {
+        let home = std::env::var("HOME")
+            .or_else(|_| std::env::var("USERPROFILE"))
+            .map_err(|_| Error::KeydbParse)?;
+        Ok(PathBuf::from(home)
+            .join(".config")
+            .join("freemkv")
+            .join("keydb.cfg"))
+    }
 }
 
 /// Download a KEYDB from a URL, verify, save to the standard path.

src/mux/resolve.rs

@@ -284,6 +284,15 @@ pub fn input(url: &str, opts: &InputOptions) -> io::Result<Box<dyn crate::pes::S
                 disc.decrypt_with(crate::disc::Key::Unit(opts.unit_keys.clone()), &[])
                     .map_err(|e| -> io::Error { e.into() })?;
             }
+            // CSS scrambled-but-uncracked guard (Fix 6): the scan saw scrambled
+            // sectors but recovered no title key, so `disc.css` is None yet the
+            // content IS encrypted. Without this, `css.is_none()` would be read
+            // as "unencrypted" and the scrambled MPEG would mux as plaintext
+            // garbage at exit 0. Surface the recorded hard error instead.
+            // `--raw` skips decryption, so it is exempt.
+            if !opts.raw && disc.css_error.is_some() {
+                return Err(crate::error::Error::CssKeyMissing.into());
+            }
             // No-key guard: if decryption is requested (not --raw) and the disc
             // is AACS-encrypted but key resolution yielded no usable key, FAIL
             // here — muxing an undecryptable stream produces ~100 MB of garbage

src/mux/tsmux.rs

@@ -46,6 +46,12 @@ pub struct TsMuxer<W: Write> {
     /// the audio/video offset is preserved. Frames that arrive before it
     /// is set saturate to 0.
     base_pts_ns: Option<i64>,
+    /// Count of PES frames actually emitted (a frame dropped as non-key
+    /// before the first keyframe does NOT count). `finish()` returns
+    /// [`Error::MuxEmpty`](crate::error::Error::MuxEmpty) when this is zero,
+    /// so a header-only `m2ts://` output can't be reported as success —
+    /// mirroring `MkvMuxer.frame_count`.
+    frame_count: u64,
 }
 
 impl<W: Write> TsMuxer<W> {
@@ -58,6 +64,7 @@ impl<W: Write> TsMuxer<W> {
             codec_privates: vec![None; n],
             params_written: vec![false; n],
             base_pts_ns: None,
+            frame_count: 0,
         }
     }
 
@@ -164,6 +171,10 @@ impl<W: Write> TsMuxer<W> {
                 first_pes = false;
             }
         }
+        // A frame that survived the pre-keyframe drop guard above and reached
+        // the writer counts as emitted. `finish()` checks this so a zero-frame
+        // mux fails loudly instead of producing a header-only "success".
+        self.frame_count += 1;
         Ok(())
     }
 
@@ -284,7 +295,17 @@ impl<W: Write> TsMuxer<W> {
 
     /// Flush the underlying writer. BD-TS needs no stream trailer, so this
     /// only drains buffering; the muxer remains usable afterwards.
+    ///
+    /// Returns [`Error::MuxEmpty`](crate::error::Error::MuxEmpty) when not a
+    /// single frame was emitted: an `m2ts://` sink that wrote only the FMKV
+    /// header (e.g. undecryptable ciphertext yielded no demuxable frames, or
+    /// every frame was dropped before the first keyframe) would otherwise be a
+    /// header-only file reported as a successful rip. Mirrors the zero-frame
+    /// guard in `MkvMuxer::finish`.
     pub fn finish(&mut self) -> io::Result<()> {
+        if self.frame_count == 0 {
+            return Err(crate::error::Error::MuxEmpty.into());
+        }
         self.writer.flush()
     }
 }
@@ -562,7 +583,12 @@ mod tests {
             let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
             let p = fake_hevc_nal(1, 80);
             mux.write_frame(0, 0, false, &p).unwrap();
-            mux.finish().unwrap();
+            // The single non-key frame was dropped (no keyframe to anchor),
+            // so finish() now reports MuxEmpty rather than producing a
+            // header-only "success". The drop behaviour itself is still
+            // verified by the empty packet list below.
+            let err = mux.finish().unwrap_err();
+            assert_eq!(err.kind(), std::io::ErrorKind::InvalidData);
         }
         // Nothing should be emitted for that PID.
         let packets = parse_bd_ts(&sink);
@@ -572,6 +598,43 @@ mod tests {
         );
     }
 
+    #[test]
+    fn finish_with_zero_frames_errors_mux_empty() {
+        // Fix 4: a TsMuxer that never emitted a frame must NOT report a
+        // clean finish — an m2ts:// sink that wrote only the FMKV header
+        // (undecryptable ciphertext → no demuxable frames) would otherwise
+        // be a header-only file published as a successful rip.
+        let mut sink: Vec<u8> = Vec::new();
+        let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
+        let err = mux.finish().unwrap_err();
+        assert_eq!(
+            err.kind(),
+            std::io::ErrorKind::InvalidData,
+            "zero-frame finish must surface MuxEmpty (E9023 → InvalidData)"
+        );
+        // The MuxEmpty variant carries the E9023 code, and its io::Error
+        // mapping is InvalidData (matching the kind above). Asserting both
+        // pins the variant ⇄ code ⇄ kind wiring without a lossy round-trip
+        // (From<Error> for io::Error → from-io goes back to IoError/E5000).
+        assert_eq!(
+            crate::error::Error::MuxEmpty.code(),
+            crate::error::E_MUX_EMPTY
+        );
+        let mapped: std::io::Error = crate::error::Error::MuxEmpty.into();
+        assert_eq!(mapped.kind(), std::io::ErrorKind::InvalidData);
+    }
+
+    #[test]
+    fn finish_after_real_frame_succeeds() {
+        // The counterpart: once a genuine keyframe is emitted, finish() is Ok.
+        let mut sink: Vec<u8> = Vec::new();
+        let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID]);
+        let idr = fake_hevc_nal(19, 50);
+        mux.write_frame(0, 0, true, &idr).unwrap();
+        mux.finish()
+            .expect("a written keyframe makes finish succeed");
+    }
+
     const AUDIO_PID: u16 = 0x1100;
 
     /// Decode the 33-bit PTS from the first PUSI packet on `pid`. Assumes

src/scsi/mod.rs

@@ -600,6 +600,82 @@ pub fn build_read10_fua(lba: u32, count: u16) -> [u8; 10] {
     ]
 }
 
+/// Round a pointer/address `p` up to the next boundary that satisfies an
+/// SPTI-style `AlignmentMask` (`STORAGE_ADAPTER_DESCRIPTOR::AlignmentMask`,
+/// ntddscsi.h / winioctl.h).
+///
+/// `mask` is a *mask*, not a power-of-two alignment value: `0` means "no
+/// alignment requirement" (any address is fine), `1` means 2-byte, `3`
+/// means DWORD (4-byte), `7` means 8-byte, etc. — always one less than the
+/// required alignment. An address is acceptable iff `(addr & mask) == 0`.
+///
+/// Returns the smallest `addr >= p` with `(addr & mask) == 0`. The
+/// standard branch-free idiom `(p + mask) & !mask` works for any valid
+/// (`2^n - 1`) mask, including `mask == 0` (where it is the identity).
+///
+/// Lives here, compiled on every platform, so the Windows SPTI bounce
+/// buffer in `windows.rs` can share it and so the arithmetic gets unit
+/// coverage on macOS/Linux CI even though the SPTI path only builds on
+/// Windows.
+#[allow(dead_code)]
+pub(crate) fn align_up(p: usize, mask: usize) -> usize {
+    (p + mask) & !mask
+}
+
+#[cfg(test)]
+mod align_tests {
+    use super::align_up;
+
+    #[test]
+    fn mask_zero_is_identity() {
+        // AlignmentMask 0 (USB optical bridges) — no alignment required.
+        for p in [0usize, 1, 2, 3, 7, 8, 13, 4096, 0x7fff_ffff] {
+            assert_eq!(align_up(p, 0), p);
+        }
+    }
+
+    #[test]
+    fn already_aligned_is_unchanged() {
+        // DWORD (mask 3): multiples of 4 are already aligned.
+        assert_eq!(align_up(0, 3), 0);
+        assert_eq!(align_up(4, 3), 4);
+        assert_eq!(align_up(8, 3), 8);
+        // 8-byte (mask 7): multiples of 8.
+        assert_eq!(align_up(0, 7), 0);
+        assert_eq!(align_up(16, 7), 16);
+    }
+
+    #[test]
+    fn rounds_up_to_next_boundary() {
+        // mask 1 (2-byte): odd → next even.
+        assert_eq!(align_up(1, 1), 2);
+        assert_eq!(align_up(3, 1), 4);
+        // mask 3 (DWORD): 1,2,3 → 4; 5,6,7 → 8.
+        assert_eq!(align_up(1, 3), 4);
+        assert_eq!(align_up(2, 3), 4);
+        assert_eq!(align_up(3, 3), 4);
+        assert_eq!(align_up(5, 3), 8);
+        // mask 7 (8-byte): 1..=7 → 8; 9 → 16.
+        assert_eq!(align_up(1, 7), 8);
+        assert_eq!(align_up(7, 7), 8);
+        assert_eq!(align_up(9, 7), 16);
+    }
+
+    #[test]
+    fn result_always_satisfies_mask() {
+        for &mask in &[0usize, 1, 3, 7, 15, 31, 63] {
+            for p in 0usize..256 {
+                let a = align_up(p, mask);
+                assert!(a >= p, "align_up({p},{mask})={a} went backwards");
+                assert_eq!(a & mask, 0, "align_up({p},{mask})={a} not aligned");
+                // Smallest such value: anything in (p-1-mask, a) would be < p
+                // or unaligned; check a - p never exceeds mask.
+                assert!(a - p <= mask, "align_up({p},{mask})={a} overshot");
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod parse_sense_tests {
     //! Unit tests for [`parse_sense`]. Covers both SPC-4 sense data

src/scsi/windows.rs

@@ -140,6 +140,17 @@ pub struct SptiTransport {
     /// [`WINDOWS_MIN_TRANSFER_BYTES`]. A single READ larger than this fails
     /// `DeviceIoControl` outright, so [`crate::Drive::read`] chunks to it.
     max_transfer: usize,
+    /// Adapter `AlignmentMask` (STORAGE_ADAPTER_DESCRIPTOR, ntddscsi.h /
+    /// winioctl.h), queried alongside `max_transfer`. It is a *mask*: `0`
+    /// (the common case on USB optical bridges) means the DataBuffer may
+    /// sit at any address; `3` means DWORD-aligned, `7` 8-byte, etc. —
+    /// always one less than the required alignment. SCSI/SAS HBAs report
+    /// nonzero masks, and IOCTL_SCSI_PASS_THROUGH_DIRECT rejects a
+    /// misaligned `DataBuffer` (DeviceIoControl fails → all reads return
+    /// transport failure / status 0xFF). When set and the caller's buffer
+    /// is misaligned, `execute()` bounces through an aligned scratch
+    /// buffer (see there).
+    alignment_mask: u32,
 }
 
 // SptiTransport's only field is an isize HANDLE, so the compiler
@@ -202,11 +213,12 @@ impl SptiTransport {
             });
         }
 
-        let max_transfer = query_max_transfer_bytes(handle);
+        let (max_transfer, alignment_mask) = query_adapter_descriptor(handle);
 
         Ok(SptiTransport {
             handle,
             max_transfer,
+            alignment_mask,
         })
     }
 
@@ -311,14 +323,24 @@ impl Drop for SptiTransport {
     }
 }
 
-/// Query the storage adapter's `MaximumTransferLength` (bytes) via
-/// IOCTL_STORAGE_QUERY_PROPERTY / StorageAdapterProperty. On any failure
-/// (IOCTL failed, short reply, or a nonsensical zero) returns the
-/// conservative [`WINDOWS_MIN_TRANSFER_BYTES`]; otherwise clamps the
-/// reported value up to that floor. Never returns 0.
-fn query_max_transfer_bytes(handle: isize) -> usize {
+/// Query the storage adapter descriptor via IOCTL_STORAGE_QUERY_PROPERTY /
+/// StorageAdapterProperty and return `(max_transfer_bytes, alignment_mask)`.
+///
+/// `max_transfer_bytes`: the adapter's `MaximumTransferLength`. On any
+/// failure (IOCTL failed, short reply, or a nonsensical zero) falls back to
+/// the conservative [`WINDOWS_MIN_TRANSFER_BYTES`]; otherwise clamped up to
+/// that floor. Never 0.
+///
+/// `alignment_mask`: the adapter's `AlignmentMask` (offset 16 in
+/// STORAGE_ADAPTER_DESCRIPTOR). `0` means no alignment requirement (the
+/// common case for USB optical bridges). A nonzero mask (SCSI/SAS HBAs)
+/// forces `execute()` to bounce the DataBuffer through an aligned scratch
+/// buffer. If the reply is too short to include `AlignmentMask`, returns
+/// `0` (no requirement) — the safe default, since any address satisfies a
+/// zero mask and the descriptor's leading fields are read first regardless.
+fn query_adapter_descriptor(handle: isize) -> (usize, u32) {
     if handle == INVALID_HANDLE_VALUE {
-        return WINDOWS_MIN_TRANSFER_BYTES;
+        return (WINDOWS_MIN_TRANSFER_BYTES, 0);
     }
     let query = StoragePropertyQuery {
         PropertyId: STORAGE_ADAPTER_PROPERTY,
@@ -341,14 +363,25 @@ fn query_max_transfer_bytes(handle: isize) -> usize {
     };
     // MaximumTransferLength sits at offset 8; need at least that many bytes
     // written for the field to be valid.
-    let valid = ok != 0
+    let max_valid = ok != 0
         && bytes_returned as usize
             >= std::mem::offset_of!(StorageAdapterDescriptor, MaximumTransferLength)
                 + std::mem::size_of::<u32>();
-    if !valid || desc.MaximumTransferLength == 0 {
-        return WINDOWS_MIN_TRANSFER_BYTES;
-    }
-    (desc.MaximumTransferLength as usize).max(WINDOWS_MIN_TRANSFER_BYTES)
+    let max_transfer = if !max_valid || desc.MaximumTransferLength == 0 {
+        WINDOWS_MIN_TRANSFER_BYTES
+    } else {
+        (desc.MaximumTransferLength as usize).max(WINDOWS_MIN_TRANSFER_BYTES)
+    };
+
+    // AlignmentMask sits at offset 16; only trust it if the reply is long
+    // enough. Otherwise assume 0 (no alignment requirement).
+    let align_valid = ok != 0
+        && bytes_returned as usize
+            >= std::mem::offset_of!(StorageAdapterDescriptor, AlignmentMask)
+                + std::mem::size_of::<u32>();
+    let alignment_mask = if align_valid { desc.AlignmentMask } else { 0 };
+
+    (max_transfer, alignment_mask)
 }
 
 impl ScsiTransport for SptiTransport {
@@ -396,11 +429,49 @@ impl ScsiTransport for SptiTransport {
         // sub-second resolution; biasing toward "more time" is safer than
         // truncating (truncation broke 1500ms fast-reads on Drive::read).
         sptwb.spt.TimeOutValue = ((timeout_ms + 999) / 1000).max(1);
-        sptwb.spt.DataBuffer = if data.is_empty() {
+
+        // AlignmentMask bounce buffer.
+        //
+        // IOCTL_SCSI_PASS_THROUGH_DIRECT requires `DataBuffer` to satisfy
+        // the adapter's `AlignmentMask` (`(ptr & mask) == 0`). On USB
+        // optical bridges the mask is 0, so the caller's buffer is always
+        // acceptable and we point straight at it (zero-copy fast path).
+        // On SCSI/SAS HBAs the mask can be 3/7/… ; if the caller's buffer
+        // happens to be misaligned the IOCTL fails outright (status 0xFF /
+        // all reads fail). In that case we transfer through an aligned
+        // scratch buffer: over-allocate by `mask` extra bytes so an aligned
+        // base is guaranteed to exist inside it, align the base with
+        // [`crate::scsi::align_up`], and use that as `DataBuffer`. For a
+        // FROM-device transfer the result is copied back into `data` after
+        // the IOCTL; for a TO-device transfer `data` is copied in before.
+        //
+        // `bounce` is kept alive for the whole `execute()` body so the
+        // aligned pointer we hand the driver stays valid across the IOCTL.
+        let mask = self.alignment_mask as usize;
+        let needs_bounce =
+            !data.is_empty() && mask != 0 && (data.as_mut_ptr() as usize) & mask != 0;
+        let mut bounce: Vec<u8> = Vec::new();
+        let data_ptr: *mut u8 = if data.is_empty() {
             std::ptr::null_mut()
+        } else if needs_bounce {
+            // Over-allocate by `mask` so an aligned start exists within.
+            bounce = vec![0u8; data.len() + mask];
+            let base = bounce.as_mut_ptr() as usize;
+            let aligned = crate::scsi::align_up(base, mask);
+            let aligned_ptr = aligned as *mut u8;
+            // For writes (ToDevice) prime the aligned region with the
+            // caller's payload before the IOCTL. (FromDevice copies back
+            // after.)
+            if direction == DataDirection::ToDevice {
+                unsafe {
+                    std::ptr::copy_nonoverlapping(data.as_ptr(), aligned_ptr, data.len());
+                }
+            }
+            aligned_ptr
         } else {
             data.as_mut_ptr()
         };
+        sptwb.spt.DataBuffer = data_ptr;
         sptwb.spt.SenseInfoOffset = std::mem::offset_of!(SptwbDirect, sense) as u32;
         sptwb.spt.Cdb[..cdb_len].copy_from_slice(&cdb[..cdb_len]);
 
@@ -452,15 +523,28 @@ impl ScsiTransport for SptiTransport {
             });
         }
 
+        // Clamp to the caller's buffer length, matching Linux/macOS: a
+        // driver that reports DataTransferLength > data.len() must never let
+        // callers read past the buffer they handed in.
+        let transferred = (sptwb.spt.DataTransferLength as usize).min(data.len());
+
+        // If we bounced a FROM-device read, copy the aligned scratch back
+        // into the caller's buffer (only the bytes actually transferred).
+        if needs_bounce && direction == DataDirection::FromDevice {
+            let aligned_ptr = data_ptr; // points inside `bounce`
+            unsafe {
+                std::ptr::copy_nonoverlapping(aligned_ptr, data.as_mut_ptr(), transferred);
+            }
+        }
+        // `bounce` is dropped here, after the last use of `data_ptr`.
+        drop(bounce);
+
         let mut sense = [0u8; 32];
         sense.copy_from_slice(&sptwb.sense);
 
         Ok(ScsiResult {
             status: sptwb.spt.ScsiStatus,
-            // Clamp to the caller's buffer length, matching Linux/macOS: a
-            // driver that reports DataTransferLength > data.len() must never
-            // let callers read past the buffer they handed in.
-            bytes_transferred: (sptwb.spt.DataTransferLength as usize).min(data.len()),
+            bytes_transferred: transferred,
             sense,
         })
     }