Skip to content

Commit b933614

Browse files
committed
fix(core): clips that meet within float rounding count as meeting in edits and playback
1 parent 162de1a commit b933614

16 files changed

Lines changed: 387 additions & 33 deletions

File tree

‎packages/cli/src/timeline/a2Shared.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
} from "@hyperframes/studio-server";
77
import type { AppliedFileMutation, PatchOperation } from "@hyperframes/studio-server";
88
import { fpsToNumber, parseFpsWithDefault } from "@hyperframes/core";
9+
import { isInsideSpan, sameInstant, spansShareTime } from "@hyperframes/core/clip-facts";
910
import { readCompositionFps } from "../utils/compositionFps.js";
1011
import { readFileSync } from "node:fs";
1112
import type { ProjectTimeline, TimelineRow } from "./describeProject.js";
@@ -147,7 +148,7 @@ function overlap(
147148
candidate !== row &&
148149
candidate.file === row.file &&
149150
candidate.trackIndex === row.trackIndex &&
150-
Math.max(start, candidate.start) < Math.min(end, candidate.end),
151+
spansShareTime(start, end, candidate.start, candidate.end),
151152
);
152153
}
153154

@@ -263,8 +264,9 @@ function finishTrim(
263264
): MutationDecision | { ok: true; nextStart: number; nextDuration: number } {
264265
if (end && !end.ok) return end;
265266
if (duration && !duration.ok) return duration;
266-
const nextDuration = duration?.seconds ?? (end ? end.seconds - nextStart : context.row.duration);
267-
if (nextDuration <= 0) {
267+
const nextDuration =
268+
duration?.seconds ?? (end ? durationUntil(nextStart, end.seconds) : context.row.duration);
269+
if (nextDuration <= 0 || sameInstant(nextStart, nextStart + nextDuration)) {
268270
return {
269271
ok: false,
270272
reason: "trim duration must be positive",
@@ -279,6 +281,19 @@ function trimStart(context: MutationContext, expression: string | undefined) {
279281
return parseMutationTime(context, expression, "pass a valid time expression");
280282
}
281283

284+
function durationUntil(start: number, end: number): number {
285+
let duration = end - start;
286+
while (duration > 0 && start + duration > end) duration = nextSmaller(duration);
287+
return duration;
288+
}
289+
290+
const float = new DataView(new ArrayBuffer(8));
291+
function nextSmaller(positive: number): number {
292+
float.setFloat64(0, positive);
293+
float.setBigUint64(0, float.getBigUint64(0) - 1n);
294+
return float.getFloat64(0);
295+
}
296+
282297
function trimEnd(context: MutationContext, expression: string | undefined) {
283298
if (!expression) return undefined;
284299
return parseMutationTime(context, expression, "pass a valid time expression");
@@ -413,8 +428,7 @@ export function mutationConflict(
413428
(candidate) =>
414429
candidate.file === row.file &&
415430
candidate.trackIndex === row.trackIndex &&
416-
candidate.start < nextStart &&
417-
nextStart < candidate.end,
431+
isInsideSpan(nextStart, candidate.start, candidate.end),
418432
);
419433
if (!conflict) return null;
420434
return {

‎packages/cli/src/timeline/timeline.e2e.test.ts‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join, resolve } from "node:path";
55
import { fileURLToPath } from "node:url";
6+
import { isClipVisibleAt } from "@hyperframes/core";
67
import { describe, expect, it } from "vitest";
78

89
const cliEntry = resolve(fileURLToPath(import.meta.url), "..", "..", "cli.ts");
@@ -102,6 +103,135 @@ describe("timeline edit command", () => {
102103
}
103104
});
104105

106+
const meeting = (aStart: string, aDuration: string, bStart: string, fps = "") =>
107+
`<div data-composition-id="main"${fps} data-duration="40"><div id="a" data-hf-id="a" data-start="${aStart}" data-duration="${aDuration}" data-track-index="0"></div><div id="b" data-hf-id="b" data-start="${bStart}" data-duration="2" data-track-index="0"></div></div>`;
108+
const clip = (html: string, id: string) => {
109+
const tag = new RegExp(`<div[^>]*\\sid="${id}"[^>]*>`).exec(html)?.[0] ?? "";
110+
const attr = (name: string) => new RegExp(`${name}="([^"]+)"`).exec(tag)?.[1];
111+
return { start: attr("data-start"), duration: attr("data-duration") };
112+
};
113+
const endOf = (html: string, id: string) =>
114+
Number(clip(html, id).start) + Number(clip(html, id).duration);
115+
const visibleAt = (html: string, time: number, ids: string[]) =>
116+
ids.filter((id) => isClipVisibleAt(time, Number(clip(html, id).start), endOf(html, id), 40));
117+
118+
const composition = (fps: string, ...clips: [string, string, string][]) =>
119+
`<div data-composition-id="main"${fps} data-duration="40">${clips
120+
.map(
121+
([id, start, duration]) =>
122+
`<div id="${id}" data-hf-id="${id}" data-start="${start}" data-duration="${duration}" data-track-index="0"></div>`,
123+
)
124+
.join("")}</div>`;
125+
126+
it.each([
127+
[
128+
"move",
129+
["#b", "20f"],
130+
composition(' data-fps="24"', ["a", "0", String(20 / 24 + 5e-7)], ["b", "2", "0.25"]),
131+
],
132+
["duplicate", ["#c", "--at", "1"], composition("", ["a", "0", "1.0000005"], ["c", "3", "1"])],
133+
])("refuses a %s that lands half a microsecond inside another clip", (verb, args, html) => {
134+
const dir = project();
135+
try {
136+
writeFileSync(join(dir, "index.html"), html);
137+
expect(run(dir, verb, ...args).status).not.toBe(0);
138+
expect(readFileSync(join(dir, "index.html"), "utf8")).toBe(html);
139+
} finally {
140+
rmSync(dir, { recursive: true, force: true });
141+
}
142+
});
143+
144+
it.each([
145+
[
146+
"a clip wholly before the insertion point",
147+
["#x", "--at", "5"],
148+
composition("", ["x", "0", "1"], ["t", "4.9999995", "0.0000002"]),
149+
4.9999995,
150+
],
151+
[
152+
"a clip half a microsecond after it",
153+
["#a"],
154+
composition("", ["a", "0", "1"], ["t", "1.0000005", "1"]),
155+
1.0000005 + 1,
156+
],
157+
])("duplicates without snapping %s to the copy", (_, args, html, tStart) => {
158+
const dir = project();
159+
try {
160+
writeFileSync(join(dir, "index.html"), html);
161+
expect(run(dir, "duplicate", ...args).status).toBe(0);
162+
expect(Number(clip(readFileSync(join(dir, "index.html"), "utf8"), "t").start)).toBe(tStart);
163+
} finally {
164+
rmSync(dir, { recursive: true, force: true });
165+
}
166+
});
167+
168+
it("trims a clip that only meets the one before it, ending where asked", () => {
169+
const dir = project();
170+
try {
171+
writeFileSync(join(dir, "index.html"), meeting("19.8", "6.4", "26.2"));
172+
expect(run(dir, "trim", "#b", "--end", "29").status).toBe(0);
173+
const html = readFileSync(join(dir, "index.html"), "utf8");
174+
expect(clip(html, "b")).toEqual({ start: "26.2", duration: "2.8000000000000007" });
175+
expect(endOf(html, "b")).toBe(29);
176+
} finally {
177+
rmSync(dir, { recursive: true, force: true });
178+
}
179+
});
180+
181+
it("trims up to the next clip's start without ending past it, where no duration lands exactly", () => {
182+
const dir = project();
183+
try {
184+
writeFileSync(join(dir, "index.html"), meeting("4.74", "10", "25.74"));
185+
expect(run(dir, "trim", "#a", "--end", "25.74").status).toBe(0);
186+
const html = readFileSync(join(dir, "index.html"), "utf8");
187+
expect(endOf(html, "a")).toBeLessThanOrEqual(25.74);
188+
expect(visibleAt(html, 25.74, ["a", "b"])).toEqual(["b"]);
189+
} finally {
190+
rmSync(dir, { recursive: true, force: true });
191+
}
192+
});
193+
194+
it("trims to frame 20 at 30 fps so that frame shows the next clip and not this one", () => {
195+
const dir = project();
196+
try {
197+
writeFileSync(join(dir, "index.html"), meeting("0", "2", "2", ' data-fps="30"'));
198+
expect(run(dir, "trim", "#a", "--end", "20f").status).toBe(0);
199+
expect(run(dir, "trim", "#b", "--start", "20f").status).toBe(0);
200+
const html = readFileSync(join(dir, "index.html"), "utf8");
201+
expect([clip(html, "a").duration, clip(html, "b").start]).toEqual([
202+
"0.6666666666666666",
203+
"0.6666666666666666",
204+
]);
205+
expect(visibleAt(html, 19 / 30, ["a", "b"])).toEqual(["a"]);
206+
expect(visibleAt(html, 20 / 30, ["a", "b"])).toEqual(["b"]);
207+
} finally {
208+
rmSync(dir, { recursive: true, force: true });
209+
}
210+
});
211+
212+
it.each([
213+
["19.8", "6.4", "26.2"],
214+
["0", "0.6666666666666666", "0.6666666666666666"],
215+
["0.1", "1.1", "1.2"],
216+
])(
217+
"duplicates a clip at %s lasting %s up against the clip at %s, each boundary exact",
218+
(aStart, aDuration, bStart) => {
219+
const dir = project();
220+
try {
221+
writeFileSync(join(dir, "index.html"), meeting(aStart, aDuration, bStart));
222+
expect(run(dir, "duplicate", "#a").status).toBe(0);
223+
const html = readFileSync(join(dir, "index.html"), "utf8");
224+
const ids = ["a", "a-copy", "b"];
225+
expect(Number(clip(html, "a-copy").start)).toBe(endOf(html, "a"));
226+
expect(Number(clip(html, "b").start)).toBe(endOf(html, "a-copy"));
227+
expect(visibleAt(html, endOf(html, "a"), ids)).toEqual(["a-copy"]);
228+
expect(visibleAt(html, endOf(html, "a-copy"), ids)).toEqual(["b"]);
229+
} finally {
230+
rmSync(dir, { recursive: true, force: true });
231+
}
232+
},
233+
);
234+
105235
it("refuses an ambiguous reference", () => {
106236
const dir = project();
107237
try {
@@ -257,6 +387,36 @@ describe("timeline edit command", () => {
257387
}
258388
});
259389

390+
it("refuses a trim that would leave a clip one float rounding step long", () => {
391+
const dir = mkdtempSync(join(tmpdir(), "hf-timeline-cli-"));
392+
try {
393+
const indexPath = join(dir, "index.html");
394+
writeFileSync(indexPath, meeting("19.8", "6.4", "26.2"));
395+
const result = run(dir, "trim", "#b", "--end", "26.200000000000003");
396+
expect(result.status).toBe(2);
397+
expect(result.stderr).toContain("trim duration must be positive");
398+
expect(clip(readFileSync(indexPath, "utf8"), "b").duration).toBe("2");
399+
} finally {
400+
rmSync(dir, { recursive: true, force: true });
401+
}
402+
});
403+
404+
it("moves a clip over pending media, whose unknown length counts as zero", () => {
405+
const dir = mkdtempSync(join(tmpdir(), "hf-timeline-cli-"));
406+
try {
407+
const indexPath = join(dir, "index.html");
408+
writeFileSync(
409+
indexPath,
410+
`<div data-composition-id="main" data-duration="12"><div id="a" data-hf-id="a" data-start="3" data-duration="1" data-track-index="0"></div><video id="pending" data-hf-id="pending" src="https://example.com/v.mp4" data-start="1" data-track-index="0"></video></div>`,
411+
);
412+
const result = run(dir, "move", "#a", "0.5");
413+
expect(result.status, result.stderr).toBe(0);
414+
expect(clip(readFileSync(indexPath, "utf8"), "a").start).toBe("0.5");
415+
} finally {
416+
rmSync(dir, { recursive: true, force: true });
417+
}
418+
});
419+
260420
it("refuses duplicate insertion inside a spanning clip", () => {
261421
const dir = project();
262422
try {

‎packages/core/src/clipFacts.test.ts‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import { describe, expect, it } from "vitest";
2-
import { byStart, formatClipLine, type ClipFact } from "./clipFacts.js";
2+
import {
3+
byStart,
4+
formatClipLine,
5+
isInsideSpan,
6+
sameInstant,
7+
spansOverlap,
8+
spansShareTime,
9+
type ClipFact,
10+
} from "./clipFacts.js";
311

412
const clip = (over: Partial<ClipFact> = {}): ClipFact => ({
513
id: "a",
@@ -60,3 +68,53 @@ describe("byStart", () => {
6068
expect(rows.sort(byStart).map((r) => r.id)).toEqual(["a", "b", "c"]);
6169
});
6270
});
71+
72+
describe("spansOverlap", () => {
73+
it("counts clips that only meet as apart, though the float sum of the first overshoots", () => {
74+
expect(19.8 + 6.4).toBeGreaterThan(26.2);
75+
expect(spansOverlap(19.8, 19.8 + 6.4, 26.2, 29)).toBe(false);
76+
expect(spansOverlap(26.2, 29, 19.8, 19.8 + 6.4)).toBe(false);
77+
});
78+
79+
it("counts clips that share time as overlapping, down to half a microsecond", () => {
80+
expect(spansOverlap(0, 2, 1.9, 3)).toBe(true);
81+
expect(spansOverlap(1, 2, 0, 5)).toBe(true);
82+
expect(spansOverlap(0, 20 / 24 + 5e-7, 20 / 24, 20 / 24 + 0.25)).toBe(true);
83+
});
84+
85+
it("counts a zero-length clip inside another, so the timeline gives it a lane of its own", () => {
86+
expect(spansOverlap(1, 1, 0.5, 1.5)).toBe(true);
87+
});
88+
});
89+
90+
describe("spansShareTime", () => {
91+
it("never counts a zero-length span, such as pending media, as sharing time", () => {
92+
expect(spansShareTime(1, 1, 0.5, 1.5)).toBe(false);
93+
expect(spansShareTime(0.5, 1.5, 1, 1)).toBe(false);
94+
});
95+
96+
it("agrees with spansOverlap on spans with length, float sums included", () => {
97+
expect(spansShareTime(19.8, 19.8 + 6.4, 26.2, 29)).toBe(false);
98+
expect(spansShareTime(0, 20 / 24 + 5e-7, 20 / 24, 20 / 24 + 0.25)).toBe(true);
99+
expect(spansShareTime(1, 2, 0, 5)).toBe(true);
100+
});
101+
});
102+
103+
describe("isInsideSpan", () => {
104+
it("counts a time strictly inside, and not one on either edge within float rounding", () => {
105+
expect(isInsideSpan(3, 1, 5)).toBe(true);
106+
expect(isInsideSpan(26.2, 19.8, 19.8 + 6.4)).toBe(false);
107+
expect(isInsideSpan(1, 1, 5)).toBe(false);
108+
});
109+
});
110+
111+
describe("sameInstant", () => {
112+
it("joins times a float rounding step apart, and nothing wider", () => {
113+
expect(sameInstant(19.8 + 6.4, 26.2)).toBe(true);
114+
expect(sameInstant(0.1 + 1.1, 1.2)).toBe(true);
115+
expect(sameInstant(3600.1 + 0.2, 3600.3)).toBe(true);
116+
expect(sameInstant(20 / 24 + 5e-7, 20 / 24)).toBe(false);
117+
expect(sameInstant(1.0000005, 1)).toBe(false);
118+
expect([sameInstant(5, Infinity), sameInstant(Infinity, Infinity)]).toEqual([false, true]);
119+
});
120+
});

‎packages/core/src/clipFacts.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,33 @@ const roundTo3 = (n: number) => Math.round(n * 1000) / 1000;
3434

3535
const num = (n: number) => String(roundTo3(n));
3636

37+
export const instantTolerance = (time: number) => 4 * Number.EPSILON * Math.max(1, Math.abs(time));
38+
39+
/** Float sums like 19.8 + 6.4 miss 26.2 by a rounding step; a few steps, scaled to the time, are one instant. */
40+
export function sameInstant(a: number, b: number): boolean {
41+
if (!Number.isFinite(a) || !Number.isFinite(b)) return a === b;
42+
return Math.abs(a - b) <= instantTolerance(Math.max(Math.abs(a), Math.abs(b)));
43+
}
44+
45+
const isBefore = (a: number, b: number) => a < b && !sameInstant(a, b);
46+
47+
/** For timeline lanes: a zero-length clip inside another still counts, so it gets a lane of its own. */
48+
export function spansOverlap(aStart: number, aEnd: number, bStart: number, bEnd: number): boolean {
49+
return isBefore(aStart, bEnd) && isBefore(bStart, aEnd);
50+
}
51+
52+
export function spansShareTime(
53+
aStart: number,
54+
aEnd: number,
55+
bStart: number,
56+
bEnd: number,
57+
): boolean {
58+
return isBefore(Math.max(aStart, bStart), Math.min(aEnd, bEnd));
59+
}
60+
61+
export const isInsideSpan = (time: number, start: number, end: number) =>
62+
isBefore(start, time) && isBefore(time, end);
63+
3764
export function formatClipLine(clip: ClipFact): string {
3865
const parts = [
3966
`${clip.kind} "${clip.id}"`,

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ export {
320320
// (verify:packed-manifests catches exactly that).
321321
export { createRuntimeStartTimeResolver } from "./runtime/startResolver.js";
322322
// Also exposed via the ./runtime/clip-window subpath; re-exported here for the same dist-emit reason.
323-
export { isClipVisibleAt, isInClipWindow } from "./runtime/clipWindow.js";
323+
export { hasClipStarted, isClipVisibleAt, isInClipWindow } from "./runtime/clipWindow.js";
324324
export {
325325
normalizePlaybackRate,
326326
normalizeRateSpec,

‎packages/core/src/runtime/clipWindow.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@ describe("isInClipWindow", () => {
55
it("includes the start and excludes the end", () => {
66
expect([1, 1.5, 2].map((t) => isInClipWindow(t, 1, 2))).toEqual([true, true, false]);
77
});
8+
9+
it("hands the instant a float sum misses by a rounding step to the next clip only", () => {
10+
expect([isInClipWindow(26.2, 19.8, 19.8 + 6.4), isInClipWindow(26.2, 26.2, 28.2)]).toEqual([
11+
false,
12+
true,
13+
]);
14+
expect(isInClipWindow(20 / 24, 0, 20 / 24 + 5e-7)).toBe(true);
15+
});
16+
17+
it("keeps a clip with no known end in its window", () => {
18+
expect(isInClipWindow(5, 0, Number.POSITIVE_INFINITY)).toBe(true);
19+
});
820
});
921

1022
describe("isClipVisibleAt", () => {

‎packages/core/src/runtime/clipWindow.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
/** Half-open: two back-to-back clips never both hold the shared boundary instant. */
1+
import { sameInstant } from "../clipFacts";
2+
3+
export const hasClipStarted = (time: number, start: number) =>
4+
time >= start || sameInstant(time, start);
5+
6+
/** Half-open: two back-to-back clips never both hold the shared boundary instant, float sums included. */
27
export const isInClipWindow = (time: number, start: number, end: number): boolean =>
3-
time >= start && time < end;
8+
hasClipStarted(time, start) && time < end && !sameInstant(time, end);
49

510
const TERMINAL_EPSILON_SECONDS = 1e-6;
611

@@ -15,6 +20,6 @@ export const isClipVisibleAt = (
1520
compositionDuration: number,
1621
): boolean =>
1722
isInClipWindow(time, start, end) ||
18-
(time >= start &&
23+
(hasClipStarted(time, start) &&
1924
compositionDuration > 0 &&
2025
end >= compositionDuration - TERMINAL_EPSILON_SECONDS);

0 commit comments

Comments
 (0)