Skip to content

Spotify Connect: clearer transport errors and automatic stall recovery#4010

Merged
marcelveldt merged 6 commits into
devfrom
fix-spotify-connect-followups
May 28, 2026
Merged

Spotify Connect: clearer transport errors and automatic stall recovery#4010
marcelveldt merged 6 commits into
devfrom
fix-spotify-connect-followups

Conversation

@marcelveldt
Copy link
Copy Markdown
Member

@marcelveldt marcelveldt commented May 28, 2026

What does this implement/fix?

Three small follow-ups to #4001 that make Spotify Connect playback control feel less fragile:

  • Friendlier errors for transport commands. When MA is no longer the active Spotify device, next/previous/seek/volume now surface the same "select Music Assistant in the Spotify app" hint as play. Previously only play caught this — the others fell through to a generic 403.
  • Self-recovery from pipe stalls. Occasionally librespot would start producing audio before MA attached a consumer; the FIFO would fill, librespot's write would block, and the device looked frozen until Spotify timed out (~14 s). A short watchdog now drains the FIFO once and lets librespot proceed on its own.
  • Codec-independent readiness sentinel. The "librespot is up" check no longer keys on a PCM-specific log line, so a future backend or codec change doesn't silently break startup detection.

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
  • New music/player/metadata/plugin provider — new-provider
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — documentation
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Checklist

  • The code change is tested and works locally.
  • pre-commit run --all-files passes.
  • pytest passes, and tests have been added/updated under tests/ where applicable.
  • For changes to shared models, the companion PR in music-assistant/models is linked.
  • For changes affecting the UI, the companion PR in music-assistant/frontend is linked.
  • I have read and complied with the project's AI Policy for any AI-assisted contributions.

…odec

- Apply the active-device check from get_stream_details at the top of
  on_source_control and on_volume_change so next/previous/seek/volume hit
  the same user-friendly "not the active device" error instead of a
  Spotify Web API 403
- Start librespot with --passthrough so the FIFO carries the raw
  Ogg-Vorbis bytes from Spotify; set audio_format to OGG/VORBIS at 320
  kbps so the frontend shows the real source codec. Drop --dither and
  --enable-volume-normalisation since both only apply when librespot is
  decoding
Copilot AI review requested due to automatic review settings May 28, 2026 07:48
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

This PR updates Spotify Connect handling so inactive-device transport commands fail with a user-facing error and Spotify Connect streams are advertised as Ogg-Vorbis instead of PCM.

Changes:

  • Adds active-device guards for source control and volume changes.
  • Starts librespot with --passthrough.
  • Updates the advertised AudioFormat to OGG/VORBIS at 320 kbps.

Comment thread music_assistant/providers/spotify_connect/__init__.py Outdated
Copy link
Copy Markdown
Contributor

@MarvinSchenkel MarvinSchenkel left a comment

Choose a reason for hiding this comment

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

Automated review (no CRITICAL/PROBLEM findings).

Verified:

  • AudioFormat OGG/VORBIS + bit_rate=320 matches the new --passthrough librespot mode.
  • Active-device guards in on_source_control / on_volume_change mirror the existing check in get_stream_details.
  • Removed --dither and --enable-volume-normalisation are decode-only flags, correctly dropped under passthrough.

Thanks @marcelveldt 🎉

Note: the PR Labels workflow failed because the bugfix checkbox uses escaped backticks (`bugfix`) instead of plain backticks — the label regex won't match, so the bugfix label won't auto-apply. Worth fixing in the description or applying the label manually.

When session_connected isn't followed by a 'playing' or 'paused' event
within 8 s the daemon is almost always blocked on a pipe write (no
consumer attached yet). Read+discard from the FIFO for up to 2 s to
unblock librespot's writer; the normal flow resumes on its own once
'playing' fires. Bails out early if librespot recovers or the regular
stream pipeline is about to attach so we never split bytes with ffmpeg.
@marcelveldt marcelveldt changed the title Guard Spotify Connect transport commands and emit Ogg-Vorbis source codec Spotify Connect reliability follow-ups May 28, 2026
Match on "Connecting to AP" instead of the PCM-specific StdoutSink line so
the daemon-started signal still fires regardless of audio backend or
passthrough mode and doesn't trip the "unable to initialize" unload path
if librespot ever changes its backend startup logging.
Copilot AI review requested due to automatic review settings May 28, 2026 10:50
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

librespot emits a separate Ogg bitstream per Spotify track in passthrough
mode and ffmpeg can't demux chained Ogg, so stop+play within the same
session was failing with "Codec not found" once a new bitstream began.

Expose chain_ogg_pages() in the existing ogg_handler (the radio-stream
chained-Ogg stitcher) so any async byte source can use it, switch the
provider's stream_type to CUSTOM and implement get_audio_stream() that
pipes read_named_pipe through chain_ogg_pages. ffmpeg now sees one
continuous Ogg bitstream regardless of how many Spotify tracks are
concatenated.
@marcelveldt marcelveldt marked this pull request as draft May 28, 2026 11:40
Chained-Ogg stitching solved the stop+restart "Codec not found" but the
multi-stage buffering (kernel FIFO -> StreamReader -> chain_ogg_pages
page buffer -> ffmpeg stdin -> -re pacing) added several seconds of
latency on track changes, which is much worse than the codec badge being
wrong. Back to librespot's decoded PCM on the FIFO; the codec-display
question can be revisited as a UI hint or via the planned librespot-go
migration with its dedicated WebSocket API.
@marcelveldt marcelveldt changed the title Spotify Connect reliability follow-ups Spotify Connect: transport-command gating, stall watchdog, robust readiness signal May 28, 2026
@marcelveldt marcelveldt changed the title Spotify Connect: transport-command gating, stall watchdog, robust readiness signal Spotify Connect: clearer transport errors and automatic stall recovery May 28, 2026
@marcelveldt marcelveldt requested a review from Copilot May 28, 2026 19:17
@marcelveldt marcelveldt marked this pull request as ready for review May 28, 2026 19:17
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@marcelveldt marcelveldt merged commit bde1d43 into dev May 28, 2026
11 of 12 checks passed
@marcelveldt marcelveldt deleted the fix-spotify-connect-followups branch May 28, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants