ingest: preserve same-named camera takes during ingest

Co-authored-by: Codex <[email protected]>

crates/engine/src/proxies.rs

@@ -378,6 +378,56 @@ pub fn create(s: &mut Session, p: &Value) -> Result<Value> {
     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<String> {
+    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<Value> {
     let ing = s.project.settings.ingest.clone();
@@ -393,21 +443,10 @@ pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result<Value> {
                 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<Value> {
             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<Value> {
                     _ => 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)?;

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<Vec<u8>> = 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::<Vec<_>>()})).unwrap();
+    assert_eq!(result["errors"], json!([]), "{result}");
+    let items: Vec<ItemId> = 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::<Vec<_>>()})).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`.

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