diff --git a/crates/engine/src/proxies.rs b/crates/engine/src/proxies.rs index d602102f..f7ca3cfa 100644 --- a/crates/engine/src/proxies.rs +++ b/crates/engine/src/proxies.rs @@ -378,6 +378,56 @@ pub fn create(s: &mut Session, p: &Value) -> Result { Ok(json!({"job": job, "preset": pr.id, "outputs": outputs, "skipped": skipped})) } +/// Copy to a new file, numbering collisions without replacing another take. Open the input +/// first and create the output exclusively, so a failed read or a competing copy cannot replace +/// an existing destination. Relink only after the copy's identity has been checked. +fn copy_for_ingest(s: &mut Session, item: ItemId, path: &str, dir: &Path) -> Result { + let source = Path::new(path); + let name = source.file_name().ok_or_else(|| bad("ingest", "source has no file name"))?; + let mut input = std::fs::File::open(source).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; + let mut number = 1u64; + let (out, mut output) = loop { + let mut next = if number == 1 { name.to_os_string() } else { source.file_stem().unwrap_or(name).to_os_string() }; + if number != 1 { + next.push(format!(" {number}")); + if let Some(extension) = source.extension() { + next.push("."); + next.push(extension); + } + } + let out = dir.join(next); + match std::fs::OpenOptions::new().write(true).create_new(true).open(&out) { + Ok(output) => break (out, output), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + number = number.checked_add(1).ok_or_else(|| bad("ingest", "no unused destination name"))?; + } + Err(e) => return Err(EngineError::Other(format!("{}: {e}", out.display()))), + } + }; + let copied = std::io::copy(&mut input, &mut output).and_then(|_| output.set_permissions(input.metadata()?.permissions())); + drop(output); + let out_s = out.to_string_lossy().into_owned(); + let linked = copied.map_err(|e| EngineError::Other(format!("copying {path}: {e}"))).and_then(|_| { + let a = crate::relink::identity_of(&*s.services, path).map_err(|e| EngineError::Other(e.to_string()))?; + let b = crate::relink::identity_of(&*s.services, &out_s).map_err(|e| EngineError::Other(e.to_string()))?; + if a != b { + return Err(EngineError::Other(format!("the copy of {} doesn't match the original", name.to_string_lossy()))); + } + // The generated suffix may change the name; every other relink check still applies. + crate::relink::relink(s, &json!({"item": item.0, "path": out_s, "relinkOthers": false, "match": {"fileName": false}}))?; + Ok(()) + }); + if let Err(error) = linked { + // Only this operation's exclusively created file can be removed on failure. + if let Err(cleanup) = std::fs::remove_file(&out) { + return Err(EngineError::Other(format!("{error}; removing incomplete copy {}: {cleanup}", out.display()))); + } + return Err(error); + } + Ok(out_s) +} + /// Ingest freshly imported items per the project's ingest settings. pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result { let ing = s.project.settings.ingest.clone(); @@ -393,21 +443,10 @@ pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result { Some(d) if !d.is_empty() => PathBuf::from(d), _ => Path::new(&path).parent().unwrap_or(Path::new(".")).join("Ingested Media"), }; - let fname = Path::new(&path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - let out = dir.join(&fname); - if Path::new(&path) == out { + if Path::new(&path).file_name().is_some_and(|name| Path::new(&path) == dir.join(name)) { continue; } - std::fs::create_dir_all(&dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?; - std::fs::copy(&path, &out).map_err(|e| EngineError::Other(format!("copying {fname}: {e}")))?; - let out_s = out.to_string_lossy().into_owned(); - // verify the copy before switching to it - let a = crate::relink::identity_of(&*s.services, &path).map_err(|e| EngineError::Other(e.to_string()))?; - let b = crate::relink::identity_of(&*s.services, &out_s).map_err(|e| EngineError::Other(e.to_string()))?; - if a != b { - return Err(EngineError::Other(format!("the copy of {fname} doesn't match the original"))); - } - crate::relink::relink(s, &json!({"item": i.0, "path": out_s, "relinkOthers": false}))?; + let out_s = copy_for_ingest(s, i, &path, &dir)?; copied.push(json!({"item": i.0, "path": out_s})); } } @@ -423,6 +462,7 @@ pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result { let pr = preset(if ing.preset.is_empty() { DEFAULT_TRANSCODE_PRESET } else { ing.preset.as_str() }).ok_or_else(|| bad("ingest", "unknown preset"))?; let mut work = Vec::new(); + let mut taken = Vec::new(); for &i in items { let Some((clip, path, name)) = media_path(&s.project, i) else { continue }; if clip.info.video.is_none() { @@ -433,7 +473,9 @@ pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result { _ => Path::new(&path).parent().unwrap_or(Path::new(".")).join("Ingested Media"), }; let stem = Path::new(&path).file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or(name); - work.push((i, unique_path(&dir, &stem, "", pr.extension, &[]).to_string_lossy().into_owned(), pr)); + let out = unique_path(&dir, &stem, "", pr.extension, &taken).to_string_lossy().into_owned(); + taken.push(out.clone()); + work.push((i, out, pr)); } if !work.is_empty() { let id = start_job(s, format!("Ingest: transcode {} clip(s)", work.len()), work, OnDone::ReplaceMedia, false)?; diff --git a/crates/engine/src/proxies_tests.rs b/crates/engine/src/proxies_tests.rs index 6cff74e3..712834cf 100644 --- a/crates/engine/src/proxies_tests.rs +++ b/crates/engine/src/proxies_tests.rs @@ -124,6 +124,155 @@ fn ingest_copies_and_creates_proxies_on_import() { let _ = std::fs::remove_dir_all(&root); } +/// Import two different camera takes with the same basename into a folder that already has +/// that name. Check the saved project with a new media pool so cached decoders cannot hide a +/// replaced file. +fn check_ingest_collision(action: &str) { + use filmcraft_media::FrameRequest; + use filmcraft_project::{ItemId, MediaRef}; + use filmcraft_time::FrameRate; + use std::collections::BTreeSet; + use std::sync::atomic::Ordering; + + let root = tmp_dir(&format!("ingest-collision-{action}")); + let paths = [root.join("Card A/A001.mov"), root.join("Card B/A001.mov")]; + for (path, scene) in paths.iter().zip([DemoScene::Aurora, DemoScene::Plasma]) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + make_movie(path, scene, 160, 96, 4); + } + let original: Vec> = paths.iter().map(|p| std::fs::read(p).unwrap()).collect(); + assert_ne!(original[0], original[1], "different source takes"); + let dest = root.join("Ingest"); + std::fs::create_dir_all(&dest).unwrap(); + let existing = dest.join("A001.mov"); + let existing_bytes = b"existing destination must survive ingest"; + std::fs::write(&existing, existing_bytes).unwrap(); + let mut s = Session::default(); + s.execute("project.ingestSettings", json!({"enabled": true, "action": action, "destination": dest.to_string_lossy(), "preset": "prores_proxy_half"})) + .unwrap(); + let result = s.execute("file.import", json!({"paths": paths.iter().map(|p| p.to_string_lossy()).collect::>()})).unwrap(); + assert_eq!(result["errors"], json!([]), "{result}"); + let items: Vec = result["items"].as_array().unwrap().iter().map(|i| ItemId(i.as_u64().unwrap())).collect(); + assert_eq!(items.len(), 2, "both takes imported"); + if let Some(id) = result["ingest"]["job"]["job"].as_u64() { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + let job = s.jobs.iter().find(|j| j.id == id).unwrap(); + if job.progress.finished.load(Ordering::Relaxed) && job.result.lock().unwrap().is_some() { + assert!(job.result.lock().unwrap().as_ref().unwrap().is_ok(), "ingest job failed"); + break; + } + assert!(std::time::Instant::now() < deadline, "ingest job did not finish"); + std::thread::sleep(std::time::Duration::from_millis(25)); + } + s.poll_persistence(); + } + assert_eq!(std::fs::read(&existing).unwrap(), existing_bytes, "ingest must preserve existing destination files"); + let project_path = root.join("ingested.fcproj"); + s.execute("file.save", json!({"path": project_path.to_string_lossy()})).unwrap(); + let mut reopened = Session::default(); + reopened.execute("file.open", json!({"path": project_path.to_string_lossy()})).unwrap(); + let request = FrameRequest { time: FrameRate::FPS_24.tick_of(2), scale: 1.0 }; + let mut linked = BTreeSet::new(); + let mut proxies = BTreeSet::new(); + for (index, item) in items.iter().enumerate() { + let media = reopened.project.item(*item).unwrap().as_media().unwrap(); + let MediaRef::File { path } = &media.media else { panic!("ingest must retain file media") }; + assert!(linked.insert(path.clone()), "different takes must have different ingest paths"); + assert_eq!(std::path::Path::new(path).parent(), Some(dest.as_path())); + assert_eq!(std::fs::read(&paths[index]).unwrap(), original[index], "source card is unchanged"); + if action != "transcode" { + assert_eq!(std::fs::read(path).unwrap(), original[index], "each copy retains its own take"); + assert_eq!(media.identity, Some(crate::relink::identity_of_bytes(&original[index]))); + } + let source = reopened.media.open_file(&paths[index].to_string_lossy(), &*reopened.services).unwrap(); + let expected = source.video_frame(request).unwrap(); + let ingested = reopened.source(*item).unwrap().video_frame(request).unwrap(); + let quality = |frame: &filmcraft_frame::VideoFrame| { + let rgba = image::RgbaImage::from_raw(frame.width, frame.height, frame.to_rgba8()).unwrap(); + let expected = image::RgbaImage::from_raw(expected.width, expected.height, expected.to_rgba8()).unwrap(); + let expected = image::imageops::resize(&expected, frame.width, frame.height, image::imageops::FilterType::Triangle); + psnr(rgba.as_raw(), expected.as_raw()) + }; + assert!(quality(&ingested) > 30.0, "saved project retains the correct picture for take {index}"); + if action == "copyAndCreateProxies" { + let Some(MediaRef::File { path }) = &media.proxy else { panic!("copy should have its proxy attached") }; + assert!(proxies.insert(path.clone()), "different takes must have different proxy paths"); + let proxy = reopened.media.open_file(path, &*reopened.services).unwrap().video_frame(request).unwrap(); + assert!(quality(&proxy) > 30.0, "proxy retains the correct picture for take {index}"); + } + } + let _ = std::fs::remove_dir_all(root); +} + +#[test] +fn ingest_copy_preserves_same_named_takes_and_existing_files() { + check_ingest_collision("copy"); +} + +#[test] +fn ingest_copy_and_proxy_preserves_same_named_takes_and_existing_files() { + check_ingest_collision("copyAndCreateProxies"); +} + +#[test] +fn ingest_transcode_preserves_same_named_takes_and_existing_files() { + check_ingest_collision("transcode"); +} + +#[test] +fn ingest_copy_keeps_sources_already_at_destination_and_extensionless_names() { + use filmcraft_project::{ItemId, MediaRef}; + + for (name, numbered) in [("Take.wav", "Take 2.wav"), ("Take", "Take 2")] { + let root = tmp_dir("ingest-copy-existing-source"); + let dest = root.join("Ingest"); + let card = root.join("Card"); + std::fs::create_dir_all(&dest).unwrap(); + std::fs::create_dir_all(&card).unwrap(); + let paths = [dest.join(name), card.join(name)]; + let takes = [filmcraft_media::wav::write_wav16(&[0.25; 960], 1, 48_000), filmcraft_media::wav::write_wav16(&[-0.25; 960], 1, 48_000)]; + for (path, bytes) in paths.iter().zip(&takes) { + std::fs::write(path, bytes).unwrap(); + } + let mut s = Session::default(); + s.execute("project.ingestSettings", json!({"enabled": true, "action": "copy", "destination": dest.to_string_lossy()})).unwrap(); + let result = s.execute("file.import", json!({"paths": paths.iter().map(|p| p.to_string_lossy()).collect::>()})).unwrap(); + assert_eq!(result["errors"], json!([]), "{result}"); + assert_eq!(result["items"].as_array().unwrap().len(), 2); + assert_eq!(result["ingest"]["copied"].as_array().unwrap().len(), 1, "source already at destination is kept"); + for (index, path) in [dest.join(name), dest.join(numbered)].iter().enumerate() { + let item = ItemId(result["items"][index].as_u64().unwrap()); + let media = s.project.item(item).unwrap().as_media().unwrap(); + assert_eq!(media.media, MediaRef::File { path: path.to_string_lossy().into_owned() }); + assert_eq!(std::fs::read(path).unwrap(), takes[index]); + assert_eq!(std::fs::read(&paths[index]).unwrap(), takes[index]); + } + assert_eq!(std::fs::read_dir(&dest).unwrap().count(), 2, "no unnecessary copies or trailing-dot filenames"); + let _ = std::fs::remove_dir_all(root); + } +} + +#[test] +fn ingest_copy_does_not_create_a_destination_when_the_source_is_missing() { + use filmcraft_project::ItemId; + + let root = tmp_dir("ingest-copy-missing-source"); + let source = root.join("Take.wav"); + std::fs::write(&source, filmcraft_media::wav::write_wav16(&[0.25; 960], 1, 48_000)).unwrap(); + let mut s = Session::default(); + let imported = s.execute("file.import", json!({"paths": [source.to_string_lossy()]})).unwrap(); + let item = ItemId(imported["items"][0].as_u64().unwrap()); + let dest = root.join("Ingest"); + s.execute("project.ingestSettings", json!({"enabled": true, "action": "copy", "destination": dest.to_string_lossy()})).unwrap(); + let before = s.project.clone(); + std::fs::rename(&source, root.join("Moved.wav")).unwrap(); + assert!(crate::proxies::ingest(&mut s, &[item]).is_err()); + assert_eq!(s.project, before, "failed copy does not relink the item"); + assert!(!dest.exists(), "no destination is created before the source can be opened"); + let _ = std::fs::remove_dir_all(root); +} + /// Measure: Program-monitor playback cost of 4K H.264 with and without proxies (sequential frames /// at ½ resolution). Run with `RAYON_NUM_THREADS=1` so wall time ≈ CPU time: /// `cargo test --release -p filmcraft-engine proxy_playback_perf_4k -- --ignored --nocapture`. diff --git a/docs/project-files.md b/docs/project-files.md index 0298571c..752f5f23 100644 --- a/docs/project-files.md +++ b/docs/project-files.md @@ -362,6 +362,11 @@ destination?, preset?}` (Project Settings ▸ Ingest) acts on every `file.import - **createProxies** makes proxies in the background and attaches them; - **copyAndCreateProxies** does both. +Copied files keep their names when available; collisions receive a numbered suffix before the +extension (`A001 2.mov`) and never replace an existing file. Transcodes reserve distinct output +names for every clip in the import batch. Copies keep their source fingerprint, and each proxy +stays attached to its corresponding take. + `file.import` reports this under `ingest`. ## Project Manager