Skip to content

mtmd: correcting duplicate empty audio chunks for short inputs - #26536

Merged
ngxson merged 2 commits into
ggml-org:masterfrom
Abhinaykrishna2:fix/voxtral-duplicate-audio-chunk
Aug 4, 2026
Merged

mtmd: correcting duplicate empty audio chunks for short inputs#26536
ngxson merged 2 commits into
ggml-org:masterfrom
Abhinaykrishna2:fix/voxtral-duplicate-audio-chunk

Conversation

@Abhinaykrishna2

@Abhinaykrishna2 Abhinaykrishna2 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Overview

Audio preprocessor padded all inputs under 31 sec up to the full 31 sec. This when combined with the spectrogram's 30 sec trailing padding, caused the short audio file to produce about 61 sec of frames, which resulted in an unnecessary extra padding only chunk sent to the encoder.
My fix changes the minimum input length to 201 samples (exact minimum for FFT reflection padding), which ensures short audio generates only necessary chunks and saves computation.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: Yes, I used Codex to verify my idea and implemented the changes on my own

@Abhinaykrishna2
Abhinaykrishna2 requested a review from a team as a code owner August 3, 2026 20:09
@github-actions github-actions Bot added the mtmd Related to multimodal functionality (video/image/audio) label Aug 3, 2026
@Abhinaykrishna2 Abhinaykrishna2 changed the title correcting duplicate empty audio chunks for short inputs mtmd: correcting duplicate empty audio chunks for short inputs Aug 3, 2026
Comment thread tools/mtmd/tests.sh Outdated
Comment on lines +207 to +209
if [[ "$hf" == "ggml-org/Voxtral-Mini-3B-2507-GGUF:Q4_K_M" ]] \
&& ! grep -Fq "encoding mtmd batch, n_chunks = 1 (done = 1, total = 3)" <<< "$output"; then
result="$prefix \033[31mFAIL\033[0m: $hf (expected one audio chunk)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is too hacky, please revert

@ngxson

ngxson commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

/bot review

@ggml-gh-bot

ggml-gh-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Automated code review

I reviewed the diff and traced the full audio path (mtmd_audio_preprocessor_whisper::preprocess -> log_mel_spectrogram -> chunk split loop) and the test harness. Here is my review.

Scope

Two files changed: tools/mtmd/mtmd-audio.cpp (whisper preprocessor min-padding) and tools/mtmd/tests.sh (Voxtral regression guard). Area: multimodal audio preprocessor. No security-sensitive surface (no GGUF parsing, no server/RPC, no untrusted index arithmetic). The change is small and self-contained.

Correctness

The fix is sound. In log_mel_spectrogram the whisper branch sets params.center_padding=false / no_padding=false, so it lands in the else path with stage_2_pad = frame_size/2 = audio_n_fft/2 and rejects short input with:

if (n_samples < stage_2_pad + 1) { return false; }
std::reverse_copy(samples + 1, samples + 1 + stage_2_pad, samples_padded.begin());

That reflection copy needs n_samples >= stage_2_pad + 1 = audio_n_fft/2 + 1. The new min_samples = audio_n_fft/2 + 1 is exactly that threshold, so the pre-pad guarantees the guard passes (and the reflection indices stay in bounds). The trailing stage_1_pad = sample_rate*30 of zeros is untouched, so for the smallest input (201 samples) out_full.n_len = 3002, which still satisfies GGML_ASSERT(out_full.n_len > 3000) and yields exactly one 3000-frame chunk; the spurious all-silence second chunk for <30s audio is gone. Parity of audio_n_fft does not matter: frame_size = audio_n_fft (or audio_n_fft-1 for odd), and stage_2_pad == audio_n_fft/2 in both cases, so min_samples matches the guard exactly. No blocking correctness issues.

Will slow the review

(point 1) The behavior change is broader than the PR description implies. mtmd_audio_preprocessor_whisper is shared by Qwen2A, Qwen25O, Voxtral, Music Flamingo, Ultravox, GLMA and Meralion (tools/mtmd/mtmd.cpp:683-711), all with audio_chunk_len=30/audio_n_fft=400 (tools/mtmd/clip.cpp:1588-1607). Every one of these now emits one chunk instead of two for short (<30s) audio. The description and tests only cover Voxtral. A maintainer will want confirmation that Qwen2-Audio / Ultravox / etc. were run with short audio and did not regress (output should move closer to the reference extractor, but it should be stated and checked, not assumed).

(point 2) The new test assertion is brittle. It greps the exact literal "encoding mtmd batch, n_chunks = 1 (done = 1, total = 3)" (tools/mtmd/tests.sh:208). The done=1/total=3 values encode a specific prompt-template layout (text, audio, text). Any template or chunk-count change breaks the assertion even though the actual fix (one audio chunk) still holds. Consider matching just n_chunks = 1 for the audio batch, or counting occurrences of the audio-encode log line, so the guard tests the invariant rather than the surrounding layout. Also note the guard only fires for the Q4_K_M quant string; if other Voxtral quants are added later they silently skip this check.

Nits

(point 3) The new comment // reflection padding needs one sample plus half an FFT window is accurate and concise - good. No style/ASCII issues in the diff.

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

@ngxson

ngxson commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

(point 1) The behavior change is broader than the PR description implies. mtmd_audio_preprocessor_whisper is shared by Qwen2A, Qwen25O, Voxtral, Music Flamingo, Ultravox, GLMA and Meralion (tools/mtmd/mtmd.cpp:683-711), all with audio_chunk_len=30/audio_n_fft=400 (tools/mtmd/clip.cpp:1588-1607). Every one of these now emits one chunk instead of two for short (<30s) audio. The description and tests only cover Voxtral. A maintainer will want confirmation that Qwen2-Audio / Ultravox / etc. were run with short audio and did not regress (output should move closer to the reference extractor, but it should be stated and checked, not assumed).

please carefully test & validate all models using the same preprocessor class

older whisper-based model always expect input to be padded

@Abhinaykrishna2

Copy link
Copy Markdown
Contributor Author

Verified one representative GGUF for each affected model family.
The tested GGUF repositories and quantizations are:

  1. ggml-org/ultravox-v0_5-llama-3_2-1b-GGUF:Q8_0
  2. ggml-org/Qwen2.5-Omni-3B-GGUF:Q4_K_M
  3. ggml-org/Voxtral-Mini-3B-2507-GGUF:Q4_K_M
  4. concedo/GLM-ASR-Nano-2512-GGUF:Q4_K
  5. MERaLiON/MERaLiON-2-3B-GGUF:Q4_K_M
  6. mradermacher/Qwen2-Audio-7B-Instruct-GGUF:Q4_K_M
  7. henry1477/music-flamingo-gguf:Q2_K

All seven families passed both validation cases.
The 17.44-second clip produced one audio encode.
The 30.97-second control produced two audio encodes.

All runs exited successfully without assertions or preprocessing errors.

@ngxson

This comment was marked as outdated.

@ngxson

ngxson commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

please ignore my last message, I misunderstood the intention of this fix

IIRC the behavior was a bit buggy due to a security fix. I need to verify this one more time to make sure this fix is safe to merge

@ngxson
ngxson merged commit 474c92e into ggml-org:master Aug 4, 2026
21 of 26 checks passed
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
…org#26536)

* correcting duplicate empty audio chunks for short inputs

* tests.sh code restored
brittlewis12 pushed a commit to brittlewis12/llama.cpp that referenced this pull request Aug 17, 2026
…org#26536)

* correcting duplicate empty audio chunks for short inputs

* tests.sh code restored
thecodacus pushed a commit to thecodacus/llama.cpp that referenced this pull request Sep 7, 2026
…org#26536)

* correcting duplicate empty audio chunks for short inputs

* tests.sh code restored
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mtmd Related to multimodal functionality (video/image/audio)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants