whisper : fix int overflow in whisper_full_parallel chunk offsets - #4044
Open
kmadiar wants to merge 1 commit into
Open
whisper : fix int overflow in whisper_full_parallel chunk offsets#4044kmadiar wants to merge 1 commit into
kmadiar wants to merge 1 commit into
Conversation
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.
Fixes #4039.
whisper_full_parallel()computed the per-chunk time offset as100 * ((i + 1) * n_samples_per_processor) / WHISPER_SAMPLE_RATEinintbefore adding it to theint64_t offset_t. For chunk boundaries later than ~22 min at 16 kHz (100 * samples > INT_MAX) the multiplication overflows, corrupting the merged segmentt0/t1and the split times printed in the log.This PR computes the offset once per chunk in
int64_t(100LL * ...) and reuses it for both the segment merge and the split-time log.Verification — 46 min file (
samples/jfk.wavlooped),--processors 2, boundary at 23:00:Before, built with
-fsanitize=undefined:After: no overflow reports,
split 1 - 00:23:00.000, all segmentoffsetsin the JSON output non-negative and monotonic.--processors 4(Release) reports splits at 00:11:30 / 00:23:00 / 00:34:30.Note: on Apple clang Release builds the UB happens to yield the correct value, which is why this is mostly seen on MSVC/Windows (as in the issue); the UBSan build reproduces it on macOS.
I have a small regression test (
whisper_full_parallelon a 45 min silent buffer withduration_ms = 1000, checking the logged split time viawhisper_log_set; ~3 s, fails on master with00:-22:-14.-350). Happy to add it to this PR or a follow-up if you'd like it.Token-level timestamps in parallel mode (#2036, #3726) are a separate issue and unchanged here.