Repository navigation
[Model] Cap the decoded size of request images and videos - #510
Conversation
arun-elr
changed the title
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| * - ``MSTAR_MAX_IMAGE_PIXELS`` | ||
| - ``89478485`` | ||
| - Pixels one request image may decode to, checked from the header | ||
| before decoding; larger images get a 400. A GIF is read as its first | ||
| frame. Applies behind either frontend. Pillow refuses images over | ||
| twice its own limit (about 179M pixels) whatever this is set to. | ||
| * - ``MSTAR_MAX_VIDEO_PIXELS`` | ||
| - ``268435456`` | ||
| - Pixels one request video may decode to, summed over the frames the | ||
| model's loader decodes (about 10 s of 720p30 for loaders that keep | ||
| every frame); larger videos get a 400. Applies behind either frontend. |
There was a problem hiding this comment.
[P2] The new rows don't name the function that reads each variable.
AGENTS.md invariant 9 asks each new MSTAR_* row to name its reader. Here that is mstar.model.media_limits.max_image_pixels and max_video_pixels. The other rows do this with :func:. These two also sit under "Serving (Python frontend) — Read by mstar-serve / mstar.api_server.entrypoint". In fact the model loaders read them in the data worker's preprocess thread, and the rows themselves say they apply behind either frontend.
There was a problem hiding this comment.
Fixed: both rows now sit in their own "Request media" section, which says they are read by the model loaders in the data worker's preprocess thread, and each row names its reader (mstar.model.media_limits.max_image_pixels / max_video_pixels).
| def _positive_int_env(name: str, default: int) -> int: | ||
| raw = os.environ.get(name) | ||
| if raw is None or raw.strip() == "": | ||
| return default | ||
| value = int(raw) | ||
| if value <= 0: | ||
| raise ValueError(f"{name} must be a positive integer; got {raw!r}") | ||
| return value |
There was a problem hiding this comment.
[P2] A misconfigured limit comes back as a client 400 on every media request, and the server still starts.
max_image_pixels() / max_video_pixels() are read per request, inside check_image_size and VideoBudget(). Say MSTAR_MAX_VIDEO_PIXELS=lots or 0. The server starts fine. Then every video request raises ValueError in the preprocess thread, and _fail_request (data_worker.py:688) turns that into a 400. The client is told its request was bad, and the error text is a server env var message. Image requests behave the same way for MSTAR_MAX_IMAGE_PIXELS. Reading and validating both values once, at import or at data-worker startup, would make a bad deploy fail at boot.
There was a problem hiding this comment.
Fixed: PreprocessWorker now reads and validates both values when it is built (check_media_limits), so an invalid value such as lots or 0 stops the server at startup behind either frontend. The error names the variable, and a test covers it.
Automated reviewThe PR adds Findings: 2 inline (2 × P2). The description is accurate and says which tests were run and what they returned. The tightened default for the base full-decode video loader, which Cosmos3-Edge and any other model relying on Questions
Advisory, not a merge gate. Rules: |
Only the Cosmos3 family (Cosmos3, Cosmos3-Edge and their variants), through
The table is in the description under "Video default: why 2^30". Sampling before decoding in Cosmos3 would bound this much lower and could follow separately. |
Live test: [Model] Cap the decoded size of request images and videosThe image caps work as described (400 naming the limit, decompression bomb refused in 0.1 s where main spent 20 s and 4.3 GB of API server RSS, GIFs fixed, bad env value stops startup). One open question: Qwen3-Omni video replies moved on the PR and not on main. 1x H200, Claims
Other
|
|
Thanks for the live test.
On the torchcodec path the PR only adds a header read (
Added to a new Not tested line. The V-JEPA 2 refusal was served on a B200 (in the description); the others rest on the CPU tests. |
9687e9a to
b0e359a
Compare
Each decoded frame costs about 2 KB of tensor and frame objects whatever its size, so a 4.2 MB clip of 300k 2x2 frames took 596 MB through the Cosmos3 PyAV fallback, and the default 2^30 cap admitted about 268M such frames. |
There was a problem hiding this comment.
Tested b0e359ad merged into main on h200s (Qwen3-Omni, Cosmos3-Edge, V-JEPA 2, Qwen3.5) + the CPU tier on Laude, 3bab74e1 (64x64 minimum per frame) re-checked on CPU. Caps work on every loader and normal images decode identical to main
Also settled the Qwen3-Omni question: frames are torch.equal to main on 7 clips, and two main servers differ from each other on the same 6/24 prompts as main vs PR, so it's cross-process noise main already has
A few things:
- The cap is per file, not per request. 8 copies of a 236 KB 9000x9000 PNG in one chat request takes the API server to 13 GB (same on main). A per-request total in
_process_input(data_worker.py:513) + an attachment cap would close it, here or in a follow-up MSTAR_MAX_IMAGE_PIXELSabove ~179M doesn't do anything, Pillow's own bomb check fires first but the 400 still names the knob's limit. SettingImage.MAX_IMAGE_PIXELSto it incheck_media_limits()fixes it- BMP / TIFF still 500 in
decode_image(media_limits.py:98),formats=("PNG", "JPEG", "WEBP", "GIF")on the check'sImage.openturns them into the 400 - worth putting the 1080p ceilings in the
MSTAR_MAX_VIDEO_PIXELSrow: Cosmos3 refuses 1080p30 over 17 s, Qwen3-Omni over 259 s - nits:
read_video_whole_budgetedbelongs in qwen3_omni/,probe_videoscans the whole file (9.6 vs 2.0 ms withseek_mode="approximate"), vjepa2_model.py:635 checks after allocating
fyi qwen-omni-utils' own fallback reader is already broken on main (torchvision 0.27.1 has no read_video), so yours is the only one that works, worth a line in the description
LGTM otherwise
3bab74e to
3836bc8
Compare
|
Thanks for testing this so thoroughly. Changes since the approval: rebased on main, plus the five points below. A request with one PNG, JPEG, still WebP or GIF file, or one video, is handled as before.
Fixed:
Fixed:
Fixed: the header check opens with
Done: the row now gives both (Cosmos3 over about 17 s, Qwen3-Omni over about 259 s, at 1080p30).
Done, all three: the reader is now
Added to the description. Tests: |
efc39be to
1a31158
Compare
cc760e3 to
2130af3
Compare
|
@vasilevklart, two gaps in this PR's loaders came up while checking #647, which is stacked on it. Both are fixed in the top two commits:
Tests: 5 new cases in A quick re-look at the two commits would be much appreciated. |
108c690 to
3ae04fe
Compare
merceod
left a comment
merceod
left a comment
There was a problem hiding this comment.
Served on klone (4ec834f for the cells; the two newest commits, the FLUX.2-klein BMP/TIFF fix and the Qwen3-Omni clip without a header frame count, were read, not served). Qwen3-Omni: the 20000x20000 PNG bomb is a 400 in 9 ms where main took 2.45 s and +4.3 GB and then a 500; 16-bit PNGs read right after 4ec834f (the earlier heads turned them white). Cosmos3: the full-decode loaders refuse after decoding up to the cap, +3.1 GB against main's +26.9 GB. CPU tests pass, ruff clean. Not run: Qwen3-Omni video (the venv lacks qwen_omni_utils), FLUX.2-klein, Waypoint and V-JEPA 2.
Notes: the 89.5 MP default refuses 108 and 200 MP phone photos, as documented. The full-decode loaders bound memory after the fact rather than before, documented too.
5fb1685 to
de029d3
Compare
Check an image's header against MSTAR_MAX_IMAGE_PIXELS before decoding it, and count decoded video frames against MSTAR_MAX_VIDEO_PIXELS, so a request is refused with a 400 before its media is held in memory.
…he video default to 2^30 A bad MSTAR_MAX_IMAGE_PIXELS or MSTAR_MAX_VIDEO_PIXELS now stops the data worker at startup instead of turning every media request into a 400. The docs rows move to their own section and name the functions that read them. 2^28 refused 31 of the 34 clips in a default Video-MME run (640x360, up to 2 min) on Cosmos3, whose loader decodes every frame. 2^30 fits all of them.
qwen-omni-utils' fetch_video falls back to its torchvision reader when torchcodec or decord raises, and FORCE_QWENVL_VIDEO_READER=torchvision selects it. That reader decodes every frame, while the Qwen3-Omni loader only checked the frames it samples, and a header can understate the clip. The reader is replaced with one that decodes the same frames with PyAV and samples them the same way, counting each frame against the budget as it is decoded.
The video budget summed pixels only, but each decoded frame costs about 2 KB of tensor and frame objects whatever its size: a 4.2 MB clip of 300k 2x2 frames took 596 MB through the PyAV fallback, and the default 2^30-pixel cap admitted about 268M such frames. A frame now counts as at least MIN_FRAME_PIXELS (64x64), which bounds the frame count at 262k under the default cap and leaves frames of that size or larger unchanged.
The image and video pixel caps now hold for all of a request's files together, and MSTAR_MAX_MEDIA_FILES (default 16) refuses a request with more files before any is decoded (mstar-project#590). The startup check lifts Pillow's own bomb check to the image cap, the header check only opens PNG, JPEG, WebP and GIF, and an animated WebP is read with Pillow as its first frame. Also: the whole-clip reader moves to qwen3_omni/, probe_video reads torchcodec's header without a scan, and V-JEPA 2 checks before decoding when the header has no frame size, instead of after.
…in's loader reads
…n's teardown reads
de029d3 to
ee90a11
Compare
What does this PR do?
Request images and videos are decoded in the API server's preprocess thread, and on main nothing bounds what they decode to: a 236 KB PNG can declare 9000x9000, and a still clip decodes to many times its size. Any client, with no credentials (#506), can send such media, or many files in one request (#590), and stall preprocessing and output delivery for every other user, or run the API server out of memory, which fails every in-flight request. This PR checks what a request's media will decode to before decoding it and answers an oversized request with a 400 that names the limit.
MSTAR_MAX_IMAGE_PIXELS(media_limits.py, image loaders); PNG, JPEG, WebP and GIF only, where BMP or TIFF was a 500; FLUX.2 klein, which decodes with Pillow, still reads BMP and TIFF.MSTAR_MAX_VIDEO_PIXELS).MSTAR_MAX_MEDIA_FILES) in_process_input(part of [Data worker][Security] Media limits apply per file, so many files in one request multiply them #590).qwen3_omni/video_reader.py); the original decodes everything and fails on torchvision 0.27. A clip whose header gives no frame count (an MKV or WebM without one) is decoded by it too, not by the sampled readers, which would decode it uncounted.check_media_limits()and documented; Pillow's bomb check, which masked caps over 179M pixels, is lifted to the image cap.How was it tested?
On 5fb1685 (head):
ruff check .clean.python -m pytest -n 4 @test/cpu-core.txt(GPU hidden, CI's install) on 4f15aab9f, the same commits on main 8e9c61f (the two main commits since do not touch media loading): 2273 passed, 363 skipped, 3 failed (test_wan_vae_decode.py2; both fail on main 56c6fbd here;test_eval_framework.py's SIGTERM case timed out under host load and passes alone, here and on main).python -m pytest test/modular/test_media_decode_limits.py: 42 passed (the 16-bit PNG cases fail on efc39be; the FLUX.2 klein BMP and TIFF cases and the header-less MKV case fail on 2130af3). Served on one B200 (Qwen3.5-0.8B images, V-JEPA 2 videos): ordinary inputs 200, inputs over a lowered limit 400, the server keeps serving; on 9687e9a, one file per request, which the later changes count the same way.Real-user checks (head
4ec834f00): 39 threshold cases through the data worker's real loaders on CPU (images at, under and over a lowered cap, alone and summed over a request; 16 and 17 files; videos at 100 and 101 minimum frames through the base, Cosmos3 torchcodec and PyAV, V-JEPA 2 and Qwen3-Omni readers, with 2x2, 16x16 and 5,000 tiny frames) all behave as designed and no message names a path; served on one H200 through Wan 2.2 TI2V withMSTAR_MAX_IMAGE_PIXELS=1000000, 1000x1000 decodes and 1000x1001 is a 400, a request 1,000 pixels over the summed cap or with 17 files is a 400, and BMP, TIFF, empty files and animated GIF or WebP get a 400 or decode where main answers 500.Not tested, and not here
Not tested: a served refusal on Cosmos3 or Qwen3-Omni, and the Waypoint and FLUX.2 klein loaders served (CPU tests only).
Not here: a decoded total for audio, and per-model file counts checked before decoding, stay in #590.
Depends on
Depends on: none.
Design
RequestMediaBudget.file(), and every image or video check also charges the request. A file is charged the largest single check made for it, so Qwen3-Omni's header estimate and the whole-clip fallback that may follow are not added together. Outside the data worker (direct loader calls) each file is checked alone against the same caps.MSTAR_MAX_VIDEO_PIXELSrow.torchvision.io.read_video, which torchvision 0.27.1 removed. The replacement here is the only whole-clip reader that works.ValueError, which_fail_requestalready maps to a 400 error chunk.Video default: why 2^30
Only the Cosmos3 family reaches the full-decode
Model.load_video(Cosmos3Model.load_videocalls it); V-JEPA 2 and Qwen3-Omni sample before decoding, and Qwen3.5 refuses video. The repo's own video-to-text eval,benchmark.runner --dataset video_mme, sends Video-MME's short clips: the 34 clips of a default 100-request run are 640x360, 31 to 118 s, 170M to 788M decoded pixels. 31 of the 34 exceed 2^28 and none exceed 2^30.Cosmos3-Edge on one H200, measured before the request total and file count were added (a single-clip request is counted the same way since),
/v1/chat/completionswith one clip and a one-sentence question, one request at a time:XWfqBKeC0g8, 31 s (0.64)fFjv93ACGo8, 74 s (1.91)TPS22HRRY1k, 114 s (2.93)At 2^30 the replies match main's word for word, at the same peak RSS. Sampling before decoding in Cosmos3, as V-JEPA 2 does, would bound this far lower and could follow separately.
Correctness
probe_video,seek_mode="approximate"so it does not scan the file) and a pixel count before the samefetch_videocall. The review found the framestorch.equalto main's on 7 clips, and two main servers differing from each other on the same prompts as main vs this PR, so the reply differences in the earlier live test are cross-process variation main already has.fetch_video, and refused the AVI above at a 10-frame budget. With qwen-omni-utils 0.0.9 and decord 0.6.0, a 30-frame MKV whose header has no frame count now goes to this reader: refused at a 3-frame budget, and at the default it loads with the same shape as before, charged for all 30 frames (it was charged 0).Served on a B200
On 9687e9a, one B200 (torch 2.12.1, CUDA 13.0), through the normal servers, one file per request. To show a refusal without a special file, the second run of each model lowers the limit so that an ordinary larger input is over it.
/v1/chat/completionsMSTAR_MAX_IMAGE_PIXELS=1000000image is 1600x1200 (1920000 pixels); the limit is 1000000 pixels/generateMSTAR_MAX_VIDEO_PIXELS=8000000video decodes to more than 8000000 pixels; send a shorter or smaller clip (the limit is MSTAR_MAX_VIDEO_PIXELS)The server keeps serving after a refusal. Streaming requests get the same 400 as their final in-band error chunk.
Tests in detail
test/modular/test_media_decode_limits.py, intest/cpu-core.txt:MSTAR_MAX_IMAGE_PIXELS=400000000admits a 200M-pixel header after the startup check (Pillow alone refuses it), and a lowered value leaves Pillow's limit alone;MSTAR_MAX_MEDIA_FILESare refused before any is loaded;probe_videoopens torchcodec withseek_mode="approximate";The image cases use header-only files, so nothing large is allocated.
Checklist
ruff check .passesAI assistance: written with AI coding agent