v0.13.16 — single Progress trait + PassProgress (RIP_DESIGN.md §16)
Pre-0.13.16 the rip API leaked internal mapfile concepts (pos,
bytes_good, work_done, bytes_pending, Finished/NonTrimmed) into per-pass
positional callbacks Fn(u64, u64, u64). Consumers reinvented the math
each time, and v0.13.15's UI bug surfaced exactly because of this —
autorip's web JS computed pct from bytes_good while the backend
computed from pos, silent drift, frozen UI bar.
This release replaces both Disc::copy::on_progress and
Disc::patch::on_progress callbacks with a single Progress trait +
PassProgress struct (new progress module).
pub struct PassProgress {
pub kind: PassKind, // Sweep | Trim {reverse} | Scrape {reverse} | Mux
pub work_done: u64,
pub work_total: u64,
pub bytes_good_total: u64,
pub bytes_total_disc: u64,
}
pub trait Progress {
fn report(&self, p: &PassProgress);
}
impl<F: Fn(&PassProgress)> Progress for F { ... } // closures work directly
CopyOptions::on_progress and PatchOptions::on_progress are renamed to
progress: Option<&dyn Progress>. Closure callers update trivially via
the blanket impl; struct callers gain a clean named-field shape with no
positional-arg confusion.
PassKind carries the semantic (sweep vs trim vs scrape vs mux) so
consumers can label phases without reinventing detection logic.
Disc::patch reports Trim {reverse} for retry passes with block_sectors
>= 2 and Scrape {reverse} when block_sectors == 1. Direction comes
through reverse: bool. Mux variant is reserved for v0.13.17 when the
mux pipeline emits progress.
Tests + clippy clean across all 4 crates.
This commit is contained in:
@@ -1,5 +1,50 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## 0.13.16 (2026-04-26)
|
||||||
|
|
||||||
|
### Architecture: single `Progress` trait + `PassProgress` struct
|
||||||
|
|
||||||
|
Pre-0.13.16 the rip API leaked internal mapfile concepts (`pos`,
|
||||||
|
`bytes_good`, `work_done`, `bytes_pending`, `Finished`/`NonTrimmed`)
|
||||||
|
into per-pass positional callbacks. Consumers reinvented the math each
|
||||||
|
time, and the v0.13.15 UI bug surfaced exactly because of this — autorip's
|
||||||
|
web JS computed `progress_pct` from `bytes_good` while the backend
|
||||||
|
computed from `pos`, silent drift, frozen UI bar.
|
||||||
|
|
||||||
|
This release replaces both `Disc::copy::on_progress` and
|
||||||
|
`Disc::patch::on_progress` `Fn(u64, u64, u64)` callbacks with a single
|
||||||
|
`Progress` trait and `PassProgress` struct (new `progress` module).
|
||||||
|
|
||||||
|
```rust
|
||||||
|
pub struct PassProgress {
|
||||||
|
pub kind: PassKind, // Sweep | Trim {reverse} | Scrape {reverse} | Mux
|
||||||
|
pub work_done: u64,
|
||||||
|
pub work_total: u64,
|
||||||
|
pub bytes_good_total: u64,
|
||||||
|
pub bytes_total_disc: u64,
|
||||||
|
}
|
||||||
|
|
||||||
|
pub trait Progress {
|
||||||
|
fn report(&self, p: &PassProgress);
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<F: Fn(&PassProgress)> Progress for F { ... }
|
||||||
|
```
|
||||||
|
|
||||||
|
Both `CopyOptions::on_progress` and `PatchOptions::on_progress` are
|
||||||
|
renamed to `progress: Option<&dyn Progress>`. Closure callers update
|
||||||
|
trivially; struct callers gain a clean named-field shape with no
|
||||||
|
positional-arg confusion.
|
||||||
|
|
||||||
|
`PassKind` carries the semantic (sweep vs trim vs scrape vs mux) so
|
||||||
|
consumers can label phases without reinventing detection logic. The
|
||||||
|
`Mux` variant is reserved for v0.13.17 when the mux pipeline emits
|
||||||
|
progress; not yet emitted by libfreemkv code.
|
||||||
|
|
||||||
|
`Disc::patch` reports `Trim {reverse}` for retry passes with
|
||||||
|
`block_sectors >= 2` and `Scrape {reverse}` when `block_sectors == 1`
|
||||||
|
(the per-sector final pass). Direction comes through `reverse: bool`.
|
||||||
|
|
||||||
## 0.13.15 (2026-04-26)
|
## 0.13.15 (2026-04-26)
|
||||||
|
|
||||||
### Breaking: `on_progress` callback gains `pos` parameter
|
### Breaking: `on_progress` callback gains `pos` parameter
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "libfreemkv"
|
name = "libfreemkv"
|
||||||
version = "0.13.15"
|
version = "0.13.16"
|
||||||
edition = "2024"
|
edition = "2024"
|
||||||
rust-version = "1.86"
|
rust-version = "1.86"
|
||||||
license = "AGPL-3.0-only"
|
license = "AGPL-3.0-only"
|
||||||
|
|||||||
+45
-15
@@ -1418,9 +1418,15 @@ impl Disc {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some(cb) = opts.on_progress {
|
if let Some(reporter) = opts.progress {
|
||||||
let stats = map.stats();
|
let stats = map.stats();
|
||||||
cb(stats.bytes_good, pos, total_bytes);
|
reporter.report(&crate::progress::PassProgress {
|
||||||
|
kind: crate::progress::PassKind::Sweep,
|
||||||
|
work_done: pos,
|
||||||
|
work_total: total_bytes,
|
||||||
|
bytes_good_total: stats.bytes_good,
|
||||||
|
bytes_total_disc: total_bytes,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1470,13 +1476,11 @@ pub struct CopyOptions<'a> {
|
|||||||
/// `skip_on_error`. The skipped region is marked `non-trimmed` for later
|
/// `skip_on_error`. The skipped region is marked `non-trimmed` for later
|
||||||
/// trimming/scraping by `Disc::patch`.
|
/// trimming/scraping by `Disc::patch`.
|
||||||
pub skip_forward: bool,
|
pub skip_forward: bool,
|
||||||
/// Callback fired per inner-loop iteration with
|
/// Per-iteration progress reporter. v0.13.16 architecture: the library
|
||||||
/// `(bytes_good, pos, total_bytes)`. `pos` is the current sweep
|
/// emits a single `PassProgress` shape via the `Progress` trait;
|
||||||
/// position — true Pass 1 progress, including skipped-forward NonTrimmed
|
/// consumers compute their own derived percentages / ETAs from it. No
|
||||||
/// ranges. `bytes_good` is the count of `Finished` (clean) sectors,
|
/// more positional `(bytes_good, pos, total)` callbacks.
|
||||||
/// which doesn't advance through bad zones. UI should display `pos`
|
pub progress: Option<&'a dyn crate::progress::Progress>,
|
||||||
/// for "swept" progress and `bytes_good` for "real data recovered".
|
|
||||||
pub on_progress: Option<&'a dyn Fn(u64, u64, u64)>,
|
|
||||||
pub halt: Option<std::sync::Arc<std::sync::atomic::AtomicBool>>,
|
pub halt: Option<std::sync::Arc<std::sync::atomic::AtomicBool>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1520,10 +1524,8 @@ pub struct PatchOptions<'a> {
|
|||||||
/// bad zone and won't recover during this attempt. `0` disables the
|
/// bad zone and won't recover during this attempt. `0` disables the
|
||||||
/// guard (run to completion or halt).
|
/// guard (run to completion or halt).
|
||||||
pub wedged_threshold: u64,
|
pub wedged_threshold: u64,
|
||||||
/// Callback fired per inner-loop iteration with
|
/// Per-iteration progress reporter. See `CopyOptions::progress`.
|
||||||
/// `(bytes_good, pos, total_bytes)`. `pos` is the current LBA-byte
|
pub progress: Option<&'a dyn crate::progress::Progress>,
|
||||||
/// position within the patch walk; for reverse passes it counts down.
|
|
||||||
pub on_progress: Option<&'a dyn Fn(u64, u64, u64)>,
|
|
||||||
pub halt: Option<std::sync::Arc<std::sync::atomic::AtomicBool>>,
|
pub halt: Option<std::sync::Arc<std::sync::atomic::AtomicBool>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1604,6 +1606,12 @@ impl Disc {
|
|||||||
if opts.reverse {
|
if opts.reverse {
|
||||||
bad_ranges.reverse();
|
bad_ranges.reverse();
|
||||||
}
|
}
|
||||||
|
// work_total = sum of all bad-range bytes. on_progress's third arg
|
||||||
|
// is this value; the second arg (work_done) is incremented per block
|
||||||
|
// attempted. UI consumers (autorip) compute pass_progress_pct =
|
||||||
|
// work_done / work_total — true 0..100% per pass per RIP_DESIGN.md §16.
|
||||||
|
let work_total: u64 = bad_ranges.iter().map(|(_, sz)| *sz).sum();
|
||||||
|
let mut work_done: u64 = 0;
|
||||||
tracing::trace!(
|
tracing::trace!(
|
||||||
target: "freemkv::disc",
|
target: "freemkv::disc",
|
||||||
phase = "patch_start",
|
phase = "patch_start",
|
||||||
@@ -1612,6 +1620,7 @@ impl Disc {
|
|||||||
reverse = opts.reverse,
|
reverse = opts.reverse,
|
||||||
wedged_threshold = opts.wedged_threshold,
|
wedged_threshold = opts.wedged_threshold,
|
||||||
num_ranges = bad_ranges.len(),
|
num_ranges = bad_ranges.len(),
|
||||||
|
work_total,
|
||||||
"Disc::patch entered"
|
"Disc::patch entered"
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -1694,9 +1703,30 @@ impl Disc {
|
|||||||
break 'outer;
|
break 'outer;
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some(cb) = opts.on_progress {
|
// Track work done in this pass for the per-pass progress bar.
|
||||||
|
// Each block iterated counts as work, regardless of read
|
||||||
|
// outcome — a failed retry is still progress through the
|
||||||
|
// bad-range walk.
|
||||||
|
work_done = work_done.saturating_add(block_bytes);
|
||||||
|
|
||||||
|
if let Some(reporter) = opts.progress {
|
||||||
let s = map.stats();
|
let s = map.stats();
|
||||||
cb(s.bytes_good, pos, total_bytes);
|
let kind = if block_sectors == 1 {
|
||||||
|
crate::progress::PassKind::Scrape {
|
||||||
|
reverse: opts.reverse,
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
crate::progress::PassKind::Trim {
|
||||||
|
reverse: opts.reverse,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
reporter.report(&crate::progress::PassProgress {
|
||||||
|
kind,
|
||||||
|
work_done,
|
||||||
|
work_total,
|
||||||
|
bytes_good_total: s.bytes_good,
|
||||||
|
bytes_total_disc: total_bytes,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -88,6 +88,7 @@ pub mod mux;
|
|||||||
pub mod pes;
|
pub mod pes;
|
||||||
pub(crate) mod platform;
|
pub(crate) mod platform;
|
||||||
pub mod profile;
|
pub mod profile;
|
||||||
|
pub mod progress;
|
||||||
pub mod scsi;
|
pub mod scsi;
|
||||||
pub mod sector;
|
pub mod sector;
|
||||||
pub(crate) mod speed;
|
pub(crate) mod speed;
|
||||||
|
|||||||
@@ -0,0 +1,70 @@
|
|||||||
|
//! Pipeline-progress reporting for the rip pipeline.
|
||||||
|
//!
|
||||||
|
//! v0.13.16 architecture rule: ONE progress signal type. Every long-running
|
||||||
|
//! pipeline operation (`Disc::copy`, `Disc::patch`, mux) emits the same
|
||||||
|
//! `PassProgress` shape via the `Progress` trait. Consumers (autorip) compute
|
||||||
|
//! a single `PipelineStats` derived view and never reach into per-pass
|
||||||
|
//! internals.
|
||||||
|
//!
|
||||||
|
//! Why this matters: pre-0.13.16 the API leaked `pos`, `bytes_good`,
|
||||||
|
//! `work_done`, `bytes_pending`, `Finished/NonTrimmed` mapfile semantics —
|
||||||
|
//! and consumers reinvented the math each time they wanted a percentage.
|
||||||
|
//! UIs ended up reading one source while server-side computed from another,
|
||||||
|
//! producing wrong percentages without anyone noticing.
|
||||||
|
|
||||||
|
/// Identifies which pipeline phase the progress event belongs to.
|
||||||
|
///
|
||||||
|
/// Consumers can render a phase-specific label (e.g. "Sweep", "Trim
|
||||||
|
/// (reverse)", "Scrape", "Mux") or just use a generic "Pass N" label.
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
pub enum PassKind {
|
||||||
|
/// `Disc::copy` — initial sweep across the entire disc.
|
||||||
|
Sweep,
|
||||||
|
/// `Disc::patch` retry pass with `block_sectors >= 2`. `reverse=true`
|
||||||
|
/// means walking bad ranges from highest to lowest LBA.
|
||||||
|
Trim { reverse: bool },
|
||||||
|
/// `Disc::patch` final pass at 1 sector per block.
|
||||||
|
Scrape { reverse: bool },
|
||||||
|
/// Demux ISO → output (MKV / M2TS / network). Single phase that runs
|
||||||
|
/// after all rip passes complete.
|
||||||
|
Mux,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// One progress sample from a pipeline phase.
|
||||||
|
///
|
||||||
|
/// `work_done / work_total` is the per-pass percentage — always 0..=100%
|
||||||
|
/// regardless of which kind of pass is running. `bytes_good_total` is the
|
||||||
|
/// cumulative count of confirmed-clean bytes across the whole rip; useful
|
||||||
|
/// for the "data recovered" stat the user sees.
|
||||||
|
#[derive(Debug, Clone, Copy)]
|
||||||
|
pub struct PassProgress {
|
||||||
|
pub kind: PassKind,
|
||||||
|
/// Bytes processed in this pass so far. Monotonically non-decreasing.
|
||||||
|
pub work_done: u64,
|
||||||
|
/// Total bytes this pass will process. Constant for the duration of
|
||||||
|
/// the pass.
|
||||||
|
pub work_total: u64,
|
||||||
|
/// Cumulative bytes confirmed clean (`Finished` mapfile state) across
|
||||||
|
/// every pass run on this rip. Doesn't change across pass boundaries.
|
||||||
|
pub bytes_good_total: u64,
|
||||||
|
/// Total disc capacity in bytes. Constant.
|
||||||
|
pub bytes_total_disc: u64,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A consumer of pipeline progress events. Library code calls
|
||||||
|
/// `Progress::report` once per inner-loop iteration (throttling is the
|
||||||
|
/// consumer's job — `report` is cheap; the library doesn't gate it).
|
||||||
|
///
|
||||||
|
/// No `Send`/`Sync` bound — `report` is always called from the same thread
|
||||||
|
/// running the rip pipeline, so closures with non-`Sync` captures (e.g.
|
||||||
|
/// `RefCell<PassProgressState>`) work directly. Blanket impl below lets
|
||||||
|
/// callers pass closures without explicit struct types.
|
||||||
|
pub trait Progress {
|
||||||
|
fn report(&self, p: &PassProgress);
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<F: Fn(&PassProgress)> Progress for F {
|
||||||
|
fn report(&self, p: &PassProgress) {
|
||||||
|
(self)(p)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -175,17 +175,25 @@ fn test_disc_copy_progress_callback_fires() {
|
|||||||
|
|
||||||
let calls = Arc::new(AtomicU64::new(0));
|
let calls = Arc::new(AtomicU64::new(0));
|
||||||
let last_bytes = Arc::new(AtomicU64::new(0));
|
let last_bytes = Arc::new(AtomicU64::new(0));
|
||||||
let calls_cb = calls.clone();
|
|
||||||
let last_bytes_cb = last_bytes.clone();
|
|
||||||
|
|
||||||
let progress = move |bytes: u64, _pos: u64, _total: u64| {
|
struct CountingReporter {
|
||||||
calls_cb.fetch_add(1, Ordering::Relaxed);
|
calls: Arc<AtomicU64>,
|
||||||
last_bytes_cb.store(bytes, Ordering::Relaxed);
|
last_bytes: Arc<AtomicU64>,
|
||||||
|
}
|
||||||
|
impl libfreemkv::progress::Progress for CountingReporter {
|
||||||
|
fn report(&self, p: &libfreemkv::progress::PassProgress) {
|
||||||
|
self.calls.fetch_add(1, Ordering::Relaxed);
|
||||||
|
self.last_bytes.store(p.bytes_good_total, Ordering::Relaxed);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let reporter = CountingReporter {
|
||||||
|
calls: calls.clone(),
|
||||||
|
last_bytes: last_bytes.clone(),
|
||||||
};
|
};
|
||||||
|
|
||||||
let opts = CopyOptions {
|
let opts = CopyOptions {
|
||||||
decrypt: false,
|
decrypt: false,
|
||||||
on_progress: Some(&progress),
|
progress: Some(&reporter),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user