fix(editor): a crop of a video with wide pixels keeps what its frame showed

The browser reports such a video by its shown size and ffmpeg's crop counts stored pixels, so a
frame over the middle half of a 320 stored, 640 shown video kept the whole width. A crop now carries
the size it was measured on and is scaled to the file's own pixels. A spec from before it did is cut
as it always was. Also drops a stale note in downloads/quota.py: edit jobs are held to the cap.

backend/app/downloads/edit.py

@@ -60,6 +60,31 @@ class EditAborted(Exception):
 _MAX_SEGMENTS = 100
 
 
+def _crop_frame(crop: dict) -> dict | None:
+    """The picture size a crop was measured on, or None when it has none it could be scaled by (a
+    spec from before it was sent, which `crop_filter` cuts in the file's own pixels)."""
+    try:
+        w, h = int(crop["frame_w"]), int(crop["frame_h"])
+    # silent-ok: no frame is a crop's older form, and it is still the crop that was asked for.
+    except (KeyError, TypeError, ValueError):
+        return None
+    return {"frame_w": w, "frame_h": h} if w > 0 and h > 0 else None
+
+
+def crop_filter(crop: dict) -> str:
+    """The ffmpeg `crop` of an edit. Its numbers count the picture as the browser reports it, which
+    is its SHOWN size: a video with wide pixels (320 stored, 640 shown) would be cut off to one side
+    in ffmpeg's stored ones, so they are scaled to `iw` / `ih` by the size they were measured on. A
+    rotated file needs nothing more: both the browser and ffmpeg's filters see it upright."""
+    frame_w, frame_h = crop.get("frame_w"), crop.get("frame_h")
+    if not frame_w or not frame_h:
+        return f"crop={crop['w']}:{crop['h']}:{crop['x']}:{crop['y']}"
+    return (
+        f"crop=iw*{crop['w']}/{frame_w}:ih*{crop['h']}/{frame_h}"
+        f":iw*{crop['x']}/{frame_w}:ih*{crop['y']}/{frame_h}"
+    )
+
+
 def normalize_edit_spec(spec: dict | None) -> dict:
     """Canonicalize a raw edit spec: clamp/round trim, coerce crop to ints, drop empties.
 
@@ -81,7 +106,8 @@ def normalize_edit_spec(spec: dict | None) -> dict:
             )
             c = None
         if c and c["w"] > 0 and c["h"] > 0 and c["x"] >= 0 and c["y"] >= 0:
-            out["crop"] = c
+            frame = _crop_frame(crop)
+            out["crop"] = {**c, **frame} if frame else c
 
     # Multi-segment JOIN (cut-list → one concatenated file). Takes precedence over `trim`.
     segs = spec.get("segments")
@@ -268,7 +294,7 @@ def build_edit_cmd(src: Path, dest: Path, spec: dict, out_ext: str) -> list[str]
     if reencode:
         # Input -ss + re-encode is frame-accurate (decodes from the prior keyframe, discards up to
         # the exact cut). A crop adds the filter; an accurate trim only evens the frame.
-        cropf = f"crop={crop['w']}:{crop['h']}:{crop['x']}:{crop['y']}," if crop else ""
+        cropf = f"{crop_filter(crop)}," if crop else ""
         cmd += ["-vf", cropf + EVEN_FRAME, *X264_ARGS, *AAC_ARGS]
     else:
         cmd += ["-c", "copy", "-avoid_negative_ts", "make_zero"]
@@ -313,7 +339,7 @@ def build_concat_plan(src: Path, dest: Path, spec: dict, out_ext: str, staging:
     ]
 
     if needs_reencode(spec):
-        cropf = f",crop={crop['w']}:{crop['h']}:{crop['x']}:{crop['y']}" if crop else ""
+        cropf = f",{crop_filter(crop)}" if crop else ""
         parts, labels = [], []
         for i, s in enumerate(segs):
             st, en = s["start_s"], s["end_s"]

backend/app/downloads/quota.py

@@ -16,7 +16,8 @@ That is a per-job ceiling, not an aggregate ledger: concurrent jobs of one user
 remainder (in-flight bytes are not `ready`, so `footprint` cannot see them), so a wave of
 max_concurrent downloads can still land over the cap — the deliberate trade, since a download's size
 is unknown until it finishes. CLIP jobs consult it before their fetch or encode and again with
-their output's real size before it is placed. Edit jobs do not consult it at all (tracked as SEC-19).
+their output's real size before it is placed. Edit jobs consult it with their output's real size
+before it is placed (`test_edit_cap.py`).
 """
 
 from dataclasses import dataclass

backend/tests/test_edit_crop_frame.py

@@ -0,0 +1,112 @@
+"""The editor's crop lands on the part of the picture the frame was drawn on, also when a video's
+pixels are not square. The browser reports such a video by its shown size (320 stored, 640 shown),
+and ffmpeg's `crop` counts stored pixels: measured on a real clip, a frame over the middle half kept
+half the frame's red surroundings. So a crop carries the size it was measured on.
+
+Real ffmpeg: the pixels of the output are the oracle.
+"""
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+from app.config import settings
+from app.downloads import edit, pipeline, service, storage
+from app.downloads import upload as uploadmod
+from app.downloads.models import DownloadJob, MediaAsset
+
+pytestmark = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available")
+
+
+def _source(path: Path, size: str, box: str, sar: int) -> None:
+    """Red, with the picture's middle half green: a crop of that middle holds no red."""
+    subprocess.run(
+        [
+            "ffmpeg", "-y", "-loglevel", "error",
+            "-f", "lavfi", "-i", f"color=c=red:s={size}:d=2:r=10",
+            "-f", "lavfi", "-i", "sine=duration=2",
+            "-vf", f"drawbox={box}:c=lime:t=fill,setsar={sar}",
+            "-c:v", "libx264", "-pix_fmt", "yuv420p", "-c:a", "aac", str(path),
+        ],
+        check=True,
+    )  # fmt: skip
+
+
+def _red_and_green(path: Path) -> tuple[int, int]:
+    raw = subprocess.run(
+        ["ffmpeg", "-loglevel", "error", "-i", str(path), "-frames:v", "1",
+         "-f", "rawvideo", "-pix_fmt", "rgb24", "-"],
+        check=True,
+        capture_output=True,
+    ).stdout  # fmt: skip
+    red = green = 0
+    for i in range(0, len(raw) - 2, 3):
+        if raw[i] > 180 and raw[i + 1] < 80:
+            red += 1
+        elif raw[i + 1] > 180 and raw[i] < 80:
+            green += 1
+    return red, green
+
+
+def _cut(db, make_user, tmp_path, src: Path, crop: dict) -> Path:
+    user = make_user()
+    source = uploadmod.ingest_upload(
+        db, user.id, filename="clip.mp4", src_path=src, size=src.stat().st_size
+    )
+    assert source.status == "done"
+    job = service.enqueue_edit(db, user.id, source, {"crop": crop})
+    asset = db.get(MediaAsset, job.asset_id)
+    job.status, asset.status = "running", "downloading"  # the worker's claim
+    db.commit()
+
+    pipeline._process_edit(job.id, asset.id)
+
+    db.expire_all()
+    job, asset = db.get(DownloadJob, job.id), db.get(MediaAsset, job.asset_id)
+    assert (job.status, job.error, asset.status) == ("done", None, "ready")
+    return storage.abs_path(str(tmp_path), asset.rel_path)
+
+
+def test_a_crop_of_a_video_with_wide_pixels_keeps_what_the_frame_showed(
+    db, make_user, tmp_path, monkeypatch
+):
+    monkeypatch.setattr(settings, "files_root", str(tmp_path))
+    src = tmp_path / "in.mp4"
+    _source(src, "320x240", "x=80:y=60:w=160:h=120", sar=2)
+
+    # What the editor sends for the middle half of this video: the browser shows it at 640 x 240.
+    final = _cut(
+        db, make_user, tmp_path, src,
+        {"x": 160, "y": 60, "w": 320, "h": 120, "frame_w": 640, "frame_h": 240},
+    )  # fmt: skip
+
+    red, green = _red_and_green(final)
+    assert (red, green) == (0, 160 * 120)
+
+
+def test_a_crop_of_square_pixels_is_cut_where_it_was_drawn(db, make_user, tmp_path, monkeypatch):
+    monkeypatch.setattr(settings, "files_root", str(tmp_path))
+    src = tmp_path / "in.mp4"
+    _source(src, "480x640", "x=120:y=160:w=240:h=320", sar=1)
+
+    final = _cut(
+        db, make_user, tmp_path, src,
+        {"x": 120, "y": 160, "w": 240, "h": 320, "frame_w": 480, "frame_h": 640},
+    )  # fmt: skip
+
+    assert _red_and_green(final) == (0, 240 * 320)
+
+
+def test_a_crop_stored_before_it_carried_its_frame_is_cut_as_it_always_was():
+    spec = edit.normalize_edit_spec({"crop": {"x": 4, "y": 6, "w": 100, "h": 50}})
+    assert spec["crop"] == {"x": 4, "y": 6, "w": 100, "h": 50}
+    cmd = edit.build_edit_cmd(Path("in.mp4"), Path("out.mp4"), spec, "mp4")
+    assert cmd[cmd.index("-vf") + 1].startswith("crop=100:50:4:6,")
+
+
+@pytest.mark.parametrize("frame", [{"frame_w": 0, "frame_h": 240}, {"frame_w": "x", "frame_h": 2}])
+def test_a_frame_that_cannot_be_read_is_not_divided_by(frame):
+    spec = edit.normalize_edit_spec({"crop": {"x": 4, "y": 6, "w": 100, "h": 50, **frame}})
+    assert spec["crop"] == {"x": 4, "y": 6, "w": 100, "h": 50}

frontend/src/lib/api/downloads.ts

@@ -70,7 +70,9 @@ export interface ClipRequest {
 export interface EditSpec {
   trim?: { start_s: number; end_s?: number };
   segments?: { start_s: number; end_s: number }[];
-  crop?: { x: number; y: number; w: number; h: number };
+  // In the picture's size as the browser reports it, which `frame_w` x `frame_h` says: the server
+  // scales them to the file's own pixels (a video's stored and shown sizes can differ).
+  crop?: { x: number; y: number; w: number; h: number; frame_w?: number; frame_h?: number };
   accurate?: boolean;
 }
 

frontend/src/modules/files/VideoEditor.crop.dom.test.tsx

@@ -0,0 +1,56 @@
+// @vitest-environment jsdom
+//
+// A crop's numbers count the picture as the browser reports it, its shown size, which differs from the
+// file's own on a video with wide pixels. The size goes along, or the server cuts the wrong part.
+import { describe, it, expect, vi, beforeAll, afterEach } from "vitest";
+import { render, screen, fireEvent, waitFor, cleanup } from "@testing-library/react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+
+vi.mock("react-i18next", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("react-i18next")>()),
+  useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
+}));
+
+import VideoEditor from "./VideoEditor";
+import { api, type DownloadJob } from "../../lib/api";
+
+beforeAll(() => {
+  class Stub {
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+    takeRecords() {
+      return [];
+    }
+  }
+  globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
+  globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
+});
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+const job = {
+  id: 7,
+  display_name: "Talk",
+  asset: { duration_s: 100, width: 1920, height: 1080, title: "Talk" },
+} as unknown as DownloadJob;
+
+describe("VideoEditor crop", () => {
+  it("sends the size the crop was measured on with it", async () => {
+    const enqueue = vi
+      .spyOn(api, "enqueueEdit")
+      .mockResolvedValue({ id: 8 } as unknown as DownloadJob);
+    render(
+      <QueryClientProvider client={new QueryClient()}>
+        <VideoEditor job={job} onClose={() => {}} />
+      </QueryClientProvider>,
+    );
+    fireEvent.click(screen.getByRole("button", { name: "editor.cropOff" }));
+    fireEvent.click(screen.getByRole("button", { name: "editor.create" }));
+    await waitFor(() => expect(enqueue).toHaveBeenCalledTimes(1));
+    const crop = enqueue.mock.calls[0]?.[0].edit_spec.crop;
+    expect(crop).toMatchObject({ frame_w: 1920, frame_h: 1080, w: 1536, h: 864 });
+  });
+});

frontend/src/modules/files/VideoEditor.tsx

@@ -226,14 +226,16 @@ export default function VideoEditor({ job, onClose }: { job: DownloadJob; onClos
   const willJoin = output === "join" && kept.length > 1;
   const reencode = cropOn || accurate;
 
-  const cropPx = (): { x: number; y: number; w: number; h: number } | undefined => {
+  const cropPx = (): EditSpec["crop"] => {
     const v = videoRef.current;
     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 px = cropToSourcePx(crop, vw, vh);
+    // The size they count in goes along: it is the shown one, and the file's own can differ.
+    return px ? { ...px, frame_w: vw, frame_h: vh } : undefined;
   };
 
   const create = useMutation({