Repository navigation
Define the time filter algorithm in the spec - #303
Merged
Merged
Conversation
# Conflicts: # README.md # messaging.md
# Conflicts: # README.md # messaging.md
kahrendt
reviewed
Oct 14, 2026
Matches the reference implementation change: Sendspin/time-filter#10
kahrendt
approved these changes
Oct 21, 2026
kahrendt
left a comment
Contributor
There was a problem hiding this comment.
Thanks, looks good to me!
7 of 9 tasks
1 of 9 tasks
maximmaxim345
added a commit
to Sendspin/sendspin-js
that referenced
this pull request
Oct 22, 2026
# What does this implement/fix? <!-- Quick description and explanation of changes. --> The client didn't follow the spec's rules for `client/time` bursts. It fed the time filter the median of the three fastest measurements instead of the one with the lowest `max_error`, and bursts always started slightly more than 10 seconds apart. Now each burst only feeds its lowest `max_error` measurement to the filter, `measurement` and `max_error` are truncated to whole microseconds, and bursts are scheduled every 9 seconds so a late timer can't push them past the 10 second limit. Bursts can still end up more than 10 seconds apart when the previous burst is still running at the next tick (round trips averaging over about 1.1 s), or when Chrome throttles timers in a tab that has been hidden and silent for a while. Servers must not close the connection over this. **Related issue or spec PR (if applicable):** - Sendspin/spec#303 ## Types of changes <!-- Tick exactly one box. CI (.github/workflows/pr-labels.yaml) applies the matching label, and the release notes use it to sort this change. --> - [x] Bugfix (non-breaking change which fixes an issue): `bugfix` - [ ] New feature (non-breaking change which adds functionality): `new-feature` - [ ] Enhancement to an existing feature: `enhancement` - [ ] Breaking change (changes the public API or the protocol in a way that is not backwards compatible): `breaking-change` - [ ] Refactor (no behavior change): `refactor` - [ ] Documentation only: `documentation` - [ ] Maintenance / chore: `maintenance` - [ ] CI / workflow change: `ci` - [ ] Dependencies bump: `dependencies`
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.
Moves the time filter algorithm from the time-filter repository into Clock Synchronization, so the spec defines it directly instead of only linking to it.
The algorithm itself stays a MUST, like before. The parameter values are now a SHOULD, where before they were only the library's defaults. The burst strategy from the library README is now a MUST instead of a known-good baseline, with at least 8 messages per burst and at most 10 s between burst starts. Servers must not close the connection only because bursts are late, since a network stall can delay them.
The filter now ignores samples with a
max_errorof 0 or less, the same as the reference implementation:I dropped the
aiosendspinlink since it doesn't follow the burst pattern.Open question: Should the parameter values be a MUST?
The player sync accuracy floor is measured against the client's own filter, not the server clock, so a badly tuned filter can be compliant but out of sync. This PR makes the values a SHOULD, but I'm not sure that's the right level.
MUST:
sendspin-jsandsendspin-rsuse other values today.SHOULD (this PR):
No keyword, like before:
Implementations
Only
sendspin-cppmatches the reference everywhere:sendspin-cppsendspin-dotnetSendspinDroidmax_erroraiosendspinSsendspin-jssendspin-rs>=in the drift test, conversions refused above 0.2 driftmeasurementrounds halves away from zerosendspin-jsusesprocess_std_dev0.01,forget_factor1.1,drift_process_std_dev0,adaptive_cutoff2.0 and nomax_error_scale.sendspin-rsusesprocess_std_dev0.01,forget_factor1.001,drift_process_std_dev0,adaptive_cutoff0.75 and nomax_error_scale.Closes #293