diff --git a/frontend/src/components/VideoEditor.tsx b/frontend/src/components/VideoEditor.tsx index bab983f6..042956c9 100644 --- a/frontend/src/components/VideoEditor.tsx +++ b/frontend/src/components/VideoEditor.tsx @@ -7,6 +7,7 @@ import clsx from "clsx"; import Modal from "./Modal"; import { notify } from "../lib/notifications"; import { api, type DownloadJob, type EditSpec } from "../lib/api"; +import { contentRect, cropToSourcePx, type Frac } from "../lib/cropGeometry"; import { inputCls, btnCls } from "./ui/form"; import { Button } from "./ui/Button"; @@ -31,7 +32,6 @@ function parseTc(v: string): number | null { } type Seg = { id: number; start: number; end: number; keep: boolean }; -type Frac = { x: number; y: number; w: number; h: number }; type Drag = { type: "seek" } | { type: "boundary"; bi: number }; const numCls = @@ -51,6 +51,11 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos const [current, setCurrent] = useState(0); const [playing, setPlaying] = useState(false); const [trackW, setTrackW] = useState(0); + // Intrinsic (source) video dims and the video element's rendered box, both needed to map the + // crop marquee onto the letterboxed picture. `vdim` starts from asset metadata so the overlay + // can size before the media decodes; `box` is measured off the element (max-h can clamp it). + const [vdim, setVdim] = useState({ w: job.asset?.width ?? 0, h: job.asset?.height ?? 0 }); + const [box, setBox] = useState({ w: 0, h: 0 }); // The cut-list: contiguous segments covering [0, duration]. Starts as the whole video, kept. const [segments, setSegments] = useState([{ id: 0, start: 0, end: srcDur, keep: true }]); @@ -83,11 +88,28 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos return () => ro.disconnect(); }, []); + // Measure the video element's rendered box (max-h can shrink it below its intrinsic size, which + // then pillarboxes the picture). The crop overlay is anchored to the picture, not this box. + useEffect(() => { + const v = videoRef.current; + if (!v || typeof ResizeObserver === "undefined") return; + const ro = new ResizeObserver(() => setBox({ w: v.clientWidth, h: v.clientHeight })); + ro.observe(v); + setBox({ w: v.clientWidth, h: v.clientHeight }); + return () => ro.disconnect(); + }, []); + + // The picture's rendered rectangle inside the (possibly letterboxed) video box — the overlay's + // frame, so a crop fraction is a fraction of the real picture, not of the black bars. + const va = vdim.w && vdim.h ? vdim.w / vdim.h : aspect; + const picRect = contentRect(box.w, box.h, va); + const onLoaded = () => { const v = videoRef.current; if (!v) return; const d = v.duration && isFinite(v.duration) ? v.duration : srcDur; setDuration(d); + if (v.videoWidth && v.videoHeight) setVdim({ w: v.videoWidth, h: v.videoHeight }); // Reconcile the still-pristine full-length segment to the REAL decoded duration. The initial // segment ends at the (integer, often-rounded) metadata `srcDur`; once the media loads we know // the true length (e.g. 213.44 vs 213). Without this the untouched last segment stays short, so @@ -258,16 +280,12 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos const cropPx = (): { x: number; y: number; w: number; h: number } | undefined => { const v = videoRef.current; - const vw = v?.videoWidth || job.asset?.width || 0; - const vh = v?.videoHeight || job.asset?.height || 0; - if (!cropOn || !vw || !vh) return undefined; - const even = (n: number) => Math.max(2, Math.round(n / 2) * 2); - return { - x: even(crop.x * vw), - y: even(crop.y * vh), - w: even(crop.w * vw), - h: even(crop.h * vh), - }; + const vw = v?.videoWidth || vdim.w; + const vh = v?.videoHeight || vdim.h; + if (!cropOn) return undefined; + // crop fractions are relative to the picture (the overlay is anchored to it), so this is a + // straight fraction→source-px scale; see cropGeometry for the even-rounding + in-bounds clamp. + return cropToSourcePx(crop, vw, vh) ?? undefined; }; const create = useMutation({ @@ -322,11 +340,20 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos onTimeUpdate={onTimeUpdate} onPlay={() => setPlaying(true)} onPause={() => setPlaying(false)} - className="w-full max-h-[42vh] mx-auto block" + className="w-full max-h-[42vh] mx-auto block object-contain" playsInline /> - {cropOn && ( -
+ {cropOn && picRect && ( +
{ + it("returns the full box when box aspect matches the video aspect", () => { + // 16:9 picture in a 16:9 box → no bars, content == box + expect(contentRect(1600, 900, 16 / 9)).toEqual({ + left: 0, + top: 0, + width: 1600, + height: 900, + }); + }); + + it("pillarboxes (bars left/right) when the box is wider than the picture", () => { + // 16:9 picture in a 2:1 box → full height, narrower centered width + const r = contentRect(2000, 1000, 16 / 9)!; + expect(r.height).toBe(1000); + expect(r.width).toBeCloseTo(1000 * (16 / 9), 3); + expect(r.top).toBe(0); + expect(r.left).toBeCloseTo((2000 - 1000 * (16 / 9)) / 2, 3); + }); + + it("letterboxes (bars top/bottom) when the box is taller than the picture", () => { + // 16:9 picture in a 1:1 box → full width, shorter centered height + const r = contentRect(1000, 1000, 16 / 9)!; + expect(r.width).toBe(1000); + expect(r.height).toBeCloseTo(1000 / (16 / 9), 3); + expect(r.left).toBe(0); + expect(r.top).toBeCloseTo((1000 - 1000 / (16 / 9)) / 2, 3); + }); + + it("returns null for unmeasurable inputs", () => { + expect(contentRect(0, 900, 16 / 9)).toBeNull(); + expect(contentRect(1600, 0, 16 / 9)).toBeNull(); + expect(contentRect(1600, 900, 0)).toBeNull(); + expect(contentRect(1600, 900, NaN)).toBeNull(); + }); +}); + +describe("cropToSourcePx (fraction-of-content → even source px)", () => { + it("scales content fractions straight to source pixels", () => { + // A marquee anchored to the content rect: a fraction is a fraction of the real picture. + expect(cropToSourcePx({ x: 0.25, y: 0.25, w: 0.5, h: 0.5 }, 1920, 1080)).toEqual({ + x: 480, + y: 270, + w: 960, + h: 540, + }); + }); + + it("rounds to even dimensions", () => { + // 0.1 * 1920 = 192 (even); 0.333 * 1080 = 359.64 → 360 + const c = cropToSourcePx({ x: 0.1, y: 0.333, w: 0.2, h: 0.111 }, 1920, 1080)!; + expect(c.w % 2).toBe(0); + expect(c.h % 2).toBe(0); + expect(c.x % 2).toBe(0); + expect(c.y % 2).toBe(0); + }); + + it("allows a top-left origin (x/y may be 0)", () => { + const c = cropToSourcePx({ x: 0, y: 0, w: 0.5, h: 0.5 }, 1920, 1080)!; + expect(c.x).toBe(0); + expect(c.y).toBe(0); + }); + + it("clamps the window inside the frame so even-rounding can't overflow", () => { + // A full-frame crop must stay exactly at the source size, not round past it. + const c = cropToSourcePx({ x: 0, y: 0, w: 1, h: 1 }, 1921, 1081)!; + expect(c.w).toBeLessThanOrEqual(1921); + expect(c.h).toBeLessThanOrEqual(1081); + expect(c.x + c.w).toBeLessThanOrEqual(1921); + expect(c.y + c.h).toBeLessThanOrEqual(1081); + }); + + it("keeps x+w within the frame for an off-square region near the right edge", () => { + const srcW = 1280; + const srcH = 720; + const c = cropToSourcePx({ x: 0.7, y: 0.1, w: 0.3, h: 0.8 }, srcW, srcH)!; + expect(c.x + c.w).toBeLessThanOrEqual(srcW); + expect(c.y + c.h).toBeLessThanOrEqual(srcH); + }); + + it("returns null for an unusable source size", () => { + expect(cropToSourcePx({ x: 0, y: 0, w: 1, h: 1 }, 0, 1080)).toBeNull(); + expect(cropToSourcePx({ x: 0, y: 0, w: 1, h: 1 }, 1920, 0)).toBeNull(); + }); +}); diff --git a/frontend/src/lib/cropGeometry.ts b/frontend/src/lib/cropGeometry.ts new file mode 100644 index 00000000..eebd268f --- /dev/null +++ b/frontend/src/lib/cropGeometry.ts @@ -0,0 +1,48 @@ +// Crop geometry for the video editor — the display-px ↔ source-px mapping, isolated as pure +// functions so the math is unit-testable without a DOM. +// +// The bug this fixes (E7 S1→S2): the crop marquee is drawn in fractions of the *video element box*, +// but a letterboxed video (object-fit: contain) does NOT fill that box — pillar/letterbox bars sit +// between the box edge and the actual picture. Multiplying box-fractions by the source resolution +// therefore emitted an ffmpeg crop shifted/scaled off from what the selector showed. The fix is to +// anchor the overlay to the *content* rect (below), so a fraction of the overlay is a fraction of +// the real picture, and cropToSourcePx() is then a straight fraction→pixel scale. + +export type Frac = { x: number; y: number; w: number; h: number }; +type Rect = { left: number; top: number; width: number; height: number }; +type CropPx = { x: number; y: number; w: number; h: number }; + +/** + * The rendered video-content rectangle inside an element box of `boxW × boxH`, given the video's + * intrinsic aspect ratio, when the video is laid out `object-fit: contain` (letterboxed). Returns + * null while any input is unmeasurable (0/NaN), so callers can skip drawing until the video sizes. + */ +export function contentRect(boxW: number, boxH: number, videoAspect: number): Rect | null { + if (!(boxW > 0) || !(boxH > 0) || !(videoAspect > 0)) return null; + const boxAspect = boxW / boxH; + if (boxAspect > videoAspect) { + // box wider than the picture → pillarbox (bars left/right), full height + const width = boxH * videoAspect; + return { left: (boxW - width) / 2, top: 0, width, height: boxH }; + } + // box taller than the picture → letterbox (bars top/bottom), full width + const height = boxW / videoAspect; + return { left: 0, top: (boxH - height) / 2, width: boxW, height }; +} + +/** + * A crop expressed as fractions of the video *content* → integer, even source-pixel crop args + * (ffmpeg `crop=w:h:x:y`). Even dimensions keep chroma-subsampled encoders happy; the window is + * clamped inside the frame so even-rounding can never push `x+w`/`y+h` past the source resolution + * (which ffmpeg rejects as "crop area out of bounds"). Returns null for an unusable source size. + */ +export function cropToSourcePx(crop: Frac, srcW: number, srcH: number): CropPx | null { + if (!(srcW > 0) || !(srcH > 0)) return null; + const evenDim = (n: number) => Math.max(2, Math.round(n / 2) * 2); // w/h ≥ 2 + const evenOff = (n: number) => Math.max(0, Math.round(n / 2) * 2); // x/y ≥ 0 (top-left is valid) + const w = Math.min(evenDim(crop.w * srcW), srcW); + const h = Math.min(evenDim(crop.h * srcH), srcH); + const x = Math.min(evenOff(crop.x * srcW), srcW - w); + const y = Math.min(evenOff(crop.y * srcH), srcH - h); + return { x, y, w, h }; +}