Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion src/sound-design-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2028,6 +2028,9 @@ export class SoundDesignSession {
this.pause();
const project = this.project;
const frame = frameDuration(project);
this.#reading = new AbortController();
const reading = this.#reading;
this.#readingFrom = performance.now();
this.#store.set({
detect: { ...emptyDetection(), status: 'scanning', sensitivity: this.#store.state.detect.sensitivity },
status: 'reading the video…',
Expand All @@ -2038,6 +2041,7 @@ export class SoundDesignSession {
samples = await analyseMotion(url, {
fps: project.fps,
onProgress: (fraction) => this.#progress('scanning', fraction),
signal: reading.signal,
});
} catch (error) {
this.#store.set({
Expand All @@ -2047,6 +2051,14 @@ export class SoundDesignSession {
return;
}

/*
* Called off part way. What was read is thrown away rather than half
* offered: a pass over the first third of a clip would suggest sounds for
* the first third and say nothing about the rest, which reads as the app
* having found nothing there.
*/
if (reading.signal.aborted) return;

if (!samples.length) {
this.#store.set({ detect: emptyDetection(), status: 'nothing found in that video' });
return;
Expand Down Expand Up @@ -2080,6 +2092,7 @@ export class SoundDesignSession {
detect: {
status: 'ready',
progress: 1,
secondsLeft: null,
samples,
candidates,
peaks,
Expand All @@ -2092,12 +2105,46 @@ export class SoundDesignSession {
});
}

/** When the current read began, so how long is left can be worked out. */
#readingFrom = 0;

#progress(status: 'scanning' | 'pinning', progress: number): void {
const detect = this.#store.state.detect;
if (detect.status !== status) return;
this.#store.set({ detect: { ...detect, progress } });

/*
* How long is left, from the rate so far rather than from a guess.
*
* Nothing is said until a twentieth of the way in, because before that
* the rate is mostly the cost of starting and the answer swings about by
* minutes. Rounded up to whole seconds, since a number that changes
* several times a second is a number nobody can read.
*/
const gone = (performance.now() - this.#readingFrom) / 1000;
const secondsLeft =
progress > 0.05 && gone > 0.5 ? Math.ceil((gone / progress) * (1 - progress)) : null;

this.#store.set({ detect: { ...detect, progress, secondsLeft } });
}

/**
* Stop a read that is going, keeping nothing.
*
* Reading takes about half the length of the clip, so a ten minute video is
* five minutes, and there was no way out of it: the button that starts the
* read is the one that shows the progress, and it was greyed out for the
* duration. Loading the wrong file meant waiting it out.
*/
stopFindingHits(): void {
const { status } = this.#store.state.detect;
if (status !== 'scanning' && status !== 'pinning') return;
this.#reading?.abort();
this.#store.set({ detect: emptyDetection(), status: 'stopped reading the video' });
}

/** How to call off the read that is going, if one is. */
#reading: AbortController | null = null;

/** Show more or fewer of what was already found. Does not read the video. */
setSensitivity(sensitivity: number): void {
const detect = this.#store.state.detect;
Expand Down
11 changes: 11 additions & 0 deletions src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ export interface Detection {
status: 'idle' | 'scanning' | 'pinning' | 'ready';
/** 0 to 1 while working. */
progress: number;
/**
* Roughly how many seconds are left, or null before there is enough to say.
*
* A percentage on its own does not answer the question somebody actually
* has, which is whether to wait or go and do something else. Reading runs at
* a steady rate — about half the length of the clip, every time — so a few
* seconds in there is a real answer available, and it costs nothing to work
* out.
*/
secondsLeft: number | null;
/** Every measurement taken, used for the strip under the ruler. */
samples: MotionSample[];
/** Every moment found, before the sensitivity is applied. */
Expand Down Expand Up @@ -188,6 +198,7 @@ export function emptyDetection(): Detection {
return {
status: 'idle',
progress: 0,
secondsLeft: null,
samples: [],
candidates: [],
peaks: [],
Expand Down
10 changes: 10 additions & 0 deletions src/styles/sound-design.css
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,16 @@
margin-bottom: 6px;
}

/*
* The read button while it is reading.
*
* Marked rather than greyed out, since it is live: it is the way to stop.
*/
.chip.is-working {
border-color: var(--ac-line);
color: var(--ac);
}

/*
* Asking something, over the top of the work.
*
Expand Down
48 changes: 40 additions & 8 deletions src/ui/sound-design/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,47 @@ export function createTimeline(
}, [content]);

// ---------- finding hits ----------
/**
* One button for starting the read and for calling it off.
*
* It used to grey itself out for the duration and show a percentage, which
* is the worst of both: a read takes about half the length of the clip, so
* a ten minute video is five minutes with no way out of it and no way to
* know how much of that was left. The same button is the way out, because
* that is where somebody is already looking.
*/
const findButton = button(
{
class: 'chip chip--sm',
title: 'Read the video and suggest where sounds belong',
on: { click: () => void session.findHits() },
on: {
click: () => {
const { status } = session.store.state.detect;
if (status === 'scanning' || status === 'pinning') session.stopFindingHits();
else void session.findHits();
},
},
},
['Find hits'],
);

/**
* What the button says while it works.
*
* How long is left rather than how far through, because that is the
* question somebody actually has: whether to wait or go and do something
* else. A percentage answers a different one. It falls back to the
* percentage for the first fraction of a second, before there is a rate to
* work an answer out from, and for the pinning pass, which is short.
*/
function readingLabel(detect: AppState['detect']): string {
const what = detect.status === 'scanning' ? 'Reading' : 'Pinning';
const left = detect.secondsLeft;
if (left === null) return `${what} ${Math.round(detect.progress * 100)}%`;
if (left >= 90) return `${what} · ${Math.round(left / 60)} min left`;
return `${what} · ${left}s left`;
}

const sensitivity = el('input', {
class: 'range strip__sensitivity',
type: 'range',
Expand Down Expand Up @@ -1907,13 +1939,13 @@ export function createTimeline(

const { detect } = state;
const working = detect.status === 'scanning' || detect.status === 'pinning';
findButton.disabled = working || !state.videoReady;
setText(
findButton,
working
? `${detect.status === 'scanning' ? 'Reading' : 'Pinning'} ${Math.round(detect.progress * 100)}%`
: 'Find hits',
);
// Live while it works, because it is the way to stop it.
findButton.disabled = !working && !state.videoReady;
toggleClass(findButton, 'is-working', working);
findButton.title = working
? 'Stop reading. Nothing is kept.'
: 'Read the video and suggest where sounds belong';
setText(findButton, working ? readingLabel(detect) : 'Find hits');

const ready = detect.status === 'ready';
detectGroup.classList.toggle('is-ready', ready);
Expand Down
33 changes: 32 additions & 1 deletion src/video/analyse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ export interface AnalyseOptions {
onProgress?(fraction: number): void;
/** Frames per second, used to decide how finely to sample. */
fps: number;
/**
* Give up early, for somebody who did not mean to start.
*
* Reading takes about half the length of the clip, so a ten minute video is
* five minutes of waiting, and there was no way out of it: the button that
* starts the read is the one that shows the progress, and it was disabled
* for the duration. Loading the wrong file meant waiting it out or
* reloading the page.
*/
signal?: AbortSignal;
}

/** Width the picture is reduced to before comparing. Height follows the shape. */
Expand Down Expand Up @@ -80,6 +90,27 @@ export async function analyseMotion(
stall = window.setTimeout(() => resolve(), STALL_MS);
};

/*
* Stopping is resolving rather than throwing.
*
* What has been read so far is real, and the caller decides what to do
* with it. An error here would make "I have changed my mind" arrive at
* the same place as "this file cannot be read", which are not the same
* thing to say to somebody.
*/
if (options.signal) {
if (options.signal.aborted) {
window.clearTimeout(stall);
resolve();
return;
}
options.signal.addEventListener('abort', () => {
window.clearTimeout(stall);
video.pause();
resolve();
}, { once: true });
}

const onFrame: VideoFrameRequestCallback = (_now, metadata) => {
bump();
ctx.drawImage(video, 0, 0, SAMPLE_WIDTH, height);
Expand All @@ -89,7 +120,7 @@ export async function analyseMotion(
}
previous = frame;
options.onProgress?.(duration ? Math.min(1, metadata.mediaTime / duration) : 0);
if (!video.ended) video.requestVideoFrameCallback(onFrame);
if (!video.ended && !options.signal?.aborted) video.requestVideoFrameCallback(onFrame);
};

video.addEventListener('ended', () => {
Expand Down
109 changes: 109 additions & 0 deletions test/browser/scan.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { expect, test, type Page } from '@playwright/test';
import { loadClip, open } from './app.ts';

/**
* Waiting for the video to be read, which is the longest wait in the app.
*
* Reading runs at about half the length of the clip — measured at 0.53 to
* 0.58 times realtime across several clips — so a three minute video is a
* minute and a half and a ten minute one is five. That rate is not a
* carelessness: sampling half as often halves the wait and finds nine fewer
* of forty moments on a dense clip, which was measured before touching it and
* is why the rate is left alone.
*
* What was wrong was everything around the wait. The button that starts the
* read showed a percentage and greyed itself out, so there was no way to know
* how much longer and no way to stop — loading the wrong file meant waiting it
* out or reloading the page.
*/

const findButton = (page: Page) => page.locator('.tl__detect button').first();

test.describe('reading the video', () => {
/*
* Longer than the default, because these wait for real work.
*
* Reading a twenty second clip is about eleven seconds on top of making and
* loading it, and two of these read it twice.
*/
test.describe.configure({ timeout: 180_000 });

test.beforeEach(async ({ page }) => {
await open(page);
await loadClip(page, { seconds: 20 });
});

/*
* How long is left, not how far through.
*
* A percentage answers a question nobody has. The one somebody does have is
* whether to wait or go and do something else.
*/
test('says how long is left', async ({ page }) => {
await findButton(page).click();

await expect
.poll(async () => (await findButton(page).innerText()).trim(), {
message: 'it started saying how long is left',
})
.toMatch(/Reading · \d+s left/);

// And it counts down rather than sitting on one number.
const first = Number((await findButton(page).innerText()).match(/(\d+)s/)![1]);
await expect
.poll(async () => {
const now = (await findButton(page).innerText()).match(/(\d+)s/);
return now ? Number(now[1]) : 0;
})
.toBeLessThan(first);
});

/* The way out, which is the button already under the pointer. */
test('can be stopped, and keeps nothing', async ({ page }) => {
await findButton(page).click();
await expect(findButton(page), 'live while it works, because it is the way out')
.toBeEnabled();
await expect.poll(async () => (await findButton(page).innerText()).trim()).toContain('Reading');

await findButton(page).click();
await expect(findButton(page)).toHaveText('Find hits');
await expect(page.locator('.tl__status')).toContainText('stopped reading');
// Nothing half-offered: a pass over part of a clip would suggest sounds
// for that part and say nothing about the rest, which reads as the app
// having found nothing there.
await expect(page.locator('.dock--right .dock__body')).toContainText('Nothing found yet');

/*
* And it stays stopped.
*
* Resetting what is on screen is not stopping. A read that was only
* forgotten about carries on in the background and hands its results in
* half a minute later, so moments appear out of nowhere long after
* somebody pressed stop — which is worse than not having stopped at all.
* Waited out past the point the whole clip would have been read.
*/
await page.waitForTimeout(15_000);
await expect(findButton(page), 'nothing came back').toHaveText('Find hits');
await expect(page.locator('.dock--right .dock__body')).toContainText('Nothing found yet');
await expect(page.locator('.tl__status')).not.toContainText('moments found');
});

test('and can be started again afterwards', async ({ page }) => {
await findButton(page).click();
await expect.poll(async () => (await findButton(page).innerText()).trim()).toContain('Reading');
await findButton(page).click();
await expect(findButton(page)).toHaveText('Find hits');

await findButton(page).click();
await expect(findButton(page), 'it read the whole clip the second time')
.toHaveText('Find hits', { timeout: 120_000 });
await expect(page.locator('.tl__status')).toContainText('moments found');
});

test('finds the moments when it is left to finish', async ({ page }) => {
await findButton(page).click();
await expect(findButton(page)).toHaveText('Find hits', { timeout: 120_000 });
await expect(page.locator('.tl__status')).toContainText('moments found');
await expect(page.locator('.dock--right .dock__body')).toContainText('waiting on you');
});
});
Loading