Make iso:// a destination for any image source, not just a drive

Writing an iso:// meant a raw sector copy off a drive, so an existing
image could only ever be a source. Decrypting one you already had meant
finding the disc again.

The engine's copy is the recovery path — mapfile, multipass, damage-jump,
auto-resume — and all of it exists because optical media returns read
errors. A file does not, so a non-drive source gets write_image instead:
sectors in, bytes out, once, no recovery machinery. Keeping them apart is
not just tidiness. The mapfile identity check compares AACS unit keys and
the VID, both empty for an already-decrypted source, so identity passes
for any such source — a second run with different input to the same
output path would resume over the previous image and report success.

A short read is an error rather than a zero-fill: padding a truncated
source yields an image that looks complete and is not, which is the worst
outcome for a copy someone means to keep.

src/error.rs

@@ -243,6 +243,8 @@ pub const E_SYNC_WORKER_LOST: u16 = 9057;
 /// READ CAPACITY returned a short or overflowing transfer.
 pub const E_DISC_CAPACITY_MALFORMED: u16 = 9047;
 pub const E_DRIVE_INQUIRY_SHORT: u16 = 9058;
+pub const E_SHORT_IMAGE_READ: u16 = 9059;
+pub const E_EMPTY_IMAGE: u16 = 9060;
 
 // ── Error enum ──────────────────────────────────────────────────────────────
 
@@ -371,6 +373,22 @@ pub enum Error {
         index: usize,
         count: usize,
     },
+    /// An image-level write read fewer bytes than the sector count it asked for.
+    ///
+    /// Distinct from [`Error::IoError`] on purpose: the read SUCCEEDED and simply
+    /// returned less than a whole sector run, which for a file-backed source means
+    /// the source is shorter than its declared capacity. Zero-filling the gap
+    /// would produce an image that looks complete and is not — the worst outcome
+    /// for a copy someone intends to keep — so it is an error instead.
+    ShortImageRead {
+        lba: u32,
+        expected: u32,
+        got: u32,
+    },
+    /// An image-level write was asked for zero sectors. A zero-byte image is
+    /// never the intent, and reporting it here names the problem at the point it
+    /// is knowable rather than leaving an empty file behind.
+    EmptyImage,
     IfoParse,
     /// A title produced NO muxable frames: the mux driver's pump ended without
     /// any video track's `codec_private` resolving, or the MKV muxer reached
@@ -744,6 +762,8 @@ impl Error {
             Error::UdfNotFilesystem => E_UDF_NOT_FILESYSTEM,
             Error::UdfBufferTooSmall => E_UDF_BUFFER_TOO_SMALL,
             Error::DiscTitleRange { .. } => E_DISC_TITLE_RANGE,
+            Error::ShortImageRead { .. } => E_SHORT_IMAGE_READ,
+            Error::EmptyImage => E_EMPTY_IMAGE,
             Error::IfoParse => E_IFO_PARSE,
             Error::MkvInvalid => E_MKV_INVALID,
             Error::MkvSourceInvalid => E_MKV_SOURCE_INVALID,

src/io/image_writer.rs

@@ -0,0 +1,270 @@
+//! `write_image` — write an image-level source out as a sector image.
+//!
+//! This is the plain image writer: sectors in from any [`SectorSource`], bytes
+//! out to a file, in order, once. It is what an `iso://` DESTINATION means when
+//! the source is not a physical drive.
+//!
+//! # Why this is not `freemkv_engine::copy`
+//!
+//! The engine's `copy` is the RECOVERY path — mapfile sidecar, `--multipass`
+//! sweep/patch, damage-jump, ECC-aware batching, auto-resume. Every one of those
+//! exists because an optical drive returns read errors on marginal media. A
+//! file-backed or synthesized source has no marginal media: a read either
+//! succeeds or the underlying file is broken, and retrying it is pointless.
+//!
+//! Routing a non-drive source through the recovery path is not merely wasteful,
+//! it is wrong. Its mapfile identity check compares AACS unit keys and the VID,
+//! both of which are empty for an already-decrypted source, so identity passes
+//! for ANY such source: a second run with a different input to the same output
+//! path would resume over the previous image and produce wrong content at exit
+//! zero. Keeping the two paths separate makes that unrepresentable.
+//!
+//! So: drive sources get `freemkv_engine::copy`. Everything else gets this.
+
+use crate::error::{Error, Result};
+use crate::halt::Halt;
+use crate::sector::SectorSource;
+use std::fs::File;
+use std::io::{BufWriter, Write};
+use std::path::Path;
+
+/// Sectors per read/write batch. 4 MiB — large enough that per-call overhead
+/// disappears against a file-backed source, small enough that the buffer is not
+/// a notable allocation and cancellation stays responsive.
+const BATCH_SECTORS: u32 = 2048;
+
+/// Bytes per sector. Fixed for every medium this crate reads.
+const SECTOR_BYTES: usize = 2048;
+
+/// Write `total_sectors` sectors from `reader` to `dest`.
+///
+/// Reads sequentially from LBA 0 and writes in order, so the output is a faithful
+/// image of whatever the source presents — decrypted if the caller wrapped the
+/// source in a [`DecryptingSectorSource`](crate::sector::decrypting::DecryptingSectorSource),
+/// ciphertext if it did not. This function performs no decryption itself and makes
+/// no decryption decision; that belongs to the caller, which knows whether the run
+/// is `--raw`.
+///
+/// `on_progress` is called after each batch with the cumulative byte count, for
+/// front-end progress reporting. It must not block.
+///
+/// `halt` is checked once per batch. On cancellation the partial file is left in
+/// place — the caller decides whether a partial image is worth keeping, and
+/// deleting a multi-gigabyte file the user may want to inspect is not this
+/// function's call to make.
+///
+/// Returns the number of bytes written.
+///
+/// # Errors
+///
+/// - [`Error::Halted`] if `halt` was cancelled.
+/// - [`Error::IoError`] if the destination cannot be created or written.
+/// - Whatever the source's `read_sectors` returns. A short read is an error, not
+///   a zero-fill: silently padding a truncated source produces an image that
+///   looks complete and is not.
+pub fn write_image(
+    reader: &mut dyn SectorSource,
+    dest: &Path,
+    total_sectors: u32,
+    halt: &Halt,
+    mut on_progress: impl FnMut(u64),
+) -> Result<u64> {
+    if total_sectors == 0 {
+        return Err(Error::EmptyImage);
+    }
+
+    let file = File::create(dest).map_err(|source| Error::IoError { source })?;
+    let mut out = BufWriter::with_capacity(BATCH_SECTORS as usize * SECTOR_BYTES, file);
+
+    let mut buf = vec![0u8; BATCH_SECTORS as usize * SECTOR_BYTES];
+    let mut written: u64 = 0;
+    let mut lba: u32 = 0;
+
+    while lba < total_sectors {
+        if halt.is_cancelled() {
+            return Err(Error::Halted);
+        }
+        let count = BATCH_SECTORS.min(total_sectors - lba);
+        let want = count as usize * SECTOR_BYTES;
+        // `recovery = false`: a file-backed source ignores the flag, and a
+        // retry loop over a local file would only re-read the same bytes.
+        let got = reader.read_sectors(lba, count as u16, &mut buf[..want], false)?;
+        if got != want {
+            return Err(Error::ShortImageRead {
+                lba,
+                expected: want as u32,
+                got: got as u32,
+            });
+        }
+        out.write_all(&buf[..want])
+            .map_err(|source| Error::IoError { source })?;
+        written += want as u64;
+        lba += count;
+        on_progress(written);
+    }
+
+    out.flush().map_err(|source| Error::IoError { source })?;
+    Ok(written)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::error::Result as FmResult;
+
+    /// A source that yields a deterministic byte per sector, so the written
+    /// image can be checked positionally rather than just by length.
+    struct PatternSource {
+        sectors: u32,
+        /// Sectors after which `read_sectors` reports a short read.
+        short_after: Option<u32>,
+    }
+
+    impl SectorSource for PatternSource {
+        fn capacity_sectors(&self) -> u32 {
+            self.sectors
+        }
+        fn read_sectors(
+            &mut self,
+            lba: u32,
+            count: u16,
+            buf: &mut [u8],
+            _recovery: bool,
+        ) -> FmResult<usize> {
+            let want = count as usize * SECTOR_BYTES;
+            if self.short_after.is_some_and(|after| lba >= after) {
+                return Ok(want - 1);
+            }
+            for s in 0..count as usize {
+                let byte = ((lba as usize + s) % 251) as u8;
+                buf[s * SECTOR_BYTES..(s + 1) * SECTOR_BYTES].fill(byte);
+            }
+            Ok(want)
+        }
+    }
+
+    fn tmp(name: &str) -> std::path::PathBuf {
+        let mut p = std::env::temp_dir();
+        p.push(format!("fmkv-image-writer-{name}-{}", std::process::id()));
+        p
+    }
+
+    /// The written image is byte-for-byte what the source presented, at the
+    /// right offsets — not merely the right length.
+    #[test]
+    fn writes_every_sector_in_order() {
+        let dest = tmp("order");
+        let mut src = PatternSource {
+            sectors: 5000,
+            short_after: None,
+        };
+        let n = write_image(&mut src, &dest, 5000, &Halt::new(), |_| {}).expect("write");
+        assert_eq!(n, 5000 * SECTOR_BYTES as u64);
+
+        let data = std::fs::read(&dest).expect("read back");
+        assert_eq!(data.len(), 5000 * SECTOR_BYTES);
+        // Spot-check across batch boundaries (BATCH_SECTORS = 2048): the last
+        // sector of batch 0, the first of batch 1, and the final sector.
+        for lba in [0usize, 2047, 2048, 4095, 4096, 4999] {
+            let want = (lba % 251) as u8;
+            assert_eq!(
+                data[lba * SECTOR_BYTES],
+                want,
+                "sector {lba} head byte wrong — batching lost or duplicated a sector"
+            );
+            assert_eq!(
+                data[(lba + 1) * SECTOR_BYTES - 1],
+                want,
+                "sector {lba} tail"
+            );
+        }
+        let _ = std::fs::remove_file(&dest);
+    }
+
+    /// A tail shorter than a full batch must still be written whole — the
+    /// classic off-by-one when `total_sectors` is not a batch multiple.
+    #[test]
+    fn writes_a_partial_final_batch() {
+        let dest = tmp("tail");
+        let mut src = PatternSource {
+            sectors: 2049,
+            short_after: None,
+        };
+        let n = write_image(&mut src, &dest, 2049, &Halt::new(), |_| {}).expect("write");
+        assert_eq!(n, 2049 * SECTOR_BYTES as u64);
+        assert_eq!(
+            std::fs::metadata(&dest).expect("stat").len(),
+            2049 * SECTOR_BYTES as u64
+        );
+        let _ = std::fs::remove_file(&dest);
+    }
+
+    /// A short read is an error. Zero-filling would yield an image that looks
+    /// complete and is not — the single worst outcome for an archival copy.
+    #[test]
+    fn short_read_is_an_error_not_a_zero_fill() {
+        let dest = tmp("short");
+        let mut src = PatternSource {
+            sectors: 4096,
+            short_after: Some(2048),
+        };
+        let err = write_image(&mut src, &dest, 4096, &Halt::new(), |_| {}).expect_err("must fail");
+        assert!(
+            matches!(err, Error::ShortImageRead { lba: 2048, .. }),
+            "got {err:?}"
+        );
+        let _ = std::fs::remove_file(&dest);
+    }
+
+    /// Cancellation stops the run and reports it, rather than finishing quietly
+    /// or reporting success on a partial image.
+    #[test]
+    fn cancellation_halts_and_reports() {
+        let dest = tmp("halt");
+        let mut src = PatternSource {
+            sectors: 100_000,
+            short_after: None,
+        };
+        let halt = Halt::new();
+        halt.cancel();
+        let err = write_image(&mut src, &dest, 100_000, &halt, |_| {}).expect_err("must halt");
+        assert!(matches!(err, Error::Halted), "got {err:?}");
+        let _ = std::fs::remove_file(&dest);
+    }
+
+    /// Progress is cumulative and monotonic, and its final value equals the
+    /// returned byte count — a front-end that trusts the callback must not end
+    /// up disagreeing with the return value.
+    #[test]
+    fn progress_is_cumulative_and_ends_at_the_total() {
+        let dest = tmp("progress");
+        let mut src = PatternSource {
+            sectors: 5000,
+            short_after: None,
+        };
+        let mut seen: Vec<u64> = Vec::new();
+        let n = write_image(&mut src, &dest, 5000, &Halt::new(), |b| seen.push(b)).expect("write");
+        assert!(
+            seen.windows(2).all(|w| w[1] > w[0]),
+            "not monotonic: {seen:?}"
+        );
+        assert_eq!(*seen.last().expect("at least one callback"), n);
+        let _ = std::fs::remove_file(&dest);
+    }
+
+    /// A zero-sector source is a caller error, not a zero-byte image: an empty
+    /// ISO is never what anyone wanted, and failing here names the problem.
+    #[test]
+    fn zero_sectors_is_an_error() {
+        let dest = tmp("empty");
+        let mut src = PatternSource {
+            sectors: 0,
+            short_after: None,
+        };
+        let err = write_image(&mut src, &dest, 0, &Halt::new(), |_| {}).expect_err("must fail");
+        assert!(matches!(err, Error::EmptyImage), "got {err:?}");
+        // The destination must not have been created — a failed run leaves no
+        // stub for a later run to mistake for output.
+        assert!(!dest.exists(), "empty run created a file");
+    }
+}

src/io/mod.rs

@@ -31,6 +31,7 @@ pub(crate) mod bounded;
 pub mod byte_prefetcher;
 pub mod file_sector_source;
 pub mod fsync;
+pub mod image_writer;
 pub mod sink;
 mod writeback;
 mod writeback_file;

src/lib.rs

@@ -191,6 +191,11 @@ pub use io::pipeline::{
 // continuously instead of bursting. General I/O infra, not recovery policy;
 // promoted to `pub` so freemkv-engine's relocated sweep/patch can use it too.
 pub use io::WritebackFile;
+/// Write an image-level source out as a sector image — what an `iso://`
+/// DESTINATION means for any source that is not a physical drive. Drive sources
+/// go through `freemkv_engine::copy`, which is the recovery path; see
+/// [`io::image_writer`] for why the two are deliberately separate.
+pub use io::image_writer::write_image;
 
 // ─── Drive events (low-level callbacks) ─────────────────────────────────────
 pub use event::{BatchSizeReason, Event, EventKind};