paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(uploads): keep videos when thumbnail generation fails (#845)

Browse Source 
* fix(uploads): keep videos when thumbnail generation fails

processUploadedVideo() (ffmpeg probe + thumbnail) was unguarded in both
pipeline paths, while the image branch next to each already survives its
thumbnail failures:

- processUploadedPhotos (sync): the throw failed the whole upload — the
  video was lost.
- processPhoto (async worker, the path real uploads take): the throw
  marked the row 'failed', and the guest gallery only lists 'complete' —
  the video became permanently invisible despite being fully uploaded.

Both call sites now fall back to extractVideoMetadata() alone and keep
the video without a preview; if even the probe fails, the video is kept
with no metadata. Idea from the munin92 fork (2026-07-02), reimplemented
for both paths + regression test.

* fix(uploads): placeholder thumbnail for rescued videos (codex review of #845)

A completed video with a NULL thumbnail made the gallery grid fetch the
ORIGINAL video file as an <img> blob (thumbnail_url || url) — a
potentially multi-GB download for a broken tile. Both fallback paths now
generate the existing sharp-rendered play-button placeholder
(generateVideoPlaceholder — ffmpeg-free), so rescued videos get a real
tile. Test asserts the placeholder key lands in thumbnail_path.
This commit is contained in:
Paul Nothaft
2026-07-19 22:00:08 +02:00
committed by GitHub
parent 8060fedf6a
commit 0310c46fdd
2 changed files with 93 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/__tests__/services/photoProcessor.processPhoto.test.js
+40
View File
@@ -71,6 +71,7 @@ jest.mock('../../src/services/imageProcessor', () => {
const mockExtractCaptureDate = jest.fn();
return {
generateThumbnail: mockGenerateThumbnail,
generateVideoPlaceholder: jest.fn(async (filename) => `thumbnails/thumb_${filename.replace(/\.[^.]+$/, '')}.jpg`),
extractCaptureDate: mockExtractCaptureDate,
withLocalCopy: jest.fn(async (key, fn) =>
fn(`/tmp/local-copy-${require('path').basename(key)}`)
@@ -87,6 +88,7 @@ jest.mock('../../src/services/imageProcessor', () => {
jest.mock('../../src/services/videoProcessor', () => ({
processUploadedVideo: jest.fn(),
extractVideoMetadata: jest.fn(),
isVideoMimeType: (mime) => typeof mime === 'string' && mime.startsWith('video/'),
}));
@@ -212,6 +214,44 @@ describe('photoProcessor.processPhoto', () => {
expect(watermarkService.generateForPhoto).not.toHaveBeenCalled();
});
it('keeps a video complete with a placeholder thumbnail when ffmpeg fails', async () => {
dbModule.__setPhoto({
id: 203,
event_id: 9,
filename: 'drone-clip.mp4',
original_filename: 'drone.mp4',
mime_type: 'video/mp4',
media_type: 'video',
size_bytes: 12345,
captured_at: null,
});
dbModule.__setEvent({ id: 9, slug: 'wedding', event_name: 'Wedding' });
// ffmpeg thumbnail pipeline throws (e.g. unsupported pixel format)…
videoProcessor.processUploadedVideo.mockRejectedValueOnce(new Error('ffmpeg exited with code 1'));
// …but a plain probe still works.
videoProcessor.extractVideoMetadata.mockResolvedValueOnce({
duration: 42,
videoCodec: 'hevc',
audioCodec: 'aac',
width: 3840,
height: 2160,
});
const { processPhoto } = require('../../src/services/photoProcessor');
await processPhoto(203);
const finalUpdate = dbModule.__recorded().updateCalls.pop();
// The row must complete — 'failed' rows are invisible to guests.
expect(finalUpdate.data.processing_status).toBe('complete');
// Placeholder instead of NULL: a completed video without thumbnail would
// make the grid fetch the original video file for the tile (#845 review).
expect(finalUpdate.data.thumbnail_path).toBe('thumbnails/thumb_drone-clip.jpg');
expect(imageProcessor.generateVideoPlaceholder).toHaveBeenCalledWith('drone-clip.mp4');
expect(finalUpdate.data.duration).toBe(42);
expect(finalUpdate.data.video_codec).toBe('hevc');
});
it('throws when the photo row no longer exists', async () => {
dbModule.__setPhoto(null);
dbModule.__setEvent({ id: 1 });
backend/src/services/photoProcessor.js
+53 -13
View File
@@ -1,9 +1,9 @@
const path = require('path');
const fs = require('fs').promises;
const { db } = require('../database/db');
const { generateThumbnail, extractCaptureDate, withLocalCopy, withProcessableImage } = require('./imageProcessor');
const { generateThumbnail, generateVideoPlaceholder, extractCaptureDate, withLocalCopy, withProcessableImage } = require('./imageProcessor');
const { generatePhotoFilename } = require('../utils/filenameSanitizer');
const { processUploadedVideo, isVideoMimeType } = require('./videoProcessor');
const { processUploadedVideo, extractVideoMetadata, isVideoMimeType } = require('./videoProcessor');
const { getStorage } = require('./storage');
const { resolvePhotoStorageKey } = require('./photoResolver');
const logger = require('../utils/logger');
@@ -141,9 +141,27 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ
'thumbnails',
`thumb_${newFilename.replace(/\.[^.]+$/, '.jpg')}`
);
const result = await processUploadedVideo(tempPath, videoThumbnailKey);
videoMetadata = result.metadata;
thumbnailPath = result.thumbnailKey;
// A thumbnail/probe failure must not lose the video: without this
// guard the whole upload errors here, while the image branch below
// already survives its thumbnail failures. Fall back to metadata-only
// plus the static play-button placeholder — a completed video with a
// NULL thumbnail would make the grid fetch the ORIGINAL video file
// as an <img> blob (thumbnail_url || url), i.e. a multi-GB download
// for a broken tile (codex review of #845).
try {
const result = await processUploadedVideo(tempPath, videoThumbnailKey);
videoMetadata = result.metadata;
thumbnailPath = result.thumbnailKey;
} catch (videoErr) {
logger.warn(`Video processing failed for ${file.originalname}, using placeholder thumbnail:`, videoErr.message);
try {
videoMetadata = await extractVideoMetadata(tempPath);
} catch (metaErr) {
logger.warn(`Video metadata extraction also failed for ${file.originalname}:`, metaErr.message);
}
// ffmpeg-free (sharp-rendered SVG); returns null on failure.
thumbnailPath = await generateVideoPlaceholder(newFilename);
}
} else {
// RAW/DNG can't be fed to sharp directly (no raw loader), so extract the
// embedded JPEG preview first and thumbnail/measure THAT. Pass-through
@@ -457,14 +475,36 @@ async function processPhoto(photoId) {
'thumbnails',
`thumb_${photo.filename.replace(/\.[^.]+$/, '.jpg')}`
);
const result = await processUploadedVideo(localPath, videoThumbnailKey);
updateData.thumbnail_path = result.thumbnailKey;
if (result.metadata) {
if (result.metadata.duration != null) updateData.duration = result.metadata.duration;
if (result.metadata.videoCodec) updateData.video_codec = result.metadata.videoCodec;
if (result.metadata.audioCodec) updateData.audio_codec = result.metadata.audioCodec;
if (result.metadata.width) updateData.width = result.metadata.width;
if (result.metadata.height) updateData.height = result.metadata.height;
// A thumbnail/probe failure must not fail the row: processPhoto's caller
// marks failed rows 'failed' and the guest gallery only lists 'complete',
// so the video would become permanently invisible. The image branch below
// already survives its thumbnail failures — mirror that: fall back to
// metadata-only plus the static play-button placeholder. A completed
// video with a NULL thumbnail would make the grid fetch the ORIGINAL
// video file as an <img> blob (thumbnail_url || url) — a multi-GB
// download for a broken tile (codex review of #845).
let videoResult = null;
try {
videoResult = await processUploadedVideo(localPath, videoThumbnailKey);
} catch (videoErr) {
logger.warn(`processPhoto: video processing failed for ${photoId}, using placeholder thumbnail`, { error: videoErr.message });
try {
videoResult = { metadata: await extractVideoMetadata(localPath) };
} catch (metaErr) {
logger.warn(`processPhoto: video metadata extraction also failed for ${photoId}`, { error: metaErr.message });
}
// ffmpeg-free (sharp-rendered SVG); returns null on failure.
const placeholderKey = await generateVideoPlaceholder(photo.filename);
if (placeholderKey) videoResult = { ...(videoResult || {}), thumbnailKey: placeholderKey };
}
if (videoResult?.thumbnailKey) updateData.thumbnail_path = videoResult.thumbnailKey;
if (videoResult?.metadata) {
const m = videoResult.metadata;
if (m.duration != null) updateData.duration = m.duration;
if (m.videoCodec) updateData.video_codec = m.videoCodec;
if (m.audioCodec) updateData.audio_codec = m.audioCodec;
if (m.width) updateData.width = m.width;
if (m.height) updateData.height = m.height;
}
} else {
// RAW/DNG can't be sharp-decoded directly — extract the embedded JPEG