diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index adde1d52b..721fe7902 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -171,15 +171,36 @@ pub(crate) unsafe fn walk_composited_timeline( // vraiment une caméra — sinon la boîte PiP est dessinée avec, derrière, le décodeur de // repli, c'est-à-dire l'écran lui-même recopié dans son propre coin (issue #248). // La preview vive fait exactement ça dans `live.rs` ; c'est ici l'équivalent export. - comp.set_has_webcam(webcam_is_real(&clip.webcam, &clip.screen)); + // Source webcam, clé de cache et dessin de la PiP sont décidés ENSEMBLE, sinon + // ils divergent : + // + // - Un clip SANS caméra arrive avec un chemin webcam vide, que `Decoder::open` + // refuse. Le décodeur n'existe que parce que `compose_frame` échantillonne + // deux flux inconditionnellement, donc on lui redonne l'écran (même repli que + // `live.rs::open_and_seek_clip`) et la PiP n'est pas dessinée. Sans ça, + // exporter un projet sans caméra échouerait net — le cas le plus courant + // (issue #348). + // - La clé DOIT être le fichier réellement ouvert. Tous les clips sans caméra + // portent le même chemin vide : indexer dessus faisait que le deuxième + // récupérait le décodeur du premier, donc l'écran d'un AUTRE clip. Pas + // anodin même sans PiP, `webcam_available_duration` plus bas borne + // `source_end_sec` — un clip de 60s derrière un clip de 41s finissait à 41s. + // - Un chemin NON vide qui refuse de s'ouvrir n'est pas un repli : c'est une + // caméra que le document réclame et qu'on ne peut pas fournir. L'erreur + // remonte, comme avant l'ajout du repli. La rattraper par l'écran donnerait + // exactement #265 — `webcam_is_real` reste vrai pour ce chemin, donc l'écran + // serait recopié dans sa propre vignette. + let has_camera = webcam_is_real(&clip.webcam, &clip.screen); + comp.set_has_webcam(has_camera); + let webcam_key = if has_camera { &clip.webcam } else { &clip.screen }; if !screen_decs.contains_key(&clip.screen) { screen_decs.insert(clip.screen.clone(), Decoder::open(&clip.screen, gpu)?); } - if !webcam_decs.contains_key(&clip.webcam) { - webcam_decs.insert(clip.webcam.clone(), Decoder::open(&clip.webcam, gpu)?); + if !webcam_decs.contains_key(webcam_key) { + webcam_decs.insert(webcam_key.clone(), Decoder::open(webcam_key, gpu)?); } let sdec = screen_decs.get_mut(&clip.screen).unwrap(); - let wdec = webcam_decs.get_mut(&clip.webcam).unwrap(); + let wdec = webcam_decs.get_mut(webcam_key).unwrap(); let screen_available_duration = sdec.available_duration_sec(); let webcam_available_duration = wdec.available_duration_sec(); diff --git a/electron/ai-edition/document-service.test.ts b/electron/ai-edition/document-service.test.ts index 30f5fd718..2b5782aa6 100644 --- a/electron/ai-edition/document-service.test.ts +++ b/electron/ai-edition/document-service.test.ts @@ -68,6 +68,77 @@ describe("DocumentService", () => { await expect(service.getProject("proj/with/slash")).rejects.toBeInstanceOf(ProjectFileError); }); + // Issue #348 — recording with no camera AND no microphone is the default for + // anyone capturing a screen demo, and the failure lands at REOPEN, where the + // recording exists on disk but the user cannot get to it. The recorder writes + // no audio stream at all in that configuration (confirmed with ffprobe on real + // captures) and `cameraTrack: null`, so this is the exact on-disk shape. + describe("camera-less, microphone-less recordings", () => { + // Windows path separators on purpose: the reporter is on Windows 11 and + // `path.join` gives us the host's, so this stays honest on all three. + async function writeCamlessProject(originalPath: string, sizeBytes?: number) { + const doc = await service.createProject("Screen demo, no cam no mic"); + const asset: AxcutAsset = { + id: "asset_camless", + kind: "video", + label: path.basename(originalPath), + originalPath, + sizeBytes, + // No `audio` (the probe never populates it) and no camera link. + cameraTrack: null, + transcriptionFailure: { + kind: "no-audio", + message: "No audio track found in this video.", + }, + }; + await service.saveProject({ + ...doc, + assets: [asset], + project: { ...doc.project, primaryAssetId: asset.id }, + }); + return doc.project.id; + } + + it("reopens, and stays listed", async () => { + const screenPath = path.join(mediaDir, "screen-demo.mp4"); + await fs.writeFile(screenPath, "screen bytes", "utf8"); + const projectId = await writeCamlessProject(screenPath); + + const reopened = await service.getProject(projectId); + expect(reopened.assets[0]?.cameraTrack).toBeNull(); + expect(reopened.assets[0]?.originalPath).toBe(screenPath); + // A document that throws here is dropped by listProjects' skip-on-error + // catch, which presents to the user as "my project vanished" rather than + // as an error — so the absence of a throw is not enough to assert. + const summaries = await service.listProjects(); + expect(summaries.map((s) => s.id)).toContain(projectId); + // Re-decided on every open, so it must survive the round trip or the + // whole recording is re-extracted for transcription each time. + expect(reopened.assets[0]?.transcriptionFailure?.kind).toBe("no-audio"); + }); + + it("does not hand the relinker's webcam to an asset that never had one", async () => { + // The relink only runs when something is actually broken, so move the + // screen file — and register a link that DOES carry a webcam, which is + // the shape that produced #265 (screen recording used as the webcam). + const screenBytes = "screen bytes"; + const screenPath = path.join(mediaDir, "moved-screen-demo.mp4"); + const webcamPath = path.join(mediaDir, "moved-screen-demo-webcam.mp4"); + await fs.writeFile(screenPath, screenBytes, "utf8"); + await fs.writeFile(webcamPath, "webcam bytes", "utf8"); + await registerMediaLinks(mediaDir, screenPath, { webcamVideoPath: webcamPath }); + + const projectId = await writeCamlessProject( + path.join(mediaDir, "gone", "moved-screen-demo.mp4"), + Buffer.byteLength(screenBytes), + ); + + const reopened = await service.getProject(projectId); + expect(reopened.assets[0]?.originalPath).toBe(screenPath); + expect(reopened.assets[0]?.cameraTrack).toBeNull(); + }); + }); + // Issue #212 — a project authored on another machine opens with every asset // pointing at a path that does not exist here. The relink runs on this read, // not on import, so a document already saved broken still recovers. diff --git a/electron/media/cursorSidecar.test.ts b/electron/media/cursorSidecar.test.ts index 400553819..88ad3f60e 100644 --- a/electron/media/cursorSidecar.test.ts +++ b/electron/media/cursorSidecar.test.ts @@ -17,6 +17,7 @@ import { readCursorSidecar, readCursorTelemetryFile, } from "./cursorSidecar"; +import { whenRegistryIdle } from "./mediaLinksRegistry"; let dir: string; @@ -25,6 +26,12 @@ beforeEach(async () => { }); afterEach(async () => { + // The registry fallback below starts a path-refresh write that the lookup + // deliberately does not await, so it can still be queued when the test ends. + // Removing the tree underneath it made `fs.rm` fail with ENOTEMPTY — the write + // recreating an entry between rm's recursive walk and its final rmdir — which + // failed this hook, intermittently, only in the full parallel suite. + await whenRegistryIdle(); await fs.rm(dir, { recursive: true, force: true }); vi.restoreAllMocks(); }); diff --git a/electron/media/mediaLinksRegistry.test.ts b/electron/media/mediaLinksRegistry.test.ts index 0e50905db..7c76a7580 100644 --- a/electron/media/mediaLinksRegistry.test.ts +++ b/electron/media/mediaLinksRegistry.test.ts @@ -7,6 +7,7 @@ import { findMediaLinksByFingerprint, findRelocatedMediaByStoredPath, registerMediaLinks, + whenRegistryIdle, } from "./mediaLinksRegistry"; async function makeTempDir(): Promise { @@ -262,6 +263,12 @@ describe("mediaLinksRegistry", () => { process.on("unhandledRejection", onRejection); try { await fn(); + // The refresh these cases are about is deliberately not awaited by the + // lookup, so `fn` returns while it is still queued. Waiting for the + // queue to drain is what makes "did the refresh warn / write?" + // answerable at all — the 50 ms below used to be doing that job by + // accident, and lost the race whenever the suite ran under load. + await whenRegistryIdle(); // Node decides a rejection is unhandled a tick after the microtask // queue drains, so the assertion needs a real timer, not a flush. await new Promise((resolve) => setTimeout(resolve, 50)); @@ -304,6 +311,26 @@ describe("mediaLinksRegistry", () => { } }); + // The drain the two cases above rely on. Without it there is no way to know + // the refresh has landed: the lookup returns while the write is still + // queued, so a caller that removes the directory races it and a test that + // asserts on its outcome is asserting on a coin flip. Both were real + // intermittent failures in the full suite (this file, and cursorSidecar's + // `afterEach` failing with ENOTEMPTY), green in isolation every time. + it("whenRegistryIdle waits for a refresh the lookup did not await", async () => { + const { original, moved } = await registerThenMove(); + const recorded = async () => + JSON.parse(await fs.readFile(path.join(tempDir, "media-links.registry.json"), "utf-8")) + .entries[0].lastKnownPath; + + expect(await recorded()).toBe(original); + await findMediaLinksByFingerprint(tempDir, moved); + await whenRegistryIdle(tempDir); + + // Durably on disk, not "probably by now". + expect(await recorded()).toBe(moved); + }); + it("survives the directory disappearing while the refresh is queued", async () => { // The CI shape: a suite's `afterEach` removes its temp dir while a write // is still in the queue. Whoever wins the race is fine — what must not diff --git a/electron/media/mediaLinksRegistry.ts b/electron/media/mediaLinksRegistry.ts index c2507d138..ec0ad76de 100644 --- a/electron/media/mediaLinksRegistry.ts +++ b/electron/media/mediaLinksRegistry.ts @@ -180,6 +180,33 @@ function withWriteLock(baseDir: string, fn: () => Promise): Promise { return result; } +/** + * Resolves once every write queued for `baseDir` — or for every directory, with + * no argument — has drained. + * + * `findMediaLinksByFingerprint` refreshes a drifted path WITHOUT awaiting it, on + * purpose (a lookup must not pay for a write it does not need). That leaves work + * running after the call that started it returned, and nothing could wait for it: + * a caller that then removed the directory raced the write, and a test that + * asserted on the write's outcome was asserting on a coin flip. Both showed up as + * intermittent failures in the full suite and passed in isolation, which is the + * signature of exactly this. + * + * The queue tails never reject (see `withWriteLock`), so this never throws — it is + * "the writes are done", not "the writes succeeded". + */ +export async function whenRegistryIdle(baseDir?: string): Promise { + for (;;) { + const tails = baseDir ? [writeQueues.get(baseDir)] : [...writeQueues.values()]; + const pending = tails.filter((t): t is Promise => t !== undefined); + if (pending.length === 0) return; + // A drained write can have queued another behind it, so loop rather than + // await once. `withWriteLock` drops its own key when the chain goes idle, + // which is what eventually empties the map and ends this. + await Promise.all(pending); + } +} + async function updateRegistry( baseDir: string, mutator: (file: MediaLinksRegistryFile) => MediaLinksRegistryFile, diff --git a/src/cli/CliExportRunner.tsx b/src/cli/CliExportRunner.tsx index f9c57c682..5b72eee1c 100644 --- a/src/cli/CliExportRunner.tsx +++ b/src/cli/CliExportRunner.tsx @@ -23,6 +23,7 @@ import { import { applyProbedDuration } from "@/lib/ai-edition/document/timeline"; import type { AxcutDocument } from "@/lib/ai-edition/schema"; import { getEditorSettings } from "@/lib/ai-edition/store/editorSettings"; +import { assetCameraSource } from "@/lib/ai-edition/timeline/camera"; import { resolveClipSourceEndSec } from "@/lib/ai-edition/timeline/clipDuration"; import { DEFAULT_ZOOM_DEPTH, ZOOM_DEPTH_SCALES } from "@/lib/ai-edition/timeline/zoom-scale"; import { buildAutoZoomSuggestions } from "@/lib/ai-edition/timeline/zoom-suggestions"; @@ -83,15 +84,15 @@ function buildNativeClipList(axcutDocument: AxcutDocument): CompositorClipInput[ if (!asset?.originalPath) { return []; } - const cam = asset.cameraTrack; + const camera = assetCameraSource(asset); const sourceEndSec = resolveClipSourceEndSec(clip, asset); return [ { screenPath: asset.originalPath, - webcamPath: cam?.sourcePath ?? asset.originalPath, + webcamPath: camera.path, sourceStartSec: clip.sourceStartSec, sourceEndSec, - webcamOffsetSec: cam ? (cam.startMs + cam.offsetMs) / 1000 : 0, + webcamOffsetSec: camera.offsetSec, hasAudio: true, }, ]; diff --git a/src/components/ai-edition/ExportDialog.tsx b/src/components/ai-edition/ExportDialog.tsx index 901756989..e640fbd19 100644 --- a/src/components/ai-edition/ExportDialog.tsx +++ b/src/components/ai-edition/ExportDialog.tsx @@ -20,6 +20,7 @@ import { } from "@/lib/ai-edition/document/outputFormat"; import type { AxcutDocument } from "@/lib/ai-edition/schema"; import { getEditorSettings } from "@/lib/ai-edition/store/editorSettings"; +import { assetCameraSource } from "@/lib/ai-edition/timeline/camera"; import { resolveClipSourceEndSec } from "@/lib/ai-edition/timeline/clipDuration"; import { type ExportFormat, @@ -62,24 +63,21 @@ function buildNativeClipList(document: AxcutDocument): CompositorClipInput[] { if (!asset?.originalPath) { return []; } - const cam = asset.cameraTrack; + const camera = assetCameraSource(asset); // sourceEndSec is optional in the schema (unknown until probed) — fall back through // the single canonical precedence used by every consumer (clip.probe → asset.duration // → timeline-length guess). See `resolveClipSourceEndSec` for the full order. const sourceEndSec = resolveClipSourceEndSec(clip, asset); - // ponytail: matches the rule in `buildSceneDescription` — screen recordings - // from this app always carry a decodable audio track (the webcam path - // never does), so the only clips that reach this branch already have audio. - // If a per-asset audio-probe flag lands on the schema later, swap to - // `Boolean(asset.audio)` here too and keep these two derivation paths in - // lock-step with `buildSceneDescription` in src/native/sceneDescription.ts. + // ponytail: `hasAudio` stays optimistic for the same reason as in + // `buildSceneDescription` — nothing populates `asset.audio` yet, and the + // native side degrades cleanly on a stream-less file. return [ { screenPath: asset.originalPath, - webcamPath: cam?.sourcePath ?? asset.originalPath, + webcamPath: camera.path, sourceStartSec: clip.sourceStartSec, sourceEndSec, - webcamOffsetSec: cam ? (cam.startMs + cam.offsetMs) / 1000 : 0, + webcamOffsetSec: camera.offsetSec, hasAudio: true, }, ]; diff --git a/src/components/ai-edition/NativeCompositorOverlay.tsx b/src/components/ai-edition/NativeCompositorOverlay.tsx index 198cf89b5..710e90c74 100644 --- a/src/components/ai-edition/NativeCompositorOverlay.tsx +++ b/src/components/ai-edition/NativeCompositorOverlay.tsx @@ -3,6 +3,7 @@ import { useScopedT } from "@/contexts/I18nContext"; import { noteUiProbeClipSwitch } from "@/lib/ai-edition/perf/uiFrameProbe"; import { getEditorSettings } from "@/lib/ai-edition/store/editorSettings"; import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; +import { assetCameraSource } from "@/lib/ai-edition/timeline/camera"; import { resolveNativePosition } from "@/lib/ai-edition/timeline/timelineMap"; import { pushAllNativeParams, @@ -89,12 +90,12 @@ export function NativeCompositorOverlay() { if (!primary?.originalPath) { return {}; } + // `undefined` rather than `""` here ONLY because `useNativeCompositorView` + // treats the key's absence as "no webcam source"; the value still comes from + // the one accessor, so it can never disagree with the scene or the export. return { screenPath: primary.originalPath, - webcamPath: - primary.cameraTrack?.visible && primary.cameraTrack.sourcePath - ? primary.cameraTrack.sourcePath - : undefined, + webcamPath: assetCameraSource(primary).path || undefined, // sidecar convention (electron/ipc/handlers.ts readCursorRecordingFile) : la // télémétrie curseur vit à côté de la vidéo tant qu'elle n'a pas bougé. Absente → // le natif ignore juste le curseur (CursorTrack::load échoue silencieusement). @@ -196,8 +197,7 @@ export function NativeCompositorOverlay() { if (!asset?.originalPath) { return; } - const cam = asset.cameraTrack; - const webcamPath = cam && cam.visible && cam.sourcePath ? cam.sourcePath : ""; + const camera = assetCameraSource(asset); const targetClipId = activeClipId; // Sonde de fluidité (diagnostic) : sépare les mesures d'avant et d'après un // franchissement de clip, qui se sont déjà révélées non comparables. @@ -214,8 +214,8 @@ export function NativeCompositorOverlay() { setActiveClip( viewId, asset.originalPath, - webcamPath, - cam ? (cam.startMs + cam.offsetMs) / 1000 : 0, + camera.path, + camera.offsetSec, activeClipIndex, activeSourceTimeSec, ) diff --git a/src/hooks/useAudioPeaks.test.ts b/src/hooks/useAudioPeaks.test.ts index 0783cd2d9..a5a18f513 100644 --- a/src/hooks/useAudioPeaks.test.ts +++ b/src/hooks/useAudioPeaks.test.ts @@ -10,8 +10,12 @@ const streamingCalls = vi.fn(); const inMemoryCalls = vi.fn(); vi.mock("./streamingAudioPeaks", () => ({ - computePeaksFromFileStreaming: async () => { + computePeaksFromFileStreaming: async (file: { name: string }) => { streamingCalls(); + // A recording captured with no microphone and no system audio has no audio + // stream at all (verified with ffprobe on real camera-less captures), so + // every decode of it fails — the same way, every time. + if (file.name.startsWith("silent")) throw new Error("no audio track"); return new Float32Array([0, 1]); }, })); @@ -85,4 +89,85 @@ describe("useAudioPeaks", () => { await waitFor(() => expect(again.result.current).not.toBeNull()); expect(streamingCalls).toHaveBeenCalledOnce(); }); + + // Issue #348 — a project recorded with no camera AND no microphone. The + // recorder writes an MP4 with no audio stream at all, so the decode below can + // never succeed. Caching only successes meant this file re-read itself whole + // on every mount, forever; a recording WITH a mic paid it once. That + // asymmetry is the bug. + it("gives up on a file with no audio track once, not once per mount", async () => { + const url = "/tmp/silent-no-mic.mp4"; + const warned = vi.spyOn(console, "warn").mockImplementation(() => { + // swallowed: it is the signal this test waits on, not suite output + }); + const first = renderHook(() => useAudioPeaks(url, THIRTY_TWO_MINUTES)); + // The hook logs from its `.catch`, so this is the first observable AFTER the + // rejection settles. Waiting on `streamingCalls` instead would only prove the + // decode STARTED: the remounts below would then join the still-pending + // in-flight promise, and the test would pass even if the failure cache broke. + await waitFor(() => expect(warned).toHaveBeenCalled()); + expect(first.result.current).toBeNull(); + + act(() => first.unmount()); + const second = renderHook(() => useAudioPeaks(url, THIRTY_TWO_MINUTES)); + await waitFor(() => expect(second.result.current).toBeNull()); + act(() => second.unmount()); + renderHook(() => useAudioPeaks(url, THIRTY_TWO_MINUTES)); + + // The decode is never retried: "this file has no waveform" is a permanent + // answer and is remembered as one. The first mount's rejection has settled + // by now, so these mounts hit the cache — not a shared in-flight promise. + expect(streamingCalls).toHaveBeenCalledOnce(); + expect(warned).toHaveBeenCalledTimes(1); + warned.mockRestore(); + }); + + // The other half of #348, and the expensive half: ffmpeg answers "no audio + // track" in ~2s, and the renderer used to spend a 175 MB copy into OPFS plus + // a full Chromium decode re-discovering it — on every project open, since a + // module-scope cache starts empty each launch. + it("takes ffmpeg's word for it when a recording has no audio track", async () => { + (window as unknown as { electronAPI: unknown }).electronAPI = { + getReadableFileInfo: async () => ({ success: true, size: FILE_BYTES }), + getAudioPeaks: async () => ({ + success: false, + message: "Cannot find wanted stream in the input file", + }), + }; + const { result } = renderHook(() => + useAudioPeaks("/tmp/no-mic-recording.mp4", THIRTY_TWO_MINUTES), + ); + await waitFor(() => expect(result.current).not.toBeNull()); + // A verdict, not a gap: no browser pipeline runs at all. + expect(streamingCalls).not.toHaveBeenCalled(); + expect(inMemoryCalls).not.toHaveBeenCalled(); + expect(result.current).toHaveLength(0); + }); + + it("still falls back to a browser pipeline when the host has no ffmpeg", async () => { + (window as unknown as { electronAPI: unknown }).electronAPI = { + getReadableFileInfo: async () => ({ success: true, size: FILE_BYTES }), + // The documented "no native ffmpeg here" signal — a gap, not a verdict. + getAudioPeaks: async () => ({ success: true, peaks: null }), + }; + renderHook(() => useAudioPeaks("/tmp/no-ffmpeg-host.mp4", THIRTY_TWO_MINUTES)); + await waitFor(() => expect(streamingCalls).toHaveBeenCalledOnce()); + }); + + it("decodes nothing until the duration is known", async () => { + const url = "/tmp/pending-duration.mp4"; + const view = renderHook(({ d }: { d: number | undefined }) => useAudioPeaks(url, d), { + initialProps: { d: undefined as number | undefined }, + }); + // Without a duration `computePeaksForUrl` cannot reach the cheap native + // tier and falls through to reading the whole file into memory — for a + // waveform `ClipWaveform` could not draw anyway, since it needs the same + // duration to lay bars out. + expect(streamingCalls).not.toHaveBeenCalled(); + expect(inMemoryCalls).not.toHaveBeenCalled(); + + view.rerender({ d: THIRTY_TWO_MINUTES }); + await waitFor(() => expect(view.result.current).not.toBeNull()); + expect(streamingCalls).toHaveBeenCalledOnce(); + }); }); diff --git a/src/hooks/useAudioPeaks.ts b/src/hooks/useAudioPeaks.ts index da7512e2c..3db9e6fcb 100644 --- a/src/hooks/useAudioPeaks.ts +++ b/src/hooks/useAudioPeaks.ts @@ -95,15 +95,29 @@ async function computePeaksForUrl( // Native first. Both browser pipelines below decode the whole track in // Chromium — 12s on a 32-minute recording, whichever one runs — where ffmpeg // in the main process takes ~2s and caches the result on disk, so the second - // time it is free. Anything that stops this from working (no ffmpeg staged, - // an unapproved path, a clip with no audio) falls through rather than - // dropping the waveform. + // time it is free. + // + // Only ONE of the three replies is a reason to fall through (see + // `AudioPeaksResult`): `peaks: null` means "no native ffmpeg on this host", + // which is the gap the browser pipelines exist to cover. `success: false` + // means ffmpeg RAN and found nothing to decode — a verdict, not a gap. + // + // Falling through on that verdict is issue #348's real cost: a recording made + // with no mic and no system audio has no audio stream at all, ffmpeg says so + // in ~2s, and the renderer then spent a 175 MB copy into OPFS plus a full + // Chromium decode re-discovering it on every project open. Empty peaks rather + // than a throw, so the answer caches like any other and is never recomputed. if (!isRemoteUrl && durationSec && window.electronAPI?.getAudioPeaks) { try { const native = await window.electronAPI.getAudioPeaks(videoUrl, durationSec); - if (native.success && native.peaks && native.peaks.length > 0) return native.peaks; + if (native.success) { + if (native.peaks && native.peaks.length > 0) return native.peaks; + if (native.peaks !== null) return new Float32Array(0); + } else { + return new Float32Array(0); + } } catch { - // Fall through to the browser pipelines. + // The IPC itself failed — that IS a gap, so fall through. } } @@ -143,11 +157,18 @@ async function computePeaksForUrl( * * `inFlight` is the other half: N clips of one asset mounting together must * share a single decode instead of racing N of them. + * + * FAILURE IS CACHED TOO (`null`), which is why the value type is nullable and + * why lookups go through `has()` rather than truthiness. A file with no audio + * fails deterministically, so retrying it is pure cost — and on a host with no + * native ffmpeg that retry is the whole-file browser decode. Caching only + * successes meant a recording WITH a mic paid for its waveform once while one + * WITHOUT paid, and threw away, the same work on every mount (issue #348). */ -const peaksCache = new Map(); +const peaksCache = new Map(); const peaksInFlight = new Map>(); -function loadPeaks(videoUrl: string, durationSec?: number): Promise { +function loadPeaks(videoUrl: string, durationSec: number): Promise { const existing = peaksInFlight.get(videoUrl); if (existing) return existing; // Deliberately NOT wired to any component's AbortSignal: the work is shared, @@ -158,6 +179,12 @@ function loadPeaks(videoUrl: string, durationSec?: number): Promise { + // "This file has no waveform" is an answer, and a permanent one — record + // it so the decode is never attempted again for this file. + peaksCache.set(videoUrl, null); + throw err; + }) .finally(() => { peaksInFlight.delete(videoUrl); }); @@ -168,8 +195,14 @@ function loadPeaks(videoUrl: string, durationSec?: number): Promise(() => @@ -182,13 +215,13 @@ export function useAudioPeaks(videoUrl?: string, durationSec?: number): Float32A return; } - const cached = peaksCache.get(videoUrl); - if (cached) { - setPeaks(cached); + if (peaksCache.has(videoUrl)) { + setPeaks(peaksCache.get(videoUrl) ?? null); return; } setPeaks(null); + if (!durationSec) return; let cancelled = false; loadPeaks(videoUrl, durationSec) @@ -206,7 +239,7 @@ export function useAudioPeaks(videoUrl?: string, durationSec?: number): Float32A return () => { cancelled = true; }; - }, [videoUrl]); + }, [videoUrl, durationSec]); return peaks; } diff --git a/src/lib/ai-edition/timeline/camera.test.ts b/src/lib/ai-edition/timeline/camera.test.ts index 95aaa505a..1f7fea91b 100644 --- a/src/lib/ai-edition/timeline/camera.test.ts +++ b/src/lib/ai-edition/timeline/camera.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { AxcutAsset, AxcutClip } from "../schema"; -import { hasAnyClipWithCamera, resolveActiveCameraTrack } from "./camera"; +import { assetCameraSource, hasAnyClipWithCamera, resolveActiveCameraTrack } from "./camera"; const assetWithCamera: AxcutAsset = { id: "asset_with_camera", @@ -111,3 +111,46 @@ describe("hasAnyClipWithCamera", () => { expect(hasAnyClipWithCamera([], [])).toBe(false); }); }); + +// The single spelling of "no camera". Five producers used to answer this +// question five different ways (empty string, undefined, and — in both export +// paths — the SCREEN recording's own path, which is issue #265's defect shape). +// They all route through assetCameraSource now; this is what pins it. +describe("assetCameraSource", () => { + it("returns the camera path and its start offset in seconds", () => { + const asset: AxcutAsset = { + ...assetWithCamera, + cameraTrack: { sourcePath: "/cam-1.mp4", startMs: 500, offsetMs: -200, visible: true }, + }; + expect(assetCameraSource(asset)).toEqual({ path: "/cam-1.mp4", offsetSec: 0.3 }); + }); + + it('says "no camera" with an empty path — NEVER the screen recording', () => { + expect(assetCameraSource(assetWithoutCamera)).toEqual({ path: "", offsetSec: 0 }); + // The banned fallback: substituting originalPath makes "no camera" + // indistinguishable from "the camera IS this file", and the native side + // then has to tell them apart by string comparison. + expect(assetCameraSource(assetWithoutCamera).path).not.toBe(assetWithoutCamera.originalPath); + }); + + it("treats a hidden camera as no camera", () => { + // The export producers used to ignore `visible` while the preview and the + // scene honoured it — the same project rendered two different ways. + expect(assetCameraSource(assetWithHiddenCamera)).toEqual({ path: "", offsetSec: 0 }); + }); + + it("treats a camera track with no source path as no camera", () => { + // `cameraTrackSchema` requires a non-empty sourcePath, so a parsed document + // cannot carry this — but the accessor takes an asset, not a parse result, + // and the branch exists. Covered so it cannot quietly start returning "". + const asset: AxcutAsset = { + ...assetWithCamera, + cameraTrack: { sourcePath: "", startMs: 0, offsetMs: 0, visible: true }, + }; + expect(assetCameraSource(asset)).toEqual({ path: "", offsetSec: 0 }); + }); + + it("tolerates a missing asset", () => { + expect(assetCameraSource(undefined)).toEqual({ path: "", offsetSec: 0 }); + }); +}); diff --git a/src/lib/ai-edition/timeline/camera.ts b/src/lib/ai-edition/timeline/camera.ts index 8b1c96746..4fc576952 100644 --- a/src/lib/ai-edition/timeline/camera.ts +++ b/src/lib/ai-edition/timeline/camera.ts @@ -22,3 +22,31 @@ export function resolveActiveCameraTrack( export function hasAnyClipWithCamera(assets: AxcutAsset[], clips: AxcutClip[]): boolean { return clips.some((clip) => assets.find((a) => a.id === clip.assetId)?.cameraTrack != null); } + +/** + * THE answer to "which camera file does this asset contribute, and where does it + * start". Every producer of a `CompositorClipInput` — the scene, the preview + * overlay, the export dialog, the CLI exporter — must go through this, because + * the native side compares the webcam path against the screen path to decide + * whether a PiP gets drawn at all (`webcam_is_real`, frame_geometry.rs). + * + * `path: ""` is the ONE way to say "no camera". The alternative that used to + * live in the export producers — substituting `asset.originalPath` — is banned: + * it makes "no camera" indistinguishable from "the camera happens to be the + * screen file", and it only ever worked because both fields were filled from + * the same variable, so the two strings matched byte for byte. Any producer that + * derived one of them differently (a separator, a case, a resolved vs. raw path + * — all routine on Windows) would have re-opened issue #265, where the screen + * recording is drawn into the webcam slot. + * + * `visible: false` counts as no camera, matching what the preview and scene + * already did and what the export producers did NOT. + */ +export function assetCameraSource(asset: AxcutAsset | undefined): { + path: string; + offsetSec: number; +} { + const cam = asset?.cameraTrack; + if (!cam?.visible || !cam.sourcePath) return { path: "", offsetSec: 0 }; + return { path: cam.sourcePath, offsetSec: (cam.startMs + cam.offsetMs) / 1000 }; +} diff --git a/src/native/sceneDescription.ts b/src/native/sceneDescription.ts index ae4beb5d9..7186d66e5 100644 --- a/src/native/sceneDescription.ts +++ b/src/native/sceneDescription.ts @@ -29,6 +29,7 @@ import { pickOutputDims } from "@/lib/ai-edition/document/outputFormat"; import { resolvePlaybackSegments } from "@/lib/ai-edition/document/timeline"; import type { AxcutClip, AxcutDocument } from "@/lib/ai-edition/schema"; import { getEditorSettings } from "@/lib/ai-edition/store/editorSettings"; +import { assetCameraSource } from "@/lib/ai-edition/timeline/camera"; import { resolveClipSourceEndSec } from "@/lib/ai-edition/timeline/clipDuration"; import { projectRegionsToSource } from "@/lib/ai-edition/timeline/timelineMap"; import { @@ -417,24 +418,21 @@ export function buildSceneDescription( const clips: CompositorClipInput[] = visibleClips.flatMap((clip) => { const asset = assetById.get(clip.assetId); if (!asset?.originalPath) return []; - const cam = asset.cameraTrack; - // ponytail: screen recordings from this app always carry a decodable audio - // track (confirmed via ffprobe on real recordings); webcam files never do - // and clips only ever reference their SCREEN path for the main video. The - // `asset.audio` schema slot exists but is never populated by the probe - // pipeline today, so we can't rely on it as an "is there a track?" signal — - // matching the legacy web exporter (which just tries-and-catches in - // `decodeSegmentAudioPcm`), we default `hasAudio: true` for every clip whose - // asset has an `originalPath`. The visibleClips filter above already - // guarantees that precondition by the time we reach this branch. If a - // per-asset audio-probe flag is added later, swap to `Boolean(asset.audio)`. + const camera = assetCameraSource(asset); + // ponytail: `asset.audio` exists in the schema but the probe pipeline never + // populates it, so there is no per-asset "is there a track?" signal to read + // yet. Every consumer downstream degrades on a stream-less file (audio.rs + // returns Ok(None)), so this stays optimistic. NOT "recordings always carry + // audio" — a capture made with no mic and no system audio has no audio + // stream at all (issue #348). Swap to `Boolean(asset.audio)` the day the + // probe fills it in. return [ { screenPath: asset.originalPath, - webcamPath: cam && cam.visible && cam.sourcePath ? cam.sourcePath : "", + webcamPath: camera.path, sourceStartSec: clip.sourceStartSec, sourceEndSec: resolveClipSourceEndSec(clip, asset), - webcamOffsetSec: cam ? (cam.startMs + cam.offsetMs) / 1000 : 0, + webcamOffsetSec: camera.offsetSec, hasAudio: true, }, ]; @@ -575,10 +573,8 @@ export function buildSceneDescription( * with the clip above, so the layout and the decoder can never disagree about it. * Note this is NOT `hasAnyClipWithCamera` (which gates the Layout panel): that one * ignores `visible` on purpose, so the panel stays reachable to un-hide a camera. */ - const clipHasCamera = (clip: AxcutClip) => { - const cam = assetById.get(clip.assetId)?.cameraTrack; - return Boolean(cam?.visible && cam.sourcePath); - }; + const clipHasCamera = (clip: AxcutClip) => + assetCameraSource(assetById.get(clip.assetId)).path !== ""; /** * The layout preset is GLOBAL — one panel for the whole timeline — but the camera is * per clip: a project mixes a screen+webcam recording with a plain import that has