Skip to content

Commit dfc03ff

Browse files
committed
fix(mtmd/qwen3vl): validate image_max_tiles at library boundary, scope to Qwen3VL
Addresses review on PR #175: - Clamp the preproc_max_tiles override to [1,256] in clip_init. Bindings populate clip_context_params directly and bypass the CLI's range check; an unbounded value reached the O(max_tiles*log max_tiles) grid-fitting reserve in mtmd-image.cpp and could request hundreds of GB -> std::bad_alloc -> process crash. - Extract a shared CLIP_PREPROC_MAX_TILES_LIMIT (clip-impl.h) and use it at both the GGUF-read and override sites so the bound can't drift. - Scope the override to PROJECTOR_TYPE_QWEN3VL (the only preprocessor that reads the field live); warn and ignore for other projectors. Drop the dead InternVL min-tiles clamp (InternVL reads a candidate list frozen in load_hparams, not the live field). - Gate the "set --image-max-tiles" suggestion in the shared Qwen2/2.5/3 case to Qwen3VL only; the flag is inert for Qwen2/2.5VL (dyn_size).
1 parent f64769e commit dfc03ff

2 files changed

Lines changed: 38 additions & 16 deletions

File tree

tools/mtmd/clip-impl.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717

1818
#define MTMD_INTERNAL_HEADER
1919

20+
// Upper bound on preproc_max_tiles, enforced at every site that sets it (GGUF read,
21+
// CLI/binding override). A larger value would flow into the O(max_tiles·log max_tiles)
22+
// grid-fitting reserve in mtmd-image.cpp and can request hundreds of GB -> std::bad_alloc.
23+
constexpr int CLIP_PREPROC_MAX_TILES_LIMIT = 256;
24+
2025
#define KEY_FTYPE "general.file_type"
2126
#define KEY_NAME "general.name"
2227
#define KEY_DESCRIPTION "general.description"

tools/mtmd/clip.cpp

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,19 +1422,27 @@ struct clip_model_loader {
14221422
hparams.preproc_max_tiles = 4;
14231423
const bool has_max_tiles_key = gguf_find_key(ctx_gguf.get(), KEY_PREPROC_MAX_TILES) >= 0;
14241424
get_u32(KEY_PREPROC_MAX_TILES, hparams.preproc_max_tiles, false);
1425-
if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > 256)) {
1426-
LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,256] in GGUF — defaulting to 4\n",
1427-
__func__, hparams.preproc_max_tiles);
1425+
if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT)) {
1426+
LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,%d] in GGUF — defaulting to 4\n",
1427+
__func__, hparams.preproc_max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
14281428
hparams.preproc_max_tiles = 4;
14291429
}
14301430
if (!has_max_tiles_key && (hparams.n_head > 16 || hparams.n_layer > 24)) {
14311431
const char * model_name =
14321432
model.proj_type == PROJECTOR_TYPE_QWEN2VL ? "Qwen2VL" :
14331433
model.proj_type == PROJECTOR_TYPE_QWEN25VL ? "Qwen2.5VL" : "Qwen3VL";
1434-
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1435-
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
1436-
"If using an 8B+ model, set the correct value with --image-max-tiles.\n",
1437-
__func__, model_name, hparams.n_head, hparams.n_layer);
1434+
// preproc_max_tiles is only read by the Qwen3VL preprocessor; Qwen2/2.5VL
1435+
// use dyn_size, so --image-max-tiles is inert there and must not be suggested.
1436+
if (model.proj_type == PROJECTOR_TYPE_QWEN3VL) {
1437+
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1438+
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
1439+
"If using an 8B+ model, set the correct value with --image-max-tiles.\n",
1440+
__func__, model_name, hparams.n_head, hparams.n_layer);
1441+
} else {
1442+
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1443+
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4.\n",
1444+
__func__, model_name, hparams.n_head, hparams.n_layer);
1445+
}
14381446
}
14391447
// ref: https://huggingface.co/Qwen/Qwen2.5-VL-7B-Instruct/blob/main/preprocessor_config.json
14401448
hparams.set_limit_image_tokens(8, 4096);
@@ -2762,16 +2770,25 @@ struct clip_init_result clip_init(const char * fname, struct clip_context_params
27622770
if (loader.has_vision) {
27632771
ctx_vision = new clip_ctx(ctx_params);
27642772
loader.load_hparams(ctx_vision->model, CLIP_MODALITY_VISION);
2765-
// apply CLI overrides after load_hparams so GGUF defaults don't clobber them
2773+
// apply CLI/binding overrides after load_hparams so GGUF defaults don't clobber them.
2774+
// Only the Qwen3VL preprocessor reads preproc_max_tiles live; Qwen2/2.5VL use dyn_size
2775+
// and InternVL uses a candidate list already frozen in load_hparams, so the override is
2776+
// inert for them — apply (and validate) it only where it takes effect.
27662777
if (ctx_params.image_max_tiles != -1) {
2767-
ctx_vision->model.hparams.preproc_max_tiles = ctx_params.image_max_tiles;
2768-
LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, ctx_vision->model.hparams.preproc_max_tiles);
2769-
// models with a min-tiles floor (e.g. InternVL) would produce an empty candidate
2770-
// set if the override drops max below min; clamp min so the invariant holds.
2771-
if (ctx_vision->model.hparams.preproc_min_tiles > ctx_vision->model.hparams.preproc_max_tiles) {
2772-
LOG_WRN("%s: --image-max-tiles=%d is below the model's preproc_min_tiles=%d; clamping min\n",
2773-
__func__, ctx_params.image_max_tiles, ctx_vision->model.hparams.preproc_min_tiles);
2774-
ctx_vision->model.hparams.preproc_min_tiles = ctx_vision->model.hparams.preproc_max_tiles;
2778+
if (ctx_vision->model.proj_type != PROJECTOR_TYPE_QWEN3VL) {
2779+
LOG_WRN("%s: --image-max-tiles is only supported for Qwen3VL; ignoring for this model\n", __func__);
2780+
} else {
2781+
int max_tiles = ctx_params.image_max_tiles;
2782+
// Re-validate at this library boundary: bindings populate clip_context_params
2783+
// directly and bypass the CLI's [1,256] check. An unbounded value reaches the
2784+
// grid-fitting reserve in mtmd-image.cpp and can OOM the process.
2785+
if (max_tiles < 1 || max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT) {
2786+
LOG_WRN("%s: --image-max-tiles=%d out of range [1,%d]; clamping\n",
2787+
__func__, max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
2788+
max_tiles = std::min(std::max(max_tiles, 1), CLIP_PREPROC_MAX_TILES_LIMIT);
2789+
}
2790+
ctx_vision->model.hparams.preproc_max_tiles = max_tiles;
2791+
LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, max_tiles);
27752792
}
27762793
}
27772794
loader.load_tensors(*ctx_vision);

0 commit comments

Comments
 (0)