Fix Whisper ASR timestamp regression - #1742
Open
anshusaurav wants to merge 1 commit into
Open
Conversation
Three fixes for the timestamp regression between v3.8.1 and v4.2.0: 1. Use actual audio length for seek loop bounds: The seek loop used input_features.dims[2] (always 3000 = padded 30s) instead of generation_config.num_frames (actual audio length). For clips shorter than 30s, this caused the loop to process silence as real audio, producing hallucinated text and wrong timestamps. 2. Apply max_initial_timestamp_index constraint: A `continue` statement in WhisperTimeStampLogitsProcessor skipped the max_initial_timestamp_index check, allowing the model to generate any timestamp as its first token. The Python reference implementation has no such early exit. 3. Guard against infinite seek loop: If segment_offset is zero (e.g., from a degenerate timestamp pair), the loop ran forever.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1684 — Whisper ASR timestamp regression between v3.8.1 and v4.2.0.
Three fixes targeting the seek loop and logits processor introduced in commit
43b2662("Overdue Whisper fixes #1594"):Use actual audio length for seek loop bounds (
modeling_whisper.js): The seek loop usedinput_features.dims[2](always 3000 = padded 30s) astotal_framesinstead ofgeneration_config.num_frames(set by the ASR pipeline to the real mel frame count). For clips shorter than 30 seconds, this caused the loop to process silence/padding as real audio, producing hallucinated text and incorrect timestamps.Apply
max_initial_timestamp_indexconstraint (logits_process.js): Acontinuestatement inWhisperTimeStampLogitsProcessor._call()skipped themax_initial_timestamp_indexcheck at line 326, making it dead code. This allowed the model to generate any timestamp as its first token instead of being constrained to the allowed range. The Python reference implementation has no such early exit — both blocks execute sequentially.Guard against infinite seek loop (
modeling_whisper.js): Ifsegment_offsetcomputes to zero (e.g., from a degenerate timestamp pair), the seek loop would run forever. Added abreakwhensegment_offset <= 0.Test plan
[applause]segment is no longer droppedtest_modeling_whisper.js) still pass — they usemax_new_tokenswhich bypasses the seek loop