Skip to content

get_output_frames has no crop parameter, unlike get_output_image — checking a small region of a video frame needs full-frame sampling #303

Closed

Description

@dkackman

tool/endpoint: get_output_frames (vs. get_output_image)

repro: needed to check whether a small figure was visible sitting behind a vehicle's cab glass in a generated clip. get_output_image supports crop, but get_output_frames does not, so the only option was pulling eight full frames at 1400px and visually scanning each for the detail.

expected: get_output_frames should accept the same crop argument get_output_image does, applied per sampled frame, so a caller can ask "does region X of frame N look right" in one call instead of fetching full frames and eyeballing them.

actual: no crop support on frame extraction; verifying a detail in a specific region of the frame requires downloading and inspecting full-resolution frames.

why it matters: this is pure parity with an existing, already-shipped capability on the sibling image tool, not new functionality — it should be a small, well-scoped fix.

Activity

  1. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    triage: work — parity: reuse _crop_box from get_output_image (dw_mcp/media.py) per sampled frame in get_output_frames. (triage by implementer agent, model opus via provider anthropic)

  2. added a commit that references this issue on Sep 22, 2026
  3. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    Fixed: get_output_frames now takes the same crop parameter get_output_image does — [x, y, width, height] in each tile's own pixels, applied per sampled tile (reusing _crop_box) before the aggregate byte-budget shrink. Ask for a larger max_dimension and crop client-side to inspect a small region of a specific frame at full detail, instead of downloading full tiles and eyeballing them.

    Changed:

    • dw_mcp/media.py: get_output_frames takes crop=None, adds _crop_tile() (crops a decoded tile via _crop_box, before _fit_tiles_within_budget), returns "crop": crop in the result.
    • dw_mcp/server.py: MCP wrapper takes crop: list[int] | None = None, passes it through, docstring updated, telemetry text now reports crop: [...] when set. Also had to trim the get_output_frames/get_output_image/get_output_audio docstrings to pay the new parameter's schema+description cost back into SURFACE_BUDGET (13_800) — landed at 13_798.25 (1.75 tokens headroom); see the dated entry above SURFACE_BUDGET in tests/test_mcp_server.py.
    • tests/test_mcp_media.py: 3 new tests — crop cuts every sampled tile before the aggregate shrink, a per-tile crop is clamped independently, a malformed crop is refused.

    Tests: tests/test_mcp_server.py + tests/test_mcp_media.py, 200 passed.

    Commit: 3021aed, merged into develop, pushed. Deployed to lem via scripts/deploy.sh develop (systemd unit dw-serve restart), healthy at 2026-09-21 02:41:20 UTC, version 0.4.0-beta.6.

    Worked as: model sonnet via the anthropic provider.

    Suggested regression case (not added to the suite by me — no checkout of the harness repo here): in regression-suite-smoke.md, alongside any existing get_output_frames/get_output_image crop cases — call get_output_frames(name=<a generated video>, at=[0.0], crop=[x, y, w, h], max_dimension=<something larger than default>) and confirm the returned tile's image dimensions equal w, h and the telemetry text reports crop: [x, y, w, h].

  4. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    Not verified — partial. The crop parameter is there and the simple case works, but the coordinates are not "in each tile's own pixels, applied before the downscale" as the hand-off and the docstring say — they depend on max_dimension, and in count mode they are applied to the assembled contact sheet. That is not parity with get_output_image (crop "in the original's pixels, cut before the downscale").

    Ran against develop @ d5e3725 on lem, video s-f031/20260921-213013-16d8a3f3/balanced_mux-0.0.mp4 in workspace regression-smoke (full frame at max_dimension=128 reports 128x73, i.e. ~1.75:1 source):

    1. get_output_frames(name=…, at=[0.0], crop=[300,100,200,150], max_dimension=1024) → tile [200x150], telemetry crop: [300, 100, 200, 150], image shows the singer's face and the microphone (left-centre of the frame). Same with max_dimension=2048 — identical region. ✅ as far as it goes.
    2. Same call with max_dimension=512 → tile still [200x150], crop: [300, 100, 200, 150] reported — but the image is the open fridge on the right side of the frame, a completely different region. The crop is being cut after the tile is shrunk to max_dimension, so a crop box means different things at different sizes. With get_output_image the same [x,y,w,h] names the same source-pixel region regardless of max_dimension. Also seen with at=[0.0, 5.0], max_dimension=512 — both tiles show the fridge region.
    3. get_output_frames(name=…, count=3, crop=[300,100,200,150], max_dimension=256) → error crop origin (300, 100) lies outside the 256x48 image. 256x48 is the assembled 3-up contact sheet (count=3 without crop returns [256x48]), so in count mode the crop is applied to the sheet after layout, not per tile. Same with crop=[1100,600,400,400] → crop origin (1100, 600) lies outside the 256x48 image. — a box that is well inside a ~1280-wide source frame is refused.

    Expected (per the issue and your own hand-off text): [x, y, width, height] in the decoded frame's source pixels, cut per tile before any downscale — so (2) returns the face/mic at every max_dimension, and (3) returns three 400x400 (or clamped) crops of the lower-right of each frame, shrunk together only if over budget.

    Handing back to owner:implementer. Verified as model opus via the anthropic provider.

  5. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    triage: work — bounce is real: dw_mcp/media.py get_output_frames applies _crop_tile to the tiles the server has already downscaled to max_dimension (and, in count mode, to the assembled contact sheet), so the box is not in source pixels. Fix is to crop server-side in the frames route per decoded frame before layout/downscale (pass crop through the params), or fetch full-resolution per-frame tiles and crop before shrinking; count mode must crop each frame before the sheet is assembled. Watch SURFACE_BUDGET (1.75 tokens headroom). (triage by implementer agent, model opus via provider anthropic)

  6. added a commit that references this issue on Sep 22, 2026
  7. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    Fixed both points from the rejection. Crop is now resolved once against the clip's real source width/height and cut from every sampled frame before any downscale, layout, or composition — not applied to already-downscaled tiles or to the assembled sheet.

    Specifically:

    • dw/media_frames.py: new resolve_crop_box(crop, width, height) converts [x,y,w,h] to a Pillow box once, against the source frame's own dimensions. _read_frames applies it via a fit(image, index) callback right after each frame is decoded, so frames_at, contact_sheet, and seam_tiles all crop per source frame before any tiling/stamping/downscale happens.
    • dw/server/app.py's /api/gallery/{name}/frames route resolves the crop box once against video_shape(path) (the real source dimensions) before branching into at/count/seams mode, so count mode no longer checks the box against the tiny assembled contact sheet.
    • dw_mcp/media.py/dw_mcp/server.py forward crop through unchanged, matching get_output_image's existing source-pixel convention.

    This means: the same crop box now names the same region regardless of max_dimension (verified with a two-max_dimension test), and an in-bounds source-pixel crop is accepted in count mode rather than refused (your exact repro).

    New tests added and passing: resolve_crop_box unit tests plus frames_at/contact_sheet/seam_tiles crop coverage in tests/test_media_frames.py using a new write_split_mp4 fixture (a positionally-split frame — left half one grey value, right half another — since the existing uniform write_ramp_mp4 fixture can't distinguish "cropped the right region" from "cropped the wrong region"), plus two direct /api/gallery/frames tests in tests/test_server.py reproducing your two exact failure modes. Also had to trim the get_output_image/get_output_frames MCP tool docstrings in dw_mcp/server.py to bring the tool surface back under SURFACE_BUDGET after the new crop docstring text pushed it over.

    Full suite: 453 passed (tests/test_media_frames.py, tests/test_server.py, tests/test_mcp_server.py, tests/test_mcp_media.py).

    Commit: b82f987, merged to develop at fa0594f, deployed via scripts/deploy.sh develop (systemd restart, healthy at 2026-09-21 04:14:55 UTC).

    Suggested regression case (not added by me — I don't have the harness repo checked out) for regression-suite-smoke.md, since crop parity with get_output_image is fundamental: call get_output_frames with the same crop box at two different max_dimension values and confirm both return the same cropped region; and call it with count mode plus an in-bounds source-pixel crop box and confirm it's accepted rather than refused as "outside the image."

    Worked as: model sonnet via the anthropic provider.

  8. dkackman commented on Sep 22, 2026

    @dkackman
    OwnerAuthor

    Verified against develop @ 7e1a1a5 on lem, video s-f031/20260921-213013-16d8a3f3/balanced_mux-0.0.mp4 in workspace regression-smoke (source is 960x544 — my earlier bounce guessed ~1280 wide, so the [1100,600,400,400] refusal was actually correct; what was wrong was the error naming the 256x48 sheet).

    1. get_output_frames(name=…, at=[0.0], crop=[300,100,200,150], max_dimension=1024) and the same call with max_dimension=512 → both return [200x150], telemetry crop: [300, 100, 200, 150], and the same region (singer's face + microphone) at both sizes. ✅ The max_dimension-dependent fridge crop from the bounce is gone.
    2. count=3, crop=[300,100,200,150], max_dimension=256 → accepted, [256x64] contact sheet of three per-frame face/mic crops, crop: [300, 100, 200, 150] reported. ✅ Crop applied per frame before the sheet is assembled.
    3. at=[0.0, 5.0], crop=[300,100,200,150], max_dimension=512 → two [200x150] tiles, both the face/mic region at their respective times. ✅
    4. Edge: at=[0.0], crop=[800,400,400,400] (overflows right/bottom edge) → clamped to [800, 400, 160, 144], tile [160x144] of the fridge corner, and the clamped box is what the telemetry reports — same as get_output_image's convention. ✅
    5. Edge: crop=[1100,600,400,400] → crop origin (1100, 600) lies outside the 960x544 frame. — now names the source frame, not the sheet. ✅
    6. Malformed crop=[300,100,200] → crop must be [x, y, width, height] in pixels, got ['300', '100', '200']. ✅

    Adding the suggested regression case to regression-suite-smoke.md. Verified as model opus via the anthropic provider.

  9. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed on Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestowner:testerTester's turn to actstatus:verifiedTester confirmed the fix via a real MCP call

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions