mental32 / spotify.py

🌐 API wrapper for Spotify 🎶
https://spotifypy.readthedocs.io/en/latest/
MIT License
150 stars 38 forks source link

Player.progress_ms is only set once and not updated #38

Closed winston-yallow closed 4 years ago

winston-yallow commented 4 years ago

Player.progress_ms is only set when the player is initialised. After the initialization the value will stay the same.

Expected behaviour: The progress_ms should always be the current value

I would propose one of the following:

  1. Don't use progress_ms at all. It is not really needed as the progress can easily be obtained with User.currently_playing()['progress_ms']
  2. Define a getter method with @property to fetch the current progress
  3. Wait until there is support for realtime playback state (see https://github.com/spotify/web-api/issues/492). Maybe at some day there will be a websocket based solution publicly available. When this happens it would be possible to constantly refresh the progress_ms value with a coroutine for the websocket.

I think option 3 would be the best way. However it does not look like it will come soon.

mental32 commented 4 years ago

Spotify does have a real time [audio] streaming api, unfortunately its not official nor public and the only ways to interact with it would be through a depreciated libspotify, a mopidy server running the spotify plugin (which uses c bindings to libspotify under the hood) or one of the librespot solutions (or spotifyd).

I am currently working on an offline branch that adds spotify.audio and aims to support real time streaming/operations, but I'm mostly gluing this together from librespot's solutions and resources so it will take some time for me to get a stable working branch to push.

regarding this issue however I'm happy with just replacing progress_ms from the Player model with a comment describing the reason for its absence.

winston-yallow commented 4 years ago

Glad to hear that you are working on a streaming branch! (Oh and btw thanks for this awesome and open python module :tada:)

In the Player class I would drop the other attributes like context, device, item and so on too. They all seem to be unused currently as they would require constant updates. I probably don't have time today, but I could put a PR together in the next days if dropping them all is wanted.

mental32 commented 4 years ago

Sounds good, I can get these changes in today, or are you keen on PR'ing this yourself?

winston-yallow commented 4 years ago

Go for it, I am not keen on this PR