-
Notifications
You must be signed in to change notification settings - Fork 458
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 librespot to 0.6 #1317
base: master
Are you sure you want to change the base?
update librespot to 0.6 #1317
Conversation
Co-Authored-By: dotandl <[email protected]>
Looks good to me +1 |
* Update docker cross compile guide for librespot 0.6.0 upgrade - Enhanced log output - Use latest GH Release - Corrected compose filename * use latest rust 1 image
I was testing this PR to see if it solved #706 (which it seems like it did!) but I noticed some issues that I didn't see with the master build. I noticed that the shuffle status gets "stuck" in a certain state. The Spotify client on another device like my phone or the web player will show Spotify has shuffle on, but mpris will still report it is off:
In spite of my phone and web player showing shuffle is on, it seems like it is not shuffling, as when I send the next song command via mpris, it goes through my playlist in order. This issue is a little hit or miss for me, but it seems like more often than not shuffle is messed up. The other issue (I guess whether or not this behavior is desired is an opinion since technically there is a period of no audio) I saw is that, when going to the next song, there is a brief period where the mpris metadata returns "NoTrack", which is causing my mpris controller to show as "not playing" for a few seconds every time the next song starts. Note the timestamps
It is only for a fraction of a second, but it still throws off my mpris client. Let me know if there is any more info I can try to gather to help figure this out. |
@clstrickland Thank you for giving the updated MPRIS implementation a little more testing, that is really helpful. Regarding the shuffle, I can confirm that setting Regarding the other issue, I'm not entirely sure what is happening there, maybe it was one minor thing I missed and added now. If it's still happening I added some debug logging, could you possibly try to gather some debug logs? (in particular, the lines with |
That's too bad about shuffle, but I'd rather not have shuffle on a client that doesn't crash than how it was before.
The issue happened on the transition from "Fly (Original Mix) - Fly (Original Mix)" to "Push Through". |
Thanks for the log, that probably makes sense, it's erasing the previous metadata when loading a new song, which probably happens when the track hasn't been preloaded entirely beforehand. I'll push a fix later... |
@clstrickland Should hopefully be fixed now. :) |
That is fixed now! For the sake of not blowing up this thread, I'm attaching the logs as files. Mpris starts reporting NoTrack nonstop until the song is changed. I created this issue by skipping back about 3 songs using mpris controls. In the logs, it happened once I got to "Sleepy Eyes". |
Don't worry, the more testing the better! I didn't really think that could happen, but it turns out that one track can have multiple |
@eladyn I just checked, and that issue is resolved. I have not noticed any other problems, I hope this gets merged soon! |
This upgrades librespot to version 0.6 and reworks the dbus logic to no longer rely on the web API but just use local method calls.
This should hopefully fix #891, reports welcome.
It supersedes and closes #1309.
Note, that it does not yet fix the issue #1293, since OAuth is still not implemented.
TBD:
Spirc::load
implement OAuth(probably new PR)Some of the long-standing librespot issues that are fixed with this upgrade:
Fixes #1246, fixes #522, fixes #788, fixes #1285.