Skip to content

feat(server): send sampled video frames as ordered images - #1759

Merged
inureyes merged 4 commits into
mainfrom
feature/issue-1322-video-frames-fallback
Sep 10, 2026
Merged

inureyes merged 4 commits into
mainfrom
feature/issue-1322-video-frames-fallback

Conversation

@inureyes

@inureyes inureyes commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

A video_url block and --video were refused by every family without a temporal processor, although each of the refusing families reads a sequence of stills perfectly well. A checkpoint with a vision tower and no native video path now decodes the clip, keeps at most --video-max-frames evenly spaced frames (default 16, floor 2, first and last always kept), PNG-encodes them, and rewrites them into that many image_url parts in chronological order behind the sentence Here is a video as a sequence of N frames in chronological order.

The rewrite runs on the request itself, on one seam: after the media capability gate and before the template renders. The prompt then carries one image placeholder per frame, so the per-request image budget, the soft-token accounting and the prompt-cache multimodal digest treat the frames as the images they now are, with no downstream change. Native families (Gemma 4 VL / Unified, Inkling, Kimi-VL, Qwen-VL) are untouched and keep prepared.videos. Muse Glimmer is excluded, matching the --video refusal the CLI already carries for it, so the two fronts do not disagree about one checkpoint. ModelMediaSupport::video splits into video_native and video_frames_fallback with a video() accessor keeping the old meaning for the HTTP gate, and the native-video family list moves next to get_model_type so the CLI and the server read one list instead of two that can drift.

Only the frames that survive the cap are decoded. load_video_source_frames_fallback probes the clip, computes the count the requested fps alone would have sampled so the log line can still say "kept of sampled", and reads only min(sampled, max_frames) frames. Decoding at the fps and subsampling afterwards would hold up to FPS_MAX_FRAMES (768) full-resolution frames to keep sixteen, which at the shipped resource caps is tens of gigabytes for a long clip, and the default 2.0 fps reaches that ceiling on any clip past about six minutes. A uniform sample of a uniform sample is one, so the frames kept are the same even spread over the clip; the re-validation below confirms that against real checkpoints, to the token. Both fronts call the same helper, so a CLI run and a server request choose the same frames out of the same clip at the same settings.

Knobs, both read only on the fallback path: --video-max-frames (env MLXCEL_VIDEO_MAX_FRAMES, default 16, values below 2 raised to 2) on mlxcel generate, mlxcel serve and mlxcel-server; --video-fps (env MLXCEL_VIDEO_FPS, default 2.0) on the two server fronts, where the CLI keeps --fps for the same dial.

The frames spend the ordinary per-request image budget, and the request is refused before it is paid for. A body carrying more video_url parts than --max-images can hold is refused ahead of the first ffprobe, since every clip yields at least one frame image; past that the frame count is checked per clip, so a body that goes over budget stops at the clip that broke it rather than decoding the rest. Both refusals name the frame count, the caller's own image count and the limit, rather than letting validate_image_count report a count the caller never sent.

Two wiring points deliberately left alone

The issue's step 4 named routes/anthropic.rs and router_front.rs. Both are absent on purpose. AnthropicContentBlock has no video variant (text, image, document, tool_use, tool_result), so anthropic_request_to_chat can never produce a VideoUrl part and wiring it would add an unreachable call. route_chat refuses every request request_declares_media matches, video included, before rendering, because the disaggregated path is text-only for pool-backed families, so there is nothing there for the expansion to run on.

Changes during review

Three commits, and the second and third came out of the review and security passes. The bounded decode described above replaced a decode-then-discard that had been sized by FPS_MAX_FRAMES rather than by the cap, on both fronts. The per-request image budget, which had been enforced once after every clip in the body was decoded and PNG-encoded, gained the clip-count pre-check and the per-clip check. And the info line reporting a clip as sent moved behind the budget refusal, so a refused request no longer leaves a log claiming the clip that broke the budget was sent.

Reported and not fixed, all pre-existing or out of this issue's scope: the CLI emits one lead sentence for the summed frame count when several --video flags are passed, where the server splices one sentence per clip at that clip's own position; CLI frame PNGs land in std::env::temp_dir() at mode 0644 minus umask, matching the existing write_video_temp_file; ffmpeg_available() and video_dir_allowlist_from_env() do their blocking work on a Tokio worker, both with the existing rationale; the prompt-inspection route clones the whole request, caller images included; and the spawn_blocking decode is not cancellable on client disconnect.

Validation

Real checkpoints on M5 Max / macOS 27.0, against release binaries built from this tree. Fixture: an 8 s 448x448 25 fps H.264 clip of a 128x128 red square crossing a white background left to right, first and last frames visually confirmed. Every number below was produced twice, once before the bounded-decode change and once after it, and did not move.

CLI (mlxcel generate), four runs, all exit 0:

Case Checkpoint What ran
Fallback gemma-3-4b-it-4bit, --video-max-frames 8 model_type=Gemma3VLM has no native video path; sending 8 of 16 sampled frames ... as ordered images, then Loaded 8 image(s). at 256 tokens each; prompt opens with the lead sentence
Fallback lfm2-vl-450m-4bit, default cap model_type=Lfm2VL ... sending 16 of 16 sampled frames, Loaded 16 image(s)., inserted 16 image block(s) (3136 total image tokens)
Native control qwen2.5-vl-3b-instruct-4bit no fallback line; Loaded 1 Qwen-VL video(s) (16 total frames after sampling), 1 video block(s) (2048 video tokens)
Native control gemma-4-e4b-it-4bit no fallback line; Loaded 1 video(s) (16 total frames after sampling), expanded 1 video(s) into 16 frame slot(s)

Server (gemma-3-4b-it-4bit --video-max-frames 8, /v1/chat/completions, HTTP 200 throughout): text only 19 prompt tokens, text plus one still PNG 279, text plus video_url 2114. The 2095-token growth is 8 x 260 for the frames plus 15 for the lead sentence, to the token. gemma-4-e4b-it-4bit answered the same body natively at 1080 tokens and logged the fallback line zero times. GET /props reports modalities.video: true for both, as designed.

The control that settles it: at --video-max-frames 4 with greedy decoding, the same question through video_url (1112 prompt tokens) and with those same four frames sent by the caller as four image_url parts (1097) returned the identical answer. The 15-token delta is the lead sentence and nothing else, so the rewritten request is the plain multi-image request the model would have received anyway.

Honest limit: neither front got these small checkpoints to articulate the motion. gemma-3-4b-it-4bit describes a red square on white and calls it constant, and the native Qwen2.5-VL and Gemma 4 paths do no better on the same clip (Qwen2.5-VL calls it "a static image ... no changes or movements"). That is the checkpoints' multi-image temporal reasoning rather than the substitution, which is what the control above separates, and the docs say so too.

On the checkpoint list: the issue names Qwen2.5-VL as the fallback checkpoint to validate against, which was true when it was filed. Qwen-VL gained a native video path in #1166, so here it is a native control instead, and it is what confirms the native families were left alone; lfm2-vl-450m-4bit took its place as the second fallback family.

Gates, all green on the final tree: cargo test --profile test-fast --features metal,accelerate over --lib multimodal::video (40), --lib server::chat_request (112), --lib server::startup (78), --lib server::media (75), --lib server::routes (394), --lib server::cli_input (144), and --bin mlxcel commands::generate (85). commands::generate_tests lives in the binary crate, not the lib, so --lib commands::generate_tests matches zero tests and the --bin scope is the correct one. The two ffmpeg-backed additions are #[ignore] by repo convention (#1172) and were run for real under MLXCEL_TEST_VIDEO=1 ... -- --include-ignored: video_part_expands_through_a_real_clip and cli_video_fallback_appends_frame_images_and_clears_video both pass. Also clean: cargo clippy --lib --tests and --bins with -D warnings, cargo fmt --all -- --check, cargo check --lib --tests.

Closes #1322

@inureyes inureyes added status:review Under review type:enhancement New features, capabilities, or significant additions priority:medium Medium priority area:inference Generation, sampling, decoding (incl. speculative, DRY) area:cli Command-line interface / CLI flags modeltype:vlm Vision-language model labels Sep 10, 2026
@inureyes
inureyes force-pushed the feature/issue-1322-video-frames-fallback branch from 9ca2190 to 254bf27 Compare September 10, 2026 13:01
inureyes added a commit that referenced this pull request Sep 10, 2026
Bilingual report for the video-to-images fallback: where the native / fallback split had to live so the CLI and the server read one list, why the rewrite sits between the capability gate and the render, and the two wiring points left alone on purpose. Records the bounded decode the security pass produced, the budget checks that moved ahead of the decode work, and the measurement that says the bounded path keeps the same frames: every validation number, on four checkpoints, produced twice and unmoved.

Also records the fixture that lied. The first synthetic clip was eight seconds of blank white, and the run around it looked like a clean pass on every signal except the answers.

Refs #1322
@inureyes inureyes added status:done Completed and removed status:review Under review labels Sep 10, 2026
A `video_url` block and `--video` were refused by every family except the handful with a temporal processor, although each refusing family reads a sequence of stills perfectly well. A checkpoint with a vision tower and no native video path now decodes the clip, keeps at most `--video-max-frames` evenly spaced frames (default 16, minimum 2, first and last always kept), and rewrites them into that many `image_url` parts in chronological order behind a sentence naming the frame count.

The rewrite runs on the request itself, after the media capability gate and before the template renders, so the prompt carries one image placeholder per frame and the per-request image limit and the prompt-cache multimodal digest treat the frames as the images they now are. `ModelMediaSupport::video` splits into `video_native` and `video_frames_fallback` with a `video()` accessor keeping the old meaning for the HTTP gate, and the native-video family list moves next to `get_model_type` so the CLI and the server read one list.

Native families (Gemma 4 VL / Unified, Inkling, Kimi-VL, Qwen-VL) are untouched and keep `prepared.videos`. Muse Glimmer is excluded, matching the CLI refusal it already carries. New knobs: `--video-max-frames` on `generate`, `serve` and `mlxcel-server`, plus server-side `--video-fps`.

Refs #1322
The video-to-images fallback decoded a clip at the caller's fps and only then threw most of it away. `load_video_source` answers an fps with up to `FPS_MAX_FRAMES` (768) full-resolution frames held at once, and the fallback keeps `--video-max-frames` (16) of them, so at the default caps a 600 s 4096x4096 clip built 768 frames of RGB, tens of gigabytes, to send sixteen. The default 2.0 fps reaches that ceiling on any clip past about six minutes, so no unusual `video_url.fps` is needed to trigger it. `load_video_source_frames_fallback` probes, computes the same fps-implied sample count for the log line, and decodes only the frames that survive the cap. A uniform sample of a uniform sample is one, so the kept frames are the same even spread over the clip.

The per-request image budget was also enforced after every clip in the body had been decoded and PNG-encoded, so a body carrying more `video_url` parts than the image limit can hold paid for all of them before the refusal. The count check now runs before the first ffprobe, and the frame check runs per clip, so the work stops at the clip that breaks the budget.

Refs #1322
The CLI half of the video-to-images fallback still decoded a clip at `--fps` and threw most of it away, which is the defect the server half was just fixed for: `load_video` answers an fps with up to `FPS_MAX_FRAMES` full-resolution frames held at once, and the fallback keeps `--video-max-frames` of them. It now goes through the same `load_video_source_frames_fallback` helper via `VideoSource::from_path`. That bounds the peak, and it also makes the two fronts choose the same frames out of the same clip at the same settings, which is worth more than the memory: a CLI run and a server request were free to disagree about what the model saw.

On the server, the per-clip budget refusal ran after the line reporting the clip as sent, so a request refused for exceeding the per-request image budget still left an info log saying the clip that broke the budget had been sent as frames. The check moves ahead of the line.

Re-validated against real checkpoints after the change, and every number is unmoved: `gemma-3-4b-it-4bit` still logs `sending 8 of 16 sampled frames` and reports 19 / 279 / 2114 prompt tokens for text, text plus one image, and text plus the clip; `gemma-4-e4b-it-4bit` still answers natively at 1080 with no fallback line; and the four-frame control still returns the identical answer at 1112 tokens through `video_url` and 1097 with the frames sent as images.

Refs #1322
Bilingual report for the video-to-images fallback: where the native / fallback split had to live so the CLI and the server read one list, why the rewrite sits between the capability gate and the render, and the two wiring points left alone on purpose. Records the bounded decode the security pass produced, the budget checks that moved ahead of the decode work, and the measurement that says the bounded path keeps the same frames: every validation number, on four checkpoints, produced twice and unmoved.

Also records the fixture that lied. The first synthetic clip was eight seconds of blank white, and the run around it looked like a clean pass on every signal except the answers.

Refs #1322
@inureyes
inureyes force-pushed the feature/issue-1322-video-frames-fallback branch from 197c65d to 218bfe8 Compare September 10, 2026 13:17
@inureyes
inureyes merged commit 41a0adc into main Sep 10, 2026
13 checks passed
@inureyes
inureyes deleted the feature/issue-1322-video-frames-fallback branch September 10, 2026 13:20
inureyes added a commit that referenced this pull request Sep 10, 2026
Bilingual pre-merge report for the settings-schema roster and drafter-refusal drift repair, committed into the branch because the repository carries TECHNICAL_REPORTS/.keep-reports.

The analytical content is why a per-PR test scope cannot see cross-file contract drift. A scope derived from a diff excludes exactly the files that assert about what the diff changed, and neither defect existed on its author's branch: #1759 was correct against the roster it was written against and #1751's assertion was correct against the message on its base, so the drift is a property of the merged state alone. The two misses have different shapes, one a module-filter miss inside the same --lib target and one a target miss on an integration binary --lib never builds, and the workspace gate run on merged main is the only layer where the merged state and the full target set meet. The report also records why the third gate failure (#1769, the deterministic mxfp8 tolerance from #1742) is a host toolchain result rather than drift, and therefore keeps its own PR and its own standard of evidence.

Refs #1322, #1339, #1343, #1769.
inureyes added a commit that referenced this pull request Sep 10, 2026
## Summary

The orchestrator-side workspace gate (`cargo test --workspace --profile test-fast --features metal,accelerate`) on main `58dc9693`, after the twelve merges of 2026-09-10, reported 10981 passed and 3 failed. Two of the failures are cross-file drift that the per-PR narrow test scopes could not see; this PR repairs both. The third (`fp8_block_requantize_matches_direct_path`, deterministic on this M5 Max under macOS 27) predates these merges and is tracked in #1769.

## What changed

- `src/server/runtime_settings.rs`: `video_max_frames` (Int) and `video_fps` (Float) from #1759 join `CLASSIFIED_SERVER_CONFIG_FIELDS` in `ServerConfig` source order, with read-only values and the model-side startup reason, so the management API schema lists them.
- `src/server/runtime_settings_tests.rs`: the roster test is renamed to `server_config_schema_classifies_all_107_fields` and its three counts move from 105 to 107.
- `tests/speculative_dispatch.rs`: the `-m <drafter>` refusal assertion matches the wording #1751 introduced (`DFlash-family speculative drafter`), the same substring `detection_tests.rs` already asserts.

## Validation

`cargo test --profile test-fast --features metal,accelerate --lib server::runtime_settings` (8 passed) and `--test speculative_dispatch` (22 passed), `cargo clippy --lib --tests --features metal,accelerate -- -D warnings`, `cargo fmt --all -- --check`, all clean. The full workspace gate is re-run on main after this merges; with #1769 still open it is expected to report exactly that one failure.

Refs #1322, #1339, #1343, #1769
inureyes added a commit that referenced this pull request Sep 10, 2026
## Summary

Fixes the five follow-ups #1759 left on the video-frames fallback. The CLI and the server now render the same prompt for the same clips, and the server's request path neither blocks nor copies the body for it.

## What changed

- **Per-clip lead sentence (CLI).** `video_frames_lead_text` moved to `multimodal::video` and both fronts call it. `CliVideoFrames` holds one entry per clip. A new `CliPromptMedia` layout renders each clip's sentence immediately ahead of that clip's frames, matching `apply_video_frame_expansion`. The issue's plan to prepend the sentences to `-p` would not have worked, because the CLI renders every image before the text item. `-p` itself is no longer rewritten.
- **One flattening for templates without image items.** Both fronts now go through `flatten_template_text` (`server::chat_template`). Before this, the CLI joined the parts with `\n\n` and the server with no separator, so families such as internvl3, deepseek-vl2, fastvlm, molmo and dots.ocr got different prompts on the two fronts. The CLI's no-template path now uses the same join.
- **Private frames (CLI).** A new `PrivateTempDir` (std only) creates a per-run 0700 directory whose prefix and file names must be plain names. It writes each PNG with `create_new` at 0600 and removes the directory on drop.
- **No blocking work on a worker (server).** At startup the server resolves `MLXCEL_VIDEO_DIR_ALLOWLIST` into `AppState`, warms the ffmpeg probe, and warns about writable allowlist directories. The router does the same on each model load. `extract_chat_video_paths` checks a non-allocating `has_video_urls()` and returns early for bodies without a clip. Operator-visible change: the fallback reads the allowlist once, so a directory created later needs a restart (documented).
- **No request clone.** `render_chat_prompt` takes the request by value and expands it in place.
- **Cancellable decode.** `expand_request_video_parts` creates a `CancellationToken` only when the request actually has a clip to expand, and holds its drop guard in the handler future. The token is checked before the clip loop, before each clip, and when the blocking decode starts. A correction to the issue: the guard fires only when the future is dropped, which already stops the loop, so the check that matters in production is at decode start, where a queued decode now never begins. `Cancelled` maps to 499 as defensive code; no live route can receive it.

## Validation

- Workspace gate on 0dad212: 121 of 122 binaries ran clean (9,297 passed). The `mlxcel-core` lib binary aborted mid-run with `[METAL] Command buffer execution failed: Discarded (victim of GPU error/recovery) (kIOGPUCommandBufferCallbackErrorInnocentVictim)`, the error macOS reports when another process's GPU fault forces a recovery. Re-run alone, that binary passes 1,731/1,731. This PR does not touch `mlxcel-core`. On c5204bf the gate passed in one run: 123 binaries, 11,024 passed. Workspace clippy with `-D warnings`, fmt and the `make verify-*` contract checks are clean.
- Revert arms: each new test fails by name with its fix reverted, including the new parity tests against the old `\n\n` join and the prefix check.
- Real run with `gemma-3-4b-it-4bit`, two synthetic clips (the red square moves left to right, the blue one top to bottom; first and last frames checked by eye) and `--video-max-frames 4`:
  - After this PR: the CLI `Prompt tokens:` and the server's `usage.prompt_tokens` are both 2,129. The CLI prints one frame line per clip. The server returns HTTP 200, and `/apply-template` shows two lead sentences and eight image markers.
  - A router-mode server (`--models-dir` over the store) gives the same 2,129 for the same body.
  - Before this PR the CLI read 2,115 against the server's 2,129, the cost of one lead sentence.
- `/v1/chat/completions/input_tokens` returns 57 for this body, before and after this PR. It counts the rendered template text without expanding image soft tokens. That is a separate property of the route, not a regression here, and it is noted as a follow-up rather than changed.

## Not changed (follow-ups)

- The server's native-video `write_video_temp_file` still writes a 0644 file that a disconnect can leave behind.
- The native-video path still reads the allowlist per request (off the request thread).
- `input_tokens` does not count image soft tokens.
- A template that takes `video` or `audio` items but not `image` still receives the content list on the server and a flattened string on the CLI.

Closes #1766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:cli Command-line interface / CLI flags area:inference Generation, sampling, decoding (incl. speculative, DRY) modeltype:vlm Vision-language model priority:medium Medium priority status:done Completed type:enhancement New features, capabilities, or significant additions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server): send sampled video frames as ordered images when the model has no native video path

1 participant