fix(diashow): never project a video, however its poster turned out
`merge` decided by whether a usable still existed, so the same clip was included or dropped depending on whether ffmpeg happened to extract a poster — a video WITH a thumbnail was queued and shown as a frozen frame, one without was skipped. Keyed on the mime type instead: the projector shows stills only. The test factory casts through `as unknown as FeedUpload`, so adding a field the queue reads does not fail typechecking — it fails at runtime, which is how this surfaced as ten broken tests rather than a compile error. The factory now carries `mime_type` and the comment says why keeping it in step matters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,16 +2,20 @@ import { describe, it, expect } from 'vitest';
|
|||||||
import { SlideQueue } from './queue';
|
import { SlideQueue } from './queue';
|
||||||
import type { FeedUpload } from '$lib/types';
|
import type { FeedUpload } from '$lib/types';
|
||||||
|
|
||||||
// Minimal FeedUpload factory — the queue only reads id, user_id, preview_url, thumbnail_url.
|
// Minimal FeedUpload factory — the queue reads id, user_id, preview_url, thumbnail_url and
|
||||||
|
// mime_type. Note the `as unknown as FeedUpload` cast below: it silences the missing fields,
|
||||||
|
// so adding a field the queue reads does NOT fail typechecking here — it fails at runtime.
|
||||||
|
// Keep this factory in step with what `merge` actually touches.
|
||||||
const up = (
|
const up = (
|
||||||
id: string,
|
id: string,
|
||||||
opts: { user?: string; preview?: boolean; thumb?: boolean } = {}
|
opts: { user?: string; preview?: boolean; thumb?: boolean; video?: boolean } = {}
|
||||||
): FeedUpload =>
|
): FeedUpload =>
|
||||||
({
|
({
|
||||||
id,
|
id,
|
||||||
user_id: opts.user ?? 'u1',
|
user_id: opts.user ?? 'u1',
|
||||||
preview_url: opts.preview === false ? null : `/p/${id}`,
|
preview_url: opts.preview === false ? null : `/p/${id}`,
|
||||||
thumbnail_url: opts.thumb ? `/t/${id}` : null
|
thumbnail_url: opts.thumb ? `/t/${id}` : null,
|
||||||
|
mime_type: opts.video ? 'video/mp4' : 'image/jpeg'
|
||||||
}) as unknown as FeedUpload;
|
}) as unknown as FeedUpload;
|
||||||
|
|
||||||
const drain = (q: SlideQueue, n: number): string[] => {
|
const drain = (q: SlideQueue, n: number): string[] => {
|
||||||
@@ -32,6 +36,19 @@ describe('SlideQueue.merge', () => {
|
|||||||
expect(q.stats().known).toBe(3);
|
expect(q.stats().known).toBe(3);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('never queues a video, even when it has a usable poster frame', () => {
|
||||||
|
const q = new SlideQueue();
|
||||||
|
// The projector shows stills only. Before this was keyed on the mime type it depended
|
||||||
|
// on whether ffmpeg happened to extract a poster: a video WITH a thumbnail was shown,
|
||||||
|
// one without was dropped — the same clip in or out by luck.
|
||||||
|
const added = q.merge([up('photo'), up('clip', { video: true, thumb: true })], {
|
||||||
|
live: false
|
||||||
|
});
|
||||||
|
expect(added).toBe(1);
|
||||||
|
expect(q.has('clip')).toBe(false);
|
||||||
|
expect(q.has('photo')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('skips uploads with no preview or thumbnail (still compressing)', () => {
|
it('skips uploads with no preview or thumbnail (still compressing)', () => {
|
||||||
const q = new SlideQueue();
|
const q = new SlideQueue();
|
||||||
const added = q.merge([up('a'), up('pending', { preview: false }), up('t', { thumb: true })], {
|
const added = q.merge([up('a'), up('pending', { preview: false }), up('t', { thumb: true })], {
|
||||||
|
|||||||
@@ -26,14 +26,25 @@ export class SlideQueue {
|
|||||||
private recentlyShown: string[] = [];
|
private recentlyShown: string[] = [];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Add uploads, de-duplicated by id. Only *displayable* items (preview- or
|
* Add uploads, de-duplicated by id. `live` items jump onto the live queue (shown next);
|
||||||
* thumbnail-ready) are kept — a still-compressing upload has no image yet, so it's
|
* otherwise they join the shuffle pool. Returns the count newly added.
|
||||||
* skipped until a later sync brings it with a preview. `live` items jump onto the live
|
*
|
||||||
* queue (shown next); otherwise they join the shuffle pool. Returns the count newly added.
|
* Two things are skipped:
|
||||||
|
*
|
||||||
|
* 1. **Videos, deliberately and by mime type.** The projector shows stills only — a clip
|
||||||
|
* would either hold the slide for its full duration or be cut off mid-way, and it has
|
||||||
|
* no audio path on a room's screen. This used to happen only ACCIDENTALLY, as a side
|
||||||
|
* effect of the derivative check below: a video whose poster frame extracted fine has
|
||||||
|
* a `thumbnail_url` and so was shown as a still frame, while one whose extraction
|
||||||
|
* failed was dropped — the same upload included or excluded depending on whether
|
||||||
|
* ffmpeg happened to find a frame. Now the rule is the mime type, so it is consistent.
|
||||||
|
* 2. **Images with no derivative yet.** A still-compressing photo has no image to show;
|
||||||
|
* it is picked up by a later sync once its preview exists.
|
||||||
*/
|
*/
|
||||||
merge(uploads: FeedUpload[], opts: { live: boolean }): number {
|
merge(uploads: FeedUpload[], opts: { live: boolean }): number {
|
||||||
let added = 0;
|
let added = 0;
|
||||||
for (const u of uploads) {
|
for (const u of uploads) {
|
||||||
|
if (u.mime_type.startsWith('video/')) continue;
|
||||||
if (!u.preview_url && !u.thumbnail_url) continue;
|
if (!u.preview_url && !u.thumbnail_url) continue;
|
||||||
if (this.allKnown.has(u.id)) continue;
|
if (this.allKnown.has(u.id)) continue;
|
||||||
this.allKnown.set(u.id, u);
|
this.allKnown.set(u.id, u);
|
||||||
|
|||||||
@@ -205,6 +205,7 @@
|
|||||||
// Ids known BEFORE the scan — only these are eviction candidates, so an upload that
|
// Ids known BEFORE the scan — only these are eviction candidates, so an upload that
|
||||||
// arrives mid-scan (not in this snapshot) is never wrongly pruned.
|
// arrives mid-scan (not in this snapshot) is never wrongly pruned.
|
||||||
const before = new Set(queue.knownIds());
|
const before = new Set(queue.knownIds());
|
||||||
|
// eslint-disable-next-line svelte/prefer-svelte-reactivity -- local scan-bookkeeping set inside one reconcile() call; never stored in $state, so no reactivity is involved.
|
||||||
const seen = new Set<string>();
|
const seen = new Set<string>();
|
||||||
let cursor: string | null = null;
|
let cursor: string | null = null;
|
||||||
let complete = false;
|
let complete = false;
|
||||||
|
|||||||
Reference in New Issue
Block a user