search

mopidy / mopidy-soundcloud

Mopidy extension for playing music from SoundCloud
https://mopidy.com/ext/soundcloud/
MIT License
185 stars 59 forks source link

Mitigate error 429: too many requests #122

Closed Laurentww closed 3 years ago

Laurentww commented 3 years ago

Streams publicly available tracks using a public client id to unload use of standard app client id

kingosticks commented 3 years ago

My guess would be that Soundcloud don't make many changes and although this is scraping, it's stable enough. Would that be a fair assessment, do you think? Are public streams common? What decides if they are public or not? I'm just curious what sort of improvement we can expect from this?

kingosticks commented 3 years ago

P. S. Thanks very much for these PRs. I will look closer later.

Laurentww commented 3 years ago

Two other packages implement similar ways of streaming using public client ids: https://pypi.org/project/youtube_dl/ and https://pypi.org/project/soundcloud-lib/. Not sure what fraction of streaming is public, so hard to say what impact it has.

https://github.com/ytdl-org/youtube-dl/blob/04d4a3b136060158438c3f2c1b31c884c6961712/youtube_dl/extractor/soundcloud.py#L277-L289 and https://github.com/3jackdaws/soundcloud-lib/blob/8ef22360c07f44fc3ee27e1dea8b210b25dd47f9/sclib/util.py#L13-L28

The ‘sharing’ attribute in the track’s json shows if the track is available publicly. This is used to decide if the public client id will be used to stream the track. This json is obtained through the apps standard client id.

The scraping is done a bit provisionally, but does the work for the streams I’ve tested.

kingosticks commented 3 years ago

Seems it does break but we can see how it goes. If it does break maybe we can look at depending on soundcloud-lib and relying on those fixes.

Laurentww commented 3 years ago

Yes sounds reasonable. Ill look into passing the tests and adding a few for the new code.

kingosticks commented 3 years ago

I noticed that tracks only available with "SoundCloud Go+" now play a preview whereas previously we considered them unplayable and they were skipped (which could easily eat up our quota). We could avoid this by filtering out "/preview/progressive" strings but maybe this behaviour is better. Maybe we should mark them as previews if we keep this.

EDIT: actually, now I've gone back to the master branch it looks like the old method also plays the preview... I must have just remembered it wrong so never mind. We should consider marking these tracks as previews but that is out of scope here.

theomarkkuspaul commented 3 years ago

Pulled down your fix @Laurentww. Works great, thanks for your effort on this.

djmattyg007 commented 3 years ago

@Laurentww is this ready for merging? I recommend going through all of the comments above and clicking the "resolve conversation" button for each thread that's no longer relevant.

Laurentww commented 3 years ago

I do think it is ready to merge. Thanks for the suggestions!

PS: I have a newer branch which makes use of API-v2 and includes images. That branch has more rigorous code changes and might therefore be better to merge later on. For now this PR should get this plugin up and running.

kingosticks commented 3 years ago

Merged in https://github.com/mopidy/mopidy-soundcloud/commit/89bac5b24b537e530cc85ed2efc14cf3ac05a330