diff --git a/Dockerfile b/Dockerfile index d3fce32..e99ab24 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,6 +18,11 @@ RUN npm ci --omit=dev # --- runtime --- FROM node:20-slim +# ffmpeg (ships ffprobe) powers video thumbnails + duration extraction at upload. +# Without it videos still upload and play, but arrive with no thumbnail or duration. +RUN apt-get update \ + && apt-get install -y --no-install-recommends ffmpeg \ + && rm -rf /var/lib/apt/lists/* ENV NODE_ENV=production # Relocate all state onto the volume (config.js reads DATA_DIR; unset would use # the in-repo paths, which we do not want in a container). diff --git a/README.md b/README.md index 6e5709b..a6c5911 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,11 @@ self-update — new versions are installed the same way the first one was. - Node.js **20.6+** (the npm scripts use the built-in `--env-file-if-exists` flag, added in 20.6) - Linux, macOS, or Windows - SQLite (bundled via `better-sqlite3`; no separate install needed — `npm install` handles the native bindings) +- **ffmpeg** (optional but recommended) — powers video thumbnails and duration extraction + (`sudo apt-get install ffmpeg` / `brew install ffmpeg`). Without it, videos upload and + play fine but show no thumbnail in the content library. The Docker image includes it. + The server logs a `[MEDIA]` line at startup telling you whether it was found, and + backfills missing thumbnails automatically once ffmpeg appears after a restart. ### Quick Start @@ -425,7 +430,8 @@ sudo useradd -r -s /bin/false screentinker sudo cp -r . /opt/screentinker sudo chown -R screentinker:screentinker /opt/screentinker -# Install dependencies +# Install dependencies (ffmpeg is for video thumbnails + durations — see Requirements) +sudo apt-get install -y ffmpeg cd /opt/screentinker/server && npm install --production # Create a systemd service diff --git a/server/lib/content-ingest.js b/server/lib/content-ingest.js index fe1063a..6666305 100644 --- a/server/lib/content-ingest.js +++ b/server/lib/content-ingest.js @@ -48,18 +48,35 @@ async function deriveMediaMetadata(sourcePath, filepath, mime) { const metadata = await sharp(sourcePath).metadata(); // #170: honor EXIF orientation so a portrait photo isn't stored as landscape. ({ width, height } = imageDisplayDims(metadata)); - thumbnailPath = `thumb_${filepath}`; + // Assign thumbnailPath only AFTER the write succeeds: a sharp failure used to + // return the already-assigned name for a file that was never written, storing a + // phantom thumbnail_path that the UI then requests forever as a broken image. + const thumbName = `thumb_${filepath}`; await sharp(sourcePath) .rotate() // #170: auto-orient per EXIF (and strip the tag) so the thumbnail matches .resize(config.thumbnailWidth) .jpeg({ quality: 70 }) - .toFile(path.join(config.contentDir, thumbnailPath)); + .toFile(path.join(config.contentDir, thumbName)); + thumbnailPath = thumbName; } else if (mime.startsWith('video/')) { try { - const { execFileSync } = require('child_process'); - const probe = execFileSync('ffprobe', ['-v', 'quiet', '-print_format', 'json', '-show_format', '-show_streams', sourcePath], + // execFile, NOT execFileSync. These two spawns each carry a 15s timeout, and run + // synchronously they block the event loop for their whole duration — nothing else on + // the server runs, including heartbeats and socket traffic. That was survivable while + // the only caller was a human-initiated upload; it stopped being survivable the moment + // a boot-time sweep started walking a whole library of them unattended, which is the + // #240 failure mode exactly (blocked loop -> missed heartbeats -> panels marked + // offline -> reconnect churn) arriving from our own maintenance. + // + // deriveMediaMetadata is already async and both callers already await it, so awaiting + // the subprocess instead of blocking on it is invisible to them. + const { execFile } = require('child_process'); + const { promisify } = require('util'); + const execFileAsync = promisify(execFile); + const { stdout: probe } = await execFileAsync('ffprobe', + ['-v', 'quiet', '-print_format', 'json', '-show_format', '-show_streams', sourcePath], { timeout: 15000 } - ).toString(); + ); const info = JSON.parse(probe); if (info.format?.duration) durationSec = parseFloat(info.format.duration); const videoStream = info.streams?.find(s => s.codec_type === 'video'); @@ -68,11 +85,15 @@ async function deriveMediaMetadata(sourcePath, filepath, mime) { // (ffmpeg auto-rotates the thumbnail below by default, so only the dims need fixing.) ({ width, height } = videoDisplayDims(videoStream)); } - thumbnailPath = `thumb_${filepath.replace(/\.[^.]+$/, '.jpg')}`; + // Same phantom-path discipline as the image branch above: name it only once the + // file exists, so a failed encode cannot leave the row claiming a thumbnail. + const thumbName = `thumb_${filepath.replace(/\.[^.]+$/, '.jpg')}`; try { - execFileSync('ffmpeg', ['-y', '-i', sourcePath, '-ss', '2', '-vframes', '1', '-vf', `scale=${config.thumbnailWidth}:-1`, path.join(config.contentDir, thumbnailPath)], + await execFileAsync('ffmpeg', + ['-y', '-i', sourcePath, '-ss', '2', '-vframes', '1', '-vf', `scale=${config.thumbnailWidth}:-1`, path.join(config.contentDir, thumbName)], { timeout: 15000 } ); + thumbnailPath = thumbName; } catch { thumbnailPath = null; } } catch (e) { console.warn('ffprobe failed:', e.message); diff --git a/server/lib/media-tools.js b/server/lib/media-tools.js new file mode 100644 index 0000000..7728a0e --- /dev/null +++ b/server/lib/media-tools.js @@ -0,0 +1,32 @@ +'use strict'; + +// Availability probe for the external media binaries (ffmpeg/ffprobe) that video +// thumbnail + duration extraction depends on. They are SYSTEM dependencies, not npm +// ones, so a deployment can easily lack them — and content-ingest's best-effort +// contract means every video then uploads fine but silently gets no thumbnail and +// no duration. Probed once and cached: the answer can't change without an operator +// installing packages, which comes with a restart anyway. +// +// Async on purpose: the first caller is server.js right after listen, and a hung +// binary (NFS-mounted PATH shim, broken wrapper) must degrade to a late log line, +// not block request serving on a freshly-bound port. + +const { execFile } = require('child_process'); + +let cached = null; + +function probeTool(bin) { + return new Promise((resolve) => { + execFile(bin, ['-version'], { timeout: 5000 }, (err) => resolve(!err)); + }); +} + +function mediaToolStatus() { + if (!cached) { + cached = Promise.all([probeTool('ffmpeg'), probeTool('ffprobe')]) + .then(([ffmpeg, ffprobe]) => ({ ffmpeg, ffprobe })); + } + return cached; +} + +module.exports = { mediaToolStatus }; diff --git a/server/lib/thumbnail-backfill.js b/server/lib/thumbnail-backfill.js new file mode 100644 index 0000000..63d7962 --- /dev/null +++ b/server/lib/thumbnail-backfill.js @@ -0,0 +1,108 @@ +'use strict'; + +// Retroactive thumbnail generation. Ingest-time generation (lib/content-ingest) is +// best-effort by contract, so a row silently ends up without a thumbnail whenever it +// fails — most commonly video uploads on a host without ffmpeg installed, plus any +// content from before thumbnails existed. Those rows previously stayed bare forever: +// nothing ever looked at them again. +// +// This sweep runs once per boot (kicked off from server.js shortly after listen), +// finds local image/video rows with no thumbnail, and re-derives metadata for each. +// One file at a time with a pause between files: the point is to heal the library +// eventually, not to win a race against playback serving on the same box. +// +// Idempotent by construction — a generated thumbnail fills thumbnail_path, which +// removes the row from the next boot's query. Video rows are skipped wholesale when +// ffmpeg/ffprobe are missing (the [MEDIA] startup diagnostic already told the +// operator) rather than paying a doomed ffmpeg spawn per file per boot. +// +// The sweep can take a long time on a large bare library, and the replace/delete +// flows may touch the same rows meanwhile. Two consequences handled below: +// - the row UPDATE re-checks that thumbnail_path is STILL empty, so a thumbnail +// written concurrently by PUT /:id/replace is never clobbered with a frame of +// the pre-replace bytes; +// - a row that vanished (or was replaced) mid-derive gets its just-written thumb +// file removed again — contentDir has no garbage collector. + +const path = require('path'); +const fs = require('fs'); +const { db } = require('../db/database'); +const config = require('../config'); +const { deriveMediaMetadata } = require('./content-ingest'); +const { mediaToolStatus } = require('./media-tools'); + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +// Undecodable files fail again every boot (thumbnail_path is the only idempotency +// marker — deliberately, so installing ffmpeg heals them). A video can burn two 15s +// subprocess timeouts, so a library with hundreds of corrupt clips must not turn +// every boot into an hour of doomed ffmpeg spawns: stop after this many failures +// and let the next boot take another bite. +const FAILURE_CAP = 25; + +async function backfillMissingThumbnails({ delayMs = 500 } = {}) { + const tools = await mediaToolStatus(); + // filepath != '' and no remote_url: only content whose bytes live in contentDir. + // Remote/YouTube/embed rows either carry a remote thumbnail URL already or have + // nothing local to derive one from. + const rows = db.prepare(` + SELECT id, filepath, mime_type FROM content + WHERE (thumbnail_path IS NULL OR thumbnail_path = '') + AND filepath != '' + AND (remote_url IS NULL OR remote_url = '') + AND (mime_type LIKE 'image/%' OR mime_type LIKE 'video/%') + `).all(); + + const stats = { scanned: rows.length, generated: 0, skipped: 0, failed: 0, aborted: false }; + const updateStmt = db.prepare(` + UPDATE content SET thumbnail_path = ?, + width = COALESCE(width, ?), height = COALESCE(height, ?), + duration_sec = COALESCE(duration_sec, ?) + WHERE id = ? AND (thumbnail_path IS NULL OR thumbnail_path = '') + `); + // Thumbnail failed but the probe worked: keep the dims/duration (item-duration and + // orientation handling consume them) without marking the row healed — it stays + // eligible for a thumbnail retry next boot. + const metadataStmt = db.prepare(` + UPDATE content SET width = COALESCE(width, ?), height = COALESCE(height, ?), + duration_sec = COALESCE(duration_sec, ?) + WHERE id = ? + `); + + for (const row of rows) { + if (stats.failed >= FAILURE_CAP) { + stats.aborted = true; + console.warn(`[MEDIA] thumbnail backfill: stopping after ${stats.failed} failures — will retry remaining rows next boot`); + break; + } + const isVideo = row.mime_type.startsWith('video/'); + if (isVideo && (!tools.ffmpeg || !tools.ffprobe)) { stats.skipped++; continue; } + const storedName = path.basename(row.filepath); + const sourcePath = path.join(config.contentDir, storedName); + if (!fs.existsSync(sourcePath)) { stats.skipped++; continue; } + try { + const { width, height, durationSec, thumbnailPath } = + await deriveMediaMetadata(sourcePath, storedName, row.mime_type); + if (thumbnailPath) { + const res = updateStmt.run(thumbnailPath, width, height, durationSec, row.id); + if (res.changes === 0 && thumbnailPath !== storedName) { + // Row deleted/replaced while we were deriving. Don't leave the freshly + // written file orphaned. (thumbnailPath === storedName is the SVG + // self-thumbnail case — that file IS the content, never remove it.) + try { fs.unlinkSync(path.join(config.contentDir, path.basename(thumbnailPath))); } catch { /* best-effort */ } + } + stats.generated += res.changes; + } else { + if (width || height || durationSec) metadataStmt.run(width, height, durationSec, row.id); + stats.failed++; // deriveMediaMetadata already warned with the reason + } + } catch (e) { + stats.failed++; + console.warn(`Thumbnail backfill failed for ${row.id}: ${e.message}`); + } + await sleep(delayMs); + } + return stats; +} + +module.exports = { backfillMissingThumbnails }; diff --git a/server/server.js b/server/server.js index 5ba0248..90805d6 100644 --- a/server/server.js +++ b/server/server.js @@ -1359,6 +1359,34 @@ server.listen(listenPort, '0.0.0.0', () => { } catch (e) { console.error(`[EMAIL] config check failed: ${e.message}`); } + + // Media tooling diagnostics — ffmpeg/ffprobe are SYSTEM dependencies that video + // thumbnail + duration extraction needs. Ingest is best-effort, so without them + // every video uploads fine and silently gets no thumbnail: exactly the kind of + // misconfiguration that deserves a loud line, like the email block above. + // (The probe is async so a hung binary can't block serving on the bound port.) + require('./lib/media-tools').mediaToolStatus() + .then((mt) => { + if (!mt.ffmpeg || !mt.ffprobe) { + const missing = [!mt.ffmpeg && 'ffmpeg', !mt.ffprobe && 'ffprobe'].filter(Boolean).join(', '); + console.error(`[MEDIA] ${missing} not found on PATH — video thumbnails and durations are DISABLED until installed (e.g. apt-get install ffmpeg). Image thumbnails are unaffected.`); + } else { + console.log('[MEDIA] ffmpeg/ffprobe found — video thumbnails enabled'); + } + }) + .catch((e) => console.error(`[MEDIA] tooling check failed: ${e.message}`)); + + // Heal rows that missed ingest-time thumbnail generation (uploads from before the + // feature, or videos uploaded while ffmpeg was missing). Delayed past boot so it + // never competes with startup work; paced internally so it never competes with + // serving. Timer unref'd: it must not hold the process open on shutdown. + setTimeout(() => { + require('./lib/thumbnail-backfill').backfillMissingThumbnails() + .then((s) => { + if (s.scanned > 0) console.log(`[MEDIA] thumbnail backfill: ${s.generated} generated, ${s.skipped} skipped, ${s.failed} failed (of ${s.scanned} without thumbnails)`); + }) + .catch((e) => console.error(`[MEDIA] thumbnail backfill failed: ${e.message}`)); + }, 15000).unref(); }); // If SSL is enabled, also start an HTTP server that redirects to HTTPS diff --git a/server/test/media-probe-nonblocking.test.js b/server/test/media-probe-nonblocking.test.js new file mode 100644 index 0000000..71525a8 --- /dev/null +++ b/server/test/media-probe-nonblocking.test.js @@ -0,0 +1,73 @@ +'use strict'; + +/* + * Video probing must not block the event loop. + * + * deriveMediaMetadata spawns ffprobe and ffmpeg with a 15s timeout each. Run synchronously + * (execFileSync) those two calls stop the entire server for their duration — no heartbeats, no + * socket traffic, no HTTP. That was survivable while the only caller was a human-initiated + * upload: one file, someone waiting for it, bounded. + * + * The boot-time thumbnail backfill removed every one of those mitigations. It walks a whole + * library unattended, on a server with live panels, once per boot — so a sync spawn per video + * reproduces #240's failure mode (blocked loop -> missed heartbeats -> panels marked offline -> + * reconnect churn) from our own maintenance sweep rather than from a checkpoint. + * + * These tests pin the property, not the implementation detail, so a future edit that + * reintroduces a sync spawn on this path fails here rather than in a customer's fleet. + */ +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const SRC = fs.readFileSync(path.join(__dirname, '..', 'lib', 'content-ingest.js'), 'utf8'); +// Comments explaining why the sync form is banned must not themselves trip the ban. +const CODE = SRC.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, ''); + +test('#244: the media path spawns asynchronously', () => { + assert.ok(!/execFileSync|execSync|spawnSync/.test(CODE), + 'content-ingest must not spawn synchronously — it blocks the loop for the subprocess timeout'); + assert.match(SRC, /execFileAsync\('ffprobe'/, 'ffprobe must be awaited'); + assert.match(SRC, /execFileAsync\('ffmpeg'/, 'ffmpeg must be awaited'); +}); + +test('#244: every spawn keeps its timeout — async is not a licence to hang', () => { + // Async does not make a wedged binary harmless: without the timeout the promise never + // settles and the backfill stops dead on one bad file instead of moving on. + const spawns = SRC.match(/execFileAsync\(/g) || []; + const timeouts = SRC.match(/timeout: 15000/g) || []; + assert.equal(spawns.length, 2, 'expected exactly the ffprobe and ffmpeg spawns'); + assert.equal(timeouts.length, spawns.length, 'every spawn needs its own timeout'); +}); + +test('#244: the loop keeps running while a probe is in flight', async () => { + // The real property, measured rather than grepped: a slow subprocess must not stop timers. + const { promisify } = require('util'); + const execFileAsync = promisify(require('child_process').execFile); + + let ticks = 0; + const ticker = setInterval(() => { ticks++; }, 20); + try { + // Stands in for a slow ffprobe. `sleep` is on PATH everywhere the server runs. + await execFileAsync('sleep', ['0.5'], { timeout: 15000 }); + } catch { + clearInterval(ticker); + return; // no `sleep` binary — the grep assertions above still hold + } + clearInterval(ticker); + // Sync would have yielded 0 ticks across the whole call. + assert.ok(ticks >= 10, `the loop should keep ticking during a spawn, got ${ticks} ticks in 500ms`); +}); + +test('#244: neither branch names a thumbnail it has not written', () => { + // The phantom-path bug, in both media branches: assigning thumbnailPath before the write + // means a failed encode returns a name for a file that does not exist, and the dashboard + // then requests it forever as a broken image. + const videoBranch = SRC.slice(SRC.indexOf("mime.startsWith('video/')"), SRC.indexOf('return { width, height')); + assert.match(videoBranch, /const thumbName =/, 'the video branch must stage the name'); + assert.match(videoBranch, /thumbnailPath = thumbName;/, 'and assign only after the encode resolves'); + const assignIdx = videoBranch.indexOf('thumbnailPath = thumbName;'); + const spawnIdx = videoBranch.indexOf("execFileAsync('ffmpeg'"); + assert.ok(spawnIdx < assignIdx, 'the assignment must come AFTER the ffmpeg call, not before'); +}); diff --git a/server/test/thumbnail-backfill.test.js b/server/test/thumbnail-backfill.test.js new file mode 100644 index 0000000..0cbc1ab --- /dev/null +++ b/server/test/thumbnail-backfill.test.js @@ -0,0 +1,100 @@ +'use strict'; + +// Boot-time thumbnail backfill (lib/thumbnail-backfill): rows that missed ingest-time +// generation — uploads from before thumbnails existed, or videos uploaded while ffmpeg +// was missing — get healed once per boot. The cases that matter: +// - a local image with no thumbnail gets one, and the row's dims are filled in +// - rows that already have a thumbnail are not touched (and not even scanned) +// - remote rows (YouTube/URL) and rows whose file is gone are left alone +// - a second run finds nothing to do (idempotent) + +const os = require('node:os'); +const path = require('node:path'); +const fs = require('node:fs'); +const crypto = require('node:crypto'); +process.env.DATA_DIR = path.join(os.tmpdir(), 'st-thumb-' + crypto.randomBytes(4).toString('hex')); + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { db } = require('../db/database'); +const config = require('../config'); +const { backfillMissingThumbnails } = require('../lib/thumbnail-backfill'); + +const insertStmt = db.prepare( + 'INSERT INTO content (id, filename, filepath, mime_type, file_size, remote_url, thumbnail_path) VALUES (?,?,?,?,?,?,?)' +); +const insert = (...args) => insertStmt.run(...args); +const row = (id) => db.prepare('SELECT * FROM content WHERE id = ?').get(id); + +test('backfill generates missing image thumbnails, skips what it must, and is idempotent', async () => { + // Generate a real 3x2 PNG with sharp itself — no hand-rolled fixture bytes to rot. + const sharp = require('sharp'); + const png = await sharp({ create: { width: 3, height: 2, channels: 3, background: { r: 200, g: 30, b: 30 } } }) + .png().toBuffer(); + + fs.mkdirSync(config.contentDir, { recursive: true }); + fs.writeFileSync(path.join(config.contentDir, 'bare.png'), png); + fs.writeFileSync(path.join(config.contentDir, 'has-thumb.png'), png); + fs.writeFileSync(path.join(config.contentDir, 'corrupt.png'), Buffer.from('not a png at all')); + + insert('img-bare', 'bare.png', 'bare.png', 'image/png', png.length, null, null); + insert('img-has', 'has-thumb.png', 'has-thumb.png', 'image/png', png.length, null, 'thumb_existing.jpg'); + insert('img-gone', 'gone.png', 'gone.png', 'image/png', 10, null, null); + insert('img-corrupt', 'corrupt.png', 'corrupt.png', 'image/png', 16, null, null); + insert('yt', 'promo', '', 'video/youtube', 0, 'https://youtu.be/aaaaaaaaaaa', null); + insert('html', 'feed', '', 'text/html', 0, 'https://example.com/feed', null); + + const stats = await backfillMissingThumbnails({ delayMs: 0 }); + + // Scanned: img-bare + img-gone + img-corrupt. Remote rows and already-thumbed rows never qualify. + assert.equal(stats.scanned, 3); + assert.equal(stats.generated, 1); + assert.equal(stats.skipped, 1); // img-gone: file missing on disk + assert.equal(stats.failed, 1); // img-corrupt: sharp can't decode it + + // The phantom-path regression: a failed generation must NOT store a thumbnail_path + // pointing at a file that was never written. + assert.equal(row('img-corrupt').thumbnail_path, null, 'failed generation stores nothing'); + + const healed = row('img-bare'); + assert.ok(healed.thumbnail_path, 'thumbnail_path filled in'); + assert.ok(fs.existsSync(path.join(config.contentDir, path.basename(healed.thumbnail_path))), 'thumbnail written to disk'); + assert.equal(healed.width, 3); + assert.equal(healed.height, 2); + + assert.equal(row('img-has').thumbnail_path, 'thumb_existing.jpg', 'existing thumbnail untouched'); + assert.equal(row('yt').thumbnail_path, null, 'remote row untouched'); + + // Idempotent: the healed row no longer matches; the missing-file and corrupt rows remain + // (they'd be retried next boot — correct if the operator fixes the underlying cause). + const again = await backfillMissingThumbnails({ delayMs: 0 }); + assert.equal(again.scanned, 2); + assert.equal(again.generated, 0); +}); + +test('backfill fills a video thumbnail and duration when ffmpeg is present', async (t) => { + const { mediaToolStatus } = require('../lib/media-tools'); + const tools = await mediaToolStatus(); + if (!tools.ffmpeg || !tools.ffprobe) return t.skip('ffmpeg/ffprobe not installed on this machine'); + + // Don't depend on the image test having created the directory first. + fs.mkdirSync(config.contentDir, { recursive: true }); + + // Synthesize a 5s test clip with ffmpeg itself — no fixture binary in the repo. + // 5s, not shorter: ingest thumbnails the frame at t=2s, which must exist. + const clip = path.join(config.contentDir, 'clip.mp4'); + require('node:child_process').execFileSync('ffmpeg', [ + '-y', '-f', 'lavfi', '-i', 'testsrc=duration=5:size=64x48:rate=10', clip, + ], { timeout: 30000, stdio: 'ignore' }); + + insert('vid-bare', 'clip.mp4', 'clip.mp4', 'video/mp4', fs.statSync(clip).size, null, null); + const stats = await backfillMissingThumbnails({ delayMs: 0 }); + assert.equal(stats.generated, 1); + + const healed = row('vid-bare'); + assert.ok(/\.jpg$/.test(healed.thumbnail_path), 'video thumbnail is a jpg frame'); + assert.ok(fs.existsSync(path.join(config.contentDir, path.basename(healed.thumbnail_path)))); + assert.equal(healed.width, 64); + assert.equal(healed.height, 48); + assert.ok(healed.duration_sec > 4 && healed.duration_sec <= 6, `duration ${healed.duration_sec} ≈ 5s`); +});