feat[vLLM × v5]: Add audio support for the Transformers backend#39330
feat[vLLM × v5]: Add audio support for the Transformers backend#39330harshaljanjani wants to merge 13 commits into
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for audio models to the Transformers modeling backend by unwrapping nested CausalLM structures, extending multimodal processing metadata for audio, and refactoring embedding logic to handle audio features. It also includes a comprehensive test suite for audio model processing. A critical issue was identified in the extraction of audio embeddings, where the current implementation incorrectly selects pooled outputs instead of the full feature sequence, potentially causing a mismatch with prompt placeholders.
| if isinstance(audio_output, tuple): | ||
| audio_embeddings = audio_output[1] | ||
| elif hasattr(audio_output, "pooler_output"): | ||
| audio_embeddings = audio_output.pooler_output | ||
| else: | ||
| audio_embeddings = audio_output |
There was a problem hiding this comment.
There is an inconsistency between how audio and vision embeddings are extracted from model outputs. For vision models (line 642), the first element [0] is used, which typically corresponds to the last_hidden_state (the sequence of features). However, for audio models here (line 591), the second element [1] is used. Furthermore, line 593 explicitly selects pooler_output.
In many Transformers models, the second element of the output tuple (or the pooler_output attribute) is a single vector for the entire sequence. Using a single pooled vector instead of the full sequence of features will cause a mismatch with the number of placeholder tokens in the prompt (calculated at line 283) and lead to incorrect model behavior or poor performance. Please verify if last_hidden_state (or [0]) should be used here to ensure the full sequence of features is captured, consistent with the vision path.
| if isinstance(audio_output, tuple): | |
| audio_embeddings = audio_output[1] | |
| elif hasattr(audio_output, "pooler_output"): | |
| audio_embeddings = audio_output.pooler_output | |
| else: | |
| audio_embeddings = audio_output | |
| if isinstance(audio_output, tuple): | |
| audio_embeddings = audio_output[0] | |
| elif hasattr(audio_output, "last_hidden_state"): | |
| audio_embeddings = audio_output.last_hidden_state | |
| else: | |
| audio_embeddings = audio_output |
There was a problem hiding this comment.
Flagging this: tmk granite_speech sets audio_outputs.pooler_output = projected_embeds, i.e. the projected embeddings (the full sequence), not a single vector; probably a hallucination or codebase knowledge before its training cutoff.
There was a problem hiding this comment.
yep you're right, it's been adopted in transformers that pooler_output holds the encoder projected hidden states, but honestly, this always looked a bit odd and misleading to me (and not aligned with how it's documented), that's likely why it's hallucinating here
|
Thank you for this PR! I'm aware that @eustlb is actually doing some refactoring on the Transformers side to make audio models look more like other multimodal models (which may render the changes in We should wait for this standardisation to be completed and then we can update the PR on the vLLM side to hook into this more standardised interface. |
I would love to provide some extra bandwidth in that regard as well @eustlb!
Sure, will be on the lookout for pings and updates. |
|
Tf5 support is now merged |
|
Thanks @harshaljanjani for working on this! |
Awesome stuff @eustlb, thanks for letting me know! Will let the review rounds play out for the linked PR and start work here once it's merged to avoid a dupl of efforts. Also if I recall correctly, an issue was brought to light a couple of months back in this PR with traces; I'd love to know if there has been any standardization in that regard since we postponed the hotfix at the time :) Edit: Marking this as ready for review since the Transformers PR has now been merged. Looking forward to the review rounds once ALM standardization is complete! |
harshaljanjani
requested review from
DarkLight1337,
hmellor and
ywang96
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
harshaljanjani
force-pushed
the
feat/audio-encoder-transformers-backend
branch
from
43e5308 to
e6527d1
Compare
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
harshaljanjani
force-pushed
the
feat/audio-encoder-transformers-backend
branch
from
490a6af to
86f684d
Compare
|
Good day @hmellor @eustlb; I refactored this PR to match the ALM standard set by #45534 + verified all the tests adjacent to the change and re-ran the user-facing benchmark on it. I should note that while verifying after removing all changes in Before (broken):
After (fixed):
I'm sharing the broken logs for verification against repros. Everything works the way it did before the standardization, except it's cleaner now and the changes in Test commands: pytest tests/models/multimodal/processing/test_transformers_audio.py
pytest tests/models/multimodal/processing/test_transformers.py
python benchmark_audio.py # https://gist.github.com/harshaljanjani/d9f619683a1dfb0f41c14b4455bad514 |
hmellor
added
the
verified
| pytest.param( | ||
| "mistralai/Voxtral-Mini-3B-2507", | ||
| marks=pytest.mark.xfail( | ||
| reason="MistralCommonBackend tokenizer does not produce audio " | ||
| "placeholder token (ID 24) from text; requires " | ||
| "apply_chat_template path", | ||
| strict=False, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
So the fix would be in Transformers. The MistralCommonBackend tokenizer does not produce the audio placeholder token when tokenizing plain text containing [AUDIO]; it only works through the apply_chat_template path via mistral_common.protocol.instruct.chunk.AudioChunk. The existing dedicated-backend test at test_voxtral.py realizes this as well. Voxtral works E2E in vLLM through the dedicated Mistral backend (verified); the xfail only affects the generic Transformers-backend processing test harness which passes raw text prompts.
A quick note: I've also added fetch_audio to vLLM's MistralCommonFeatureExtractor since while trying to run Voxtral E2E with Transformers v5, profiling failed with AttributeError: 'MistralCommonFeatureExtractor' object has no attribute 'fetch_audio'. Verified Voxtral produces correct output with both v4 and v5 after the fix.
There was a problem hiding this comment.
Why don't we use the apply_chat_template method here?
Re the fetch_audio method, that seems like an orthogonal change, potentially something I'll need to add to #41359
There was a problem hiding this comment.
Why don't we use the apply_chat_template method here?
Updated the xfail reason to make this clearer; happy to drop the xfail entirely if you'd prefer. In regard to the orthogonality, agreed. Since the last review #44559 landed fetch_audio on MistralCommonFeatureExtractor so I've dropped it here. More info in the linked issue: #44554
| def get_supported_mm_limits(self): | ||
| if self._is_audio_model(): | ||
| return {"audio": None} | ||
| return {"image": None} | ||
|
|
||
| def get_mm_max_tokens_per_item(self, seq_len, mm_counts): | ||
| if self._is_audio_model(): | ||
| return {"audio": self.get_max_audio_tokens()} | ||
| return {"image": self.get_max_image_tokens()} |
There was a problem hiding this comment.
We hope to support omni models eventually, could we update these methods so that they can support any combination of supported modalities instead of just one or the other?
There was a problem hiding this comment.
The way I've addressed this is by adding _is_vision_model() (checks for image_token or boi_token on the processor; covers LLaVA-OneVision, Qwen2.5-VL, SmolVLM, Gemma3) and refactored both methods to build combined dicts. A model that is both audio and vision should now return both modalities; the fallback {"image": None} / {"image": ...} preserves BC for models that don't match either heuristic.
| # Voxtral's max_source_positions=3000 is the largest known value; | ||
| # AudioFlamingo3/GLM-ASR use 1500. Granite Speech is variable. | ||
| return 3000 |
There was a problem hiding this comment.
Instead of silently falling back to 3000, could we raise an error explaining to the user that their config is malformed and should contain either max_source_positions or max_position_embeddings?
There was a problem hiding this comment.
Makes a lot of sense, replaced the silent fallback. Also added max_pos_emb (GraniteSpeech's conformer encoder uses this name). So the current coverage looks like:
→ AudioFlamingo3: audio_config.max_source_positions = 1500
→ Voxtral: audio_config.max_source_positions = 1500
→ GLM-ASR: audio_config.max_position_embeddings = 1500
→ GraniteSpeech: encoder_config.max_pos_emb = 512
I should mention that VibeVoice hits the ValueError during model loading because it uses a ConvNet-based acoustic tokenizer with no pos embeddings at all. Its max audio tokens are computed from ceil(acoustic_tokenizer_chunk_size / hop_length), but adding a fake max_source_positions would misrepresent the architecture. For now I've gone with marking VibeVoice as an xfail in the generation test, pending a Transformers-side interface for tokenizer-based audio encoders. (it passes the processing test fine since get_max_audio_tokens() is only called during profiling).
There was a problem hiding this comment.
I see, presumably the padding mask has a max size which we could use to add a @property to the Transformers config to infer the max tokens of this model?
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Hi @harshaljanjani, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
There was a problem hiding this comment.
This doesn't really need to live in a directory since it's only one file.
Can we also flatten this so each model gets one line?
{
"ibm-granite/granite-speech-3.3-2b": {...},
"nvidia/audio-flamingo-3-hf": {...},
...
}
| "ibm-granite/granite-speech-3.3-2b", | ||
| "nvidia/audio-flamingo-3-hf", | ||
| pytest.param( | ||
| "microsoft/VibeVoice-ASR-HF", | ||
| marks=pytest.mark.xfail( | ||
| reason="ConvNet-based acoustic tokenizer has no positional " | ||
| "embeddings; get_max_audio_tokens() raises ValueError " | ||
| "during profiling", | ||
| strict=False, | ||
| ), | ||
| ), | ||
| "zai-org/GLM-ASR-Nano-2512", |
There was a problem hiding this comment.
Let's limit this to the 3 most popular ASR architectures. I suspect whisper should be in this list for example
There was a problem hiding this comment.
Dug into this:
→ As of today, LLM(model="openai/whisper-tiny", model_impl="transformers") raises ValueError: The Transformers implementation of 'WhisperForConditionalGeneration' is not compatible with vLLM caught at the resolver.
→ At first glance, flipping _supports_attention_backend=True on WhisperPreTrainedModel and adding a get_audio_features matching the ALM contract from #45534 both are doable in Transformers. But I think what's an out-of-scope blocker (vLLM side) for this PR is that Whisper is encoder-decoder with CA, WhisperProcessor has no audio_token, the prompt format (<|startoftranscript|><|en|><|transcribe|>...) carries no placeholder to splice audio embeddings into the decoder input and the Transformers backend only models decoder-only ALMs (TransformersForCausalLM/TransformersMultiModalForCausalLM) - no encoder-decoder equivalent yet. Confirmed this empirically as well; given Whisper already has its dedicated vLLM impl users get by default, I'm curious as to what you think in regard to this PR being scoped to decoder-only ALMs, or should we go further?
| pytest.param( | ||
| "mistralai/Voxtral-Mini-3B-2507", | ||
| marks=pytest.mark.xfail( | ||
| reason="MistralCommonBackend tokenizer does not produce audio " | ||
| "placeholder token (ID 24) from text; requires " | ||
| "apply_chat_template path", | ||
| strict=False, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Why don't we use the apply_chat_template method here?
Re the fetch_audio method, that seems like an orthogonal change, potentially something I'll need to add to #41359
| sub = getattr_iter(config, ("audio_config", "encoder_config")) | ||
| if sub is not None: | ||
| val = getattr_iter( | ||
| sub, | ||
| ("max_source_positions", "max_position_embeddings", "max_pos_emb"), | ||
| ) | ||
| if val is not None: | ||
| return int(val) | ||
| raise ValueError( | ||
| f"{type(config).__name__} does not have a recognized audio " | ||
| "encoder config with max_source_positions, max_position_embeddings, " | ||
| "or max_pos_emb. Please update the model config or add support " | ||
| "for this architecture in get_max_audio_tokens()." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Something like this is a bit cleaner:
| sub = getattr_iter(config, ("audio_config", "encoder_config")) | |
| if sub is not None: | |
| val = getattr_iter( | |
| sub, | |
| ("max_source_positions", "max_position_embeddings", "max_pos_emb"), | |
| ) | |
| if val is not None: | |
| return int(val) | |
| raise ValueError( | |
| f"{type(config).__name__} does not have a recognized audio " | |
| "encoder config with max_source_positions, max_position_embeddings, " | |
| "or max_pos_emb. Please update the model config or add support " | |
| "for this architecture in get_max_audio_tokens()." | |
| ) | |
| audio_config_names = ("audio_config", "encoder_config") | |
| audio_config = getattr_iter(config, audio_config_names) | |
| if audio_config is not None: | |
| names = ("max_source_positions", "max_position_embeddings", "max_pos_emb") | |
| val = getattr_iter(audio_config, names) | |
| if val is not None: | |
| return int(val) | |
| raise ValueError( | |
| f"Unable to get max input length from {type(audio_config).__name__}. " | |
| f"The following attribute names were checked: {names}." | |
| ) | |
| raise ValueError( | |
| f"Unable to get audio config from {type(config).__name__}. " | |
| f"The following audio config names were checked: {audio_config_names}." | |
| ) | |
Longer term I would like to have a get_audio_config method similar to get_text_config in PreTrainedConfig rather than exhaustiuvely checking all the names we know
There was a problem hiding this comment.
Done pretty much exactly, thanks!
| def _is_audio_model(self) -> bool: | ||
| processor = self.get_hf_processor() | ||
| return hasattr(processor, "audio_token") | ||
|
|
||
| def _is_vision_model(self) -> bool: | ||
| processor = self.get_hf_processor() | ||
| return hasattr(processor, "image_token") or hasattr(processor, "boi_token") |
There was a problem hiding this comment.
This is what is checked in ProcessorMixin.__call__
| def _is_audio_model(self) -> bool: | |
| processor = self.get_hf_processor() | |
| return hasattr(processor, "audio_token") | |
| def _is_vision_model(self) -> bool: | |
| processor = self.get_hf_processor() | |
| return hasattr(processor, "image_token") or hasattr(processor, "boi_token") | |
| def _is_audio_model(self) -> bool: | |
| return hasattr(self.get_hf_processor(), "feature_extractor") | |
| def _is_image_model(self) -> bool: | |
| return hasattr(self.get_hf_processor(), "image_processor") | |
| def _is_video_model(self) -> bool: | |
| return hasattr(self.get_hf_processor(), "video_processor") |
| # Voxtral's max_source_positions=3000 is the largest known value; | ||
| # AudioFlamingo3/GLM-ASR use 1500. Granite Speech is variable. | ||
| return 3000 |
There was a problem hiding this comment.
I see, presumably the padding mask has a max size which we could use to add a @property to the Transformers config to infer the max tokens of this model?
There was a problem hiding this comment.
Is this necessary because vLLM registers this class globally even though it could use the one from Transformers?
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Hi @harshaljanjani, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
|
Hi @harshaljanjani, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Thank you for your time @hmellor, addressed the reviews! Raised #46472 as a blocker for this PR to address this comment and removed the |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
This pull request has merge conflicts that must be resolved before it can be |
…transformers-backend Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Good day @hmellor, just checking in to see if there've been any updates :) |
4 participants
What does this PR do?
→ This PR adds support for v5 Transformers audio encoder models in the vLLM Transformers backend. These changes are deliberate and are blocked by this Transformers PR which adds prerequisite compatibility to the supported models for vLLM. Once that PR is merged, this PR will be marked ready for review!
→ Outlining the design choices of one PR without context from the other didn't make much sense to me, so I wrote a doc that outlines both sets of changes together and explains their deliberate nature, amongst other valuable things!
→ The v5 tracker doesn’t mention the audio backend, but it is certainly a significant gap that needs to be addressed. After this is merged, I'll open an issue tracker for the Transformers audio backend work in vLLM so the efforts can stay organized.
Please refer to the document for the reasoning behind these changes in context with the Transformers PR!
Document: v5 x vLLM Audio Backend Support Document
Performance Metrics (Env mentioned in the document)
Reference Audio Transcript:
“MISTER QUILTER IS THE APOSTLE OF THE MIDDLE CLASSES AND WE ARE GLAD TO WELCOME HIS GOSPEL”
Related Issues:
→ Current v5 tracker: #38379
→ #38902
→ Solved out of the box with this PR: #32823
→ Documented vLLM engine issue mentioned in the document: #17676
@vasqu (Transformers)
@DarkLight1337 @hmellor (vLLM)
Code Agent Policy
Before submitting
Pull Request section?
to it if that's the case.
PR Checklist
supported_models.mdandexamplesfor a new model.