Skip to content

mtmd : fix output-buffer size for multi-image batches without temporal merge#24634

Closed
mayerwin wants to merge 1 commit into
ggml-org:masterfrom
mayerwin:fix-mtmd-multiimage-ntokens
Closed

mtmd : fix output-buffer size for multi-image batches without temporal merge#24634
mayerwin wants to merge 1 commit into
ggml-org:masterfrom
mayerwin:fix-mtmd-multiimage-ntokens

Conversation

@mayerwin

Copy link
Copy Markdown

Summary

mtmd_image_tokens::n_tokens() under-counts the output tokens for a multi-image batch
when the model does not use temporal merge, so clip_image_batch_encode() aborts with
Output buffer size mismatch.

Repro

Send a chat request with two images to a model whose image entries get batched into a
single clip_image_batch_encode call and that has n_temporal_merge == 1 (e.g. Gemma-4
vision):

clip.cpp: Output buffer size mismatch
clip_image_batch_encode: output buffer has 322560 elements but expected 645120
GGML_ABORT

(322560 = 1*126*2560, 645120 = 2*126*2560 — the buffer was sized for one image while the
encoder produced two; embeddings->ne[2] == entries.size() == 2.)

Root cause

mtmd_encode_impl() sizes the output buffer as n_embd_out * image_tokens->n_tokens().
n_tokens() returns nx*ny (a single entry) whenever n_temporal_merge == 1, ignoring
batch_f32.entries.size(), but clip_image_batch_encode() produces
entries.size() * nx * ny tokens.

Fix

Multiply by the entry count when there is no temporal merge. The single-entry path and the
temporal-merge branch (entries divided by the merge factor) are unchanged.

Testing

Built on Intel Arc (SYCL). Before: every 2-image Gemma-4 request aborts. After: 2-image
requests encode all entries and return normally; single-image paths (e.g. Qwen3-VL) unchanged.

@mayerwin mayerwin requested a review from a team as a code owner June 15, 2026 03:06
@ggml-gh-bot

ggml-gh-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

Hi @mayerwin, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

…l merge

mtmd_image_tokens::n_tokens() returned a single image's token count
(nx*ny) whenever n_temporal_merge == 1, even when batch_f32 held multiple
image entries. mtmd_encode_impl() uses n_tokens() to size the output
embedding buffer, but clip_image_batch_encode() produces
entries.size() * nx * ny tokens, so any request batching multiple images
(e.g. two images in one message) aborted with
"clip.cpp: Output buffer size mismatch". Reproduced with Gemma-4 vision.

Multiply by the entry count when there is no temporal merge. The
temporal-merge branch and the single-entry path are unchanged.
@mayerwin mayerwin force-pushed the fix-mtmd-multiimage-ntokens branch from d0e6f07 to 0e7f29e Compare June 15, 2026 03:28
@mayerwin

mayerwin commented Jun 15, 2026

Copy link
Copy Markdown
Author

Disclosure, per the contribution guidelines: yes, I used an AI coding assistant on this, and I'd rather be upfront about that than hide it.

What that meant in practice: the fix is four lines in mtmd_image_tokens::n_tokens(). The work was almost entirely diagnosis, many hours and, honestly, millions of tokens spent reproducing the GGML_ABORT, ruling out offload / resolution / -ub / audio-mmproj / backend (it reproduces on both SYCL and Vulkan), instrumenting clip_image_batch_encode, and tracing the Output buffer size mismatch back to the n_temporal_merge == 1 short-circuit incorrectly covering the plain multi-image / multi-tile case. The AI was a tool in that loop; the investigation, the on-hardware validation (Intel Arc, Gemma-4 vision, 2-image requests), and the decision on the correct fix were mine.

The bug is real and reproducible on current master for any 2+ image request to a non-temporal-merge model (Gemma-3/4 vision), and it's a hard abort that arguably shouldn't have been there in the first place. If you'd prefer a different formulation of the fix, I'm happy to adjust it, mostly I hope this saves the next person the time it cost me.

@ngxson

ngxson commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

the fix is incorrect, it will break qwen-vl model. I'll push another fix

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants