Skip to main content

max / audiofiles

Fix three ultra-fuzz launch-blockers: LUFS normalize, worker panics, import orphan blob - edit/normalize: measure LUFS via the BS.1770-4 meter (analysis::loudness) instead of the hand-rolled mean-square formula, and apply a flat gain with no clamp (the final encode is the single clamp point). Adds a round-trip test asserting normalized loudness lands within 0.5 LUFS of target. - workers: wrap each job body in catch_unwind so a panic decoding one corrupt file reports an error instead of killing the worker thread (which left the import/export UI wedged). Covers edit, forge (chop+conform), classifier, import, and export workers; mirrors the existing analysis worker. - store::import: unlink the freshly-written blob if the INSERT fails (guarded by needs_write so a shared/pre-existing blob is untouched), closing the orphan-on-insert-failure path. 530 core + 218 browser tests green; workspace builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-15 22:42 UTC
Commit: a6865ee7b6e490e4f97a347667f64e8168be8607
Parent: 773dc96
7 files changed, +161 insertions, -54 deletions
M crates/audiofiles-browser/src/classifier_worker.rs +7 -1
@@ -55,8 +55,14 @@ pub fn spawn_classifier_job(db_path: PathBuf, job: ClassifierJob) -> std::io::Re
55 55 let thread = thread::Builder::new()
56 56 .name("classifier-job".to_string())
57 57 .spawn(move || {
58 + // Catch panics so a failed job reports back instead of unwinding the
59 + // thread silently — otherwise the GUI's try_recv() never resolves and
60 + // the trigger buttons stay disabled forever.
58 61 let event = match Database::open(&db_path) {
59 - Ok(db) => run_job(&db, job),
62 + Ok(db) => std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| run_job(&db, job)))
63 + .unwrap_or_else(|_| {
64 + ClassifierWorkerEvent::Failed("classifier job panicked (internal error)".to_string())
65 + }),
60 66 Err(e) => {
61 67 error!("Classifier worker failed to open database: {e}");
62 68 ClassifierWorkerEvent::Failed(format!("could not open database: {e}"))
M crates/audiofiles-browser/src/export.rs +35 -24
@@ -117,39 +117,50 @@ fn worker_loop(
117 117 ExportCommand::Export { items, config } => {
118 118 let cancelled = std::sync::atomic::AtomicBool::new(false);
119 119
120 - let summary = audiofiles_core::export::run_export(
121 - &items,
122 - &config,
123 - &store,
124 - |completed, total, current_name| {
125 - // Check for cancel between files
126 - if let Ok(ExportCommand::Cancel) | Ok(ExportCommand::Shutdown) =
127 - cmd_rx.try_recv()
128 - {
129 - cancelled.store(true, std::sync::atomic::Ordering::Relaxed);
130 - return false;
131 - }
132 -
133 - let _ = event_tx.send(ExportEvent::Progress {
134 - completed,
135 - total,
136 - current_name: current_name.to_string(),
137 - });
138 -
139 - true
140 - },
141 - );
120 + // Catch panics so a bad file reports an error instead of unwinding
121 + // the worker thread — that would wedge the UI in the exporting
122 + // state, since ExportEvent::Complete is the only exit.
123 + let summary = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
124 + audiofiles_core::export::run_export(
125 + &items,
126 + &config,
127 + &store,
128 + |completed, total, current_name| {
129 + // Check for cancel between files
130 + if let Ok(ExportCommand::Cancel) | Ok(ExportCommand::Shutdown) =
131 + cmd_rx.try_recv()
132 + {
133 + cancelled.store(true, std::sync::atomic::Ordering::Relaxed);
134 + return false;
135 + }
136 +
137 + let _ = event_tx.send(ExportEvent::Progress {
138 + completed,
139 + total,
140 + current_name: current_name.to_string(),
141 + });
142 +
143 + true
144 + },
145 + )
146 + }));
142 147
143 148 match summary {
144 - Ok(ExportSummary { total, errors }) => {
149 + Ok(Ok(ExportSummary { total, errors })) => {
145 150 let _ = event_tx.send(ExportEvent::Complete { total, errors });
146 151 }
147 - Err(e) => {
152 + Ok(Err(e)) => {
148 153 let _ = event_tx.send(ExportEvent::Complete {
149 154 total: 0,
150 155 errors: vec![("export".to_string(), e.to_string())],
151 156 });
152 157 }
158 + Err(_panic) => {
159 + let _ = event_tx.send(ExportEvent::Complete {
160 + total: 0,
161 + errors: vec![("export".to_string(), "export panicked (internal error)".to_string())],
162 + });
163 + }
153 164 }
154 165 }
155 166 }
M crates/audiofiles-browser/src/import.rs +19 -4
@@ -266,22 +266,37 @@ impl ImportContext<'_> {
266 266 let name = audiofiles_core::util::get_filename(path, "unknown");
267 267 self.send_progress(name);
268 268
269 - match import_single_file(path, vfs_id, parent_id, self.store, self.db, self.loose_files) {
270 - Ok(ImportFileResult::Imported(hash, ext)) => {
269 + // Catch panics so one malformed file can't unwind and kill the import
270 + // worker thread — that would wedge the UI in the progress screen forever,
271 + // since ImportEvent::Complete (sent only at loop end) is the only event
272 + // that exits Importing. A panicked file is counted as an error and the
273 + // import continues to completion.
274 + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
275 + import_single_file(path, vfs_id, parent_id, self.store, self.db, self.loose_files)
276 + }));
277 + match outcome {
278 + Ok(Ok(ImportFileResult::Imported(hash, ext))) => {
271 279 self.imported.push((hash, ext));
272 280 *self.completed += 1;
273 281 }
274 - Ok(ImportFileResult::Duplicate) => {
282 + Ok(Ok(ImportFileResult::Duplicate)) => {
275 283 *self.duplicates += 1;
276 284 *self.completed += 1;
277 285 }
278 - Err(e) => {
286 + Ok(Err(e)) => {
279 287 *self.errors += 1;
280 288 let _ = self.event_tx.send(ImportEvent::FileError {
281 289 path: path.display().to_string(),
282 290 error: e.to_string(),
283 291 });
284 292 }
293 + Err(_panic) => {
294 + *self.errors += 1;
295 + let _ = self.event_tx.send(ImportEvent::FileError {
296 + path: path.display().to_string(),
297 + error: "import panicked (internal error)".to_string(),
298 + });
299 + }
285 300 }
286 301 }
287 302 }
M crates/audiofiles-core/src/edit/normalize.rs +46 -13
@@ -1,6 +1,7 @@
1 1 //! Normalize operations: peak normalize and LUFS normalize.
2 2
3 3 use crate::analysis::basic::peak_db;
4 + use crate::analysis::loudness::measure_lufs;
4 5 use crate::error::CoreError;
5 6
6 7 /// Peak-normalize samples to the target dBFS level.
@@ -29,36 +30,36 @@ pub fn apply_normalize_peak(samples: &mut [f32], target_db: f64) -> Result<(), C
29 30 Ok(())
30 31 }
31 32
32 - /// LUFS-normalize samples to the target loudness level.
33 + /// LUFS-normalize samples to the target integrated loudness.
33 34 ///
34 - /// Uses a simplified integrated loudness measurement (mean square of
35 - /// all samples, which approximates ITU-R BS.1770 for short samples).
35 + /// Measures integrated loudness with the ITU-R BS.1770-4 meter
36 + /// ([`measure_lufs`] — the same meter analysis reports), then applies a flat
37 + /// gain to reach `target_lufs`. Like the peak path, this applies no clipping:
38 + /// loudness normalize can legitimately push peaks past full scale, and the
39 + /// final encode is the single place that clamps. `channels` is unused because
40 + /// the meter operates on the interleaved buffer (matching how analysis
41 + /// measures loudness), but is kept for signature symmetry with the other ops.
36 42 pub fn apply_normalize_lufs(
37 43 samples: &mut [f32],
38 44 _channels: u16,
39 - _sample_rate: u32,
45 + sample_rate: u32,
40 46 target_lufs: f64,
41 47 ) -> Result<(), CoreError> {
42 48 if samples.is_empty() {
43 49 return Ok(());
44 50 }
45 51
46 - // Compute integrated loudness (simplified: RMS-based approximation)
47 - let sum_sq: f64 = samples.iter().map(|&s| (s as f64) * (s as f64)).sum();
48 - let mean_sq = sum_sq / samples.len() as f64;
49 - if mean_sq <= 1e-20 {
50 - // Silent — nothing to normalize
52 + let current_lufs = measure_lufs(samples, sample_rate);
53 + if current_lufs <= -70.0 {
54 + // Silent / below the absolute gate — nothing to normalize.
51 55 return Ok(());
52 56 }
53 57
54 - // LUFS ≈ -0.691 + 10 * log10(mean_square) for simple signals
55 - // We use this approximation for short samples
56 - let current_lufs = -0.691 + 10.0 * mean_sq.log10();
57 58 let gain_db = target_lufs - current_lufs;
58 59 let scale = 10.0_f64.powf(gain_db / 20.0) as f32;
59 60
60 61 for sample in samples.iter_mut() {
61 - *sample = (*sample * scale).clamp(-1.0, 1.0);
62 + *sample *= scale;
62 63 }
63 64
64 65 Ok(())
@@ -132,4 +133,36 @@ mod tests {
132 133 apply_normalize_lufs(&mut samples, 1, 44100, -14.0).unwrap();
133 134 assert!(samples.iter().all(|&s| s == 0.0));
134 135 }
136 +
137 + #[test]
138 + fn normalize_lufs_hits_target_within_tolerance() {
139 + // A 1kHz tone at half amplitude, long enough for the BS.1770 gate.
140 + let sr = 48000u32;
141 + let mut samples: Vec<f32> = (0..sr)
142 + .map(|i| 0.5 * (2.0 * std::f32::consts::PI * 1000.0 * i as f32 / sr as f32).sin())
143 + .collect();
144 +
145 + apply_normalize_lufs(&mut samples, 1, sr, -14.0).unwrap();
146 +
147 + let measured = measure_lufs(&samples, sr);
148 + assert!(
149 + (measured - (-14.0)).abs() < 0.5,
150 + "normalized loudness should be ~-14 LUFS, got {measured}"
151 + );
152 + }
153 +
154 + #[test]
155 + fn normalize_lufs_does_not_silently_clip_when_boosting() {
156 + // A quiet tone boosted toward 0 LUFS will exceed full scale; the LUFS
157 + // path must NOT clamp (the old code did, silently distorting the tone).
158 + let sr = 48000u32;
159 + let mut samples: Vec<f32> = (0..sr)
160 + .map(|i| 0.05 * (2.0 * std::f32::consts::PI * 1000.0 * i as f32 / sr as f32).sin())
161 + .collect();
162 +
163 + apply_normalize_lufs(&mut samples, 1, sr, 0.0).unwrap();
164 +
165 + let peak = samples.iter().fold(0.0f32, |m, &s| m.max(s.abs()));
166 + assert!(peak > 1.0, "boosting a quiet tone to 0 LUFS should overshoot full scale, got peak {peak}");
167 + }
135 168 }
M crates/audiofiles-core/src/edit/worker.rs +15 -3
@@ -140,20 +140,32 @@ fn edit_worker_loop(
140 140 continue;
141 141 }
142 142
143 - match process_edit(&path, &operation, &cancel_flag) {
144 - Ok(result_path) => {
143 + // Catch panics so a malformed file (bad decode/encode) reports an
144 + // error instead of unwinding and killing the worker thread —
145 + // which would silently stop all future edits with no UI signal.
146 + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
147 + process_edit(&path, &operation, &cancel_flag)
148 + }));
149 + match outcome {
150 + Ok(Ok(result_path)) => {
145 151 let _ = event_tx.send(EditEvent::Complete {
146 152 source_hash: hash,
147 153 result_path,
148 154 operation,
149 155 });
150 156 }
151 - Err(e) => {
157 + Ok(Err(e)) => {
152 158 let _ = event_tx.send(EditEvent::Error {
153 159 hash,
154 160 error: e.to_string(),
155 161 });
156 162 }
163 + Err(_panic) => {
164 + let _ = event_tx.send(EditEvent::Error {
165 + hash,
166 + error: "edit panicked (internal error)".to_string(),
167 + });
168 + }
157 169 }
158 170 }
159 171 }
M crates/audiofiles-core/src/forge/worker.rs +24 -6
@@ -124,8 +124,13 @@ fn forge_worker_loop(
124 124 if cancel_flag.load(Ordering::Acquire) {
125 125 continue;
126 126 }
127 - match chop_to_temp(&source_path, &base_name, &method) {
128 - Ok(staging) => {
127 + // Isolate panics: a bad file must report an error, not unwind
128 + // the worker thread and silently stop all future forge jobs.
129 + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
130 + chop_to_temp(&source_path, &base_name, &method)
131 + }));
132 + match outcome {
133 + Ok(Ok(staging)) => {
129 134 // If cancelled mid-flight, drop the staged temp files
130 135 // rather than handing them to the GUI.
131 136 if cancel_flag.load(Ordering::Acquire) {
@@ -136,11 +141,16 @@ fn forge_worker_loop(
136 141 }
137 142 let _ = event_tx.send(ForgeEvent::ChopComplete { staging });
138 143 }
139 - Err(e) => {
144 + Ok(Err(e)) => {
140 145 let _ = event_tx.send(ForgeEvent::Error {
141 146 error: e.to_string(),
142 147 });
143 148 }
149 + Err(_panic) => {
150 + let _ = event_tx.send(ForgeEvent::Error {
151 + error: "chop panicked (internal error)".to_string(),
152 + });
153 + }
144 154 }
145 155 }
146 156 ForgeCommand::Conform {
@@ -154,19 +164,27 @@ fn forge_worker_loop(
154 164 if cancel_flag.load(Ordering::Acquire) {
155 165 continue;
156 166 }
157 - match conform_to_temp(&source_path, &base_name, &target, auto_trim) {
158 - Ok(staging) => {
167 + let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
168 + conform_to_temp(&source_path, &base_name, &target, auto_trim)
169 + }));
170 + match outcome {
171 + Ok(Ok(staging)) => {
159 172 if cancel_flag.load(Ordering::Acquire) {
160 173 let _ = std::fs::remove_file(&staging.temp_path);
161 174 continue;
162 175 }
163 176 let _ = event_tx.send(ForgeEvent::ConformComplete { staging });
164 177 }
165 - Err(e) => {
178 + Ok(Err(e)) => {
166 179 let _ = event_tx.send(ForgeEvent::Error {
167 180 error: e.to_string(),
168 181 });
169 182 }
183 + Err(_panic) => {
184 + let _ = event_tx.send(ForgeEvent::Error {
185 + error: "conform panicked (internal error)".to_string(),
186 + });
187 + }
170 188 }
171 189 }
172 190 }
M crates/audiofiles-core/src/store.rs +15 -3
@@ -187,13 +187,25 @@ impl SampleStore {
187 187 }
188 188 }
189 189
190 - // Insert into DB (ignore if hash already exists)
190 + // Insert into DB (ignore if hash already exists). If this fails after we
191 + // just wrote a fresh blob, that blob is unreferenced — no row points at
192 + // it, and since `needs_write` was true nothing pointed at it before
193 + // either. Unlink it so a DB-write failure (disk full on the WAL, lock
194 + // timeout) can't leak an unreachable blob that no GC path reclaims —
195 + // upholding this module's "never orphan a blob" invariant (see remove()).
196 + // When `needs_write` was false the blob pre-existed and may be shared by
197 + // another row, so it is left untouched.
191 198 let now = unix_now();
192 - db.conn().execute(
199 + if let Err(e) = db.conn().execute(
193 200 "INSERT OR IGNORE INTO samples (hash, original_name, file_extension, file_size, import_date, last_modified, duration)
194 201 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
195 202 rusqlite::params![hash, original_name, ext, file_size, now, now, duration],
196 - )?;
203 + ) {
204 + if needs_write {
205 + let _ = fs::remove_file(&dest);
206 + }
207 + return Err(CoreError::Db(e));
208 + }
197 209
198 210 Ok(hash)
199 211 }