Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update configuration template for upstream 0.5.0 #674

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

kimtore
Copy link
Collaborator

@kimtore kimtore commented Sep 19, 2024

  • Removes LIBRESPOT_SAMPLE_RATE
  • Removes LIBRESPOT_INTERPOLATION_QUALITY
  • Sets boolean values in config to 'on' or 'off'

Fixes #673

* Removes LIBRESPOT_SAMPLE_RATE
* Removes LIBRESPOT_INTERPOLATION_QUALITY
* Sets boolean values in config to 'on' or 'off'
@MichaIng
Copy link

Do these values match the previous defaults when the variables were empty or unset?

MichaIng added a commit to MichaIng/DietPi that referenced this pull request Sep 19, 2024
- DietPi-Software | Raspotify: Resolved an issue where the service failed to start with the latest Raspotify release, since it contains a config, incompatible with the bundles librespot: dtcooper/raspotify#674
@kimtore
Copy link
Collaborator Author

kimtore commented Sep 20, 2024

@MichaIng, yes, to the best of my knowledge :)

@dtcooper dtcooper merged commit b927945 into dtcooper:master Sep 20, 2024
@codydubat
Copy link

codydubat commented Sep 20, 2024

Did you test this @kimtore? In librespot help (librespot -h) only the autoplay flag mentions it expects {on|off}. The others are just regular flags.
image

Look at --quiet for example:
image
I also tried to update the config and make a PR but I am not familiar with Rust and only from the bash files I couldn't figure out how these variables actually get parsed.
To me it looks like {on | off} applies only to autoplay while the others are just regular flags. Please correct me if I am wrong.
And even then I wasn't able to actually get autoplay working no matter if I set it as on or "on" or 'on'. The only thing I know is that no matter what you set there at least it doesn't crash on start.
If you know how the parsing works (and don't mind explaining), I can also try to pick this up.

@MichaIng
Copy link

Actually, "Defaults to following the client setting." indicates that with this commit, Raspotify indeed does change/override the defaults, and that all those empty-value settings should be commented to preserve the previous behaviour.

@bonzi9
Copy link

bonzi9 commented Sep 25, 2024

@codydubat
I suspect the same thing, where the new default autoplay behavior of librespot is not specifying any --autoplay flag to let the client setting for autoplay decide what librespot should do.

So far I can confirm that on v0.44.1 commenting out the setting like this: #LIBRESPOT_AUTOPLAY=on, the debug log shows that it switches on and off when I toggle the autoplay setting in the Spotify android app:

OFF:
Sep 25 17:23:29 DietPi-NUC librespot[1672]: [2024-09-25T15:23:29Z TRACE librespot_connect::spirc] Received attribute mutation, autoplay was 1 is now 0
Sep 25 17:23:29 DietPi-NUC librespot[1672]: [2024-09-25T15:23:29Z DEBUG librespot_playback::player] command=EmitAutoPlayChangedEvent(false)

ON:
Sep 25 17:23:50 DietPi-NUC librespot[1672]: [2024-09-25T15:23:50Z TRACE librespot_connect::spirc] Received attribute mutation, autoplay was 0 is now 1
Sep 25 17:23:50 DietPi-NUC librespot[1672]: [2024-09-25T15:23:50Z DEBUG librespot_playback::player] command=EmitAutoPlayChangedEvent(true)

When enabling the setting in the conf file to either LIBRESPOT_AUTOPLAY=on or LIBRESPOT_AUTOPLAY=off that results in librespot ignoring it as expected when toggling the autoplay option in the app:

Sep 25 17:25:39 DietPi-NUC librespot[1724]: [2024-09-25T15:25:39Z TRACE librespot_connect::spirc] Autoplay override active. Ignoring mutation.

But also like you mentioned, I also cannot get autoplay to work either by commenting out the setting or setting it to =on.

EDIT

Even though it is mainly unrelated to the config file of raspotify, I just tried again to trigger autoplay with the #LIBRESPOT_AUTOPLAY= option still commented out and it does work now, I did not change anything but after the last song in the queue is played it fills the queue again with similar songs.

@codydubat
Copy link

@bonzi9 Great! Thanks for the deep dive! Then should we remove this param completely from the default conf? @kimtore @dtcooper ?

@bonzi9
Copy link

bonzi9 commented Sep 25, 2024

If I may suggest, I would leave the option in the file but commented out so that the default behavior is to allow librespot to respect the client setting, and so it is clear that it can be forced/adjusted, but just not left empty if set active.
Something like this as default:

# Automatically play similar songs when your music ends.
# Respects client autoplay setting if commented out. Forces autoplay on or off when active.
# If active the value must be specified, or librespot will fail to start.
#LIBRESPOT_AUTOPLAY=on

kimtore added a commit that referenced this pull request Sep 26, 2024
@kimtore
Copy link
Collaborator Author

kimtore commented Sep 26, 2024

@bonzi9 thank you, these changes will be included with the next release.

@kimtore kimtore deleted the upstream-config branch September 30, 2024 08:08
This was referenced Oct 10, 2024
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.

Invalid --autoplay / -A in librespot
5 participants