Skip to content

Commit ffb6ef4

Browse files
yingying0906makaveli10
authored andcommitted
fix(mtmd/qwen3vl): validate image_max_tiles at library boundary, scope to Qwen3VL
Addresses review on PR tetherto#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 77da9b7 commit ffb6ef4

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
@@ -26,6 +26,11 @@
2626

2727
#define MTMD_INTERNAL_HEADER
2828

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

tools/mtmd/clip.cpp

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,19 +1632,27 @@ struct clip_model_loader {
16321632
hparams.preproc_max_tiles = 4;
16331633
const bool has_max_tiles_key = gguf_find_key(ctx_gguf.get(), KEY_PREPROC_MAX_TILES) >= 0;
16341634
get_u32(KEY_PREPROC_MAX_TILES, hparams.preproc_max_tiles, false);
1635-
if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > 256)) {
1636-
LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,256] in GGUF — defaulting to 4\n",
1637-
__func__, hparams.preproc_max_tiles);
1635+
if (has_max_tiles_key && (hparams.preproc_max_tiles < 1 || hparams.preproc_max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT)) {
1636+
LOG_WRN("%s: clip.vision.preproc_max_tiles=%d out of range [1,%d] in GGUF — defaulting to 4\n",
1637+
__func__, hparams.preproc_max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
16381638
hparams.preproc_max_tiles = 4;
16391639
}
16401640
if (!has_max_tiles_key && (hparams.n_head > 16 || hparams.n_layer > 24)) {
16411641
const char * model_name =
16421642
model.proj_type == PROJECTOR_TYPE_QWEN2VL ? "Qwen2VL" :
16431643
model.proj_type == PROJECTOR_TYPE_QWEN25VL ? "Qwen2.5VL" : "Qwen3VL";
1644-
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1645-
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
1646-
"If using an 8B+ model, set the correct value with --image-max-tiles.\n",
1647-
__func__, model_name, hparams.n_head, hparams.n_layer);
1644+
// preproc_max_tiles is only read by the Qwen3VL preprocessor; Qwen2/2.5VL
1645+
// use dyn_size, so --image-max-tiles is inert there and must not be suggested.
1646+
if (model.proj_type == PROJECTOR_TYPE_QWEN3VL) {
1647+
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1648+
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4. "
1649+
"If using an 8B+ model, set the correct value with --image-max-tiles.\n",
1650+
__func__, model_name, hparams.n_head, hparams.n_layer);
1651+
} else {
1652+
LOG_WRN("%s: %s large ViT detected (n_head=%d, n_layer=%d) but "
1653+
"clip.vision.preproc_max_tiles not in GGUF — defaulting to 4.\n",
1654+
__func__, model_name, hparams.n_head, hparams.n_layer);
1655+
}
16481656
}
16491657
// ref: https://huggingface.co/Qwen/Qwen2.5-VL-7B-Instruct/blob/main/preprocessor_config.json
16501658
hparams.set_limit_image_tokens(8, 4096);
@@ -3491,16 +3499,25 @@ struct clip_init_result clip_init(const char * fname, struct clip_context_params
34913499
if (loader.has_vision) {
34923500
ctx_vision = new clip_ctx(ctx_params);
34933501
loader.load_hparams(ctx_vision->model, CLIP_MODALITY_VISION);
3494-
// apply CLI overrides after load_hparams so GGUF defaults don't clobber them
3502+
// apply CLI/binding overrides after load_hparams so GGUF defaults don't clobber them.
3503+
// Only the Qwen3VL preprocessor reads preproc_max_tiles live; Qwen2/2.5VL use dyn_size
3504+
// and InternVL uses a candidate list already frozen in load_hparams, so the override is
3505+
// inert for them — apply (and validate) it only where it takes effect.
34953506
if (ctx_params.image_max_tiles != -1) {
3496-
ctx_vision->model.hparams.preproc_max_tiles = ctx_params.image_max_tiles;
3497-
LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, ctx_vision->model.hparams.preproc_max_tiles);
3498-
// models with a min-tiles floor (e.g. InternVL) would produce an empty candidate
3499-
// set if the override drops max below min; clamp min so the invariant holds.
3500-
if (ctx_vision->model.hparams.preproc_min_tiles > ctx_vision->model.hparams.preproc_max_tiles) {
3501-
LOG_WRN("%s: --image-max-tiles=%d is below the model's preproc_min_tiles=%d; clamping min\n",
3502-
__func__, ctx_params.image_max_tiles, ctx_vision->model.hparams.preproc_min_tiles);
3503-
ctx_vision->model.hparams.preproc_min_tiles = ctx_vision->model.hparams.preproc_max_tiles;
3507+
if (ctx_vision->model.proj_type != PROJECTOR_TYPE_QWEN3VL) {
3508+
LOG_WRN("%s: --image-max-tiles is only supported for Qwen3VL; ignoring for this model\n", __func__);
3509+
} else {
3510+
int max_tiles = ctx_params.image_max_tiles;
3511+
// Re-validate at this library boundary: bindings populate clip_context_params
3512+
// directly and bypass the CLI's [1,256] check. An unbounded value reaches the
3513+
// grid-fitting reserve in mtmd-image.cpp and can OOM the process.
3514+
if (max_tiles < 1 || max_tiles > CLIP_PREPROC_MAX_TILES_LIMIT) {
3515+
LOG_WRN("%s: --image-max-tiles=%d out of range [1,%d]; clamping\n",
3516+
__func__, max_tiles, CLIP_PREPROC_MAX_TILES_LIMIT);
3517+
max_tiles = std::min(std::max(max_tiles, 1), CLIP_PREPROC_MAX_TILES_LIMIT);
3518+
}
3519+
ctx_vision->model.hparams.preproc_max_tiles = max_tiles;
3520+
LOG_INF("%s: preproc_max_tiles: %d (custom value)\n", __func__, max_tiles);
35043521
}
35053522
}
35063523
loader.load_tensors(*ctx_vision);

0 commit comments

Comments
 (0)