* fix(video): try metadata extraction and thumbnail generation independently
processUploadedVideo() gated everything behind isValidVideo(), which
rejects the whole video if ffprobe can't read even one of
duration/width/height -- common on some iPhone/Lightroom-exported
MP4s (issue 1370). Callers (photoProcessor.js's processPhoto and
processUploadedPhotos) already catch that throw and fall back to a
static placeholder thumbnail plus a metadata-only retry (codex
review of #845), but that fallback never got a REAL thumbnail even
when generateVideoThumbnail() would have succeeded on its own --
thumbnailing doesn't need valid duration/width/height, it just seeks
and grabs a frame.
processUploadedVideo now tries metadata extraction and thumbnail
generation independently, keeping whichever succeeds instead of
discarding both on a single failed field. The callers' existing
throw handling stays as a backstop.
Also: extractVideoMetadata stored duration as 0 (not null) whenever
ffprobe had no duration field, masking "unknown" as a fake real
zero-second clip and defeating downstream `duration != null` checks
meant to skip an untrustworthy value.
Relates to issue 1370
* fix(video): fall back to the SVG placeholder when thumbnail generation fails
processUploadedVideo could return success with thumbnailKey: null
when only thumbnail generation failed. The gallery grid
(GridGalleryLayout/JustifiedGalleryLayout) falls back to
`photo.thumbnail_url || photo.url` when there's no thumbnail, so
AuthenticatedImage downloaded the full original video and tried to
render it as an <img> -- a broken tile and a potentially huge
fetch just from opening the gallery.
Falls back to the same ffmpeg-free SVG placeholder the callers
already generate for a total processing failure, so a bare
thumbnail-generation failure degrades to that placeholder too,
never to "no thumbnail at all".
Found by codex review.
* fix(video): avoid a SQLite connection deadlock in the placeholder fallback
generateVideoPlaceholder() unconditionally called getThumbnailSettings(),
which queries the database directly (not through any active transaction).
videoProcessor.js's new placeholder fallback can run from inside
processUploadedPhotos' open per-file SQLite transaction (chunked video
upload) -- knex's default SQLite pool has exactly one connection, so
that second, un-transacted query deadlocks against the transaction
holding it, timing out after acquireConnectionTimeout (60s). Reproduced
directly against an isolated SQLite db.
generateVideoPlaceholder now skips the settings lookup entirely when
the caller supplies explicit width/height, and the video fallback
passes the same DEFAULT_THUMBNAIL_WIDTH/HEIGHT the settings lookup
would have fallen back to anyway (now exported for reuse).
Found by codex review.
* fix(video): throw when neither a real thumbnail nor the placeholder can be produced
processUploadedVideo returned success with thumbnailKey: null when
both the real thumbnail AND the SVG placeholder failed -- a total,
systemic failure (storage backend down, disk full), not a quirk of
one file. On stable, which doesn't have the #845 call-site fallback,
this silently completed the video with no thumbnail at all instead
of the retryable 'failed' status a throw here produces. On main,
the pre-existing #845 fallback already absorbed this exact case
(no behavior change there) -- verified against codex's own
git-blame check of the pre-PR stable code before applying this.
Now throws in that case, restoring the pre-existing "let the caller
mark it failed and retryable" behavior for a genuinely unrecoverable
video, while keeping every partial-failure case (the vast majority)
resolving with whatever succeeded.
Found by codex review.
---------
Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
Paul Nothaft
1316ed05b3