Fix 20 bugs found in a bug hunt - #14197
Merged
Merged
Conversation
Mostly the previous sweeps' verified-but-unapplied agent findings, plus a
fresh pass over the libse Fix-Common-Errors rules.
Crash
- Helper.RemoveSpacesBeginLine indexed text[3] after a check that is already
satisfied by the three-character string "<i>", and text[closeIdx + 1]
unguarded. Reached by FixHyphensRemoveForSingleLine on "<i>-" (the dash is
stripped first, leaving the bare tag), taking down the whole
Fix-Common-Errors / Remove-text-for-HI run. The "{\...}" branch six lines
above already bounds its index correctly.
Destroys user data
- The Qwen3 (CrispASR) voice pass deleted the user's reference WAV and then
moved a replacement in on a "file exists and is non-empty" test. ffmpeg
writes the 44-byte header before encoding, so a failed run passed that
test and replaced the voice with a stub, permanently. VoiceSeedHelper
spells out this exact rule; Zonos hand-rolled the same loop without an
exit code, a timeout, or disposal and now goes through the helper.
Wrong results
- FixMissingSpaces: the "Ph.D" guard compared a four-character match against
the three-character "h.d", so it never fired and "his ph.d yesterday"
became "ph. d". The comma and period loops advanced with NextMatch() over
the pre-replacement string while indexing the rewritten text, so after the
first fix the exclusion list was read one character off - the "?", "!" and
":" loops in the same method re-match correctly.
- FixMusicNotation's "letter after #" chord guard used "idx < Length - 2",
skipping a '#' in the second-to-last position: "F#m" gained a music
symbol while "F#maj" was correctly left alone.
- FixOverlappingDisplayTimes reported the previous paragraph in a branch
that moves the current one, so the fix list showed a row whose before and
after were identical and hid the real change.
- The speech-to-text window wrote the forced-aligner setting on every engine
change but never read it, overwriting the aligner chosen in Import plain
text > Forced aligner setup - the only place that reads it back.
- The plain-text speech-to-text fallback synthesised a 0/0 segment, so the
whole transcript landed in one cue at 00:00:00,000 and the sentence
spreading fallback never ran. The streaming path documents the same fix.
- A model change resolved the saved language from the ElevenLabs key
whichever engine was active, so a per-engine language was lost and then
overwritten on the next Generate.
- AutoCast wrote a reference transcript built from every picked line while
the audio held only the clips that succeeded - and just the first one when
joining failed. A ref-text/ref-audio mismatch is what garbles a clone.
- "Enable VAD" could never be enabled for the cuBLAS and Vulkan backends:
the Silero lookup hard-coded the "Cpp" folder while the model is unpacked
into the selected backend's own folder.
- The CTranslate2 engine looked for whisper.cpp's binary, so with a distro
whisper.cpp installed it reported itself installed and launched whisper-cli
with faster-whisper arguments. All four engines also listed a relative
"usr/local/bin/" that File.Exists could never match.
- Space in the skip-noise-lines and detect-speakers dialogs pressed OK
instead of toggling the row, accepting every pre-checked line.
Hangs and leaks
- The model-download dialog stopped its timer and then ran the state machine
unprotected, so a throw left it frozen at 100% with no error; its twin has
carried that catch since #12127.
- Ten of eleven timer restarts in the TTS download dialog were unguarded,
so a chained step could restart a timer OnClosing had disposed.
- Piper leaked a Process per subtitle line and orphaned the child on cancel;
the TTS post-processor leaked up to four ffmpeg processes per line, and
speech-to-text leaked one per batch item.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
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.
Sweep 17. Mostly the previous sweeps' verified-but-unapplied agent findings, plus a fresh pass over the libse Fix-Common-Errors rules.
Suites: libse 1630 / libuilogic 708 / seconv 422 / UI 4157.
Crash
Helper.RemoveSpacesBeginLineindexestext[3]after a check that is already satisfied by the three-character string"<i>"— andtext[closeIdx + 1]unguarded in the<font>branch below. It is reached byFixHyphensRemoveForSingleLineon"<i>-":FixDashstrips the dash first and hands it the bare tag. That takes down a whole Fix-Common-Errors or Remove-text-for-HI run. The{\...}branch six lines above bounds its index correctly, which is what makes this unambiguous. Guarded, with a theory test that fails without the fix.Destroys user data
The Qwen3 (CrispASR) voice pass deleted the user's reference WAV and then moved a replacement in, gated on "file exists and is non-empty". ffmpeg writes the 44-byte RIFF header before it encodes, so a run that starts and fails passes that test — the original is gone and a silent stub takes its place, permanently.
VoiceSeedHelperspells out this exact rule in a comment ("a truncated WAV is still non-empty, so 'does the file exist / is it non-empty' cannot be the test here") and every other engine goes through it. Zonos hand-rolled the same loop with no exit code, no timeout and an undisposed process; it now uses the helper too.Wrong results
FixMissingSpaces: the "Ph.D" guard compared a four-character regex match against the three-character"h.d", so it could never fire —his ph.d yesterdaybecameph. d. Separately, the comma and period loops advanced withNextMatch()over the pre-replacement string while indexing the rewritten text, so after the first fix theexpectedCharsexclusion was read one character off. The?,!and:loops in the same method re-match against the updated text, which is the correct shape.FixMusicNotation: the "letter after#" chord guard usedidx < Length - 2, skipping a#in the second-to-last position —F#mgained a music symbol whileF#majwas correctly left alone.FixOverlappingDisplayTimesreported the previous paragraph in the branch that moves the current one, so the fix list showed a row whose before and after were identical and the real change was never surfaced.Segments.Count > 0, so the whole transcript landed in one cue at00:00:00,000 --> 00:00:00,000and the sentence-spreading fallback never ran. The streaming path 50 lines above documents this exact fix;OpenRouterSttServicealready did it correctly.Cppfolder, but the model is unpacked into the selected backend's own folder. The CrispASR sibling resolves its engine's folder correctly.whisper-cliwith faster-whisper arguments. All four whisper engines also listed a relative"usr/local/bin/"thatFile.Existscan never match, so a/usr/local/binbuild was never found.AddSpaceToggleinstalls a tunnelling handler on the table, so it only fires while focus is inside it — and the builder focused the OK button.ProfilesWindowstates the rule: "a focused button clicks on bare Space".Hangs and leaks
File.Moveover amodel.binlocked by a running whisper) left it frozen at 100% with no error. Its twin has carried that catch since Fix download dialogs getting stuck when the download itself fails #12127._timer.Start()calls in the TTS download dialog were unguarded, so a chained step could restart a timerOnClosinghad already disposed.Processper subtitle line and orphaned the child on cancel; the TTS post-processor leaked up to four ffmpeg processes per line; speech-to-text leaked one per batch item.A note on the flaky mpv test, and a correction
LibMpvEventLoopTestsfails intermittently on my machine. I chased this properly this time and reached a firm answer, which corrects what I wrote in the last two PRs.Measuring the two checkouts interleaved (the method I insisted on last time) gave main 0/7 failures and this branch 6/7 — which looked conclusive. Bisecting then pointed at
src/libse, which is implausible for an mpv timing test. So I ran the control I had been missing: I reverted the throwaway worktree's content until it was identical to main, and it still failed 3/4 while the other worktree passed 0/4.The variable is the worktree, not the source. My long-lived working worktree has been building all session; a freshly created one has not. Every comparison I have made across two different worktrees — including last sweep's — was measuring that, not the code. The correct control is the same worktree with alternating content. Nothing in this PR touches the video players.
🤖 Generated with Claude Code