Skip to content

Add remote per-player delay configuration#185

Merged
maximmaxim345 merged 10 commits intomainfrom
feat/delay-configuration
Apr 14, 2026
Merged

Add remote per-player delay configuration#185
maximmaxim345 merged 10 commits intomainfrom
feat/delay-configuration

Conversation

@maximmaxim345
Copy link
Copy Markdown
Member

@maximmaxim345 maximmaxim345 commented Mar 17, 2026

Implements per-player delay configuration from Sendspin spec PR #67.

TUI and daemon handle incoming set_static_delay server commands: the client library applies the delay, then the app persists it to settings and updates the UI. Both advertise supported_commands: ["set_static_delay"] in client/state.

Keyboard delay adjustment (.,]) now clamps to the valid 0-5000ms range and reports the change to the server. Old settings with negative delay values (previous sign convention) are migrated on load. The settings _load() path no longer calls _schedule_save() from inside an executor, fixing a RuntimeError: no running event loop.

Comment thread sendspin/settings.py Outdated
@balloob-travel
Copy link
Copy Markdown
Contributor

Please update the referenced sendspin-js in the serve HTML too.

Add SET_STATIC_DELAY handling in TUI and daemon, pass
state_supported_commands to SendspinClient, clamp keyboard delay
adjustment to 0-5000ms range.
Without this, TUI delay adjustments were local-only and the
server never learned about the new static_delay_ms value.
Instead of clearing the audio buffer (which the server won't resend),
shift the server timestamp cursor so sync correction gradually speeds
up or slows down playback to match the new delay. Plumb the delay
delta through the audio work queue for both server-initiated and
keyboard-initiated changes.
@maximmaxim345 maximmaxim345 force-pushed the feat/delay-configuration branch from fa7c7f9 to 439ac0a Compare April 10, 2026 13:56
Copy link
Copy Markdown
Contributor

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified and works.

@balloob balloob marked this pull request as ready for review April 11, 2026 18:47
Copilot AI review requested due to automatic review settings April 11, 2026 18:47
@balloob balloob added the breaking-change Breaks API or changes the protocol in a not backwards compatible way label Apr 11, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end support for remotely configuring a per-player static playback delay (per spec PR #67), including persistence/migration and audio-timing adjustment so delay changes don’t require clearing the buffer.

Changes:

  • Handle incoming set_static_delay server commands in both TUI and daemon, advertise support via client/state.
  • Clamp local keyboard delay adjustments to 0–5000ms and propagate delay-change notifications to the audio worker for gradual sync correction.
  • Migrate older persisted negative delay values on settings load; adjust audio playback anchoring and update dependency constraints.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sendspin/tui/keyboard.py Clamp delay adjustments; notify audio worker of delay deltas; persist delay updates.
sendspin/tui/app.py Advertise SET_STATIC_DELAY support and apply/persist server-issued delay changes in TUI.
sendspin/settings.py Change settings load path to avoid scheduling saves from executor; clamp/migrate persisted delay values.
sendspin/daemon/daemon.py Advertise SET_STATIC_DELAY support and apply/persist server-issued delay changes in daemon mode.
sendspin/audio.py Add cursor adjustment API for delay changes and refine DAC-anchored start estimation.
sendspin/audio_connector.py Plumb delay-change notifications through the sync audio worker to AudioPlayer.
README.md Update documentation/examples for the new delay sign convention and typical values.
pyproject.toml Bump aiosendspin dependency (with [server] extra) and raise minimum av/numpy versions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sendspin/settings.py
Comment thread sendspin/audio_connector.py
Comment thread sendspin/audio_connector.py
Copy link
Copy Markdown
Contributor

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At +4880ms the TUI starts buffering, increase it even further and audio doesn't play.

In server log:

WARNING:aiosendspin.server.connection.sendspin-cli-mac-1.home:Late binary: skipping 4 chunk(s); late_by_us=1591 ts_us=148084323243 now_us=148079444834 queue=1/8192
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Send summary role=player samples=47 send_gap_ms(avg=108.4 min=76.9 max=183.7) ts_gap_ms(avg=108.3 min=96.0 max=192.0) buf_ms(avg=110.3 min=96.3 max=126.2)
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Discarding late chunk: late_by=5.3ms, plays_in=-5.3ms
WARNING:aiosendspin.server.connection.sendspin-cli-mac-1.home:Late binary: skipping 1 chunk(s); late_by_us=5291 ts_us=148086435243 now_us=148081560534 queue=1/8192
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Discarding late chunk: late_by=4.3ms, plays_in=-4.3ms
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Discarding late chunk: late_by=4.1ms, plays_in=-4.1ms
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Discarding late chunk: late_by=3.4ms, plays_in=-3.4ms
DEBUG:aiosendspin.server.connection.sendspin-cli-mac-1.home:Send summary role=player samples=49 send_gap_ms(avg=104.0 min=76.8 max=184.3) ts_gap_ms(avg=103.8 min=96.0 max=192.0) buf_ms(avg=111.5 min=97.4 max=127.3)

maximmaxim345 added a commit to Sendspin/aiosendspin that referenced this pull request Apr 14, 2026
When a player advertises a large `static_delay_ms`, the server's fixed
250ms send-ahead window wasn't enough. Chunks arrived late from the
player's perspective and got dropped at startup.

`PushStream` scheduling paths now extend the send-ahead by the largest
active player's `static_delay_us` via a new
`_max_active_static_delay_us()` helper. Buffer throttling also accounts
for the extra lead so the producer isn't throttled prematurely.

Resolves the issue raised in
Sendspin/sendspin-cli#185
aiosendspin switched to CLOCK_MONOTONIC_RAW on Linux. Use
`client.now_us()` instead of `time.monotonic()` / `loop.time()`
to keep all components in the same clock domain.
Without this, a delay change during draining would fall through and be
miscast as a `_ChunkWorkItem`.
@maximmaxim345
Copy link
Copy Markdown
Member Author

At +4880ms the TUI starts buffering, increase it even further and audio doesn't play.

Fixed in aiosendspin 5.1.0 (through Sendspin/aiosendspin#212).
There still can be issues with larger static delays, but properly solving them requires finishing and implementing:

Only issue I think thats left is that large changes to the static delay trigger clearing of the buffer, but this was also the case before. Pausing/restarting fixes that.

@maximmaxim345 maximmaxim345 merged commit 201d9c1 into main Apr 14, 2026
1 check passed
@maximmaxim345 maximmaxim345 deleted the feat/delay-configuration branch April 14, 2026 14:38
maximmaxim345 added a commit that referenced this pull request Apr 14, 2026
The DAC-anchored sync fix (#226) and remote per-player delay (#185) make
the old 100-150ms recommendation obsolete. Updated the README to
recommend 0ms as the default and mention that compatible servers can
configure delay remotely.
selleronom pushed a commit to selleronom/sendspin-cli that referenced this pull request Apr 15, 2026
Implements per-player delay configuration from [Sendspin spec PR
Sendspin#67](Sendspin/spec#67).

TUI and daemon handle incoming `set_static_delay` server commands: the
client library applies the delay, then the app persists it to settings
and updates the UI. Both advertise `supported_commands:
["set_static_delay"]` in `client/state`.

Keyboard delay adjustment (`.`,`]`) now clamps to the valid 0-5000ms
range and reports the change to the server. Old settings with negative
delay values (previous sign convention) are migrated on load. The
settings `_load()` path no longer calls `_schedule_save()` from inside
an executor, fixing a `RuntimeError: no running event loop`.

---------

Co-authored-by: Paulus Schoutsen <balloob@gmail.com>
selleronom pushed a commit to selleronom/sendspin-cli that referenced this pull request Apr 15, 2026
The DAC-anchored sync fix (Sendspin#226) and remote per-player delay (Sendspin#185) make
the old 100-150ms recommendation obsolete. Updated the README to
recommend 0ms as the default and mention that compatible servers can
configure delay remotely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Breaks API or changes the protocol in a not backwards compatible way enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants