Skip to content

Fix volume reset on connect and add seek event forwarding to LMS#26

Open
hansherlighed wants to merge 1 commit intomichaelherger:spottyfrom
hansherlighed:spotty
Open

Fix volume reset on connect and add seek event forwarding to LMS#26
hansherlighed wants to merge 1 commit intomichaelherger:spottyfrom
hansherlighed:spotty

Conversation

@hansherlighed
Copy link
Copy Markdown

Fix volume reset on connect and add seek event forwarding to LMS

This adds two fixes to the Spotty LMS notification path in src/spotty.rs, improving the Spotify Connect experience when used with the Spotty LMS plugin.

  1. Suppress spurious volume reset on session connect
    When a Spotify Connect session is established, Spirc immediately emits a VolumeChanged event pushing Spotify's stored device volume. This overwrites the LMS player's current volume with an unrelated value from a previous session.

Fixed by adding a suppress_next_volume: Arc flag to the LMS struct. The flag is set when SessionConnected is received, and cleared — consuming exactly one suppression — on the immediately following VolumeChanged event.

  1. Forward seek events to LMS
    PlayerEvent::Seeked was previously unhandled (fell through to _ => {}), so seeking in the Spotify app had no effect on the LMS player position.

Added a Seeked handler that converts position_ms to seconds and sends a dedicated "seek" notification to LMS via the existing JSON-RPC notification path. The exact position from Spirc is passed directly, avoiding a Spotify REST API roundtrip — the REST API frequently lags 500ms or more behind the WebSocket path that Spirc uses, which caused stale positions to be applied.

The corresponding handler for the "seek" command is implemented on the plugin side in Connect.pm. in the fork https://github.com/hansherlighed/Spotty-Plugin

@michaelherger
Copy link
Copy Markdown
Owner

I'd have to compare this to the previous implementation. But did you basically restore that implementation and fix a few issues? Or is this a new implementation? Because when I decided to put this on ice, I thought that the event handling had changed quite a bit.

@hansherlighed
Copy link
Copy Markdown
Author

I'd have to compare this to the previous implementation. But did you basically restore that implementation and fix a few issues? Or is this a new implementation? Because when I decided to put this on ice, I thought that the event handling had changed quite a bit.

I asked Copilot to re-implement the Spotify connect feature, using the old implementation as a template/guide. I have to admit that I have no experience with RUST, so all work is done by Github Copilot. I just verified that the "Bin/i386-linux/spotty-x86_64" compiled and is working together with Spotty plugin in LMS docker image version "lmscommunity/lyrionmusicserver:stable".

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants