fabi/EventSnap
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(video): extract a real poster frame, and stop claiming one that isn't there

Browse Source 
Both the compression worker and the HTML export ran the same invocation:

    ffmpeg -i <src> -vframes 1 -ss 00:00:01 -vf scale=… -y <out>

`-ss` AFTER `-i` is an output-side seek. Against a clip of a second or less ffmpeg
exits 0 and writes nothing, and both call sites gated on the exit status:

- the worker wrote `thumbnail_path` and logged "thumbnail generated" for a file that
  was never created, so GET /upload/{id}/thumbnail 404s in the live feed;
- the export listed media/<id>_thumb.jpg in data.json while the ZIP writer skipped the
  unopenable file, so the keepsake drew a broken image tile.

Any clip at or under a second, which phones produce constantly — mis-taps, Live
Photos, boomerangs. Not data loss; the .mp4 is in both archives and plays. Every
server-side signal stayed green.

New `services/video.rs` owns the extraction for both callers, mirroring the imaging.rs
precedent (created for the same duplication, and it paid off when the max_alloc fix
landed in both workers at once). Three changes in it:

- `-ss` before `-i`, an input-side seek. NOT sufficient alone: verified against the
  production image, seeking to 1s in a 1.000s clip is still past the last frame and
  still exits 0 with no file. The 0s fallback is what actually fixes this, and 1s is
  tried first only because an opening frame makes a poor poster.
- Verify the artifact, not the exit status. This is the check both sites were missing.
- Carry compression.rs's 120s timeout. export.rs had NONE — a hung ffmpeg there would
  strand the job at `running` and the keepsake would never complete.

The worker's call used `?`. Tightening the check without also making a missing poster
non-fatal would have been far worse than the bug: every sub-second clip would fail
compression, exhaust its retries and be soft-deleted. It now logs a warning and leaves
`thumbnail_path` NULL, which FeedListCard, VirtualFeed and LightboxModal already
handle.

The export now sets `thumb: ""` and skips the manifest entry when there is no poster —
and does the same for the IMAGE branch, whose decode failure left the identical
dangling reference. No viewer change was needed: +page.svelte already guards
`{#if post.media.thumb}` and falls back to a video tile with a play glyph. The comment
claiming "viewer handles missing thumbs gracefully" was true about the viewer and false
about what the backend sent — the guard never fired because the string was never empty.

e2e/specs/06-export/export-video.spec.ts had DOCUMENTED this as intended behaviour
("the fixture clip is <1s, so ffmpeg extracts no thumbnail frame — but exits 0 …
that's the intended shape here"). sample.mp4 is exactly 1.000s, so every video test in
the suite ran at that boundary and none ever fetched the poster. That comment is now
corrected to say what it actually was.

Tests: 2 unit; a new sample-5s.mp4 fixture so the ordinary first-seek path is covered
at all; video-playback now FETCHES the poster rather than asserting the attribute (the
one extra request that nine rounds of green never made); a new spec covering both
fixtures plus the mirror that a posterless video still uploads and plays; and a
keepsake spec asserting every <img> in the opened viewer resolves — naturalWidth === 0
is exactly the broken-tile case, whatever produced it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
fabi
2026-07-30 19:40:27 +02:00
parent 2b313e67e0
commit 402215d405
8 changed files with 447 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
backend/src/services/export.rs
View File

@@ -785,38 +785,32 @@ async fn run_html_export_inner(
let full_ext = ext_from_path(&row.original_path);
let full = format!("{id_str}.{full_ext}");
// Video thumbnail via ffmpeg
// Poster frame via the shared helper, which owns the seek order, the 120s timeout
// (this call site had NONE — a hung ffmpeg would strand the export at `running`
// forever) and the artifact check.
let thumb_path = media_tmp.join(&thumb);
let ffmpeg_result = tokio::process::Command::new("ffmpeg")
.args([
"-i",
src.to_str().unwrap_or_default(),
"-vframes",
"1",
"-ss",
"00:00:01",
"-vf",
"scale=400:-1",
"-y",
thumb_path.to_str().unwrap_or_default(),
])
.output()
.await;
match ffmpeg_result {
Ok(output) if output.status.success() => {}
_ => {
tracing::warn!(
"ffmpeg thumbnail failed for upload {}, skipping thumb",
row.id
);
// Missing thumb entry — viewer handles missing thumbs gracefully.
}
let produced =
match crate::services::video::extract_poster_frame(&src, &thumb_path, 400).await {
Ok(produced) => produced,
Err(e) => {
tracing::warn!("poster extraction errored for upload {}: {e:#}", row.id);
false
}
};
if !produced {
tracing::info!(
upload_id = %row.id,
"no poster frame for this video; exporting it without one"
);
}
// Stream the video full-res straight from the original at ZIP time — no
// copy to temp (that used to transiently double disk usage per video).
(thumb, full, MediaSource::Original(src.clone()))
(
produced.then(|| thumb.clone()),
full,
MediaSource::Original(src.clone()),
)
} else {
let thumb = format!("{id_str}_thumb.jpg");
let ext = ext_from_path(&row.original_path);
@@ -842,9 +836,17 @@ async fn run_html_export_inner(
})
.await?;
if let Err(e) = thumb_result {
tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
}
// Same dangling-reference hazard as the video branch: a failure here left `thumb`
// pointing at a file the ZIP writer would then skip, so `data.json` advertised an
// entry the archive didn't contain. An undecodable image is rarer than a sub-second
// clip, but the broken tile is identical.
let thumb_ok = match thumb_result {
Ok(()) => true,
Err(e) => {
tracing::warn!("thumbnail generation failed for upload {}: {e:#}", row.id);
false
}
};
// Full variant: compress to temp if >5MB, otherwise stream the original
// as-is (no temp copy). `src_meta` was stat'd once at the top of the loop.
@@ -882,15 +884,16 @@ async fn run_html_export_inner(
MediaSource::Original(src.clone())
};
(thumb, full, full_source)
(thumb_ok.then_some(thumb), full, full_source)
};
// Register this post's two media entries. Thumbnails always come from temp
// (they're freshly generated); the full variant's source was decided above.
media_manifest.push((
thumb_name.clone(),
MediaSource::Temp(media_tmp.join(&thumb_name)),
));
// Register this post's media entries. The thumbnail is registered ONLY when one was
// actually produced: pushing a manifest entry for a file that doesn't exist made the ZIP
// writer skip it silently while `data.json` still advertised it — the viewer then drew a
// broken image tile for an entry the archive never contained.
if let Some(name) = &thumb_name {
media_manifest.push((name.clone(), MediaSource::Temp(media_tmp.join(name))));
}
media_manifest.push((full_name.clone(), full_source));
// Build comments for this upload
@@ -925,7 +928,15 @@ async fn run_html_export_inner(
} else {
"image".to_string()
},
thumb: format!("media/{thumb_name}"),
// Empty when there is no poster. The viewer already guards on this
// (`{#if post.media.thumb}` → a video tile with a play glyph, or the placeholder
// icon for an image), so telling it the truth is the entire fix — no schema
// change, no viewer rebuild. What was broken was the backend always claiming a
// thumbnail existed.
thumb: thumb_name
.as_ref()
.map(|n| format!("media/{n}"))
.unwrap_or_default(),
full: format!("media/{full_name}"),
},
});