diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index a8531c9ac..e2ae200cb 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -24,6 +24,25 @@ name: Nix build # front of every merge. Promote it once the schedule has reported a few times. on: workflow_dispatch: + # The job that actually builds the derivation now runs on the pull requests that + # can break it. It did not, and the gap was not academic: `nix-check.yml` only + # compares npmDepsHash, so a PR rewriting the addon's source filter, its RPATH + # handling or its symbols.map went green on ~18 checks without one of them + # building it, and the first real signal arrived on main half an hour after the + # merge. #371 shipped a change to `nix/compositor-view.nix` that way. + # + # Path-filtered rather than universal: this takes about half an hour, and a PR + # that touches none of these files cannot change what it produces. + pull_request: + paths: + - flake.nix + - flake.lock + - nix/** + - crates/** + - package-lock.json + # Including itself, or a PR that only edits this file gets no validation of + # the change it is making -- the same rule nix-check.yml already follows. + - .github/workflows/nix-build.yml push: branches: [main] schedule: @@ -56,7 +75,11 @@ concurrency: # That is the affordable half. Verifying each merge would need a queue this # workflow does not have, and is not worth it for a half-hour job whose purpose # is catching drift rather than gating a commit. - cancel-in-progress: false + # + # On a pull request the opposite is right: a new push makes the previous run + # answer a question nobody is asking any more, and at half an hour each they + # would pile up. `github.ref` is the PR's merge ref, so the group is per-PR. + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: build: @@ -421,84 +444,100 @@ jobs: fi # The real acceptance test. Everything above proves the package starts - # and can list a screen; none of it touches the compositor addon, which - # is what actually renders output. Record a couple of seconds, export it, - # and look at what came out. + # and can answer an enumeration call; none of it touches the compositor + # addon, which is what actually renders output. # - # This block was briefly moved ahead of the sources loop and moved back, - # so that it is not tried a third time. The theory was that position - # explained why record seemed to fail far more often than sources -- - # run_cli spawns a fresh `xvfb-run -a` each time, so record was always - # invocations 6-8, after five Xvfb servers had come and gone. The - # experiment could not answer it: by the time it ran, record had started - # succeeding from its old position anyway, so there was no contrast left - # to measure. From position 4 it succeeded, which proves nothing it was - # not already doing from position 9. + # It used to record two seconds and export the result. That never once + # worked here. `record` needs a display index and this host has no + # display to give: Chromium's X11 capturer logs + # "screen_capturer_x11.cc: Failed to initialize pixel buffer", `sources` + # answers with `displays: []`, and record dies on "Display index 0 not + # found (0 screen(s) available)". So export never ran, and the step that + # exists to vouch for the addon vouched for nothing -- runs 32707512544 + # (24/08) and 32827253816 (25/08) failed exactly here, in the 34-36 + # minutes they took to get past the build. # - # What the runs did establish is that the premise was wrong. Enumeration - # here is bimodal -- 12-31ms when it answers, no return at all when it - # does not, with nothing in between across every measurement so far -- - # and the failures cluster by run and by window within a run rather than - # by command. The apparent record-versus-sources gap was that clustering - # seen through a denominator, not a property of either path. Reopen this - # with the run_cli labels, on a run that actually fails, before assuming - # otherwise. + # Measured before rewriting it, against the built artefact under Xvfb: + # with `xvfb-run -a` as this workflow invokes it, 4 runs in 10 saw a + # display; starting Xvfb by hand and polling xdpyinfo until the server + # answered before launching the app, 3 in 10. So it is not the startup + # race it looks like -- waiting for the server changes nothing -- and + # whatever it is lives inside Chromium's X11 capturer. On this runner it + # comes up zero every time rather than a third of the time. # - # Up to three goes, because screen capture on this host is unreliable in - # its own right. One success is enough for the question being asked here. - echo "--- record then export (first run_cli here is #$((RUN_CLI_N + 1))) ---" - EXPORTED="" - # Tracked apart from EXPORTED so the verdict can name the stage that - # actually failed. For three runs every attempt died in record without - # export ever executing, while the annotation said "the export path does - # not work" -- an accusation aimed at the one component the run never - # reached, and the compositor addon is precisely what this step exists - # to vouch for. - RECORDED=0 - for i in 1 2 3; do - echo "=== export attempt $i/3 (run_cli #$((RUN_CLI_N + 1))) ===" - rm -f /tmp/demo.openscreen /tmp/demo.mp4 - RC=0 - CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/demo.openscreen >"/tmp/rec.$i.out" 2>&1 || RC=$? - # Outside the failure branch for the same reason as above: a record that - # works is exactly the measurement missing from the comparison, since - # this path has never yet produced one. - grep -a "get-sources\]" "/tmp/rec.$i.out" || true - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.openscreen ]; then - echo "record failed (rc=$RC); last lines:" - tail -5 "/tmp/rec.$i.out" || true - continue - fi - RECORDED=1 - echo "recorded. project:" - head -c 200 /tmp/demo.openscreen; echo + # So the input stops being a recording. ffmpeg synthesises two seconds of + # video, a three-line project points at it, and export renders that. + # Identical in what it proves -- the packaged compositor addon loads, + # decodes, composes through Vulkan and muxes an MP4 -- and it asks for no + # capability a headless runner is ever going to have. Measured at 6/6 + # locally where record measured 4/10. + # + # Capture is still worth watching, so one probe still runs. It is + # informational: it cannot pass here, and nothing gates on it. + echo "--- capture probe (informational; run_cli #$((RUN_CLI_N + 1))) ---" + RC=0 + CLI_TIMEOUT=120 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS record --duration 2 --project /tmp/probe.openscreen >/tmp/probe.out 2>&1 || RC=$? + if [ "$RC" -eq 0 ] && [ -f /tmp/probe.openscreen ]; then + echo "::warning::record worked on this runner. Capture is no longer broken here -- see whether the export check below should go back to using a real recording." + else + echo "capture still unavailable (rc=$RC); last lines:" + tail -3 /tmp/probe.out || true + fi - RC=0 - CLI_TIMEOUT=180 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >"/tmp/exp.$i.out" 2>&1 || RC=$? - if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then - echo "export failed (rc=$RC); last lines:" - tail -15 "/tmp/exp.$i.out" || true - continue - fi - EXPORTED=/tmp/demo.mp4 - break - done + echo "--- synthesise a clip and export it (first run_cli here is #$((RUN_CLI_N + 1))) ---" + # From the flake's own nixpkgs, for the same reason the Vulkan ICD is: + # the ambient registry drifts, and a decoder that is never the same twice + # is drift injected into a check that exists to catch it. + FFMPEG=$(nix build --no-link --print-out-paths --inputs-from . nixpkgs#ffmpeg-headless) + FFMPEG=${FFMPEG%%$'\n'*} + echo "ffmpeg: $FFMPEG" + + # H.264 in MP4 with an AAC track: the shape a real recording arrives in, + # so the export walks its ordinary decode path rather than a special one. + "$FFMPEG/bin/ffmpeg" -loglevel error -y \ + -f lavfi -i "testsrc2=size=1280x720:rate=30" \ + -f lavfi -i "sine=frequency=440:sample_rate=48000" \ + -t 2 -pix_fmt yuv420p -c:v libx264 -c:a aac -shortest /tmp/demo-src.mp4 + ls -l /tmp/demo-src.mp4 + # The whole project format the exporter needs: a media path and an empty + # editor, which normalises to a single full-length clip. This is what + # `record --project` writes, minus the parts a recording fills in. + cat > /tmp/demo.openscreen <<'JSON' + { + "version": 2, + "media": { "screenVideoPath": "/tmp/demo-src.mp4" }, + "editor": {} + } + JSON + # Parse it back before handing it over, so a future edit that breaks the + # JSON fails here with a parse error rather than 300 s later as an + # export that could not read its project. + python3 -c "import json;json.load(open('/tmp/demo.openscreen'))" + cat /tmp/demo.openscreen + + echo "--- openscreen info ---" + CLI_TIMEOUT=120 run_cli $SANDBOX $CHROME_FLAGS info /tmp/demo.openscreen || true + + rm -f /tmp/demo.mp4 + RC=0 + CLI_TIMEOUT=300 OPENSCREEN_DIAGNOSTIC=1 run_cli $SANDBOX $CHROME_FLAGS export /tmp/demo.openscreen -o /tmp/demo.mp4 >/tmp/exp.out 2>&1 || RC=$? EXPORT_OK=0 - if [ -z "$EXPORTED" ] && [ "$RECORDED" -eq 0 ]; then - echo "::error::No attempt got past record, so export never ran and the compositor addon is unproven. This is a capture failure on this host, not an export failure." - elif [ -z "$EXPORTED" ]; then - echo "::error::record produced a project but no attempt produced an MP4. The compositor addon is packaged and the export path does not work." + if [ "$RC" -ne 0 ] || [ ! -f /tmp/demo.mp4 ]; then + echo "::error::export failed (rc=$RC). The compositor addon is packaged and the export path does not work." + tail -25 /tmp/exp.out || true else - SIZE=$(wc -c < "$EXPORTED") + SIZE=$(wc -c < /tmp/demo.mp4) # An MP4 opens with a 4-byte length then 'ftyp'. A zero-length or # truncated file would otherwise pass a mere existence check. - MAGIC=$(dd if="$EXPORTED" bs=1 skip=4 count=4 2>/dev/null || true) + MAGIC=$(dd if=/tmp/demo.mp4 bs=1 skip=4 count=4 2>/dev/null || true) echo "exported $SIZE bytes, magic at offset 4: $MAGIC" if [ "$MAGIC" != "ftyp" ]; then echo "::error::output is not an MP4 (no ftyp box)" + tail -25 /tmp/exp.out || true elif [ "$SIZE" -lt 10000 ]; then echo "::error::MP4 is only $SIZE bytes, too small to hold two seconds of video" + tail -25 /tmp/exp.out || true else echo "Export works: $SIZE bytes of MP4." EXPORT_OK=1 @@ -509,7 +548,7 @@ jobs: # flaky must not hide whether export works, which is the whole point of # having packaged the compositor addon. # - # The gate is "did enumeration ever work" and "does export work", not + # The gate is "did enumeration ever answer" and "does export work", not # "did all five attempts pass". Requiring FAILED -eq 0 made the job red # by construction: the standing numbers on this runner are 1/5, 3/5 and # 4/5 ok, so a run where export is perfect and four enumerations succeed @@ -518,7 +557,11 @@ jobs: # per-attempt warnings above keep that flakiness visible without letting # it decide the build; tighten this to $ATTEMPTS once the capture failure # is understood and fixed. - echo "=== verdict: enumeration $OK/$ATTEMPTS ok, record $RECORDED, export $EXPORT_OK, $RUN_CLI_N run_cli invocations ===" + # + # Export, on the other hand, is a hard gate on every trigger again. It no + # longer depends on a capability this host does not have, so there is + # nothing left to excuse: if it fails now, the package is broken. + echo "=== verdict: enumeration $OK/$ATTEMPTS ok, export $EXPORT_OK, $RUN_CLI_N run_cli invocations ===" if [ "$EXPORT_OK" -ne 1 ] || [ "$OK" -eq 0 ]; then exit 1 fi diff --git a/crates/compositor/src/audio.rs b/crates/compositor/src/audio.rs index e51f0f8af..89da8cbec 100644 --- a/crates/compositor/src/audio.rs +++ b/crates/compositor/src/audio.rs @@ -5,7 +5,7 @@ use crate::ffi::*; use crate::regions::SpeedSegment; -use crate::scene::SceneAudio; +use crate::scene::{SceneAudio, SceneAudioTrack}; use anyhow::{bail, Result}; use std::f32::consts::PI; use std::ffi::CString; @@ -1381,6 +1381,84 @@ pub fn assemble_concatenated_pcm( output } +/// Mix imported audio tracks (issue #350) over the assembled programme. +/// +/// Each track is decoded across its trim window — already resampled to 48 kHz +/// stereo by `decode_clip_audio`, the same path a clip's own audio takes — scaled +/// by its per-track gain (the same `10^(dB/20)` law as `finish_audio`), and summed +/// into the programme at `start_sec`. The programme length is NOT extended: a +/// track that runs past the video is truncated to it, so the audio and video +/// streams stay the same length for the muxer. +/// +/// The decode window is capped up front at the room left in the programme after +/// `start_sec`, and a track starting at/after the end is skipped without decoding. +/// `decode_clip_audio` preallocates from the window, so this keeps a long track +/// pinned near a short programme's end from buffering (and clamping away) hours of +/// PCM. `trim_end_sec` must therefore be concrete — the renderer sends +/// `trimEnd ?? durationSec`. +/// +/// A track whose file has no decodable audio is skipped — the same degradation a +/// stream-less clip gets. +pub fn mix_external_tracks(mut programme: PlanarPcm, tracks: &[SceneAudioTrack]) -> PlanarPcm { + let programme_len = programme.first().map(Vec::len).unwrap_or(0); + if programme_len == 0 { + return programme; + } + for track in tracks { + let offset = (track.start_sec.max(0.0) * AUDIO_OUTPUT_SAMPLE_RATE as f64).round() as usize; + // A track that starts at or past the programme end contributes nothing — + // skip it before decoding anything. + if offset >= programme_len { + continue; + } + let trim_start = track.trim_start_sec.max(0.0); + let Some(trim_end_full) = track.trim_end_sec else { + // Without a concrete end there is no safe window to decode (see the doc + // comment); the renderer always resolves one, so this only guards a + // hand-written scene. + continue; + }; + // Cap the decode window at the room left in the programme. Everything past + // `offset` that overflows is discarded by `overlay_track_pcm` anyway, so + // decoding it only wastes time and memory — a three-hour track placed at + // second 9 of a ten-second export must not buffer three hours of PCM. + let remaining_sec = (programme_len - offset) as f64 / AUDIO_OUTPUT_SAMPLE_RATE as f64; + let trim_end = trim_end_full.min(trim_start + remaining_sec); + if trim_end <= trim_start { + continue; + } + let decoded = match decode_clip_audio(&track.path, trim_start, trim_end) { + Ok(Some(pcm)) => pcm, + _ => continue, + }; + let gain = 10.0f32.powf(track.gain_db.clamp(-12.0, 12.0) / 20.0); + overlay_track_pcm(&mut programme, &decoded, offset, gain); + } + programme +} + +/// Sum one decoded track into the programme at `offset` samples, scaled by `gain`, +/// truncated at the programme's end. Split out of `mix_external_tracks` so the +/// placement/gain/clamp math is testable without ffmpeg, exactly like +/// `mix_aligned_tracks` is split from the decode above. +fn overlay_track_pcm(programme: &mut PlanarPcm, decoded: &PlanarPcm, offset: usize, gain: f32) { + let programme_len = programme.first().map(Vec::len).unwrap_or(0); + if offset >= programme_len { + return; + } + let room = programme_len - offset; + for channel in 0..AUDIO_OUTPUT_CHANNELS { + let Some(source) = decoded.get(channel) else { + continue; + }; + let count = source.len().min(room); + let dst = &mut programme[channel]; + for k in 0..count { + dst[offset + k] += source[k] * gain; + } + } +} + /// Encodeur AAC attaché au muxer avant son header. Les paquets utilisent le même interleaver /// que la vidéo ; les pts restent en unités échantillon jusqu'au rescale vers l'AVStream. pub(crate) struct AacEncoder { @@ -1692,6 +1770,64 @@ mod tests { assert_eq!(mixed[1], vec![0.25, -0.5, 0.75]); } + // Imported audio track overlay (issue #350). + #[test] + fn overlay_sums_at_offset_with_gain() { + let mut programme = planar(&[0.1, 0.1, 0.1, 0.1]); + // ×2 gain, placed at sample offset 1. + overlay_track_pcm(&mut programme, &planar(&[0.2, 0.2]), 1, 2.0); + assert_eq!(programme[0], vec![0.1, 0.5, 0.5, 0.1]); + assert_eq!(programme[1], vec![0.1, 0.5, 0.5, 0.1]); + } + + #[test] + fn overlay_truncates_a_track_that_runs_past_the_programme() { + let mut programme = planar(&[0.0, 0.0, 0.0]); + // A 4-sample track placed at offset 2 has room for only 1 sample. + overlay_track_pcm(&mut programme, &planar(&[1.0, 1.0, 1.0, 1.0]), 2, 1.0); + assert_eq!(programme[0], vec![0.0, 0.0, 1.0]); + } + + #[test] + fn overlay_past_the_end_is_a_no_op() { + let mut programme = planar(&[0.3, 0.3]); + overlay_track_pcm(&mut programme, &planar(&[1.0]), 5, 1.0); + assert_eq!(programme[0], vec![0.3, 0.3]); + } + + #[test] + fn mix_external_tracks_skips_empty_windows() { + let programme = planar(&[0.4, 0.4]); + let tracks = vec![SceneAudioTrack { + path: "/nope.mp3".into(), + start_sec: 0.0, + gain_db: 0.0, + trim_start_sec: 2.0, + trim_end_sec: Some(1.0), // end <= start: empty window, never decoded + }]; + // The empty window is skipped before any decode, so the programme is + // untouched even though the path does not exist. + let out = mix_external_tracks(programme, &tracks); + assert_eq!(out[0], vec![0.4, 0.4]); + } + + #[test] + fn mix_external_tracks_skips_a_track_that_starts_past_the_programme() { + // 2 samples = ~0.00004 s of programme at 48 kHz; the track starts at 1 s, so + // its offset is past the end. It must be skipped before any decode is + // attempted (the path does not exist), never buffering its window. + let programme = planar(&[0.4, 0.4]); + let tracks = vec![SceneAudioTrack { + path: "/nope.mp3".into(), + start_sec: 1.0, + gain_db: 0.0, + trim_start_sec: 0.0, + trim_end_sec: Some(3600.0), + }]; + let out = mix_external_tracks(programme, &tracks); + assert_eq!(out[0], vec![0.4, 0.4]); + } + #[test] fn single_track_is_not_clamped() { // Promesse de non-régression : une source mono-piste ressort telle quelle, y compris diff --git a/crates/compositor/src/audio_jobs.rs b/crates/compositor/src/audio_jobs.rs new file mode 100644 index 000000000..8d97be40c --- /dev/null +++ b/crates/compositor/src/audio_jobs.rs @@ -0,0 +1,240 @@ +//! Décodage et étirement de l'audio d'un clip, en parallèle du parcours vidéo. +//! +//! Les trois pipelines faisaient ce travail **dans** le callback `on_clip_end` de +//! `walk_composited_timeline`, donc sur le thread de rendu et entre deux clips. Rien +//! n'appelle `progress()` pendant ce temps : la barre d'export s'arrêtait sur le +//! pourcentage de la dernière frame du clip et y restait pour toute la durée du décodage +//! et de l'étirement. C'est la moitié « reporting » du « figé à ~80 % » — la moitié +//! « coût » a été traitée par le passage à atempo, mais un clip long, un repli WSOLA ou +//! n'importe quelle étape audio future reproduisent le symptôme à l'identique. +//! +//! Y répondre en publiant une progression pendant cette phase aurait demandé de changer le +//! protocole natif → JS (il ne transporte qu'un compteur de frames absolu) et de répartir +//! un total que les deux côtés calculent séparément. Déplacer le travail est plus simple et +//! strictement meilleur : l'audio d'un clip ne dépend que de ce clip, il n'y a donc aucune +//! raison qu'il occupe le thread qui compose les frames du clip suivant. Le parcours vidéo +//! continue de rapporter sa progression sans interruption, et le temps audio disparaît du +//! mur d'export au lieu d'y être seulement mieux affiché — ce que +//! `export-pipeline.md` prétendait déjà. +//! +//! Chaque job ouvre son propre `AVFormatContext` sur le fichier du clip : libavformat +//! n'a pas d'état partagé entre contextes, et le décodeur vidéo du parcours en a un autre +//! sur le même chemin, en lecture seule lui aussi. + +use crate::audio::{decode_clip_audio, stretch_clip_pcm_by_speed, PlanarPcm}; +use crate::regions::SpeedSegment; +use std::collections::VecDeque; +use std::thread::JoinHandle; + +/// Nombre de jobs audio en vol. +/// +/// Un thread par clip serait sans plafond : une timeline de deux cents clips décoderait +/// deux cents pistes à la fois, chacune avec son contexte ffmpeg et son PCM complet en +/// mémoire. Quatre suffisent à couvrir le décodage d'un clip par le rendu du suivant, qui +/// est tout ce qu'on cherche ici. +const MAX_INFLIGHT_AUDIO_JOBS: usize = 4; + +/// Le corps d'un job : décode la fenêtre gardée du clip et l'étire sur ses spans de vitesse. +/// +/// Rend `None` quand le clip se déclare audio mais n'a pas de flux décodable, ou quand le +/// décodage échoue — dans les deux cas l'export continue et le clip sort muet, comme avant +/// que ce travail passe sur un thread. Les deux messages sont les mêmes qu'alors ; ils +/// sortent seulement d'un autre thread. +pub fn decode_and_stretch_clip_audio( + clip_index: usize, + screen_path: &str, + source_start_sec: f64, + source_end_sec: f64, + speed_segments: &[SpeedSegment], + out_fps: f64, +) -> Option { + match decode_clip_audio(screen_path, source_start_sec, source_end_sec) { + Ok(Some(pcm)) => Some(stretch_clip_pcm_by_speed(&pcm, speed_segments, out_fps)), + Ok(None) => { + eprintln!( + "[pipeline] warning: clip #{clip_index} déclaré audio mais sans flux décodable; silence conservé" + ); + None + } + Err(error) => { + eprintln!( + "[pipeline] warning: décodage audio du clip #{clip_index} échoué ({error:#}); silence conservé" + ); + None + } + } +} + +/// Collecte les résultats de jobs indexés lancés au fil du parcours, en bornant le nombre +/// de threads simultanés. +/// +/// L'ordre de restitution est celui des index, pas celui d'achèvement : `into_results` rend +/// un `Vec` de la taille annoncée où chaque case porte le résultat de son clip. +pub struct ClipAudioJobs { + inflight: VecDeque<(usize, JoinHandle)>, + results: Vec>, +} + +impl ClipAudioJobs { + pub fn new(clip_count: usize) -> Self { + Self { + inflight: VecDeque::new(), + results: (0..clip_count).map(|_| None).collect(), + } + } + + /// Lance `job` pour `clip_index`. Si le plafond est atteint, attend d'abord le plus + /// ancien job en vol — celui qui a eu le plus de temps pour finir. + pub fn spawn(&mut self, clip_index: usize, job: impl FnOnce() -> T + Send + 'static) { + while self.inflight.len() >= MAX_INFLIGHT_AUDIO_JOBS { + self.collect_oldest(); + } + self.inflight + .push_back((clip_index, std::thread::spawn(job))); + } + + /// Attend tous les jobs restants et rend les résultats rangés par index de clip. + pub fn into_results(mut self) -> Vec> { + while !self.inflight.is_empty() { + self.collect_oldest(); + } + // `mem::take` et pas un move : le `Drop` ci-dessous interdit de sortir un champ de + // `self`. Il ne trouvera plus rien à joindre, la file étant vide. + std::mem::take(&mut self.results) + } + + fn collect_oldest(&mut self) { + let Some((clip_index, handle)) = self.inflight.pop_front() else { + return; + }; + match handle.join() { + Ok(value) => { + if let Some(slot) = self.results.get_mut(clip_index) { + *slot = Some(value); + } + } + // Un panic dans un job audio ne doit pas emporter l'export : le clip sort + // muet, comme il le faisait déjà quand `decode_clip_audio` échouait. + Err(_) => eprintln!( + "[pipeline] warning: le job audio du clip #{clip_index} a paniqué; silence conservé" + ), + } + } +} + +/// Un `JoinHandle` droppé **détache** son thread. Entre le premier `spawn` et +/// `into_results` il y a des `?` — le parcours lui-même, le flush de l'encodeur — et sur +/// l'un d'eux la collection partait en fumée en laissant jusqu'à quatre décodages en vol +/// dans un addon natif que l'hôte peut décharger. On joint donc à la destruction : rien ne +/// survit à la portée, chemin d'erreur compris. +/// +/// Ce n'est pas une annulation : `decode_clip_audio` est un appel opaque et long, et +/// l'interrompre demanderait de lui passer un `AVIOInterruptCB` — un autre changement, dans +/// un autre fichier. L'attente est bornée par le plus lent des quatre, soit quelques +/// secondes depuis que le stretch passe par atempo, et elle ne coûte que sur un export qui +/// a déjà échoué. +impl Drop for ClipAudioJobs { + fn drop(&mut self) { + for (clip_index, handle) in std::mem::take(&mut self.inflight) { + if handle.join().is_err() { + eprintln!( + "[pipeline] warning: le job audio du clip #{clip_index} a paniqué pendant l'abandon de l'export" + ); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Arc; + + #[test] + fn results_are_indexed_by_clip_not_by_completion_order() { + // Le premier job est le plus lent : si on rangeait par ordre d'achèvement, le PCM + // du clip 0 atterrirait sur le clip 2 et l'export monterait l'audio dans le + // désordre sans rien signaler. + let mut jobs = ClipAudioJobs::new(3); + jobs.spawn(0, || { + std::thread::sleep(std::time::Duration::from_millis(60)); + "zero" + }); + jobs.spawn(1, || "one"); + jobs.spawn(2, || "two"); + assert_eq!( + jobs.into_results(), + vec![Some("zero"), Some("one"), Some("two")] + ); + } + + #[test] + fn a_clip_without_a_job_keeps_its_empty_slot() { + // Les clips sans audio ne lancent rien ; leur case doit rester `None` pour que + // `assemble_concatenated_pcm` y mette du silence. + let mut jobs = ClipAudioJobs::new(3); + jobs.spawn(1, || 7u32); + assert_eq!(jobs.into_results(), vec![None, Some(7), None]); + } + + #[test] + fn never_more_than_the_cap_run_at_once() { + // Sans plafond, une timeline longue ouvrirait un contexte ffmpeg et un PCM complet + // par clip, tous en même temps. + let live = Arc::new(AtomicUsize::new(0)); + let peak = Arc::new(AtomicUsize::new(0)); + let mut jobs = ClipAudioJobs::new(32); + for index in 0..32 { + let live = Arc::clone(&live); + let peak = Arc::clone(&peak); + jobs.spawn(index, move || { + let now = live.fetch_add(1, Ordering::SeqCst) + 1; + peak.fetch_max(now, Ordering::SeqCst); + std::thread::sleep(std::time::Duration::from_millis(5)); + live.fetch_sub(1, Ordering::SeqCst); + index + }); + } + let results = jobs.into_results(); + assert_eq!(results.len(), 32); + assert!(results.iter().enumerate().all(|(i, r)| *r == Some(i))); + assert!( + peak.load(Ordering::SeqCst) <= MAX_INFLIGHT_AUDIO_JOBS, + "jusqu'à {} jobs simultanés pour un plafond de {MAX_INFLIGHT_AUDIO_JOBS}", + peak.load(Ordering::SeqCst) + ); + } + + #[test] + fn dropping_the_collection_joins_its_jobs_instead_of_detaching_them() { + // Le chemin d'erreur : entre le premier `spawn` et `into_results` il y a des `?`. + // Sans le `Drop`, jusqu'à quatre décodages continuaient dans le vide après l'abandon + // de l'export, dans un addon que l'hôte peut décharger. + let finished = Arc::new(AtomicUsize::new(0)); + { + let mut jobs = ClipAudioJobs::new(4); + for index in 0..4 { + let finished = Arc::clone(&finished); + jobs.spawn(index, move || { + std::thread::sleep(std::time::Duration::from_millis(20)); + finished.fetch_add(1, Ordering::SeqCst); + }); + } + // Pas d'`into_results` : on abandonne, comme le ferait un `?`. + } + assert_eq!( + finished.load(Ordering::SeqCst), + 4, + "des jobs tournaient encore après la destruction de la collection" + ); + } + + #[test] + fn a_panicking_job_leaves_its_clip_silent_without_taking_the_export_down() { + let mut jobs = ClipAudioJobs::new(2); + jobs.spawn(0, || panic!("décodage impossible")); + jobs.spawn(1, || 42u32); + assert_eq!(jobs.into_results(), vec![None, Some(42)]); + } +} diff --git a/crates/compositor/src/lib.rs b/crates/compositor/src/lib.rs index 90dee8155..64a9c44af 100644 --- a/crates/compositor/src/lib.rs +++ b/crates/compositor/src/lib.rs @@ -28,6 +28,7 @@ //! — c'est précisément ce qui rend le port Metal possible (cf. PR #162). pub mod audio; +pub mod audio_jobs; pub mod config; pub mod cursor; pub mod ffi; diff --git a/crates/compositor/src/pipeline_linux.rs b/crates/compositor/src/pipeline_linux.rs index 910738fc0..9a58a5443 100644 --- a/crates/compositor/src/pipeline_linux.rs +++ b/crates/compositor/src/pipeline_linux.rs @@ -21,9 +21,10 @@ use std::ffi::CString; use std::ptr; use crate::audio::{ - assemble_concatenated_pcm, build_audio_concat_plan, decode_clip_audio, finish_audio, - stretch_clip_pcm_by_speed, AacEncoder, PlanarPcm, + assemble_concatenated_pcm, build_audio_concat_plan, finish_audio, mix_external_tracks, + AacEncoder, PlanarPcm, }; +use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::config::Cfg; use crate::d3d::Gpu; use crate::ffi::AVFrame; @@ -453,11 +454,17 @@ pub fn run_composited_multi( // Un PCM par clip, assemble apres la marche video (elle seule dit combien de // frames chaque clip a produit, donc combien d'audio lui revient). - let mut clip_pcm: Vec> = (0..clips.len()).map(|_| None).collect(); + let mut audio_jobs: ClipAudioJobs> = ClipAudioJobs::new(clips.len()); let mut clip_frame_counts: Vec = vec![0; clips.len()]; let scene = comp.scene_snapshot(); let audio_settings = scene.as_ref().map(|scene| scene.audio).unwrap_or_default(); + // Imported audio tracks (issue #350), cloned out of the borrowed scene so the + // mix step below owns them. Empty for a project with no imported audio. + let audio_tracks = scene + .as_ref() + .map(|scene| scene.audio_tracks.clone()) + .unwrap_or_default(); // Ring de staging a 2 : l'export ne veut que du debit, une frame de latence // ne se voit pas dans un fichier. Voir `Compositor::set_readback_depth` pour // la raison pour laquelle la preview, elle, reste a 1. @@ -497,18 +504,24 @@ pub fn run_composited_multi( clip_frame_counts[clip_index] = frames_in_clip; let clip = &clips[clip_index]; if clip.has_audio && frames_in_clip > 0 { - match decode_clip_audio(&clip.screen, clip.source_start_sec, source_end_sec) { - Ok(Some(pcm)) => { - clip_pcm[clip_index] = - Some(stretch_clip_pcm_by_speed(&pcm, speed_segments, out_fps as f64)); - } - Ok(None) => eprintln!( - "[pipeline] warning: clip #{clip_index} declare audio mais sans flux decodable; silence", - ), - Err(error) => eprintln!( - "[pipeline] warning: decodage audio clip #{clip_index} echoue ({error:#}); silence", - ), - } + // L'audio d'un clip ne dépend que de ce clip : le décoder et l'étirer ici, + // sur le thread de rendu, immobilisait la barre d'export pour toute sa + // durée — rien n'appelle `progress()` entre deux clips. Le travail part + // sur un thread et se recouvre avec la composition du clip suivant ; les + // résultats sont récupérés après le parcours, rangés par index de clip. + let path = clip.screen.clone(); + let source_start_sec = clip.source_start_sec; + let segments = speed_segments.to_vec(); + audio_jobs.spawn(clip_index, move || { + decode_and_stretch_clip_audio( + clip_index, + &path, + source_start_sec, + source_end_sec, + &segments, + out_fps as f64, + ) + }); } Ok(()) }, @@ -532,10 +545,23 @@ pub fn run_composited_multi( drain_encoder(ectx, octx, ostream, opkt)?; // Audio : le plan part des frames REELLEMENT produites par clip (un clip // raccourci voit son audio raccourci d'autant), puis un seul encode AAC. + // Récupération des jobs audio lancés pendant le parcours. `spawn` en admet quatre + // avant d'en collecter un, donc il en reste au plus quatre à attendre ici — bornés + // par le plus lent, pas par leur somme ; les autres se sont recouverts avec + // l'encodage vidéo. + let clip_pcm: Vec> = audio_jobs + .into_results() + .into_iter() + .map(|slot| slot.flatten()) + .collect(); + let declared_audio: Vec = clips.iter().map(|c| c.has_audio).collect(); let plan = build_audio_concat_plan(&clip_frame_counts, &declared_audio, out_fps as f64); audio_encoder.encode( - &finish_audio(assemble_concatenated_pcm(&clip_pcm, &plan), audio_settings), + &finish_audio( + mix_external_tracks(assemble_concatenated_pcm(&clip_pcm, &plan), &audio_tracks), + audio_settings, + ), octx, )?; crate::ffi::averr(crate::ffi::av_write_trailer(octx), "write_trailer")?; diff --git a/crates/compositor/src/pipeline_macos.rs b/crates/compositor/src/pipeline_macos.rs index 10de3fac2..f8afc95b5 100644 --- a/crates/compositor/src/pipeline_macos.rs +++ b/crates/compositor/src/pipeline_macos.rs @@ -30,9 +30,10 @@ //! décodeurs, symétrique. use crate::audio::{ - assemble_concatenated_pcm, build_audio_concat_plan, decode_clip_audio, finish_audio, - stretch_clip_pcm_by_speed, AacEncoder, PlanarPcm, + assemble_concatenated_pcm, build_audio_concat_plan, finish_audio, mix_external_tracks, + AacEncoder, PlanarPcm, }; +use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::compositor::Compositor; use crate::d3d::Gpu; use crate::timeline_walk::NextFrameTime; @@ -1064,7 +1065,7 @@ pub fn run_composited_multi( } // Un PCM par clip, assemblé après la marche vidéo : c'est elle qui dit combien de // frames chaque clip a réellement produit, donc combien d'audio lui revient. - let mut clip_pcm: Vec> = (0..clips.len()).map(|_| None).collect(); + let mut audio_jobs: ClipAudioJobs> = ClipAudioJobs::new(clips.len()); let mut clip_frame_counts: Vec = vec![0; clips.len()]; let mut opkt = unsafe { crate::ffi::av_packet_alloc() }; @@ -1077,6 +1078,11 @@ pub fn run_composited_multi( // raconte avoir déjà coûté une fois. let scene = comp.scene_snapshot(); let audio_settings = scene.as_ref().map(|scene| scene.audio).unwrap_or_default(); + // Imported audio tracks (issue #350), cloned out of the borrowed scene. + let audio_tracks = scene + .as_ref() + .map(|scene| scene.audio_tracks.clone()) + .unwrap_or_default(); frames = unsafe { crate::timeline_walk::walk_composited_timeline( clips, @@ -1097,21 +1103,24 @@ pub fn run_composited_multi( clip_frame_counts[clip_index] = frames_in_clip; let clip = &clips[clip_index]; if clip.has_audio && frames_in_clip > 0 { - match decode_clip_audio(&clip.screen, clip.source_start_sec, source_end_sec) { - Ok(Some(pcm)) => { - clip_pcm[clip_index] = Some(stretch_clip_pcm_by_speed( - &pcm, - speed_segments, - out_fps as f64, - )); - } - Ok(None) => eprintln!( - "[pipeline] warning: clip #{clip_index} déclaré audio mais sans flux décodable; silence conservé", - ), - Err(error) => eprintln!( - "[pipeline] warning: décodage audio du clip #{clip_index} échoué ({error:#}); silence conservé", - ), - } + // L'audio d'un clip ne dépend que de ce clip : le décoder et l'étirer ici, + // sur le thread de rendu, immobilisait la barre d'export pour toute sa + // durée — rien n'appelle `progress()` entre deux clips. Le travail part + // sur un thread et se recouvre avec la composition du clip suivant ; les + // résultats sont récupérés après le parcours, rangés par index de clip. + let path = clip.screen.clone(); + let source_start_sec = clip.source_start_sec; + let segments = speed_segments.to_vec(); + audio_jobs.spawn(clip_index, move || { + decode_and_stretch_clip_audio( + clip_index, + &path, + source_start_sec, + source_end_sec, + &segments, + out_fps as f64, + ) + }); } Ok(()) }, @@ -1129,10 +1138,23 @@ pub fn run_composited_multi( // Le plan part des frames RÉELLEMENT produites par clip, pas des durées demandées : // un clip raccourci (source plus courte que sa borne) doit voir son audio raccourci // d'autant, sinon la piste dérive pour tous les suivants. + // Récupération des jobs audio lancés pendant le parcours. `spawn` en admet quatre + // avant d'en collecter un, donc il en reste au plus quatre à attendre ici — bornés + // par le plus lent, pas par leur somme ; les autres se sont recouverts avec + // l'encodage vidéo. + let clip_pcm: Vec> = audio_jobs + .into_results() + .into_iter() + .map(|slot| slot.flatten()) + .collect(); + let declared_audio: Vec = clips.iter().map(|clip| clip.has_audio).collect(); let plan = build_audio_concat_plan(&clip_frame_counts, &declared_audio, out_fps as f64); audio_encoder.encode( - &finish_audio(assemble_concatenated_pcm(&clip_pcm, &plan), audio_settings), + &finish_audio( + mix_external_tracks(assemble_concatenated_pcm(&clip_pcm, &plan), &audio_tracks), + audio_settings, + ), octx, )?; diff --git a/crates/compositor/src/pipeline_windows.rs b/crates/compositor/src/pipeline_windows.rs index 11738bcd4..a3d1a7d68 100644 --- a/crates/compositor/src/pipeline_windows.rs +++ b/crates/compositor/src/pipeline_windows.rs @@ -3,9 +3,10 @@ //! tout le run, deux lectures seulement. Rien dans la boucle ne peut fausser le fps. use crate::audio::{ - assemble_concatenated_pcm, build_audio_concat_plan, decode_clip_audio, finish_audio, - stretch_clip_pcm_by_speed, AacEncoder, PlanarPcm, + assemble_concatenated_pcm, build_audio_concat_plan, finish_audio, mix_external_tracks, + AacEncoder, PlanarPcm, }; +use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::compositor::{Compositor, OUT_H, OUT_W}; use crate::config::Cfg; use crate::cpu_frames::CpuFrames; @@ -1343,6 +1344,11 @@ unsafe fn run_multi_inner( // fenêtrage par clip ; `walk_composited_timeline` s'en charge. let scene = comp.scene_snapshot(); let audio_settings = scene.as_ref().map(|scene| scene.audio).unwrap_or_default(); + // Imported audio tracks (issue #350), cloned out of the borrowed scene. + let audio_tracks = scene + .as_ref() + .map(|scene| scene.audio_tracks.clone()) + .unwrap_or_default(); // ---- encodeur (choisi à l'exécution, cf. ExportCodec::candidates) + mux ---- // Backend CPU : pas de pool D3D11 du tout. `av_hwdevice_ctx_init(D3D11VA)` échoue sur @@ -1390,8 +1396,7 @@ unsafe fn run_multi_inner( let opkt = av_packet_alloc(); let mut clip_frame_counts = vec![0u64; clips.len()]; - let mut clip_pcm: Vec> = - std::iter::repeat_with(|| None).take(clips.len()).collect(); + let mut audio_jobs: ClipAudioJobs> = ClipAudioJobs::new(clips.len()); let t0 = Instant::now(); let frames = walk_composited_timeline( @@ -1431,23 +1436,24 @@ unsafe fn run_multi_inner( clip_frame_counts[clip_index] = frames_in_clip; let clip = &clips[clip_index]; if clip.has_audio && frames_in_clip > 0 { - match decode_clip_audio(&clip.screen, clip.source_start_sec, source_end_sec) { - Ok(Some(pcm)) => { - clip_pcm[clip_index] = Some(stretch_clip_pcm_by_speed( - &pcm, - speed_segments, - out_fps as f64, - )); - } - Ok(None) => eprintln!( - "[pipeline] warning: clip #{} déclaré audio mais sans flux décodable; silence conservé", - clip_index, - ), - Err(error) => eprintln!( - "[pipeline] warning: décodage audio du clip #{} échoué ({error:#}); silence conservé", + // L'audio d'un clip ne dépend que de ce clip : le décoder et l'étirer ici, + // sur le thread de rendu, immobilisait la barre d'export pour toute sa durée + // — rien n'appelle `progress()` entre deux clips. Le travail part sur un + // thread et se recouvre avec la composition du clip suivant ; les résultats + // sont récupérés après le parcours, rangés par index de clip. + let path = clip.screen.clone(); + let source_start_sec = clip.source_start_sec; + let segments = speed_segments.to_vec(); + audio_jobs.spawn(clip_index, move || { + decode_and_stretch_clip_audio( clip_index, - ), - } + &path, + source_start_sec, + source_end_sec, + &segments, + out_fps as f64, + ) + }); } Ok(()) }, @@ -1460,6 +1466,15 @@ unsafe fn run_multi_inner( enc.send(ptr::null_mut())?; drain_encoder(ectx, octx, ostream, opkt)?; + // Récupération des jobs audio lancés pendant le parcours. `spawn` en admet quatre avant + // d'en collecter un, donc il en reste au plus quatre à attendre ici — bornés par le plus + // lent, pas par leur somme ; tous les autres se sont recouverts avec l'encodage. + let clip_pcm: Vec> = audio_jobs + .into_results() + .into_iter() + .map(|slot| slot.flatten()) + .collect(); + let declared_audio: Vec = clips.iter().map(|clip| clip.has_audio).collect(); let audio_plan = build_audio_concat_plan( &clip_frame_counts, @@ -1467,7 +1482,7 @@ unsafe fn run_multi_inner( out_fps as f64, ); let assembled_audio = finish_audio( - assemble_concatenated_pcm(&clip_pcm, &audio_plan), + mix_external_tracks(assemble_concatenated_pcm(&clip_pcm, &audio_plan), &audio_tracks), audio_settings, ); audio_encoder.encode(&assembled_audio, octx)?; diff --git a/crates/compositor/src/regions.rs b/crates/compositor/src/regions.rs index 000939708..3be707066 100644 --- a/crates/compositor/src/regions.rs +++ b/crates/compositor/src/regions.rs @@ -175,9 +175,17 @@ fn lerp(a: f32, b: f32, t: f32) -> f32 { /// `startSec` (le zoom anticipe légèrement), plein régime pendant la région, ease-out après /// `endSec`. Les temps reçus sont les temps source échantillonnés par le pipeline, donc ces /// enveloppes restent alignées quand une speed region répète ou saute des frames. +/// +/// `under_trim` coupe les enveloppes : la région vit sous une coupe, donc pleine force sur son +/// span et rien en dehors. Sans ça son ease-in (1,5 s AVANT `start_sec`) et son ease-out +/// déborderaient sur les frames GARDÉES de part et d'autre du trim — un zoom que l'export ne +/// rendra jamais, visible dans la preview juste à côté de la coupe. Cf. `SceneZoomRegion`. fn zoom_region_strength(region: &SceneZoomRegion, t: f32) -> f32 { let start = region.start_sec as f32; let end = region.end_sec as f32; + if region.under_trim { + return if t >= start && t < end { 1.0 } else { 0.0 }; + } let zoom_in_end = start + ZOOM_IN_OVERLAP_S; let lead_in_start = zoom_in_end - ZOOM_IN_TRANSITION_WINDOW_S; let lead_out_end = end + TRANSITION_WINDOW_S; @@ -311,8 +319,13 @@ fn resolve_focus(region: &SceneZoomRegion, t: f32, cursor: Option<&CursorTrack>) /// transition), en secondes. Indices dans `regions` (pas d'id nécessaire — contrairement au /// web qui matche par `region.id` car il travaille sur des objets isolés, ici tout vient du /// même slice donc les positions suffisent). +/// +/// Les régions `under_trim` sont exclues du chaînage, des DEUX côtés : leur contenu est coupé au +/// rendu, donc un pan lissé vers (ou depuis) l'une d'elles ferait bouger des frames gardées au +/// nom d'une région que l'export ne joue pas. Elles restent des régions dominantes indépendantes, +/// sèches sur leur propre span (cf. `zoom_region_strength`). fn connected_pairs(regions: &[SceneZoomRegion]) -> Vec<(usize, usize, f32, f32)> { - let mut order: Vec = (0..regions.len()).collect(); + let mut order: Vec = (0..regions.len()).filter(|&i| !regions[i].under_trim).collect(); order.sort_by(|&a, &b| regions[a].start_sec.partial_cmp(®ions[b].start_sec).unwrap()); let mut pairs = Vec::new(); for w in order.windows(2) { @@ -628,6 +641,7 @@ mod zoom_focus_tests { focus_y: 0.5, focus_mode: Some("manual".into()), rotation: None, + under_trim: false, } } @@ -678,6 +692,33 @@ mod zoom_focus_tests { assert_eq!(state.scale, 1.0); assert_eq!(state.focus, [0.5, 0.5]); } + + /// Une région sous un trim est jouée SÈCHE : pleine échelle sur son span, identité juste + /// avant et juste après. `region()` couvre [2,8] et son ease-in normal démarre 1,5 s avant + /// `start_sec` — c'est exactement ce débordement qui atteindrait les frames GARDÉES autour + /// de la coupe et ferait diverger la preview de l'export. Cf. issue #216. + #[test] + fn a_region_under_a_trim_has_no_transition_window() { + let mut r = region(2.5, 0.5); + r.under_trim = true; + let regions = [r]; + assert_eq!(zoom_state_at(®ions, 1.5, None).scale, 1.0); + assert_eq!(zoom_state_at(®ions, 2.0, None).scale, 2.5); + assert_eq!(zoom_state_at(®ions, 7.9, None).scale, 2.5); + assert_eq!(zoom_state_at(®ions, 8.0, None).scale, 1.0); + } + + /// Et elle ne se chaîne pas avec sa voisine gardée : un pan lissé vers une région que + /// l'export ne joue pas ferait bouger des frames qui, elles, sont rendues. + #[test] + fn a_region_under_a_trim_is_not_chained_with_its_neighbour() { + let mut cut = region(3.0, 0.5); + cut.under_trim = true; + cut.start_sec = 9.0; + cut.end_sec = 10.0; + // Sans le filtre, l'écart de 1 s < CHAINED_ZOOM_PAN_GAP_S apparierait [2,8] et [9,10]. + assert!(connected_pairs(&[region(2.0, 0.5), cut]).is_empty()); + } } #[cfg(test)] @@ -1021,3 +1062,63 @@ mod tilt_tests { } } } + +#[cfg(test)] +mod exporter_frame_totals { + use super::*; + use crate::scene::SceneSpeedRegion; + + fn region(start_sec: f64, end_sec: f64, speed: f64) -> SceneSpeedRegion { + SceneSpeedRegion { clip_index: None, start_sec, end_sec, speed } + } + + fn frames(start_sec: f64, end_sec: f64, regions: &[SceneSpeedRegion], fps: f64) -> u64 { + speed_segments_for_window(regions, start_sec, end_sec, fps) + .iter() + .map(|segment| segment.frame_count) + .sum() + } + + /// Le total que la barre d'export doit viser, mesuré sur ce que `walk_composited_timeline` + /// itère réellement. + /// + /// Le jumeau de ce test est `src/lib/exporter/outputFrameCount.test.ts`, avec la MÊME + /// table de chiffres. Le natif n'envoie qu'un compteur de frames brut ; le total et donc + /// le pourcentage sont calculés côté TS, et rien ne reliait les deux calculs. Résultat + /// livré : le total TS ignorait les speed regions, donc un clip entièrement en 1,25× + /// rendait 80 % des frames annoncées et la barre s'arrêtait à 80 % — le « figé à ~80 % » + /// d'OpenScreen#371, au chiffre près. Toucher un côté doit faire rougir l'autre. + #[test] + fn speed_segments_match_the_exporter_frame_totals() { + const FPS: f64 = 30.0; + assert_eq!(frames(0.0, 10.0, &[], FPS), 300, "sans région"); + assert_eq!( + frames(0.0, 10.0, &[region(0.0, 10.0, 1.25)], FPS), + 240, + "1,25× : 80 % de 300, exactement le symptôme" + ); + assert_eq!(frames(0.0, 10.0, &[region(0.0, 10.0, 0.5)], FPS), 600, "0,5×"); + assert_eq!( + frames(0.0, 10.0, &[region(2.0, 4.0, 2.0)], FPS), + 60 + 30 + 180, + "couverture partielle" + ); + assert_eq!( + frames(1.0, 5.0, &[region(0.0, 100.0, 2.0)], FPS), + 60, + "région débordant la fenêtre gardée" + ); + assert_eq!( + frames(0.0, 10.0, &[region(2.0, 6.0, 2.0), region(4.0, 8.0, 4.0)], FPS), + 60 + 60 + 15 + 60, + "recouvrement : la première région garde la portion déjà couverte" + ); + assert_eq!( + frames(0.0, 10.0, &[region(0.0, 10.0, 0.0)], FPS), + 300, + "vitesse non positive traitée comme 1×" + ); + assert_eq!(frames(4.0, 4.0, &[], FPS), 0, "fenêtre vide"); + assert_eq!(frames(0.0, 10.0, &[], 0.0), 0, "fps non positif"); + } +} diff --git a/crates/compositor/src/scene.rs b/crates/compositor/src/scene.rs index b6fb420c0..5d79ba1b6 100644 --- a/crates/compositor/src/scene.rs +++ b/crates/compositor/src/scene.rs @@ -331,6 +331,18 @@ pub struct SceneZoomRegion { pub focus_mode: Option, /// "iso" | "left" | "right" | null. pub rotation: Option, + /// La région entière tombe sur une portion qu'un trim retire. Ses temps sont donc HORS de + /// la fenêtre source de `clip_index`, qui n'est là que pour l'adresser (le segment que la + /// coupe interrompt, cf. `cutAddressingSegmentIndex` côté TS). + /// + /// Conséquence de rendu : la région est jouée SÈCHE, pleine force sur `[start_sec, end_sec)` + /// et rien en dehors — ni fenêtre d'ease-in/ease-out, ni chaînage avec une région voisine. + /// C'est ce qui garde la coupe : un export ne compose jamais de frame à ces temps source, + /// alors qu'une enveloppe de transition, elle, déborderait sur les frames gardées d'à côté. + /// L'utilisateur qui pose la tête de lecture sur le trim voit l'effet ; le rendu, non. + /// `#[serde(default)]` : absent de tout payload sans trim sous un modificateur (issue #216). + #[serde(default)] + pub under_trim: bool, } /// Une zone de vitesse portée par le temps source d'un clip. @@ -420,6 +432,29 @@ pub struct SceneAudio { pub gain_db: f32, } +/// One imported audio track (issue #350) mixed over the assembled programme — +/// voiceover / BGM / SFX. Deliberately a SEPARATE `Scene` field rather than a +/// member of `SceneAudio`, so `SceneAudio` stays `Copy` and the pipelines keep +/// copying it out of a borrow unchanged. +/// +/// `start_sec` is the track's head on the OUTPUT programme; `trim_start_sec` / +/// `trim_end_sec` window the source file (both source seconds). The renderer +/// resolves `start_sec` from the track's raw timeline position — equal to it when +/// the project has no trims/speed, which is the case this first cut mixes exactly. +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SceneAudioTrack { + pub path: String, + #[serde(default)] + pub start_sec: f64, + #[serde(default)] + pub gain_db: f32, + #[serde(default)] + pub trim_start_sec: f64, + #[serde(default)] + pub trim_end_sec: Option, +} + #[derive(Debug, Clone, Copy, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SceneOutput { @@ -488,6 +523,10 @@ pub struct Scene { /// Global audio finishing. Default keeps old scene payloads bit-for-bit compatible. #[serde(default)] pub audio: SceneAudio, + /// Imported audio tracks mixed over the programme (issue #350). `#[serde(default)]`: + /// absent from every scene written before this, and from a project with none. + #[serde(default)] + pub audio_tracks: Vec, /// Crop écran par clip, dans le même ordre que `clips` (`cropByClip` côté TS). #[serde(default)] pub crop_by_clip: Vec>, @@ -509,6 +548,13 @@ impl Scene { /// Copie de scène limitée aux régions du clip actif. `clipIndex` est l'identité fiable /// lorsque plusieurs clips réutilisent les mêmes temps source ; son absence retombe sur le /// chevauchement avec la fenêtre source pour accepter les anciens payloads. + /// + /// Les deux tests étaient jusqu'ici cumulés, ce que la phrase ci-dessus ne dit pas : le + /// chevauchement est le REPLI, pas une seconde condition. La différence n'apparaît que pour + /// une région hors fenêtre, et une seule l'est — celle qui vit sous un trim (`under_trim`, + /// cf. `SceneZoomRegion`). L'app en émet une par modificateur entièrement coupé, adressée au + /// segment que la coupe interrompt, pour que la tête de lecture posée sur le trim montre ce + /// qu'il y a dessous. Exiger le chevauchement l'aurait filtrée ici même. pub(crate) fn for_clip_window( &self, clip_index: usize, @@ -517,7 +563,9 @@ impl Scene { ) -> Scene { let belongs = |region_clip_index: Option, start_sec: f64, end_sec: f64| { let overlaps_window = end_sec > source_start_sec && start_sec < source_end_sec; - overlaps_window && region_clip_index.map(|i| i == clip_index).unwrap_or(true) + region_clip_index + .map(|i| i == clip_index) + .unwrap_or(overlaps_window) }; let mut scene = self.clone(); scene.zoom_regions.retain(|region| { @@ -780,17 +828,36 @@ mod annotation_tests { #[test] fn for_clip_window_keeps_only_the_annotations_of_the_composed_clip() { - // Même règle que les zoom/speed/camera regions : bon clip ET recouvrement de la fenêtre. + // Même règle que les zoom/speed/camera regions : `clipIndex` décide seul quand il est là. + // `under-trim` porte des temps hors fenêtre EXPRÈS (il vit sous une coupe) et doit donc + // survivre : le dessin est ensuite borné par `startSec`/`endSec`, jamais atteints par un + // export. Cf. issue #216. let json = scene_json( r##"[{"id":"keep","clipIndex":0,"startSec":1.0,"endSec":2.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}, {"id":"other-clip","clipIndex":1,"startSec":1.0,"endSec":2.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}, - {"id":"out-of-window","clipIndex":0,"startSec":50.0,"endSec":51.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}]"##, + {"id":"under-trim","clipIndex":0,"underTrim":true,"startSec":50.0,"endSec":51.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}]"##, + ); + let scene = Scene::from_json(&json).expect("parse"); + let filtered = scene.for_clip_window(0, 0.0, 10.0); + assert_eq!( + filtered.annotations.iter().map(|a| a.id.as_str()).collect::>(), + vec!["keep", "under-trim"] + ); + } + + #[test] + fn for_clip_window_still_falls_back_to_window_overlap_without_a_clip_index() { + // Vieux payload : rien ne dit à quel clip la région appartient, le chevauchement de + // fenêtre reste la seule réponse disponible. C'est le REPLI, pas une seconde condition. + let json = scene_json( + r##"[{"id":"in-window","startSec":1.0,"endSec":2.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}, + {"id":"out-of-window","startSec":50.0,"endSec":51.0,"kind":"figure","x":0,"y":0,"w":0.1,"h":0.1,"zIndex":0}]"##, ); let scene = Scene::from_json(&json).expect("parse"); let filtered = scene.for_clip_window(0, 0.0, 10.0); assert_eq!( filtered.annotations.iter().map(|a| a.id.as_str()).collect::>(), - vec!["keep"] + vec!["in-window"] ); } } diff --git a/electron/ai-edition/agent-tools.test.ts b/electron/ai-edition/agent-tools.test.ts index 8f8361e59..6f0638211 100644 --- a/electron/ai-edition/agent-tools.test.ts +++ b/electron/ai-edition/agent-tools.test.ts @@ -172,6 +172,7 @@ describe("the mutating-tool table", () => { expect([...MUTATING_TOOL_NAMES].sort()).toEqual( [ "addAnnotation", + "addAudio", "addCameraFullscreen", "addSpeed", "addTrim", @@ -184,6 +185,7 @@ describe("the mutating-tool table", () => { "removeTrim", "replaceTimeline", "setAnnotation", + "setAudio", "setCameraFullscreen", "setClipRange", "setSpeed", @@ -2054,3 +2056,191 @@ describe("setZoom answers for the focus it kept", () => { expect(result.resultJson).not.toContain("cursorAnchor"); }); }); + +// Issue #350 — the audio tools. The snapshot and the refusals are what keep the model +// from inventing an asset it cannot import, so both are pinned here beside the +// success paths. +describe("addAudio / setAudio", () => { + /** The fixture plus one imported audio asset. */ + function withAudioAsset(durationSec: number | null = 30): AxcutDocument { + const doc = fixtureDocument(); + return documentSchema.parse({ + ...doc, + assets: [ + ...doc.assets, + { + id: "audio_1", + kind: "audio", + label: "bed.mp3", + originalPath: "C:/audio/bed.mp3", + ...(durationSec == null ? {} : { durationSec }), + }, + ], + }); + } + + it("reports imported audio in the snapshot, with the asset kind beside it", () => { + // Without `kind` the model sees an asset it cannot explain and tries to place it as + // footage; without `audioRanges` it cannot see what is already on the lanes at all. + const placed = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 2, endSec: 6, kind: "voiceover" }), + ); + expect(placed.ok).toBe(true); + const snapshot = executeAgentTool(placed.document as AxcutDocument, "getCurrentDocument", ""); + const parsed = JSON.parse(snapshot.resultJson); + expect(parsed.assets.find((a: { id: string }) => a.id === "audio_1").kind).toBe("audio"); + expect(parsed.audioRanges).toHaveLength(1); + expect(parsed.audioRanges[0]).toMatchObject({ + audioAssetId: "audio_1", + kind: "voiceover", + startSec: 2, + endSec: 6, + }); + }); + + it("anchors the placed region to the clip under it", () => { + const result = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 2, endSec: 6 }), + ); + expect(result.ok).toBe(true); + const region = (result.document as AxcutDocument).audioRanges[0]; + // The anchor is what makes it travel with its clip; a bare startMs/endMs would not. + expect(region.clipId).toBe("clip_1"); + expect(region.sourceStartSec).toBeCloseTo(2, 6); + expect(region.origin).toBe("agent"); + }); + + it("plays the whole file when endSec is omitted", () => { + const result = executeAgentTool( + withAudioAsset(20), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 0, offsetSec: 5 }), + ); + expect(result.ok).toBe(true); + const region = (result.document as AxcutDocument).audioRanges[0]; + // 20s file from an in-point of 5s = 15s of span, so the model never computes it. + expect(region.endMs - region.startMs).toBe(15_000); + }); + + it("refuses an offset at or past the end of a known file", () => { + // Otherwise the omitted-end fallback mints a 0.1s region that plays silence, and the + // model reports it as having placed audio. + const result = executeAgentTool( + withAudioAsset(20), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 0, offsetSec: 20 }), + ); + expect(result.ok).toBe(false); + expect(result.resultJson).toContain("offsetSec"); + }); + + it("allows any offset while the duration is unknown", () => { + // A failed probe leaves no duration; refusing on that would block a legitimate call. + const result = executeAgentTool( + withAudioAsset(null), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 0, offsetSec: 99 }), + ); + expect(result.ok).toBe(true); + }); + + it("refuses an unknown asset and names the audio the project actually has", () => { + const result = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "nope", startSec: 0, endSec: 4 }), + ); + expect(result.ok).toBe(false); + expect(result.resultJson).toContain("audio_1"); + }); + + it("refuses a video asset, pointing at the tool that does place footage", () => { + const result = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "asset_1", startSec: 0, endSec: 4 }), + ); + expect(result.ok).toBe(false); + expect(result.resultJson).toContain("replaceTimeline"); + }); + + it("setAudio re-levels and re-lanes the pill it names", () => { + const placed = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 2, endSec: 6 }), + ); + const id = JSON.parse(placed.resultJson).audioId; + const result = executeAgentTool( + placed.document as AxcutDocument, + "setAudio", + JSON.stringify({ audioId: id, gainDb: -6, kind: "voiceover" }), + ); + expect(result.ok).toBe(true); + expect((result.document as AxcutDocument).audioRanges[0]).toMatchObject({ + gainDb: -6, + kind: "voiceover", + }); + }); + + it("setAudio applies the same offset guard as addAudio", () => { + const placed = executeAgentTool( + withAudioAsset(20), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 2, endSec: 6 }), + ); + const id = JSON.parse(placed.resultJson).audioId; + const result = executeAgentTool( + placed.document as AxcutDocument, + "setAudio", + JSON.stringify({ audioId: id, offsetSec: 25 }), + ); + expect(result.ok).toBe(false); + }); + + it("setAudio keeps a cross-clip pill whole when only gain or lane changes", () => { + // `existing` is ONE fragment; replacePillSpan removes every fragment under the + // pill and rebuilds only the span it is handed, so a gain-only edit used to + // shrink the pill to that fragment — deleting the audio on the other clip. + const placed = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 20, endSec: 40 }), + ); + expect((placed.document as AxcutDocument).audioRanges).toHaveLength(2); + const id = JSON.parse(placed.resultJson).audioId; + const result = executeAgentTool( + placed.document as AxcutDocument, + "setAudio", + JSON.stringify({ audioId: id, gainDb: -6 }), + ); + const ranges = (result.document as AxcutDocument).audioRanges; + expect(result.ok).toBe(true); + expect(ranges).toHaveLength(2); + expect(Math.min(...ranges.map((r) => r.startMs))).toBe(20_000); + expect(Math.max(...ranges.map((r) => r.endMs))).toBe(40_000); + expect(ranges.every((r) => r.gainDb === -6)).toBe(true); + }); + + it("removeModifier deletes an audio region by id, like every other kind", () => { + const placed = executeAgentTool( + withAudioAsset(), + "addAudio", + JSON.stringify({ audioAssetId: "audio_1", startSec: 2, endSec: 6 }), + ); + const id = JSON.parse(placed.resultJson).audioId; + const result = executeAgentTool( + placed.document as AxcutDocument, + "removeModifier", + JSON.stringify({ id }), + ); + expect(result.ok).toBe(true); + expect((result.document as AxcutDocument).audioRanges).toEqual([]); + // The asset went with the last region that played it. + expect((result.document as AxcutDocument).assets.some((a) => a.id === "audio_1")).toBe(false); + }); +}); diff --git a/electron/ai-edition/agent-tools.ts b/electron/ai-edition/agent-tools.ts index 5a0966398..3cf01026d 100644 --- a/electron/ai-edition/agent-tools.ts +++ b/electron/ai-edition/agent-tools.ts @@ -337,6 +337,32 @@ function droppedByEdit(before: AxcutDocument, after: AxcutDocument) { // private — callers only ever need the composed `*Args`.) const secondsSchema = z.number().finite().nonnegative(); +/** Span given to an agent-placed audio region when the asset has no probed duration yet. + * Short on purpose: a wrong guess the user has to shorten beats one that silently covers + * the whole programme. */ +const DEFAULT_AGENT_AUDIO_SEC = 10; + +/** + * "Start the file at `offsetSec`" is only answerable when there IS file left at that + * point. Past the end it yields a region that plays silence, which the model then reports + * as having placed audio — the failure worth a refusal rather than a shrug. + * + * A non-positive `durationSec` is UNKNOWN, not zero: an import whose probe failed carries + * 0 until the renderer re-probes it, and refusing on that would block a legitimate call. + */ +function audioOffsetRefusal( + asset: { id: string; durationSec?: number | null } | undefined, + offsetSec: number, +): string | null { + const duration = asset?.durationSec; + if (duration == null || !(duration > 0)) return null; + if (offsetSec < duration) return null; + return ( + `offsetSec ${offsetSec}s is at or past the end of ${asset?.id} (${duration}s), ` + + "so the region would play nothing. Pick an offset inside the file." + ); +} + export const addTrimArgs = z.object({ startSec: secondsSchema, endSec: secondsSchema, @@ -479,6 +505,24 @@ export const setAnnotationArgs = z.object({ text: z.string().optional(), }); +export const addAudioArgs = z.object({ + audioAssetId: z.string().min(1), + startSec: secondsSchema, + endSec: secondsSchema.optional(), + kind: z.enum(["voiceover", "music"]).default("music"), + offsetSec: secondsSchema.default(0), + gainDb: z.number().min(-60).max(12).default(0), +}); + +export const setAudioArgs = z.object({ + audioId: z.string().min(1), + startSec: secondsSchema.optional(), + endSec: secondsSchema.optional(), + kind: z.enum(["voiceover", "music"]).optional(), + offsetSec: secondsSchema.optional(), + gainDb: z.number().min(-60).max(12).optional(), +}); + export const addCameraFullscreenArgs = z.object({ startSec: secondsSchema, endSec: secondsSchema, @@ -541,6 +585,8 @@ export const OPENSCREEN_TOOL_NAMES = [ "setAnnotation", "addCameraFullscreen", "setCameraFullscreen", + "addAudio", + "setAudio", "removeTrim", "removeModifier", "removeClip", @@ -607,6 +653,8 @@ export const MUTATING_TOOL_NAMES: ReadonlySet = new Set([ "setAnnotation", "addCameraFullscreen", "setCameraFullscreen", + "addAudio", + "setAudio", "removeTrim", "removeModifier", "removeClip", @@ -665,7 +713,9 @@ export function documentSnapshotForModel( const autoFocusAll = legacy?.autoFocusAll === true; return { timeBaseNote: - "clips and trims are in source-time seconds; zooms, speedRegions, annotations and cameraFullscreenRegions are in virtual (edited-timeline) seconds.", + "clips and trims are in source-time seconds; zooms, speedRegions, annotations, cameraFullscreenRegions and audioRanges are in virtual (edited-timeline) seconds.", + audioNote: + "audioRanges are imported voiceover / music files laid over the recording. They are regions like every other kind — anchored to the clip they cover, so they travel with it through reorder and trim — and they play at 1x whatever a speed region does to the picture under them. addAudio places an EXISTING asset of kind 'audio'; nothing here can import a file from disk, so if the project has no audio asset, say so rather than inventing an id.", zoomNote: `renderedScale is what the viewer sees (depth is an ordinal, not a factor: ${ZOOM_DEPTH_LEGEND}). ` + "When a zoom carries customScale it wins over depth and depthIsOverridden is true — " + @@ -683,6 +733,10 @@ export function documentSnapshotForModel( assets: document.assets.map((a) => ({ id: a.id, label: a.label, + // "audio" is an imported voiceover / music file: it is never a clip, it is played + // by an audio region. Without this the model sees an asset it cannot explain and + // tries to place it on the timeline as footage. + kind: a.kind, durationSec: a.durationSec ?? null, hasCameraTrack: a.cameraTrack != null, cameraVisible: a.cameraTrack?.visible ?? false, @@ -755,6 +809,21 @@ export function documentSnapshotForModel( startSec: roundSec(c.startMs), endSec: roundSec(c.endMs), })), + // Imported audio, coalesced to whole pills like every other kind so the model + // reasons about what the user sees on the ruler rather than about the fragments a + // clip boundary happens to have split it into. + audioRanges: coalesceForAgent(document.audioRanges).map((a) => ({ + id: a.id, + startSec: roundSec(a.startMs), + endSec: roundSec(a.endMs), + audioAssetId: a.audioAssetId, + // Which lane it sits on, and part of its identity: changing it moves the region + // between lanes rather than creating a second one. + kind: a.kind, + // Where in the FILE the region starts playing, in that file's own seconds. + offsetSec: a.offsetSec, + gainDb: a.gainDb, + })), hasTranscript: document.transcripts.length > 0 || document.transcript !== null, }; } @@ -1843,6 +1912,143 @@ export function executeAgentTool( }; } + case "addAudio": { + const parsed = addAudioArgs.safeParse(args); + if (!parsed.success) return failure(parsed.error.message); + const { audioAssetId, kind, offsetSec, gainDb } = parsed.data; + const asset = document.assets.find((a) => a.id === audioAssetId); + // Two distinct refusals, because they need two different corrections: an unknown + // id is a hallucinated asset, a video id is the model reaching for footage. + if (!asset) { + const available = document.assets.filter((a) => a.kind === "audio"); + return failure( + `Unknown asset: ${audioAssetId}.` + + (available.length + ? ` Imported audio in this project: ${available.map((a) => `${a.id} (${a.label})`).join(", ")}.` + : " This project has no imported audio; a file can only be imported from the editor, not from here."), + ); + } + if (asset.kind !== "audio") { + return failure( + `Asset ${audioAssetId} is video, not audio. addAudio plays an imported audio file over the recording; to place footage use replaceTimeline.`, + ); + } + const offsetRefusal = audioOffsetRefusal(asset, offsetSec); + if (offsetRefusal) return failure(offsetRefusal); + // No endSec means "as long as the file is" — the natural span, and the one the + // editor's own add uses. Falling back to the asset duration here rather than + // making the model compute it keeps the two paths on one rule. + const startSec = parsed.data.startSec; + const endSec = + parsed.data.endSec ?? + startSec + Math.max(0.1, (asset.durationSec ?? DEFAULT_AGENT_AUDIO_SEC) - offsetSec); + const startMs = toMs(Math.min(startSec, endSec)); + const endMs = toMs(Math.max(startSec, endSec)); + const region = { + id: createId("audio"), + startMs, + endMs, + audioAssetId, + kind, + offsetSec, + gainDb, + origin: "agent" as const, + }; + const placed = anchorForAgent(region, document, "audio"); + const landing = landingOf(placed, document); + if (!landing.anchored) { + return coversNoClip("audio", startMs / 1000, endMs / 1000, document); + } + const next: AxcutDocument = { + ...document, + audioRanges: [...document.audioRanges, ...placed] as AxcutDocument["audioRanges"], + }; + return { + ok: true, + document: next, + resultJson: JSON.stringify({ + audioId: landing.ids[0], + ...landingReport(landing, startMs / 1000, endMs / 1000), + }), + summary: + `added ${kind} "${asset.label}" ${formatSec(landing.startSec)} – ${formatSec(landing.endSec)}` + + landingSuffix(landing, startMs / 1000, endMs / 1000), + }; + } + + case "setAudio": { + const parsed = setAudioArgs.safeParse(args); + if (!parsed.success) return failure(parsed.error.message); + const { audioId } = parsed.data; + const existing = document.audioRanges.find((a) => a.id === audioId); + if (!existing) return failure(`Unknown audio region: ${audioId}`); + if (parsed.data.offsetSec !== undefined) { + const refusal = audioOffsetRefusal( + document.assets.find((a) => a.id === existing.audioAssetId), + parsed.data.offsetSec, + ); + if (refusal) return failure(refusal); + } + const audioPill = new Set(resolvePillIds(document.audioRanges, audioId)); + // The span must come from the coalesced pill, not from `existing`: that is ONE + // fragment of a pill that may cross clip boundaries, while replacePillSpan below + // removes every fragment under the pill and rebuilds only the span it is handed — + // a gain/kind-only edit passing the fragment's span would shrink the pill and + // silently drop the audio on the other clips. + const existingPill = coalesceRegionsForRuler(document.audioRanges).find((pill) => + pill.ids.includes(audioId), + ); + if (!existingPill) return failure(`Unknown audio region: ${audioId}`); + const { startMs, endMs } = resolveSpanMs( + { + startMs: Math.round(existingPill.start * 1000), + endMs: Math.round(existingPill.end * 1000), + }, + parsed.data.startSec, + parsed.data.endSec, + ); + // The payload patch hits EVERY fragment under the pill before the span is + // replaced: kind, offset and gain are all part of the region identity, so + // patching one fragment of a ventilated region would split it into two pills. + const patched = document.audioRanges.map((a) => + audioPill.has(a.id) + ? { + ...a, + ...(parsed.data.kind !== undefined ? { kind: parsed.data.kind } : {}), + ...(parsed.data.offsetSec !== undefined ? { offsetSec: parsed.data.offsetSec } : {}), + ...(parsed.data.gainDb !== undefined ? { gainDb: parsed.data.gainDb } : {}), + } + : a, + ); + const rebuilt = replacePillSpan( + patched, + audioId, + startMs, + endMs, + document.timeline.clips, + () => createId("audio"), + ); + const landing = landingAfterPillEdit(document.audioRanges, rebuilt, audioPill, document); + if (!landing.anchored) { + return coversNoClip("audio", startMs / 1000, endMs / 1000, document); + } + const next: AxcutDocument = { + ...document, + audioRanges: rebuilt as AxcutDocument["audioRanges"], + }; + return { + ok: true, + document: next, + resultJson: JSON.stringify({ + audioId: landing.ids[0], + ...landingReport(landing, startMs / 1000, endMs / 1000), + }), + summary: + `updated audio ${audioId} ${formatSec(landing.startSec)} – ${formatSec(landing.endSec)}` + + landingSuffix(landing, startMs / 1000, endMs / 1000), + }; + } + case "removeTrim": { const parsed = removeTrimArgs.safeParse(args); if (!parsed.success) return failure(parsed.error.message); @@ -1872,9 +2078,10 @@ export function executeAgentTool( else if (document.annotations.some((a) => a.id === id)) kind = "annotation"; else if (speedRegions.some((s) => s.id === id)) kind = "speed"; else if (cameraFullscreenRegions.some((c) => c.id === id)) kind = "cameraFullscreen"; + else if (document.audioRanges.some((a) => a.id === id)) kind = "audio"; if (!kind) { return failure( - `No zoom / speed / annotation / full-camera modifier with id ${id}. ` + + `No zoom / speed / annotation / full-camera / audio modifier with id ${id}. ` + `For a trim use removeTrim; for a clip use removeClip.`, ); } diff --git a/electron/ai-edition/deep-agent/service.test.ts b/electron/ai-edition/deep-agent/service.test.ts index 7729bd624..2e5f66167 100644 --- a/electron/ai-edition/deep-agent/service.test.ts +++ b/electron/ai-edition/deep-agent/service.test.ts @@ -73,6 +73,11 @@ const ARGS: Record = { setAnnotation: { annotationId: "ann_nope" }, addCameraFullscreen: { startSec: 1, endSec: 2 }, setCameraFullscreen: { cameraFullscreenId: "cam_nope" }, + // The fixture has no `kind: "audio"` asset, so this exercises the refusal branch — + // which is the honest one to pin: the agent cannot import a file, only place one the + // project already has. + addAudio: { audioAssetId: "audio_nope", startSec: 1, endSec: 2 }, + setAudio: { audioId: "audio_nope" }, removeTrim: { trimRangeId: "trim_1" }, removeModifier: { id: "nope" }, removeClip: { clipId: "clip_1" }, diff --git a/electron/ai-edition/deep-agent/service.ts b/electron/ai-edition/deep-agent/service.ts index d804a3b1a..c862eab70 100644 --- a/electron/ai-edition/deep-agent/service.ts +++ b/electron/ai-edition/deep-agent/service.ts @@ -27,6 +27,7 @@ import type { AxcutDocument } from "../../../src/lib/ai-edition/schema"; import { ZOOM_DEPTH_LEGEND } from "../../../src/lib/ai-edition/timeline/zoom-scale"; import { addAnnotationArgs, + addAudioArgs, addCameraFullscreenArgs, addSpeedArgs, addTrimArgs, @@ -45,6 +46,7 @@ import { replaceTimelineArgs, resolveCursorAssetId, setAnnotationArgs, + setAudioArgs, setCameraFullscreenArgs, setClipRangeArgs, setSpeedArgs, @@ -115,6 +117,7 @@ const BASE_SYSTEM_PROMPT = [ "- Silences, pauses and dead stretches are removed as trims INSIDE the placed clip. Send them together with addTrims once you know the ranges; addTrim is for a single cut or a correction. The placed clip stays the canonical cut; it is not rebuilt to drop them.", "- Changing where a clip starts or ends within its source is setClipRange — the clip's in/out, distinct from a trim.", `- addZoom takes a virtual-timeline span (depth is an ordinal 1–6 selecting from a fixed table — ${ZOOM_DEPTH_LEGEND} — never a multiplier; focus in 0–1 frame fractions). addSpeed changes pacing over a span. addAnnotation puts text on screen. addCameraFullscreen enlarges the webcam, and only does something where assets[].hasCameraTrack is true.`, + "- addAudio lays an imported voiceover or music file over a span. It plays an asset the project already has (kind 'audio'); importing a file from disk is the editor's job, not a tool you have — so when the project has none, say so rather than naming an id that does not exist.", "- moveClip changes the order of placed clips, one call per clip that moves, preserving ids, source ranges, trims and anchored effects. replaceTimeline rebuilds the timeline from kept intervals and sorts them, so it cannot reorder anything.", "- Deleting is a first-class action, not a workaround: removeTrim, removeModifier, removeClip. Never fake a deletion by re-adding an element or zeroing it out (span 0, speed 1×) — that leaves it in the document and misreports what you did.", "If nothing in the list does what was asked, say so; do not approximate it with a bigger tool.", @@ -170,10 +173,14 @@ export const TOOL_DESCRIPTIONS: Record = { "Add a camera-fullscreen region over a span of the edited timeline (virtual seconds): the webcam fills the frame for that span. This only does something when the footage under that span comes from an asset with a linked webcam — check assets[].hasCameraTrack (or hasAnyCamera) in getCurrentDocument first. On footage with no camera the call is refused rather than storing a region that would render nothing; say so instead of retrying.", setCameraFullscreen: "Move or resize an existing camera-fullscreen region by id (virtual-timeline seconds). Only the fields you pass are changed. Refused if the new span lands on footage with no linked webcam.", + addAudio: + "Lay an ALREADY-IMPORTED audio file over the recording across a span of the edited timeline (virtual seconds): a voiceover, or a music bed. audioAssetId must name an asset whose kind is 'audio' — getCurrentDocument lists them; nothing here can import a file from disk, so if there is none, say so instead of guessing an id. Omit endSec to play the whole file from offsetSec. kind picks the lane ('voiceover' or 'music'): two regions on the SAME lane may not overlap, two on different lanes may, which is how a voiceover sits over a bed. offsetSec is where in the FILE playback starts, gainDb its level (0 is unchanged, negative ducks it).", + setAudio: + "Move, resize, re-level, re-lane, or re-point an existing audio region by id (virtual-timeline seconds). Only the fields you pass are changed. Use it to duck a bed under narration (gainDb), to shift what part of the file plays (offsetSec), or to move it between the voiceover and music lanes (kind).", removeTrim: "Delete a trim range by id — the cut is undone and that span plays/exports again. This is how you 'remove a trim'; never re-add a trim to undo one.", removeModifier: - "Delete a modifier (zoom / speed / annotation / camera-fullscreen) by id; the kind is resolved from the id. This is how you 'remove'/'delete' one — never neutralise it (span 0, speed 1×), which leaves it in the document. For a trim use removeTrim; for a clip use removeClip.", + "Delete a modifier (zoom / speed / annotation / camera-fullscreen / audio) by id; the kind is resolved from the id. This is how you 'remove'/'delete' one — never neutralise it (span 0, speed 1×), which leaves it in the document. For a trim use removeTrim; for a clip use removeClip.", removeClip: "Delete a placed clip by id; remaining clips close the gap and effects anchored to it are dropped. Use only when the user asks to remove a clip — to shorten one, use setClipRange.", }; @@ -339,6 +346,8 @@ export function buildTools( build("setAnnotation", setAnnotationArgs), build("addCameraFullscreen", addCameraFullscreenArgs), build("setCameraFullscreen", setCameraFullscreenArgs), + build("addAudio", addAudioArgs), + build("setAudio", setAudioArgs), build("removeTrim", removeTrimArgs), build("removeModifier", removeModifierArgs), build("removeClip", removeClipArgs), diff --git a/electron/ai-edition/document-service.test.ts b/electron/ai-edition/document-service.test.ts index 6cbdb97c9..d4d842325 100644 --- a/electron/ai-edition/document-service.test.ts +++ b/electron/ai-edition/document-service.test.ts @@ -280,6 +280,48 @@ describe("DocumentService", () => { expect(after.project.primaryAssetId).toBe(first.project.primaryAssetId); expect(after.assets).toHaveLength(2); }); + + // Issue #350 — external audio import (voiceover / BGM / SFX). + it("appends an audio asset without claiming the primary slot", async () => { + const doc = await service.createProject("P"); + const updated = await service.addAsset(doc.project.id, { + path: "/tmp/voiceover.mp3", + kind: "audio", + }); + expect(updated.assets).toHaveLength(1); + expect(updated.assets[0]?.kind).toBe("audio"); + // An audio-only file must never become the project's primary asset, even + // when it is the first file added to an otherwise-empty project. + expect(updated.project.primaryAssetId).toBeUndefined(); + }); + + it("keeps the existing video primary when an audio track is added", async () => { + const doc = await service.createProject("P"); + const withVideo = await service.addAsset(doc.project.id, { path: "/tmp/screen.mp4" }); + const primary = withVideo.project.primaryAssetId; + const withAudio = await service.addAsset(doc.project.id, { + path: "/tmp/bgm.wav", + kind: "audio", + }); + expect(withAudio.project.primaryAssetId).toBe(primary); + expect(withAudio.assets).toHaveLength(2); + }); + + it("rejects unsupported audio extensions", async () => { + const doc = await service.createProject("P"); + await expect( + service.addAsset(doc.project.id, { path: "/tmp/clip.mp4", kind: "audio" }), + ).rejects.toBeInstanceOf(ProjectFileError); + }); + + it("accepts a video extension under the default kind but not as audio", async () => { + const doc = await service.createProject("P"); + // The same extension routing works in reverse: an .mp3 is fine as audio + // but rejected as video (covered above), and an .mp4 is the opposite. + await expect( + service.addAsset(doc.project.id, { path: "/tmp/a.mp3", kind: "audio" }), + ).resolves.toBeDefined(); + }); }); describe("removeAsset", () => { @@ -363,6 +405,51 @@ describe("DocumentService", () => { expect(after.project.primaryAssetId).toBe(b.assets[1]?.id); }); + // Issue #350 — an audio overlay can never be primary. + it("passes primary to the next VIDEO asset, never to an audio asset", async () => { + const doc = await service.createProject("P"); + const video = await service.addAsset(doc.project.id, { path: "/tmp/screen.mp4" }); + await service.addAsset(doc.project.id, { path: "/tmp/music.mp3", kind: "audio" }); + const primaryId = video.project.primaryAssetId; + expect(primaryId).toBeTruthy(); + // Removing the only video leaves just the audio asset; primary must clear, + // not fall to the audio one. + const after = await service.removeAsset(doc.project.id, primaryId ?? ""); + expect(after.project.primaryAssetId).toBeUndefined(); + expect(after.assets).toHaveLength(1); + expect(after.assets[0]?.kind).toBe("audio"); + }); + + it("drops audio regions that played a removed audio asset", async () => { + const doc = await service.createProject("P"); + await service.addAsset(doc.project.id, { path: "/tmp/screen.mp4" }); + const withAudio = await service.addAsset(doc.project.id, { + path: "/tmp/music.mp3", + kind: "audio", + }); + const audioId = withAudio.assets.find((a) => a.kind === "audio")?.id ?? ""; + expect(audioId).toBeTruthy(); + const withTrack = await service.saveProject({ + ...withAudio, + audioRanges: [ + { + id: "audio_1", + startMs: 0, + endMs: 10_000, + audioAssetId: audioId, + kind: "music", + offsetSec: 0, + gainDb: 0, + origin: "user", + }, + ], + }); + expect(withTrack.audioRanges).toHaveLength(1); + const after = await service.removeAsset(doc.project.id, audioId); + expect(after.audioRanges).toEqual([]); + expect(after.assets.some((a) => a.id === audioId)).toBe(false); + }); + it("resequences other assets and rederives their anchored regions", async () => { const created = await service.createProject("P"); const withA = await service.addAsset(created.project.id, { path: "/tmp/a.mp4" }); diff --git a/electron/ai-edition/document-service.ts b/electron/ai-edition/document-service.ts index 3c93e3bc0..19280930a 100644 --- a/electron/ai-edition/document-service.ts +++ b/electron/ai-edition/document-service.ts @@ -38,6 +38,9 @@ export interface ProjectSummary { export interface AddAssetInput { path: string; label?: string; + // "audio" imports an external voiceover / BGM / SFX file (issue #350). + // Defaults to "video" when omitted, so existing callers are unaffected. + kind?: "video" | "audio"; } export class DocumentNotFoundError extends Error { @@ -72,6 +75,24 @@ function isSupportedVideoPath(filePath: string): boolean { return SUPPORTED_VIDEO_EXTENSIONS.has(ext); } +// Imported audio (issue #350). Decoding is handled downstream by the same +// WebCodecs / ffmpeg paths that read a video's audio track, so this list is the +// container formats decodeAudioData and the compositor can open. +const SUPPORTED_AUDIO_EXTENSIONS = new Set([ + ".mp3", + ".wav", + ".m4a", + ".aac", + ".flac", + ".ogg", + ".opus", +]); + +function isSupportedAudioPath(filePath: string): boolean { + const ext = path.extname(filePath).toLowerCase(); + return SUPPORTED_AUDIO_EXTENSIONS.has(ext); +} + function safeProjectId(raw: string): string { // ponytail: project ids are uuid-prefixed strings (e.g. "proj_"). Reject // anything that smells like path traversal before we ever touch the disk. @@ -283,7 +304,15 @@ export class DocumentService { if (!input.path) { throw new ProjectFileError("Asset path is required.", projectId); } - if (!isSupportedVideoPath(input.path)) { + const kind = input.kind ?? "video"; + if (kind === "audio") { + if (!isSupportedAudioPath(input.path)) { + throw new ProjectFileError( + `Unsupported audio extension: ${path.extname(input.path)} (supported: ${[...SUPPORTED_AUDIO_EXTENSIONS].join(", ")})`, + projectId, + ); + } + } else if (!isSupportedVideoPath(input.path)) { throw new ProjectFileError( `Unsupported video extension: ${path.extname(input.path)} (supported: ${[...SUPPORTED_VIDEO_EXTENSIONS].join(", ")})`, projectId, @@ -300,18 +329,24 @@ export class DocumentService { } const asset: AxcutAsset = { id: createId("asset"), - kind: "video", + kind, label: input.label?.trim() || path.basename(absolutePath), originalPath: absolutePath, sizeBytes, cameraTrack: null, }; + // An audio import is an overlay, never the thing the timeline is built + // around, so it must not claim the empty primaryAssetId slot — otherwise the + // first file dropped into a fresh project (a BGM track) would become its + // primary asset and the editor would try to lay out clips from a file with + // no video. + const claimsPrimary = kind !== "audio" && !doc.project.primaryAssetId; const next: AxcutDocument = { ...doc, assets: [...doc.assets, asset], project: { ...doc.project, - ...(doc.project.primaryAssetId ? {} : { primaryAssetId: asset.id }), + ...(claimsPrimary ? { primaryAssetId: asset.id } : {}), updatedAt: new Date().toISOString(), }, }; @@ -324,9 +359,13 @@ export class DocumentService { throw new ProjectFileError(`Asset ${assetId} not found in project ${projectId}.`, projectId); } const assets = doc.assets.filter((a) => a.id !== assetId); + // Primary is the thing the timeline is built around, so it must fall to the + // next VIDEO asset — never an audio overlay (issue #350), which can't be + // primary (see addAsset). Falling back to `assets[0]` would hand primary to + // an audio asset when the removed one was the last video. const primaryAssetId = doc.project.primaryAssetId === assetId - ? (assets[0]?.id ?? undefined) + ? (assets.find((a) => a.kind !== "audio")?.id ?? undefined) : doc.project.primaryAssetId; const withoutAssetClips = doc.timeline.clips .filter((clip) => clip.assetId === assetId) @@ -334,6 +373,9 @@ export class DocumentService { const next: AxcutDocument = { ...withoutAssetClips, assets, + // Drop audio regions that played the removed asset — they would otherwise + // dangle, pointing at an asset the document no longer has. + audioRanges: withoutAssetClips.audioRanges.filter((r) => r.audioAssetId !== assetId), timeline: { ...withoutAssetClips.timeline, trimRanges: withoutAssetClips.timeline.trimRanges.filter((r) => r.assetId !== assetId), diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index e140a4e37..f6132b4f6 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -289,6 +289,14 @@ interface Window { name?: string; canceled?: boolean; }>; + // Import an external audio file from the timeline toolbar (issue #350). + openAudioFilePicker: () => Promise<{ + success: boolean; + path?: string; + name?: string; + canceled?: boolean; + message?: string; + }>; setCurrentVideoPath: (path: string) => Promise<{ success: boolean }>; setCurrentRecordingSession: ( session: import("../src/lib/recordingSession").RecordingSession | null, diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index aa2014670..b3ad77fab 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -186,6 +186,32 @@ function hasAllowedImportVideoExtension(filePath: string): boolean { return ALLOWED_IMPORT_VIDEO_EXTENSIONS.has(path.extname(filePath).toLowerCase()); } +// Imported audio (issue #350). Kept separate from the video set so the two +// pickers stay honest — an audio picker must not approve a video path and vice +// versa. Mirrors SUPPORTED_AUDIO_EXTENSIONS in the document service. +const ALLOWED_IMPORT_AUDIO_EXTENSIONS = new Set([ + ".mp3", + ".wav", + ".m4a", + ".aac", + ".flac", + ".ogg", + ".opus", +]); + +function hasAllowedImportAudioExtension(filePath: string): boolean { + return ALLOWED_IMPORT_AUDIO_EXTENSIONS.has(path.extname(filePath).toLowerCase()); +} + +// Video OR audio. The type-specific pickers stay honest (see the audio set's +// comment), but the generic media READS — peaks, binary, file-info, chunk — serve +// whichever kind the document points at, so they must accept both. Gating them on +// video alone dropped every imported audio path once `approvedPaths` was empty +// (a project reopen), and the waveform was lost for good (issue #350). +function hasAllowedImportMediaExtension(filePath: string): boolean { + return hasAllowedImportVideoExtension(filePath) || hasAllowedImportAudioExtension(filePath); +} + function runProcess( command: string, args: string[], @@ -282,8 +308,13 @@ async function prepareSupplementalPreviewAudioTrack(videoPath: string) { return { success: true, path: pathToFileURL(outputPath).toString() }; } -async function approveReadableVideoPath( - filePath?: string | null, +// Shared core behind the media path approvers. `hasAllowedExtension` is the ONLY +// thing that differs between video and audio imports, so it is the single knob: +// an already-approved path passes regardless, otherwise the extension gate, +// optional trusted-dir confinement, and a stat check decide whether to approve. +async function approveReadableMediaPath( + filePath: string | null | undefined, + hasAllowedExtension: (p: string) => boolean, trustedDirs?: string[], ): Promise { const normalizedPath = normalizeVideoSourcePath(filePath); @@ -295,7 +326,7 @@ async function approveReadableVideoPath( return normalizedPath; } - if (!hasAllowedImportVideoExtension(normalizedPath)) { + if (!hasAllowedExtension(normalizedPath)) { return null; } @@ -322,6 +353,29 @@ async function approveReadableVideoPath( return normalizedPath; } +function approveReadableVideoPath( + filePath?: string | null, + trustedDirs?: string[], +): Promise { + return approveReadableMediaPath(filePath, hasAllowedImportVideoExtension, trustedDirs); +} + +function approveReadableAudioPath( + filePath?: string | null, + trustedDirs?: string[], +): Promise { + return approveReadableMediaPath(filePath, hasAllowedImportAudioExtension, trustedDirs); +} + +// For the generic media reads that accept either kind — NOT for the pickers, +// which must stay type-specific (see `hasAllowedImportMediaExtension`). +function approveReadableAvPath( + filePath?: string | null, + trustedDirs?: string[], +): Promise { + return approveReadableMediaPath(filePath, hasAllowedImportMediaExtension, trustedDirs); +} + function resolveRecordingOutputPath(fileName: string): string { const trimmed = fileName.trim(); if (!trimmed) { @@ -3590,6 +3644,8 @@ export function registerIpcHandlers( } }); + // The media tab imports VIDEO (it arranges clips). Audio is imported from the + // timeline toolbar instead (issue #350) — see `open-audio-file-picker` below. ipcMain.handle("open-video-file-picker", async () => { try { const dialogOptions = buildDialogOptions( @@ -3636,6 +3692,55 @@ export function registerIpcHandlers( } }); + // Import an external audio file (voiceover / BGM / SFX) — issue #350. Driven by + // the timeline's "Add audio" tool: audio is a timeline overlay (like an + // annotation), not a media-tab clip, so it has its own audio-only picker and the + // renderer adds it as a kind:"audio" asset + track at the playhead. + ipcMain.handle("open-audio-file-picker", async () => { + try { + const dialogOptions = buildDialogOptions( + { + title: mainT("dialogs", "fileDialogs.selectAudio"), + defaultPath: RECORDINGS_DIR, + filters: [ + { + name: mainT("dialogs", "fileDialogs.audioFiles"), + extensions: ["mp3", "wav", "m4a", "aac", "flac", "ogg", "opus"], + }, + { name: mainT("dialogs", "fileDialogs.allFiles"), extensions: ["*"] }, + ], + properties: ["openFile"], + }, + getMainWindow(), + ); + const result = await dialog.showOpenDialog(dialogOptions); + + if (result.canceled || result.filePaths.length === 0) { + return { success: false, canceled: true }; + } + + const normalizedPath = await approveReadableAudioPath(result.filePaths[0]); + if (!normalizedPath) { + return { + success: false, + message: "Selected file is not a supported readable audio file", + }; + } + + return { + success: true, + path: normalizedPath, + }; + } catch (error) { + console.error("Failed to open audio file picker:", error); + return { + success: false, + message: "Failed to open audio file picker", + error: String(error), + }; + } + }); + ipcMain.handle("reveal-in-folder", async (_, filePath: string) => { try { // showItemInFolder returns nothing, it throws on error @@ -3661,7 +3766,7 @@ export function registerIpcHandlers( ipcMain.handle("read-binary-file", async (_, filePath: string) => { try { - const normalizedPath = await approveReadableVideoPath(filePath); + const normalizedPath = await approveReadableAvPath(filePath); if (!normalizedPath) { return { success: false, @@ -3691,7 +3796,7 @@ export function registerIpcHandlers( // recording above that can never be loaded whole — see read-file-chunk). ipcMain.handle("get-readable-file-info", async (_, filePath: string) => { try { - const normalizedPath = await approveReadableVideoPath(filePath); + const normalizedPath = await approveReadableAvPath(filePath); if (!normalizedPath) { return { success: false, @@ -3727,7 +3832,7 @@ export function registerIpcHandlers( async (_, filePath: string, durationSec: number): Promise => { try { // Same approval gate as every other read of a renderer-supplied path. - const normalizedPath = await approveReadableVideoPath(filePath); + const normalizedPath = await approveReadableAvPath(filePath); if (!normalizedPath) { return { success: false, message: "File path is not approved" }; } @@ -3751,7 +3856,7 @@ export function registerIpcHandlers( // do (2 GiB cap) and a 16 GB machine cannot hold for multi-GB recordings. ipcMain.handle("read-file-chunk", async (_, filePath: string, offset: number, length: number) => { try { - const normalizedPath = await approveReadableVideoPath(filePath); + const normalizedPath = await approveReadableAvPath(filePath); if (!normalizedPath) { return { success: false, diff --git a/electron/ipc/nativeBridge.ts b/electron/ipc/nativeBridge.ts index 5d4992c10..7b01e0b2f 100644 --- a/electron/ipc/nativeBridge.ts +++ b/electron/ipc/nativeBridge.ts @@ -489,6 +489,7 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { request.payload.projectId, request.payload.path, request.payload.label, + request.payload.kind, ), ); case "document.removeAsset": diff --git a/electron/native-bridge/services/aiEditionService.ts b/electron/native-bridge/services/aiEditionService.ts index 0fbbccc9c..90088781a 100644 --- a/electron/native-bridge/services/aiEditionService.ts +++ b/electron/native-bridge/services/aiEditionService.ts @@ -151,9 +151,17 @@ export class AiEditionService { } } - async addAsset(projectId: string, path: string, label?: string): Promise { - const document = await this.options.documents.addAsset(projectId, { path, label }); - const assetId = document.project.primaryAssetId ?? document.assets.at(-1)?.id ?? ""; + async addAsset( + projectId: string, + path: string, + label?: string, + kind?: "video" | "audio", + ): Promise { + const document = await this.options.documents.addAsset(projectId, { path, label, kind }); + // The just-added asset is always the last one; primaryAssetId is only a + // fallback for the video case and would point at the wrong asset for an + // audio import (which never claims primary), so prefer the tail. + const assetId = document.assets.at(-1)?.id ?? document.project.primaryAssetId ?? ""; return { assetId, document }; } diff --git a/electron/preload.ts b/electron/preload.ts index 6aff16407..16227f110 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -276,6 +276,9 @@ contextBridge.exposeInMainWorld("electronAPI", { openVideoFilePicker: () => { return ipcRenderer.invoke("open-video-file-picker"); }, + openAudioFilePicker: () => { + return ipcRenderer.invoke("open-audio-file-picker"); + }, setCurrentVideoPath: (path: string) => { return ipcRenderer.invoke("set-current-video-path", path); }, diff --git a/flake.lock b/flake.lock index 77972fb40..dac7a986d 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1775710090, - "narHash": "sha256-ar3rofg+awPB8QXDaFJhJ2jJhu+KqN/PRCXeyuXR76E=", + "lastModified": 1788039129, + "narHash": "sha256-pa4Q0qErvCvzCaaUph7Sm37RhR4xvPrYI8Lgz6k85+A=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "4c1018dae018162ec878d42fec712642d214fdfa", + "rev": "d2f67949798825fe853f7c5d0492b8bf016d3f88", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index be01a58b4..4a8c39f2f 100644 --- a/flake.nix +++ b/flake.nix @@ -2,6 +2,17 @@ description = "OpenScreen — desktop screen recorder with built-in editor"; inputs = { + # Do not roll flake.lock BACK past nixpkgs d2f6794 (2026-08-29). Before it, + # `importCargoLock` fetched every crate from + # `https://crates.io/api/v1/crates///download`, which crates.io + # now answers with 403 — it rate-limits that endpoint to 1 req/s and points + # clients at the CDN instead (rust-lang/crates.io#13482). Every crate in the + # lockfile failed, so `nix build` died in `cargo-vendor-dir` before reaching a + # single derivation of ours: `Nix build` was red on main from 2026-08-30, and + # since `nix-check.yml` only compares npmDepsHash and `nix-build.yml` did not + # run on pull requests, the derivation itself was not being built anywhere -- + # not before a merge, and not after one either while this was red. + # d2f6794 carries the switch to `https://static.crates.io/crates`. nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; }; diff --git a/src/cli/CliExportRunner.tsx b/src/cli/CliExportRunner.tsx index 83cf31aaa..9a7f1b590 100644 --- a/src/cli/CliExportRunner.tsx +++ b/src/cli/CliExportRunner.tsx @@ -30,6 +30,7 @@ import { buildAutoZoomSuggestions } from "@/lib/ai-edition/timeline/zoom-suggest import type { CliDoneResult, CliExportRequest } from "@/lib/cliContracts"; import { GIF_SIZE_PRESETS, type GifSizePreset } from "@/lib/exporter"; import { calculateMp4ExportSettings } from "@/lib/exporter/mp4ExportSettings"; +import { outputFrameCount } from "@/lib/exporter/outputFrameCount"; import { mixVoiceoverIntoVideo } from "@/lib/exporter/voiceoverMix"; import { exportGifNative, exportMultiNative, nativeBridgeClient } from "@/native"; import type { CompositorClipInput } from "@/native/contracts"; @@ -277,13 +278,9 @@ async function runExport(request: CliExportRequest): Promise { // Progress: native pushes raw encoded-frame counts; totals and pacing are // computed here, mirroring the ExportDialog. const outFps = format === "gif" ? gifFrameRate : MP4_EXPORT_FPS; - const totalFrames = Math.max( - 1, - Math.round( - clips.reduce((sum, clip) => sum + Math.max(0, clip.sourceEndSec - clip.sourceStartSec), 0) * - outFps, - ), - ); + // Speed-adjusted, not source seconds — see `outputFrameCount`. Counting raw duration + // is what made a 1.25x timeline stop the bar at 80% (OpenScreen#371). + const totalFrames = outputFrameCount(clips, sceneDesc.speedRegions, outFps); const exportStartedAt = Date.now(); const unsubscribeProgress = window.electronAPI.onNativeExportProgress?.((frames: number) => { const elapsedSec = (Date.now() - exportStartedAt) / 1000; diff --git a/src/components/ai-edition/EditorEmptyState.test.tsx b/src/components/ai-edition/EditorEmptyState.test.tsx index acb513e4b..e175bd6bc 100644 --- a/src/components/ai-edition/EditorEmptyState.test.tsx +++ b/src/components/ai-edition/EditorEmptyState.test.tsx @@ -53,6 +53,7 @@ const sampleDoc = vi.hoisted( }, annotations: [], zoomRanges: [], + audioRanges: [], legacyEditor: null, }), ); diff --git a/src/components/ai-edition/ExportDialog.showInFolder.test.tsx b/src/components/ai-edition/ExportDialog.showInFolder.test.tsx index 835d9c8fd..6477fd3ad 100644 --- a/src/components/ai-edition/ExportDialog.showInFolder.test.tsx +++ b/src/components/ai-edition/ExportDialog.showInFolder.test.tsx @@ -17,7 +17,9 @@ vi.mock("@/native", () => ({ })); vi.mock("@/native/sceneDescription", () => ({ - buildSceneDescription: () => ({}), + // `speedRegions` is what the export dialog reads to size the progress total + // (`outputFrameCount`); an empty object here made it read `undefined`. + buildSceneDescription: () => ({ speedRegions: [] }), resolveVisibleClips: (doc: AxcutDocument) => doc.timeline.clips, })); @@ -75,6 +77,7 @@ const DOC: AxcutDocument = { }, annotations: [], zoomRanges: [], + audioRanges: [], legacyEditor: null, }; diff --git a/src/components/ai-edition/ExportDialog.test.ts b/src/components/ai-edition/ExportDialog.test.ts index 3aa8d85b5..41557e7ec 100644 --- a/src/components/ai-edition/ExportDialog.test.ts +++ b/src/components/ai-edition/ExportDialog.test.ts @@ -57,6 +57,7 @@ function doc(assets: AxcutAsset[], clips: AxcutClip[]): AxcutDocument { }, annotations: [], zoomRanges: [], + audioRanges: [], legacyEditor: null, }; } diff --git a/src/components/ai-edition/ExportDialog.tsx b/src/components/ai-edition/ExportDialog.tsx index fbc4d9362..5dff50acf 100644 --- a/src/components/ai-edition/ExportDialog.tsx +++ b/src/components/ai-edition/ExportDialog.tsx @@ -33,6 +33,7 @@ import { type GifSizePreset, } from "@/lib/exporter"; import { calculateMp4ExportSettings, wouldUpscale } from "@/lib/exporter/mp4ExportSettings"; +import { outputFrameCount } from "@/lib/exporter/outputFrameCount"; import { exportGifNative, exportMultiNative, useIsCpuCompositor } from "@/native"; import type { CompositorClipInput } from "@/native/contracts"; import { buildSceneDescription, resolveVisibleClips } from "@/native/sceneDescription"; @@ -287,15 +288,17 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { const clips = buildNativeClipList(document); // GIF runs at its own frame rate, so the progress total has to use it. const outFps = format === "gif" ? gifFrameRate : fps; - // Total frames the encoder will produce, known upfront from the timeline (sum of - // each clip's trimmed source duration) — the native side only reports frames - // AFTER encoding one (onNativeExportProgress), it doesn't know/send a total, so - // this is computed here to turn that raw count into a percentage. - const totalDurationSec = clips.reduce( - (sum, c) => sum + Math.max(0, c.sourceEndSec - c.sourceStartSec), - 0, - ); - const totalFrames = Math.max(1, Math.round(totalDurationSec * outFps)); + // Total frames the encoder will produce, known upfront from the timeline — the + // native side only reports frames AFTER composing one (onNativeExportProgress), + // it doesn't know or send a total, so this is computed here to turn that raw + // count into a percentage. + // + // It has to count SPEED-ADJUSTED frames, not source seconds: a clip under a 1.25x + // region emits 80% of `duration * fps`, which is where the "frozen at ~80%" of + // OpenScreen#371 came from — the bar climbed to 80% and the export finished + // there. `outputFrameCount` mirrors the compositor's own span arithmetic. + const sceneDesc = buildSceneDescription(document); + const totalFrames = outputFrameCount(clips, sceneDesc.speedRegions, outFps); const startedAt = Date.now(); const unsubscribeProgress = window.electronAPI?.onNativeExportProgress?.((frames) => { const elapsedS = (Date.now() - startedAt) / 1000; @@ -309,8 +312,6 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { }); }); try { - const sceneDesc = buildSceneDescription(document); - // The webcam background effect is applied by the compositor from the scene, // so the clip list needs no pre-rendering pass. const exportClips = clips; diff --git a/src/components/ai-edition/NewEditorShell.pasteRegion.test.tsx b/src/components/ai-edition/NewEditorShell.pasteRegion.test.tsx new file mode 100644 index 000000000..5d510cada --- /dev/null +++ b/src/components/ai-edition/NewEditorShell.pasteRegion.test.tsx @@ -0,0 +1,207 @@ +// @vitest-environment jsdom +// pasteRegion is a closure in the shell, so the only honest way to test it is through the +// window keydown that reaches it — same harness as NewEditorShell.dialogShortcuts.test.tsx, +// plus a seeded document (paste sits behind the `hasProject` gate). The preview is stubbed +// because the compositor canvas is the one child jsdom cannot host; nothing under test +// lives in it. +import "@testing-library/jest-dom"; +import { act, cleanup, fireEvent, render } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const openConfig = vi.fn(); + +vi.mock("@/contexts/ShortcutsContext", async () => { + const { DEFAULT_SHORTCUTS } = await import("@/lib/shortcuts"); + return { + useShortcuts: () => ({ + shortcuts: DEFAULT_SHORTCUTS, + isMac: false, + isConfigOpen: false, + openConfig, + closeConfig: () => { + /* not exercised here */ + }, + setShortcuts: () => { + /* not exercised here */ + }, + persistShortcuts: () => Promise.resolve(true), + }), + }; +}); + +vi.mock("@/contexts/I18nContext", () => ({ + useI18n: () => ({ + locale: "en", + setLocale: () => { + /* fixed locale */ + }, + }), + useScopedT: () => (key: string) => key, +})); + +vi.mock("sonner", () => ({ + toast: { + success: vi.fn(), + error: vi.fn(), + info: vi.fn(), + }, +})); + +vi.mock("./Preview", () => ({ + Preview: () => null, +})); + +import { toast } from "sonner"; +import { EditorDialogsProvider } from "@/contexts/EditorDialogsContext"; +import { type AxcutDocument, createEmptyDocument } from "@/lib/ai-edition/schema"; +import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; +import { clearRegionClipboard, copyRegion } from "@/lib/ai-edition/store/regionClipboard"; +import { NewEditorShell } from "./NewEditorShell"; + +/** One footage asset, one 0–30s clip — enough timeline for a pasted region to anchor to. */ +function seedDocument(): AxcutDocument { + const base = createEmptyDocument({ title: "Test", projectId: "proj_paste" }); + return { + ...base, + project: { ...base.project, primaryAssetId: "asset_1" }, + assets: [ + { + id: "asset_1", + kind: "video", + label: "rec.mp4", + originalPath: "C:/videos/rec.mp4", + durationSec: 60, + cameraTrack: null, + }, + ], + timeline: { + ...base.timeline, + clips: [ + { + id: "clip_1", + assetId: "asset_1", + sourceStartSec: 0, + sourceEndSec: 30, + timelineStartSec: 0, + timelineEndSec: 30, + wordRefs: [], + origin: "user" as const, + reason: "", + }, + ], + }, + }; +} + +function renderShell() { + return render( + + + , + ); +} + +/** Shortcuts are bound on `window` and read `e.target`; nothing is focused, so body. */ +function pressPaste() { + fireEvent.keyDown(document.body, { key: "v", ctrlKey: true }); +} + +beforeEach(() => { + openConfig.mockClear(); + vi.mocked(toast.success).mockClear(); + // No preload in jsdom, and no scrolling either; the chat transcript pins itself to the + // bottom on every render. + (window as unknown as { electronAPI?: unknown }).electronAPI = { + onAiEditionChatEvent: () => () => { + /* unsubscribe */ + }, + setTitleBarOverlay: () => { + /* no native titlebar */ + }, + setHasUnsavedChanges: () => { + /* no window close guard */ + }, + onRequestCloseConfirm: () => () => { + /* unsubscribe */ + }, + onRequestSaveBeforeClose: () => () => { + /* unsubscribe */ + }, + sendCloseConfirmResponse: () => { + /* nothing is closing this window */ + }, + findRecordingCamera: () => Promise.resolve(null), + preparePreviewAudioTrack: () => Promise.resolve(null), + // The timeline asks for waveform bytes as it mounts; there is no file behind the + // seeded asset, and the failed decode is noise, not a failure of anything here. + readBinaryFile: () => Promise.resolve(null), + }; + Element.prototype.scrollTo = () => { + /* no scrolling in jsdom */ + }; + // jsdom ships neither; the stage and the timeline both measure themselves. + (globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver = class { + observe() { + /* never fires: nothing has a layout in jsdom */ + } + unobserve() { + /* see observe */ + } + disconnect() { + /* see observe */ + } + }; + useProjectStore.getState().clear(); + useProjectStore.setState({ document: seedDocument(), currentTimeSec: 0 }); + // A zoom on the clipboard: the cheapest pasteable payload, and one whose landing is + // directly observable in the document. + copyRegion({ kind: "zoom", region: { startMs: 0, endMs: 1000, depth: 2 } }); +}); + +afterEach(() => { + cleanup(); + clearRegionClipboard(); + (window as unknown as { electronAPI?: unknown }).electronAPI = undefined; +}); + +describe("pasteRegion", () => { + it("does not claim a paste the store refused to write", async () => { + // saveDocument resolves false (rather than rejecting) when the write fails, and the + // failure is already reported by the store — the toast must stay silent rather than + // announce a paste that did not land. + const saveDocument = vi.fn(async () => false); + useProjectStore.setState({ saveDocument }); + renderShell(); + + await act(async () => { + pressPaste(); + }); + + expect(saveDocument).toHaveBeenCalledTimes(1); + expect(toast.success).not.toHaveBeenCalled(); + }); + + it("pastes onto the document current when the queued write runs, not a pre-await snapshot", async () => { + // The paste awaits dynamic imports before saving; two quick Ctrl+V both used to read + // the same snapshot and the second save clobbered the first — one pasted region for + // two presses. The read lives inside the enqueue chain now, so the second write + // builds on the first's result. + const saveDocument = vi.fn(async (next: AxcutDocument) => { + useProjectStore.setState({ document: next }); + return true; + }); + useProjectStore.setState({ saveDocument }); + renderShell(); + + await act(async () => { + pressPaste(); + pressPaste(); + }); + + const ranges = (useProjectStore.getState().document as AxcutDocument).zoomRanges; + expect(saveDocument).toHaveBeenCalledTimes(2); + expect(ranges).toHaveLength(2); + expect(ranges[0].id).not.toBe(ranges[1].id); + expect(toast.success).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index b371d100c..a9311b12b 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -327,7 +327,12 @@ export function NewEditorShell() { }; }, [promptUnsaved, saveDocument]); - const videoSources = useMemo(() => { + // Every asset with a resolvable URL, split below. It is deliberately NOT handed to + // anything as-is: `videoSources` reaching a consumer that treats its entries as + // footage is how an imported mp3 became a timeline clip (an audio-only project has no + // `primaryAssetId`, and both `handleLoadedMetadata` and `replaceTimeline` fall back to + // `assets[0]`, which the preview had already mounted as a