Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Enable download buffering to improve playback performance #238

Closed
wants to merge 2 commits into from
Closed

Enable download buffering to improve playback performance #238

wants to merge 2 commits into from

Conversation

jjok
Copy link
Contributor

@jjok jjok commented May 8, 2020

Fixes #161
Fixes #160

Relies on mopidy/mopidy#1888

@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #238 into master will decrease coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
- Coverage   60.42%   60.40%   -0.03%     
==========================================
  Files           9        9              
  Lines         748      750       +2     
==========================================
+ Hits          452      453       +1     
- Misses        296      297       +1     
Impacted Files Coverage Δ
mopidy_gmusic/playback.py 92.30% <50.00%> (-7.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4a7846...e049565. Read the comment docs.

@belak
Copy link
Contributor

belak commented May 9, 2020

Does this mean we should bump the required modify version? Thanks for pushing this across the finish line btw!

@jjok
Copy link
Contributor Author

jjok commented May 9, 2020

Oh, I hadn't thought of that. It's not actually a requirement. It's just that this new method won't mean anything with an older version.
I guess it's required of you want this extension to work well, but we'd have to wait for a Mopidy release before merging this.
What do you think?

@kingosticks
Copy link
Member

FYI I'm just adding the uri parameter to should_download, might as well get that in there now.

@jjok
Copy link
Contributor Author

jjok commented May 9, 2020

Ok. Is that just so it has the same interface as is_live() or can you see a situation where the method might need to do some logic based on the uri?

@kingosticks
Copy link
Member

Mostly the second. There might be a day when we tell this workaround isn't required for certain tracks, or where we want to avoid doing it for really long tracks. I see no reason not to pass this extra information in and make the API more flexible, the backend does not have to use it.

@jjok
Copy link
Contributor Author

jjok commented May 9, 2020

I'll add that param here too then, yeah?

@kingosticks
Copy link
Member

I'll add that param here too then, yeah?

Sorry, yes. We've merged mopidy/mopidy#1907 and added the new API to the v3.1 milestone, so that's the new minimum Mopidy version you must target if using should_download. There's only currently one other thing to finish off in v3.1 before releasing so hopefully it won't be long.

@jodal
Copy link
Member

jodal commented Dec 6, 2020

Closing because Google Play Music has been shut down, and this project is being discontinued.

@jodal jodal closed this Dec 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Randomly stops playing after few minutes Playing stops after prolonged pause
4 participants