Guard TimeAxis against non-finite tempo values - #2347
Open
rokujyushi wants to merge 1 commit into
Open
Conversation
TimeAxis lookups use First(predicate) with predicates that cannot match a NaN or infinite input, so a single bad value kills the render thread with "Sequence contains no matching element". Such values are produced by BuildSegments itself: a bpm of 0, NaN or infinity makes msPerTick infinite or NaN, which poisons msPos of every following segment. The existing fallback only tested `bpm == 0`, which NaN silently passes. A phoneme whose PositionMs lands in a good segment and whose EndMs lands in a poisoned one then gets a non-finite DurationMs, and PhonemeCanvas.Render throws while drawing its envelope. - Reject any bpm that cannot be divided by, not just zero, and fall back to 120 when the first segment has none. - Default to 4/4 when a project has no time signature, and to a positive beat unit and beat per bar, both of which divided by zero. - Replace every First(predicate) lookup with a helper that clamps to the last segment instead of throwing. - Clamp MsPosToTickPos's result instead of relying on the saturating cast of NaN to int, which silently returned 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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.
While testing the synthesis process of another PR, NaN (Not a Number) was passed to PhonemeCanvas during envelope rendering in a project set to 300 BPM, triggering an InvalidOperationException. The cause was that the lookup process in TimeAxis used
First(predicate)with a condition that could not match NaN or infinity inputs. Consequently, if even a single inappropriate value was included, the "Sequence contains no matching element" exception was thrown, causing the rendering thread to stop.The cause of the NaN is currently under investigation (the BPM was saved correctly in the project file itself). If NaN (or infinity) is passed to
BuildSegments,msPerTickbecomes infinite or NaN, contaminating themsPosof all subsequent segments. The previous fallback process only checked forbpm == 0, allowing NaN to slip through this condition. As a result, phonemes whose start position (PositionMs) was in a normal segment and end position (EndMs) overlapped with a contaminated segment ended up with a non-finiteDurationMs(length), causingPhonemeCanvas.Renderto throw an exception during envelope rendering.Fixes included in this PR:
First(predicate)lookups with a helper process that clamps to the last segment's value instead of throwing an exception.MsPosToTickPos.