feat: re-enable Spotify Connect integration#226
Open
stiefenm wants to merge 3 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR re-enables Spotify Connect support in the Spotty plugin by implementing daemon-managed helper processes, LMS↔Spotify state synchronization, playlist context tracking, and device-scoped player control APIs. The implementation maintains LMS as the playback controller while exposing each player as a controllable endpoint in the Spotify app. ChangesSpotify Connect Reintegration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
Connect/DaemonManager.pm (1)
30-30: ⚡ Quick win
Time::HiRes::timeavailability is ensured viaConnect.pmload order (optional cleanup)
Connect.pmexplicitly doesuse Time::HiRes;and is the module thatrequires/initializesPlugins::Spotty::Connect::DaemonManager(soTime::HiResis loaded before the timers inConnect/DaemonManager.pmrun).- The other
require Plugins::Spotty::Connect::DaemonManagercall is gated behindcanSpotifyConnect(), whichrequire Plugins::Spotty::Connect;(and thus also loadsTime::HiRes).Optional: add
use Time::HiRes;toConnect/DaemonManager.pmto make the dependency self-contained.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Connect/DaemonManager.pm` at line 30, The timer call in Slim::Utils::Timers::setTimer uses Time::HiRes::time but relies on Connect.pm loading Time::HiRes indirectly; make the dependency explicit by adding a top-level use Time::HiRes; in Connect/DaemonManager.pm so Time::HiRes::time is always available when initHelpers is scheduled (this keeps the setTimer call in the file-safe and self-contained regardless of load order involving Plugins::Spotty::Connect::DaemonManager and Connect.pm).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@API.pm`:
- Around line 401-402: The devices() helper currently calls the callback with no
arguments (it invokes $cb->() if $cb), which discards the fetched API result;
update the callback invocation inside devices() to pass the retrieved devices
payload (the variable holding the API response) to $cb so callers receive the
device list, e.g. invoke $cb->($devices_payload) when $cb is set and ensure any
error/empty-case branches similarly forward the appropriate value to $cb.
In `@Connect.pm`:
- Around line 77-97: The new Slim::Control::Request::subscribe calls (the named
handler _onPlaylistJump and the anonymous pre-buffer subscriber) are not removed
on shutdown; store the coderefs returned by subscribe (or assign the anonymous
subscriber to a lexical variable) and unregister them in the module
shutdown/cleanup path using Slim::Control::Request::unsubscribe with the same
handler references so re-init doesn't stack duplicate handlers (apply the same
change for the other subscribe pair around the 283-287 area).
- Around line 174-177: Guard access to $state->{item}->{uri} before calling
$song->streamUrl to avoid deref errors: check that $state->{item} and
$state->{item}->{uri} are defined, and only call
$song->streamUrl($state->{item}->{uri}) when present; if the uri is missing,
skip setting the stream URL (optionally log or set a safe fallback) but still
proceed with $class->setSpotifyConnect($client, $state), clear the pluginData
via $client->pluginData(newTrack => 0), and invoke $successCb->() so transition
state is not left dirty.
In `@Connect/Context.pm`:
- Line 205: The shared in-memory LRU cache (%memCache) is limited to ENTRIES =>
10 causing active contexts to evict each other; increase the capacity (e.g., set
a much larger fixed value or read ENTRIES from a config/constant) where the
cache is tied using Tie::Cache::LRU::Expires so it can hold all active players’
multiple keys without premature eviction, and ensure the change is applied to
the tie call for %memCache and validated in integration tests.
In `@Connect/Daemon.pm`:
- Around line 53-61: Guard against Slim::Player::Client::getClient($self->mac)
returning undef before dereferencing: after assigning my $client =
Slim::Player::Client::getClient($self->mac) check defined $client and only call
$client->isSynced, $client->model, $client->name or
Slim::Player::Sync::syncname($client) when $client is defined; if undefined,
skip changing $self->name or set a safe fallback (e.g. keep existing $self->name
or use an empty/placeholder string) so Plugins::Spotty::Helper->get() startup
won't dereference undef and crash.
In `@Connect/DaemonManager.pm`:
- Around line 85-89: The change hook registered via
preferences('server')->setChange only watches 'authorize' and 'username' so
Connect helpers aren't restarted when the LMS password changes; update that call
to include 'password' in the watched keys (the same setChange invocation that
currently lists 'authorize' and 'username') so that $class->shutdown() and
initHelpers() run on password rotation and running daemons pick up new
credentials.
In `@Settings.pm`:
- Around line 109-110: The template is hiding the checkbox because Settings.pm
uses the runtime-enabled check as a capability flag by calling
Plugins::Spotty::Plugin->canSpotifyConnect(1); change this to query the
capability (not the runtime enable state) by calling
Plugins::Spotty::Plugin->canSpotifyConnect() (or canSpotifyConnect(0) if the
method explicitly accepts a flag to ignore runtime state) and assign that result
to $paramRef->{canSpotifyConnect} so the UI can still show the control when
Spotify Connect is globally disabled.
In `@Settings/Player.pm`:
- Around line 34-36: The settings handler is performing an initializing
capability check by calling Plugins::Spotty::Plugin->canSpotifyConnect() which
can trigger Connect component initialization; change the call to
Plugins::Spotty::Plugin->canSpotifyConnect(1) so the check is non-initializing,
leaving the rest of the logic that sets $params->{errorString} =
$client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE') intact.
---
Nitpick comments:
In `@Connect/DaemonManager.pm`:
- Line 30: The timer call in Slim::Utils::Timers::setTimer uses
Time::HiRes::time but relies on Connect.pm loading Time::HiRes indirectly; make
the dependency explicit by adding a top-level use Time::HiRes; in
Connect/DaemonManager.pm so Time::HiRes::time is always available when
initHelpers is scheduled (this keeps the setTimer call in the file-safe and
self-contained regardless of load order involving
Plugins::Spotty::Connect::DaemonManager and Connect.pm).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e6de831-a8ce-4e50-ae13-f28b28f2a9c1
📒 Files selected for processing (14)
API.pmBin/aarch64-linux/spottyBin/i386-linux/spotty-x86_64Connect.pmConnect/Context.pmConnect/Daemon.pmConnect/DaemonManager.pmHTML/EN/plugins/Spotty/settings/basic.htmlHTML/EN/plugins/Spotty/settings/player.htmlPlugin.pmProtocolHandler.pmSettings.pmSettings/Player.pmstrings.txt
Comment on lines
+401
to
+402
| $cb->() if $cb; | ||
| }, |
There was a problem hiding this comment.
Return the fetched devices payload to the callback.
devices() currently swallows the API result and always invokes $cb without args, which breaks callers that need the device list from this helper.
Suggested fix
sub devices {
my ( $self, $cb ) = `@_`;
@@
$self->_call('me/player/devices',
sub {
my ($result) = `@_`;
@@
- $cb->() if $cb;
+ $cb->($result) if $cb;
},
GET => {
_nocache => 1,
}
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $cb->() if $cb; | |
| }, | |
| sub devices { | |
| my ( $self, $cb ) = `@_`; | |
| $self->_call('me/player/devices', | |
| sub { | |
| my ($result) = `@_`; | |
| $cb->($result) if $cb; | |
| }, | |
| GET => { | |
| _nocache => 1, | |
| } | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@API.pm` around lines 401 - 402, The devices() helper currently calls the
callback with no arguments (it invokes $cb->() if $cb), which discards the
fetched API result; update the callback invocation inside devices() to pass the
retrieved devices payload (the variable holding the API response) to $cb so
callers receive the device list, e.g. invoke $cb->($devices_payload) when $cb is
set and ensure any error/empty-case branches similarly forward the appropriate
value to $cb.
|
|
||
| use Tie::Cache::LRU::Expires; | ||
|
|
||
| tie my %memCache, 'Tie::Cache::LRU::Expires', EXPIRES => 86400 * 7, ENTRIES => 10; |
There was a problem hiding this comment.
Increase shared MemoryCache capacity to avoid active-context eviction.
Line 205 caps a global shared cache at 10 entries. Because each context uses multiple keys, a handful of active players can evict each other’s state and break last-track/history decisions.
Suggested fix
-tie my %memCache, 'Tie::Cache::LRU::Expires', EXPIRES => 86400 * 7, ENTRIES => 10;
+tie my %memCache, 'Tie::Cache::LRU::Expires', EXPIRES => 86400 * 7, ENTRIES => 256;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| tie my %memCache, 'Tie::Cache::LRU::Expires', EXPIRES => 86400 * 7, ENTRIES => 10; | |
| tie my %memCache, 'Tie::Cache::LRU::Expires', EXPIRES => 86400 * 7, ENTRIES => 256; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Connect/Context.pm` at line 205, The shared in-memory LRU cache (%memCache)
is limited to ENTRIES => 10 causing active contexts to evict each other;
increase the capacity (e.g., set a much larger fixed value or read ENTRIES from
a config/constant) where the cache is tied using Tie::Cache::LRU::Expires so it
can hold all active players’ multiple keys without premature eviction, and
ensure the change is applied to the tie call for %memCache and validated in
integration tests.
Comment on lines
+53
to
+61
| my $helperPath = Plugins::Spotty::Helper->get(); | ||
| my $client = Slim::Player::Client::getClient($self->mac); | ||
|
|
||
| # Spotify can't handle long player names | ||
| $self->name(substr( | ||
| ($client->isSynced() && $client->model ne 'group') | ||
| ? Slim::Player::Sync::syncname($client) | ||
| : $client->name, | ||
| 0, 60)); |
There was a problem hiding this comment.
Guard against missing client before dereferencing.
getClient($self->mac) can return undef during disconnect/reconnect races, and the next calls dereference it immediately. That can crash helper startup.
Proposed fix
my $helperPath = Plugins::Spotty::Helper->get();
my $client = Slim::Player::Client::getClient($self->mac);
+ if (!$client) {
+ $log->warn("Cannot start Spotty Connect daemon: no client for " . $self->mac);
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| my $helperPath = Plugins::Spotty::Helper->get(); | |
| my $client = Slim::Player::Client::getClient($self->mac); | |
| # Spotify can't handle long player names | |
| $self->name(substr( | |
| ($client->isSynced() && $client->model ne 'group') | |
| ? Slim::Player::Sync::syncname($client) | |
| : $client->name, | |
| 0, 60)); | |
| my $helperPath = Plugins::Spotty::Helper->get(); | |
| my $client = Slim::Player::Client::getClient($self->mac); | |
| if (!$client) { | |
| $log->warn("Cannot start Spotty Connect daemon: no client for " . $self->mac); | |
| return; | |
| } | |
| # Spotify can't handle long player names | |
| $self->name(substr( | |
| ($client->isSynced() && $client->model ne 'group') | |
| ? Slim::Player::Sync::syncname($client) | |
| : $client->name, | |
| 0, 60)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Connect/Daemon.pm` around lines 53 - 61, Guard against
Slim::Player::Client::getClient($self->mac) returning undef before
dereferencing: after assigning my $client =
Slim::Player::Client::getClient($self->mac) check defined $client and only call
$client->isSynced, $client->model, $client->name or
Slim::Player::Sync::syncname($client) when $client is defined; if undefined,
skip changing $self->name or set a safe fallback (e.g. keep existing $self->name
or use an empty/placeholder string) so Plugins::Spotty::Helper->get() startup
won't dereference undef and crash.
Comment on lines
+85
to
+89
| preferences('server')->setChange(sub { | ||
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | ||
| $class->shutdown(); | ||
| initHelpers(); | ||
| }, 'authorize', 'username'); |
There was a problem hiding this comment.
Restart helpers when LMS password changes.
The auth-change hook handles authorize and username, but not password. After password rotation, running daemons can keep stale auth and fail callbacks until another restart trigger occurs.
Proposed fix
preferences('server')->setChange(sub {
main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers");
$class->shutdown();
initHelpers();
- }, 'authorize', 'username');
+ }, 'authorize', 'username', 'password');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| preferences('server')->setChange(sub { | |
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | |
| $class->shutdown(); | |
| initHelpers(); | |
| }, 'authorize', 'username'); | |
| preferences('server')->setChange(sub { | |
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | |
| $class->shutdown(); | |
| initHelpers(); | |
| }, 'authorize', 'username', 'password'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Connect/DaemonManager.pm` around lines 85 - 89, The change hook registered
via preferences('server')->setChange only watches 'authorize' and 'username' so
Connect helpers aren't restarted when the LMS password changes; update that call
to include 'password' in the watched keys (the same setChange invocation that
currently lists 'authorize' and 'username') so that $class->shutdown() and
initHelpers() run on password rotation and running daemons pick up new
credentials.
Comment on lines
+391
to
+399
| if ($helper) { | ||
| my @pids = split /\n/, `pgrep -f $helper 2>/dev/null`; | ||
| my @orphans = grep { $_ && !$connectPids{$_} } @pids; | ||
| kill('TERM', @orphans) if @orphans; | ||
| } | ||
| unless ($helper && $helper =~ /spotty-custom/) { | ||
| my @pids = split /\n/, `pgrep -f spotty-custom 2>/dev/null`; | ||
| my @orphans = grep { $_ && !$connectPids{$_} } @pids; | ||
| kill('TERM', @orphans) if @orphans; |
There was a problem hiding this comment.
Avoid shell-interpolated pgrep with helper path.
Interpolating $helper directly into backticks is unsafe and can also match unintended processes due to regex interpretation. Use list-form process execution and an escaped pattern.
Proposed fix
- if ($helper) {
- my `@pids` = split /\n/, `pgrep -f $helper 2>/dev/null`;
+ if ($helper) {
+ my $pattern = quotemeta($helper);
+ open my $pgrep_fh, '-|', 'pgrep', '-f', '--', $pattern;
+ my `@pids` = grep { /\S/ } map { chomp; $_ } <$pgrep_fh>;
+ close $pgrep_fh;
my `@orphans` = grep { $_ && !$connectPids{$_} } `@pids`;
kill('TERM', `@orphans`) if `@orphans`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ($helper) { | |
| my @pids = split /\n/, `pgrep -f $helper 2>/dev/null`; | |
| my @orphans = grep { $_ && !$connectPids{$_} } @pids; | |
| kill('TERM', @orphans) if @orphans; | |
| } | |
| unless ($helper && $helper =~ /spotty-custom/) { | |
| my @pids = split /\n/, `pgrep -f spotty-custom 2>/dev/null`; | |
| my @orphans = grep { $_ && !$connectPids{$_} } @pids; | |
| kill('TERM', @orphans) if @orphans; | |
| if ($helper) { | |
| my $pattern = quotemeta($helper); | |
| open my $pgrep_fh, '-|', 'pgrep', '-f', '--', $pattern; | |
| my `@pids` = grep { /\S/ } map { chomp; $_ } <$pgrep_fh>; | |
| close $pgrep_fh; | |
| my `@orphans` = grep { $_ && !$connectPids{$_} } `@pids`; | |
| kill('TERM', `@orphans`) if `@orphans`; | |
| } | |
| unless ($helper && $helper =~ /spotty-custom/) { | |
| my `@pids` = split /\n/, `pgrep -f spotty-custom 2>/dev/null`; | |
| my `@orphans` = grep { $_ && !$connectPids{$_} } `@pids`; | |
| kill('TERM', `@orphans`) if `@orphans`; |
Comment on lines
+109
to
+110
| $paramRef->{canDiscovery} = Plugins::Spotty::Plugin->canDiscovery(); | ||
| $paramRef->{canSpotifyConnect} = Plugins::Spotty::Plugin->canSpotifyConnect(1); |
There was a problem hiding this comment.
Do not use runtime enable-state as capability flag.
canSpotifyConnect(1) returns false when Spotify Connect is globally disabled, so the template treats it as unsupported and hides the very checkbox needed to re-enable it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Settings.pm` around lines 109 - 110, The template is hiding the checkbox
because Settings.pm uses the runtime-enabled check as a capability flag by
calling Plugins::Spotty::Plugin->canSpotifyConnect(1); change this to query the
capability (not the runtime enable state) by calling
Plugins::Spotty::Plugin->canSpotifyConnect() (or canSpotifyConnect(0) if the
method explicitly accepts a flag to ignore runtime state) and assign that result
to $paramRef->{canSpotifyConnect} so the UI can still show the control when
Spotify Connect is globally disabled.
Comment on lines
+34
to
+36
| if ( !Plugins::Spotty::Plugin->canSpotifyConnect() ) { | ||
| $params->{errorString} = $client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE'); | ||
| } |
There was a problem hiding this comment.
Use a non-initializing capability check in settings handler.
This settings path should not initialize Connect components. Use canSpotifyConnect(1) here to avoid side effects from a read-only page load.
Proposed fix
- if ( !Plugins::Spotty::Plugin->canSpotifyConnect() ) {
+ if ( !Plugins::Spotty::Plugin->canSpotifyConnect(1) ) {
$params->{errorString} = $client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( !Plugins::Spotty::Plugin->canSpotifyConnect() ) { | |
| $params->{errorString} = $client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE'); | |
| } | |
| if ( !Plugins::Spotty::Plugin->canSpotifyConnect(1) ) { | |
| $params->{errorString} = $client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Settings/Player.pm` around lines 34 - 36, The settings handler is performing
an initializing capability check by calling
Plugins::Spotty::Plugin->canSpotifyConnect() which can trigger Connect component
initialization; change the call to Plugins::Spotty::Plugin->canSpotifyConnect(1)
so the check is non-initializing, leaving the rest of the logic that sets
$params->{errorString} = $client->string('PLUGIN_SPOTTY_NEED_HELPER_UPDATE')
intact.
This was referenced May 23, 2026
…dispatch, API control Adds Spotify Connect support (receiver mode) to the Spotty plugin. Herger removed Connect in 75ff6bf (2025-08-15) citing fragile track-by-track state synchronization. This re-addition addresses that fundamental issue via a completely different transport architecture (see commit 2). Changes in this commit (Phases 15-20 — initial integration): - Connect.pm: event loop, sync-group resilience, _connectEvent callbacks, LMS CLI dispatch (pause, volume, track change, position seek) - Connect/Context.pm: per-player context (iconCode, port, MAC mapping) - Connect/Daemon.pm: daemon lifecycle — start/stop/restart, FIFO stream path, crash-restart watchdog, stream-backoff constants - Connect/DaemonManager.pm: multi-player manager, MAC-to-deviceId mapping, checkAPIConnectPlayer, stopForSync differential restart (v3.1 resilience) - Plugin.pm: Connect route registration, canSpotifyConnect version guard - Settings.pm / Settings/Player.pm: Connect prefs UI, player assignment - HTML/EN/plugins/Spotty/settings/basic.html: Connect settings section - HTML/EN/plugins/Spotty/settings/player.html: per-player Connect assignment - ProtocolHandler.pm: stream-mode formatOverride branch - API.pm: Connect API methods — playerTransfer, playerPause, playerNext, playerPrevious, playerSeek, playerVolume, idFromMac, withIdFromMac, devices - API/Cache.pm: remove() method for account cleanup - AccountHelper.pm: recursion-safe getCredentials (unlink before recursive call) - custom-convert.conf / custom-types.conf: spotty-connect transcoding entry - strings.txt: Connect-related strings
…HTTP server Replaces the FIFO-based stream path with an embedded HTTP server in the spotty binary (HttpStreamSink + http_stream_server). This resolves the fundamental fragility that led to Connect removal in 75ff6bf: Architecture: Rust (binary): HttpStreamSink writes continuous S16LE PCM to an HTTP server on localhost. Port is printed to stdout at startup (stream_port=NNNN). Perl DaemonManager: pipe-based launch reads stream_port from binary stdout. Perl ProtocolHandler: canDirectStream returns http://127.0.0.1:<port>/stream. LMS handles buffering, seeking, and rate-control natively — no Spotify session state in LMS, no fragile per-track synchronization. Phase 22-23 changes (HTTP transport core + code review): - Connect/DaemonManager.pm: pipe-based start, _streamPort attribute, streamPortForClient accessor, IO::Select port-read (replaces alarm/SIGALRM), null-guard getAPIHandler at all call sites, remove dead FIFO branch - Connect/Daemon.pm: remove fifoPathForClient, suppress encode_base64 newline, unsubscribe _onPlaylistJump on shutdown - Connect.pm: canDirectStream activation — returns HTTP stream URL, spc PCM passthrough, progress-bar fix, guard undef $state in getNextTrack, WR-02 getAPIHandler call consolidation, WR-03 userId pass fix - ProtocolHandler.pm: canDirectStream returns HTTP URL for Connect players - Plugin.pm: remove dead fifoPathForClient reference, correct settings guard - API.pm: remove orphaned playerQueue method Requires companion PR michaelherger#28 (michaelherger/librespot) which provides the HttpStreamSink + http_stream_server in the spotty binary.
Phase 24-25 polish changes:
Progress bar (Phase 24):
- Connect.pm: streamingProgressBar calls in getNextTrack and _connectEvent,
reset startOffset/playPoint on track change, defer seek on new connect join
- Uses $song->streamUrl (HTTP URL) as cache key, guarded by duration_ms check
Autoplay + stability (Phase 25):
- Connect/Daemon.pm: --autoplay on/off flag wired to per-player enableAutoplay
pref, gated on getCapability('autoplay') for binary compatibility
- AccountHelper.pm: remove duplicate unlink in getCredentials corruption handler
(STB-01) — the file is already unlinked before deleteCacheFolder is called,
second unlink was a no-op that left an audit gap in logs
Binary update (Phase 24/26):
- Bin/i386-linux/spotty-x86_64: v2.2.0 (x86_64-linux-musl static-pie)
- Bin/aarch64-linux/spotty: v2.2.0 (aarch64-linux-musl static)
Built from michaelherger/librespot spotty branch + pr/lms-glue patches.
Binary adds HttpStreamSink, http_stream_server, watch-channel seek-flush,
--autoplay flag. Required companion to this PR.
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
1 participant
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.
Summary
Re-adds Spotify Connect support (receiver mode) to Spotty, built on a fundamentally different
architecture than the implementation removed in commit 75ff6bf (2025-08-15).
Herger removed the previous Connect implementation citing fragile track-by-track state
synchronization between two competing state machines (LMS and librespot). This re-addition
addresses that root cause: the new implementation replaces the FIFO pipe with an embedded
HTTP server in the spotty binary (HttpStreamSink). LMS consumes a single continuous PCM
stream over HTTP using native
canDirectStream. The binary owns the full Spirc session; LMSjust plays audio. No per-track synchronization. No fragile FIFO lifetime management.
Architecture
Three-layer design:
Rust binary (PR #28 on michaelherger/librespot):
HttpStreamSink+http_stream_server: listens on a random localhost port, serves acontinuous S16LE PCM audio stream
stream_port=NNNNon stdout--autoplay on/offflag for queue continuation after playlist endsPerl Daemon layer (
Connect/Daemon.pm,Connect/DaemonManager.pm):stream_portfrom binary stdout (IO::Select, no SIGALRM)Perl ProtocolHandler + Connect layer:
canDirectStream: returnshttp://127.0.0.1:<port>/streamfor Connect playersstreamingProgressBarcalls ingetNextTrackand_connectEventfor accurate progress displayWhat Is Included
This PR covers everything except the API routing/authentication fix (that is PR #225):
streamingProgressBarwired for accurate seek/duration display (Phase 24)--autoplay on/offflag wired to per-playerenableAutoplaypref (Phase 25)AccountHelper.pm::getCredentials(unlink before recursive call)Bin/i386-linux/spotty-x86_64andBin/aarch64-linux/spottyat v2.2.0Commit Structure
feat(connect): Spotify Connect integration — daemon lifecycle, event dispatch, API controlfeat(connect): HTTP streaming transport — replace FIFO with embedded HTTP serverfix(connect): progress bar, autoplay, stability polish + binary v2.2.0Dependencies
Required (binary): PR #28 on
michaelherger/librespot— provides theHttpStreamSink+http_stream_serverin the spotty binary. Without it,canDirectStreamreturns a URL that nothing is listening on. Pre-built binaries are included inBin/for convenience.Recommended (merge order): Merge PR #225 first. PR #225 adds the dual-OAuth dispatch (own Dev ID for
me/*, bundled-default forbrowse/*) that allows Connect and API browsing to coexist on different auth flavors. If this PR is merged without #225, Connect still works, but users may see 429 errors on Liked Songs browsing under heavy load.Tested On