Skip to content

Add --clock-source dac for bit-perfect single-room playback - #318

Open
manuel-alcocer wants to merge 2 commits into
Sendspin:mainfrom
manuel-alcocer:feat/clock-source-dac
Open

manuel-alcocer wants to merge 2 commits into
Sendspin:mainfrom
manuel-alcocer:feat/clock-source-dac

Conversation

@manuel-alcocer

Copy link
Copy Markdown

What does this implement/fix?

Adds an opt-in --clock-source dac option (also clock_source in the settings file) for bit-perfect single-room playback.

By default the player follows the server clock. The DAC crystal and the server clock never run at exactly the same rate, so drift is corrected by dropping or inserting single frames. That keeps multi-room playback in sync, but it means the samples reaching the DAC differ from the source.

With --clock-source dac the stream is aligned to the server timeline when it starts, and then the DAC clock drives playback:

  • No frames are dropped or inserted while playing. The sync error is still measured and logged at debug level.
  • If the start is slightly late (up to the existing re-anchor threshold), the lateness is kept as a constant offset instead of skipping the first samples of the stream.
  • On stream/end, a small buffered remainder (up to 1 s, from accumulated drift) is played out before the stream is closed. A larger remainder, for example from an explicit stop, is still discarded immediately.

The default stays server, so existing behavior does not change. The trade-off (slow drift from the server timeline, so not suitable for grouped players) is documented in the README.

How it was verified

A 120 s 192 kHz/24-bit white-noise reference was played from Music Assistant 2.10.9 to a player on an Orange Pi 3B with a USB DAC. Everything sent to the DAC was captured through an ALSA type file tap and compared sample by sample with the decoded reference:

Mode Inserted frames Dropped frames Result
server (default) 1772 (1406 in the first 10 s, then bursts as drift reaches the 2 ms deadband) 0 not bit-perfect
dac, PCM 0 0 100% bit-identical, including first and last sample

The measured drift between the DAC and the server was about 35 ppm, roughly 2 ms per minute.

While testing I also found that the last partial chunk of each stream never reaches the client, with both PCM and FLAC. This looks like a server-side issue (the player transformer's flush() is not called on stream end) and is not addressed here.

Related issue (if applicable):

  • n/a

Types of changes

  • 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 CLI options, settings, hooks 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

By default the player follows the server clock and corrects drift by
dropping or inserting single frames. That keeps multi-room playback in
sync but alters the sample stream reaching the DAC.

The new `--clock-source dac` option (also `clock_source` in the settings
file) aligns the stream to the server timeline at start and then lets the
DAC clock run free:

- no frames are dropped or inserted while playing
- a slightly late start (up to the re-anchor threshold) is kept as a
  constant offset instead of skipping the first samples
- on stream end, the small buffered remainder caused by accumulated drift
  (up to 1 s) is played out before the stream is closed

The default remains `server`, so existing behavior is unchanged.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 09:58
@github-actions github-actions Bot added the new-feature Adds new functionality label Oct 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The tail-drain loop blocks stop, clear, and mute requests and can exceed the teardown timeout, allowing audio to continue and the old worker to retain the device after disconnect.

3 open issues
What changed in this PR

Adds opt-in DAC-clock playback to the synchronized audio client for bit-perfect single-room listening, while keeping server-clock synchronization as the default.

Changes:

  • Wires --clock-source and its settings-file equivalent through TUI and daemon playback.
  • Disables ongoing drift correction in DAC mode, preserves slightly late starts, and adds bounded end-of-stream playout.
  • Documents playback trade-offs and adds focused audio and settings tests.
File Description
tests/​test_settings.py Tests clock-source loading and fallback values.
tests/​test_audio.py Tests drift correction, late starts, and playout limits.
tests/​test_audio_connector.py Updates the worker stub to accept clock source.
sendspin/​tui/​app.py Passes clock source into TUI audio handling.
sendspin/​settings.py Adds and validates the clock-source setting.
sendspin/​daemon/​daemon.py Passes clock source and logs DAC mode.
sendspin/​cli.py Adds CLI options and applies settings defaults.
sendspin/​audio.py Implements DAC-clock playback behavior.
sendspin/​audio_connector.py Propagates clock source and waits for tail playout.
README.md Documents configuration, usage, and synchronization trade-offs.

Review effort: Balanced

Comment thread sendspin/audio_connector.py Outdated
Comment on lines +229 to +231
deadline = time.monotonic() + playout_us / 1_000_000 + 0.5
while not player.is_drained() and time.monotonic() < deadline:
time.sleep(0.005)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 419bd31. The playout no longer sleeps on the worker thread: it keeps reading the work queue, so stop and clear end it immediately, volume/mute and delay changes are applied, and other items are kept in order for after the close. A stop now finishes well within the 1 s join. Covered by test_stop_interrupts_end_of_stream_playout and test_playout_applies_volume_and_ends_on_clear.

Comment thread sendspin/audio.py
Comment on lines +380 to +382
if self._queued_duration_us > self._DAC_END_PLAYOUT_MAX_US:
return 0
return max(self._queued_duration_us, 1)
Comment thread sendspin/audio_connector.py Outdated
Comment on lines +226 to +228
playout_us = player.end_of_stream_playout_us()
if playout_us > 0:
# DAC clock: finish the drift remainder before closing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 419bd31. With the DAC clock, the stop event is now emitted when the worker reports the stream closed (tracked by a per-close sequence number), so stop hooks no longer cut the tail. If a new stream starts first, the pending stop and the following start are both suppressed, and a disconnect still emits the pending stop. Covered by the new test_dac_* and test_disconnect_emits_pending_stop_event tests.

The DAC-clock tail playout slept on the worker thread, so stop, clear and
volume requests waited for it, and a stop could outlast the worker join
timeout while the old worker still held the audio device.

- Serve the work queue during the playout: stop and clear end it at once,
  volume and delay changes apply, and later items keep their order.
- Emit the stop event once the worker has closed the stream, so a stop
  hook (amplifier off, mute) no longer cuts the tail. A stream starting
  before that keeps the hooks in the playing state.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-feature Adds new functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants