Skip to main content

max / audiofiles

fix: wire real cancel flag through streaming export (G2) run_export passed a throwaway `&AtomicBool::new(false)` to try_stream_wav_export / try_stream_aiff_export, so the streaming encoders' mid-file cancel checks (stream.rs) could never fire in production — a large single-file streamed export could only be cancelled at file boundaries, not during the file. Only tests ever passed a live flag. Thread a real `cancel: &AtomicBool` through run_export -> export_single_item -> the streaming encoders. The export worker now passes ctx.cancel_flag() (the same flag its between-files callback already checks). run_export also polls the flag directly between items, and treats a mid-stream `CoreError::Cancelled` as a clean batch stop rather than a per-item error. +2 tests: a mid-file cancel aborts the stream with no output file and no recorded error; the un-cancelled control produces a valid RIFF file. Updated the ~17 existing run_export call sites (core tests + e2e) for the new arg. cargo test --workspace: 1092 pass; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-04 20:06 UTC
Commit: 0a8d70177df41aad8d5f70fe2121d51866cf178a
Parent: ee8ffcd
4 files changed, +126 insertions, -22 deletions
M crates/audiofiles-browser/src/export.rs +1
@@ -95,6 +95,7 @@ fn export_step(store: &mut SampleStore, cmd: ExportCommand, ctx: &WorkerCtx<Expo
95 95 &items,
96 96 &config,
97 97 store,
98 + ctx.cancel_flag(),
98 99 |completed, total, current_name| {
99 100 // Cooperative cancel between files (flag set by the handle on Cancel
100 101 // or by the runtime on Drop).
M crates/audiofiles-core/src/export/mod.rs +105 -15
@@ -251,6 +251,7 @@ mod tests {
251 251 use std::fs;
252 252 use std::io::Write;
253 253 use std::path::Path;
254 + use std::sync::atomic::AtomicBool;
254 255
255 256 fn setup_vfs_with_samples(db: &Database, store: &SampleStore, dir: &Path) -> crate::VfsId {
256 257 let vfs_id = vfs::create_vfs(db, "TestVFS").unwrap();
@@ -333,7 +334,7 @@ mod tests {
333 334 name_overrides: None,
334 335 };
335 336
336 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
337 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
337 338 assert_eq!(summary.total, 1);
338 339 assert!(summary.errors.is_empty());
339 340
@@ -373,7 +374,7 @@ mod tests {
373 374 name_overrides: None,
374 375 };
375 376
376 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
377 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
377 378 assert_eq!(summary.total, 1);
378 379 assert!(summary.errors.is_empty());
379 380
@@ -412,7 +413,7 @@ mod tests {
412 413 name_overrides: None,
413 414 };
414 415
415 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
416 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
416 417 assert!(summary.errors.is_empty());
417 418
418 419 let exported = dest_dir.join("Drums").join("kick.wav");
@@ -446,7 +447,7 @@ mod tests {
446 447 name_overrides: None,
447 448 };
448 449
449 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
450 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
450 451 assert!(summary.errors.is_empty());
451 452
452 453 // Should be flat (no Drums/ subdirectory)
@@ -492,7 +493,7 @@ mod tests {
492 493 name_overrides: None,
493 494 };
494 495
495 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
496 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
496 497 assert!(summary.errors.is_empty());
497 498
498 499 let sidecar = dest_dir.join("Drums").join("kick.wav.audiofiles.json");
@@ -529,7 +530,7 @@ mod tests {
529 530 name_overrides: None,
530 531 };
531 532
532 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
533 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
533 534 assert!(summary.errors.is_empty());
534 535
535 536 let exported = dest_dir.join("Drums").join("kick.wav");
@@ -593,7 +594,7 @@ mod tests {
593 594 name_overrides: None,
594 595 };
595 596
596 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
597 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
597 598 assert!(summary.errors.is_empty());
598 599
599 600 // "kick" uppercased -> "KICK", truncated to 8 (already short enough)
@@ -627,7 +628,7 @@ mod tests {
627 628 name_overrides: None,
628 629 };
629 630
630 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
631 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
631 632 assert_eq!(summary.errors.len(), 1);
632 633 assert!(summary.errors[0].1.contains("exceeds device file size limit"));
633 634
@@ -676,7 +677,7 @@ mod tests {
676 677 name_overrides: None,
677 678 };
678 679
679 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
680 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
680 681 assert!(summary.errors.is_empty());
681 682
682 683 // Both sanitize to "KICK" -> second gets "_2" suffix
@@ -719,7 +720,7 @@ mod tests {
719 720 name_overrides: None,
720 721 };
721 722
722 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
723 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
723 724 assert!(summary.errors.is_empty());
724 725
725 726 // Pattern produces "01_kick", rules uppercase to "01_KICK"
@@ -904,7 +905,7 @@ mod tests {
904 905 name_overrides: None,
905 906 };
906 907
907 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
908 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
908 909 assert_eq!(summary.total, 1);
909 910 assert!(summary.errors.is_empty());
910 911
@@ -950,7 +951,7 @@ mod tests {
950 951 };
951 952
952 953 // Cancel after the first item
953 - let summary = run_export(&items, &config, &store, |completed, _, _| completed < 1).unwrap();
954 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |completed, _, _| completed < 1).unwrap();
954 955 // Total reflects all items, but only 1 should have been processed
955 956 assert_eq!(summary.total, items.len());
956 957
@@ -962,6 +963,95 @@ mod tests {
962 963 }
963 964
964 965 #[test]
966 + fn streaming_export_cancel_is_wired_through() {
967 + // Regression for the dead-wired streaming cancel: run_export used to pass
968 + // a throwaway AtomicBool to the streaming encoder, so a mid-file cancel
969 + // could never fire. With the real flag threaded through, requesting cancel
970 + // before the item streams aborts it cleanly — no output file, no error.
971 + let dir = tempfile::tempdir().unwrap();
972 + let db = Database::open_in_memory().unwrap();
973 + let store = SampleStore::new(dir.path().join("store")).unwrap();
974 + // A PCM WAV with an exact frame count takes the streaming export path.
975 + let src = dir.path().join("long.wav");
976 + let samples: Vec<f32> = (0..8000).map(|i| ((i % 100) as f32 / 100.0) - 0.5).collect();
977 + write_test_wav(&src, 1, 44100, &samples);
978 + let hash = store.import(&src, &db).unwrap();
979 + let vfs_id = vfs::create_vfs(&db, "v").unwrap();
980 + vfs::create_sample_link(&db, vfs_id, None, "long.wav", &hash).unwrap();
981 + let items = collect_export_items(&db, vfs_id, None).unwrap();
982 +
983 + let dest_dir = dir.path().join("export_stream_cancel");
984 + let config = ExportConfig {
985 + format: ExportFormat::Wav,
986 + sample_rate: None,
987 + bit_depth: Some(16),
988 + channels: ExportChannels::Original,
989 + naming_pattern: None,
990 + flatten: true,
991 + metadata_sidecar: false,
992 + destination: dest_dir.clone(),
993 + device_profile: None,
994 + naming_rules: None,
995 + max_file_size_bytes: None,
996 + name_overrides: None,
997 + };
998 +
999 + // The callback requests cancel just before the item streams; the streaming
1000 + // encoder polls the same flag and returns Cancelled on its first check.
1001 + let cancel = AtomicBool::new(false);
1002 + let summary = run_export(&items, &config, &store, &cancel, |_, _, _| {
1003 + cancel.store(true, std::sync::atomic::Ordering::Relaxed);
1004 + true
1005 + })
1006 + .unwrap();
1007 +
1008 + // Clean stop: not recorded as a per-item failure...
1009 + assert!(summary.errors.is_empty(), "cancel should be clean, got {:?}", summary.errors);
1010 + // ...and the streamed file was never completed (write_atomic drops the tmp).
1011 + let out = dest_dir.join("long.wav");
1012 + assert!(!out.exists(), "cancelled stream must leave no output file");
1013 + }
1014 +
1015 + #[test]
1016 + fn streaming_export_completes_when_not_cancelled() {
1017 + // Control for the test above: the same streaming path produces a valid
1018 + // file when cancel stays false.
1019 + let dir = tempfile::tempdir().unwrap();
1020 + let db = Database::open_in_memory().unwrap();
1021 + let store = SampleStore::new(dir.path().join("store")).unwrap();
1022 + let src = dir.path().join("long.wav");
1023 + let samples: Vec<f32> = (0..8000).map(|i| ((i % 100) as f32 / 100.0) - 0.5).collect();
1024 + write_test_wav(&src, 1, 44100, &samples);
1025 + let hash = store.import(&src, &db).unwrap();
1026 + let vfs_id = vfs::create_vfs(&db, "v").unwrap();
1027 + vfs::create_sample_link(&db, vfs_id, None, "long.wav", &hash).unwrap();
1028 + let items = collect_export_items(&db, vfs_id, None).unwrap();
1029 +
1030 + let dest_dir = dir.path().join("export_stream_ok");
1031 + let config = ExportConfig {
1032 + format: ExportFormat::Wav,
1033 + sample_rate: None,
1034 + bit_depth: Some(16),
1035 + channels: ExportChannels::Original,
1036 + naming_pattern: None,
1037 + flatten: true,
1038 + metadata_sidecar: false,
1039 + destination: dest_dir.clone(),
1040 + device_profile: None,
1041 + naming_rules: None,
1042 + max_file_size_bytes: None,
1043 + name_overrides: None,
1044 + };
1045 +
1046 + let summary =
1047 + run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
1048 + assert!(summary.errors.is_empty());
1049 + let out = dest_dir.join("long.wav");
1050 + assert!(out.exists(), "un-cancelled stream must produce output");
1051 + assert_eq!(&fs::read(&out).unwrap()[0..4], b"RIFF");
1052 + }
1053 +
1054 + #[test]
965 1055 fn export_missing_source_records_error() {
966 1056 let dir = tempfile::tempdir().unwrap();
967 1057 let store = SampleStore::new(dir.path().join("store")).unwrap();
@@ -996,7 +1086,7 @@ mod tests {
996 1086 name_overrides: None,
997 1087 };
998 1088
999 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
1089 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
1000 1090 assert_eq!(summary.total, 1);
1001 1091 assert_eq!(summary.errors.len(), 1);
1002 1092 assert_eq!(summary.errors[0].0, "missing.wav");
@@ -1034,7 +1124,7 @@ mod tests {
1034 1124 name_overrides: None,
1035 1125 };
1036 1126
1037 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
1127 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
1038 1128 assert!(summary.errors.is_empty());
1039 1129
1040 1130 let sidecar_path = dest_dir.join("kick.wav.audiofiles.json");
@@ -1079,7 +1169,7 @@ mod tests {
1079 1169 name_overrides: None,
1080 1170 };
1081 1171
1082 - let summary = run_export(&items, &config, &store, |_, _, _| true).unwrap();
1172 + let summary = run_export(&items, &config, &store, &AtomicBool::new(false), |_, _, _| true).unwrap();
1083 1173 assert!(summary.errors.is_empty());
1084 1174
1085 1175 // Should be in Drums/ subdirectory
M crates/audiofiles-core/src/export/runner.rs +18 -5
@@ -2,6 +2,7 @@
2 2
3 3 use std::fs;
4 4 use std::path::{Path, PathBuf};
5 + use std::sync::atomic::{AtomicBool, Ordering};
5 6
6 7 use crate::error::{io_err, Result};
7 8 use crate::rename::RenamePattern;
@@ -60,12 +61,16 @@ fn write_sidecar(dest: &Path, item: &ExportItem) -> Result<()> {
60 61 /// Run the export pipeline: for each item, resolve source, optionally convert, write.
61 62 ///
62 63 /// `progress_callback` is called with `(completed, total, current_name)`.
63 - /// Return `false` from the callback to cancel.
64 + /// Return `false` from the callback to cancel between files. `cancel` is polled
65 + /// both between files and *inside* the streaming encoder, so a single large
66 + /// streamed export can be interrupted mid-file rather than only at file
67 + /// boundaries.
64 68 #[instrument(skip_all)]
65 69 pub fn run_export(
66 70 items: &[ExportItem],
67 71 config: &ExportConfig,
68 72 store: &SampleStore,
73 + cancel: &AtomicBool,
69 74 mut progress_callback: impl FnMut(usize, usize, &str) -> bool,
70 75 ) -> Result<ExportSummary> {
71 76 let total = items.len();
@@ -82,7 +87,9 @@ pub fn run_export(
82 87 let output_names = resolve_output_names(items, config, pattern.as_ref());
83 88
84 89 for (i, item) in items.iter().enumerate() {
85 - if !progress_callback(i, total, &item.name) {
90 + // Honor the cancel flag directly (the streaming encoder polls the same
91 + // flag mid-file), in addition to the callback's cooperative return.
92 + if cancel.load(Ordering::Relaxed) || !progress_callback(i, total, &item.name) {
86 93 break; // cancelled
87 94 }
88 95
@@ -141,7 +148,12 @@ pub fn run_export(
141 148 // Avoid silently overwriting existing files in the user's export dir.
142 149 let dest = resolve_collision(&dest);
143 150
144 - if let Err(e) = export_single_item(&source, &dest, item, config) {
151 + if let Err(e) = export_single_item(&source, &dest, item, config, cancel) {
152 + // A mid-file streaming cancel surfaces as `Cancelled`; stop the batch
153 + // cleanly rather than recording it as a per-item export failure.
154 + if matches!(e, crate::error::CoreError::Cancelled) {
155 + break;
156 + }
145 157 errors.push((item.name.clone(), e.to_string()));
146 158 continue;
147 159 }
@@ -207,6 +219,7 @@ fn export_single_item(
207 219 dest: &Path,
208 220 _item: &ExportItem,
209 221 config: &ExportConfig,
222 + cancel: &AtomicBool,
210 223 ) -> Result<()> {
211 224 match config.format {
212 225 ExportFormat::Original => {
@@ -229,7 +242,7 @@ fn export_single_item(
229 242 &config.channels,
230 243 config.sample_rate,
231 244 bit_depth,
232 - &std::sync::atomic::AtomicBool::new(false),
245 + cancel,
233 246 )?;
234 247 if streamed {
235 248 return Ok(());
@@ -257,7 +270,7 @@ fn export_single_item(
257 270 &config.channels,
258 271 config.sample_rate,
259 272 bit_depth,
260 - &std::sync::atomic::AtomicBool::new(false),
273 + cancel,
261 274 )?;
262 275 if streamed {
263 276 return Ok(());
M crates/audiofiles-core/tests/e2e_pipeline.rs +2 -2
@@ -305,7 +305,7 @@ fn e2e_import_analyze_search_tag_export() {
305 305 name_overrides: None,
306 306 };
307 307
308 - let summary = run_export(&items, &export_config, &env.store, |_, _, _| true).unwrap();
308 + let summary = run_export(&items, &export_config, &env.store, &std::sync::atomic::AtomicBool::new(false), |_, _, _| true).unwrap();
309 309 assert_eq!(summary.total, 1);
310 310 assert!(summary.errors.is_empty(), "export errors: {:?}", summary.errors);
311 311
@@ -348,7 +348,7 @@ fn e2e_import_analyze_search_tag_export() {
348 348 };
349 349
350 350 let summary_wav =
351 - run_export(&items, &export_config_wav, &env.store, |_, _, _| true).unwrap();
351 + run_export(&items, &export_config_wav, &env.store, &std::sync::atomic::AtomicBool::new(false), |_, _, _| true).unwrap();
352 352 assert!(summary_wav.errors.is_empty(), "WAV export errors: {:?}", summary_wav.errors);
353 353
354 354 let exported_wav = export_dest_wav.join("test_sine_440.wav");