Skip to content

Commit 1e18a6a

Browse files
superkc2026EtienneLescot
authored andcommitted
fix(review): address OpenScreen#371 blocking review (EtienneLescot)
- #1: avfilter_atempo_stretch returns None (-> WSOLA fallback) when atempo drains fewer than 90% of target samples, instead of padding the near-empty output to target_samples and exporting silence on short speed spans (gaps between regions, single video frames). - #3: avfilter is now a fully-known vendoring/packaging dependency: * fetch-ffmpeg.mjs probes ALL six shared DLLs (was: any av*.dll) so a warm tree with the five pre-avfilter DLLs re-vendors avfilter-11.dll. * before-pack.cjs lists avfilter on Linux, Windows and macOS (mac atLeast 3 -> 4). * build-linux-compositor-addon.mjs header + build-and-packaging.md note the sixth ffmpeg soname. - #2 (decode loop budget guard) removed here and split into its own PR to keep this one single-concern (atempo stretch).
1 parent 3eb690b commit 1e18a6a

5 files changed

Lines changed: 40 additions & 31 deletions

File tree

‎crates/compositor/src/audio.rs‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -320,30 +320,10 @@ unsafe fn decode_clip_audio_inner(
320320
let mut frame = av_frame_alloc();
321321
let mut input_eof = false;
322322

323-
// Garde anti-boucle : un conteneur dont la piste audio est tronquée ou corrompue en fin
324-
// de flux peut faire que `av_read_frame` ne renvoie jamais AVERROR_EOF, empêchant
325-
// `decoder_eof` de se propager — la boucle tourne alors à 100 % CPU pour toujours.
326-
// Un budget TEMPS plutôt qu'un compteur d'itérations : `av_read_frame` peut être lent
327-
// sur un flux corrompu, un compteur serait soit trop grand soit trop petit. Le budget
328-
// est dimensionné sur la durée demandée (facteur 8, plancher 60 s) : un long clip sur
329-
// un stockage lent reste couvert, seule une vraie boucle infinie est coupée.
330-
let loop_start = std::time::Instant::now();
331-
let loop_budget_secs = (((source_end_sec - source_start_sec).max(0.0) * 8.0) as u64).max(60);
332-
let loop_budget = std::time::Duration::from_secs(loop_budget_secs);
333-
334323
// Une seule passe de démux alimente tous les décodeurs : chaque paquet est routé vers la
335324
// piste dont il porte l'index. On continue tant qu'AU MOINS une piste a encore quelque
336325
// chose à produire.
337326
while tracks.iter().any(|t| !t.reached_end && !t.decoder_eof) {
338-
if loop_start.elapsed() > loop_budget {
339-
eprintln!(
340-
"[openscreen-compositor] decode_clip_audio: boucle plafonnée à {loop_budget_secs} s (source_end={source_end_sec} s), sortie forcée"
341-
);
342-
for track in tracks.iter_mut() {
343-
track.decoder_eof = true;
344-
}
345-
break;
346-
}
347327
if !input_eof {
348328
let read = av_read_frame(fmt, packet);
349329
if read == AVERROR_EOF {
@@ -1089,6 +1069,16 @@ unsafe fn avfilter_atempo_stretch(
10891069
}
10901070
av_frame_free(&mut frame);
10911071

1072+
// OpenScreen#371 review (EtienneLescot): atempo needs a full analysis window
1073+
// before it emits anything — a span shorter than that (e.g. a 30 ms gap between
1074+
// two speed regions, or a single video frame) drains to ZERO samples. Padding
1075+
// that emptiness up to `target_samples` would export silence, while the contract
1076+
// of `stretch_pcm_to_length` promises a `None` -> WSOLA fallback on failure. Bail
1077+
// out so the WSOLA path runs and genuinely stretches these spans.
1078+
if stretched[0].len() < target_samples * 9 / 10 {
1079+
return None;
1080+
}
1081+
10921082
// Recadrage exact : la longueur rendue par atempo diffère de `target_samples` de quelques
10931083
// échantillons de flush ; on tronque ou on padde, comme le faisait le chemin WSOLA.
10941084
let mut result: PlanarPcm = Vec::with_capacity(AUDIO_OUTPUT_CHANNELS);

‎scripts/before-pack.cjs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ const MAC_REQUIRED = [
9696
fix: FIX_MAC,
9797
},
9898
{
99-
match: (name) => /^libav(codec|format|util)\.\d+\.dylib$/.test(name),
99+
match: (name) => /^libav(codec|format|util|filter)\.\d+\.dylib$/.test(name),
100100
what: "the LGPL ffmpeg dylibs the compositor links",
101101
breaks: "the compositor addon cannot be loaded at all (dyld error at require())",
102102
fix: FIX_MAC,
103-
atLeast: 3,
103+
atLeast: 4,
104104
},
105105
{
106106
match: (name) => name === "whisper-stt-server",
@@ -162,7 +162,7 @@ const LINUX_REQUIRED = [
162162
// pendant qu'une autre manquait. Le paquet passait alors la garde et le
163163
// compositeur ne chargeait pas : exactement le mode de panne que cette garde
164164
// existe pour attraper.
165-
...["avcodec", "avformat", "avutil", "swresample", "swscale"].map((library) => ({
165+
...["avcodec", "avformat", "avutil", "swresample", "swscale", "avfilter"].map((library) => ({
166166
match: (name) => new RegExp(`^lib${library}\\.so\\.\\d+$`).test(name),
167167
what: `the symbol-renamed lib${library} shared object the compositor links`,
168168
breaks: "the compositor addon cannot be loaded at all (ld.so error at require())",
@@ -276,7 +276,7 @@ const WIN_REQUIRED = [
276276
// (avcodec-60/61/62.dll left by an earlier fetch) would satisfy a combined count
277277
// while another library was missing entirely, and the addon would still fail to
278278
// load.
279-
...["avcodec", "avformat", "avutil"].map((library) => ({
279+
...["avcodec", "avformat", "avutil", "avfilter"].map((library) => ({
280280
match: (name) => new RegExp(`^${library}-\\d+\\.dll$`).test(name),
281281
what: `the ${library} DLL the compositor links`,
282282
breaks: "the addon cannot be loaded at all under MSIX, which ignores PATH",

‎scripts/build-linux-compositor-addon.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// (ensureFfmpegSharedDllsOnPath), but glibc reads LD_LIBRARY_PATH once at
1717
// process start, so the equivalent trick cannot work after Electron is
1818
// already running. Instead the addon is linked with `-rpath,$ORIGIN` and
19-
// the five ffmpeg sonames are copied next to it, which makes the .node
19+
// the six ffmpeg sonames are copied next to it, which makes the .node
2020
// self-contained wherever it is installed — no env var, no PATH surgery.
2121

2222
import { spawnSync } from "node:child_process";

‎scripts/fetch-ffmpeg.mjs‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,13 +407,32 @@ async function fetchSharedDlls(tag, binDir) {
407407
return;
408408
}
409409

410-
// probe for any previously vendored DLL by name; re-download is driven by
411-
// --force same as the static exe, checked once we know what we'd extract.
412-
const alreadyVendored =
413-
process.platform === "win32" &&
410+
// Completeness probe, not a mere existence probe. The compositor addon now
411+
// links six shared ffmpeg DLLs — see crates/compositor/build.rs
412+
// (avcodec, avformat, avutil, swresample, swscale, avfilter). A warm dev/CI
413+
// tree that already holds the five pre-avfilter DLLs would satisfy an "any
414+
// av*.dll is present" check and let `avfilter-11.dll` go un-vendored, breaking
415+
// require() at runtime (OpenScreen#371 review, EtienneLescot). Require all
416+
// six explicitly so a missing one forces a re-vendor.
417+
const REQUIRED_SHARED_DLLS = [
418+
"avcodec",
419+
"avformat",
420+
"avutil",
421+
"swresample",
422+
"swscale",
423+
"avfilter",
424+
];
425+
const vendoredFiles = new Set(
414426
fs
415427
.readdirSync(binDir, { withFileTypes: true })
416-
.some((e) => e.isFile() && isSharedLib(e.name) && /^(lib)?av/i.test(e.name));
428+
.filter((e) => e.isFile())
429+
.map((e) => e.name),
430+
);
431+
const alreadyVendored =
432+
process.platform === "win32" &&
433+
REQUIRED_SHARED_DLLS.every((lib) =>
434+
[...vendoredFiles].some((f) => new RegExp(`^${lib}-\\d+\\.dll$`).test(f)),
435+
);
417436
// The build-time SDK comes out of this same archive, so a tree that has the
418437
// DLLs but not the SDK must still re-download — otherwise we skip here and
419438
// the compositor build fails afterwards on the missing FFMPEG_DIR.

‎technical-documentation/engineering/build-and-packaging.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ The hook now reads `electron/native/bin/darwin-<arch>/` — the directory `mac.e
204204
| Required | Without it |
205205
|---|---|
206206
| `compositor_view.node` | preview and every export render nothing |
207-
| `libavcodec/libavformat/libavutil.*.dylib` | the addon cannot load at all (dyld error at `require()`) |
207+
| `libavcodec/libavformat/libavutil/libavfilter.*.dylib` | the addon cannot load at all (dyld error at `require()`) |
208208
| `whisper-stt-server` | transcription and captions fail with a developer error shown to end users |
209209
| `libggml*.dylib` | the helper dies in dyld before `main()`; STT times out with no diagnostic |
210210
| `openscreen-screencapturekit-helper` | native screen capture unavailable |

0 commit comments

Comments
 (0)