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

Implement yt-dlp support and backups for overrides #93

Merged
merged 10 commits into from
Nov 26, 2024

Conversation

falko17
Copy link
Contributor

@falko17 falko17 commented Nov 8, 2024

This PR adds support for the YouTube downloader yt-dlp as an alternative to Invidious instances. Additionally, support for downloading and for creating backups of the overrides have been added. Hence, this should resolve #27, resolve #92, and should provide a stable enough alternative to Invidious that we might consider closing #87 and #89 as well.

Details

  • If enabled, yt-dlp is also used to search for videos. This is done using the ytsearch10 syntax (i.e., searching for the first 10 matching videos) while restricting results to videos that are not longer than 20 minutes to remove unlikely candidates.
    • The search items are retrieved one by one, so users don't have to wait for all 10 items to load before seeing anything.
  • Users can set the plugin up to either download the music or stream it. In the former case, the plugin should also be able to play the music when the device is offline.
  • A section has been added to the settings where users can create backups of the list of overrides. When restoring, they can choose from a list of existing backups to go back to.
  • A lot of structural changes to the code were made to accommodate these changes (such as introducing an abstract AudioResolver class, or moving types into the types/ directory), I hope none of these are too jarring compared to the old codebase.
  • The search text can now be edited while a search is still going on. This was mainly added because yt-dlp searches can be quite slow.
  • Additionally, the search text now reflects what's actually being searched (i.e., appName followed by "Theme Music" initially).
  • A confirmation dialog has been added before deleting all overrides.

Caveats

  • Downloading is currently only supported for yt-dlp. I tried implementing it for Invidious as well by simply downloading the audio URL from the Python backend, but got SSL certificate errors and stopped trying for now1. I've kept the code general, so it should be pretty easy to add that functionality in, assuming the SSL error can be fixed.
  • Since decky doesn't seem to have support for external Python libraries, I had to add the binary from the yt-dlp releases directly to the repository. I've also configured it as a "remote binary" in the package.json, as is recommended in the decky docs.2
  • I only speak English and German, so I've only added translations for those languages (and I've fixed some minor details for the German translation in general).
  • I haven't worked with TypeScript nor React before, so I may have implemented some things in unidiomatic ways. Please tell me if you notice anything like that during the review.

Footnotes

  1. Currently only one public instance works (the nerdvpn one), so maybe it's just broken on there? But then I'm not sure why streaming it would work.

  2. I'm still not exactly sure how this works though, when will the remote binary be downloaded? When testing, I couldn't remove the yt-dlp binary from the defaults folder. Will it only be downloaded on production builds?

This way, users have a stable backend to use instead of having to rely
on potentially unstable Invidious instances.
I also tried to implement downloading functionality for Invidious, but
for the only publicly accessible instance at the moment (nerdvpn), the
downloads would always fail with an SSL certificate error, so I removed
that functionality for now.
@OMGDuke
Copy link
Owner

OMGDuke commented Nov 10, 2024

Hey @falko17 thanks so much for the PR! I've given it a quick look over and everything looks great. I'll find some time to do some testing in the next few days then get it merged in.

Thanks again!

@eXhumer
Copy link
Contributor

eXhumer commented Nov 14, 2024

I tried implementing it for Invidious as well by simply downloading the audio URL from the Python backend, but got SSL certificate errors and stopped trying for now1. I've kept the code general, so it should be pretty easy to add that functionality in, assuming the SSL error can be fixed.

How exactly did you try to download the audio in the Python backend? From what I remember, you had to specify an SSL context for SSL connections to work correctly. See Deckcord or decky-loader for example.

@falko17
Copy link
Contributor Author

falko17 commented Nov 14, 2024

How exactly did you try to download the audio in the Python backend? From what I remember, you had to specify an SSL context for SSL connections to work correctly. See Deckcord or decky-loader for example.

Thanks for the pointers! I just very naively used urllib.urlretrieve, but using the same method that your given examples use, it works flawlessly. Hence, as of e0c1f03, Individious downloads are now possible as well.

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
@OMGDuke
Copy link
Owner

OMGDuke commented Nov 16, 2024

Installed your branch onto my deck to test. Using yt-dlp with download disabled. Opening a game page with a song that was already set plays fine, but opening search I cant get any songs to load.

When performing a search I'm getting this error

Nov 16 16:15:47 steamdeck PluginLoader[1023]: [loader][ERROR]: Method search_yt of plugin Game Theme Music failed with the following exception:
Nov 16 16:15:47 steamdeck PluginLoader[1023]: Traceback (most recent call last):
Nov 16 16:15:47 steamdeck PluginLoader[1023]:   File "decky_loader/loader.py", line 210, in handle_plugin_method_call
Nov 16 16:15:47 steamdeck PluginLoader[1023]:   File "decky_loader/plugin/plugin.py", line 108, in execute_method
Nov 16 16:15:47 steamdeck PluginLoader[1023]:   File "decky_loader/plugin/messages.py", line 35, in wait_for_result
Nov 16 16:15:47 steamdeck PluginLoader[1023]: Exception

Any ideas?

@falko17
Copy link
Contributor Author

falko17 commented Nov 16, 2024

Any ideas?

My bad, I didn't test the changes enough when switching to asyncio, and it appears there were some concurrency issues. The critical sections are now guarded by locks.

package.json Outdated Show resolved Hide resolved
defaults/yt-dlp Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Show resolved Hide resolved
main.py Show resolved Hide resolved
main.py Show resolved Hide resolved
@falko17
Copy link
Contributor Author

falko17 commented Nov 19, 2024

I've added another two commits: 37681c5 allows localizations to use placeholders, and e823815 allows downloading all existing overrides.

@eXhumer
Copy link
Contributor

eXhumer commented Nov 20, 2024

Sorry for not getting back at this earlier, I was traveling and didn't have reliable internet to be able to do any sort of code reviews.

@falko17
Copy link
Contributor Author

falko17 commented Nov 20, 2024

Sorry for not getting back at this earlier, I was traveling and didn't have reliable internet to be able to do any sort of code reviews.

No problem, thanks for your reviews!

@eXhumer
Copy link
Contributor

eXhumer commented Nov 24, 2024

@falko17 Have a look at the PR I opened on your repo. Mostly style changes, translation updates & binary update.

* style: small changes to shut prettier up
* chore: upgrade yt-dlp binary to latest at time of commit
* chore: update .gitignore to ignore files produced as a result of decky CLI
* missing keys for translations added with English translation as value
@libewa
Copy link

libewa commented Nov 26, 2024

We should maybe consider having a slider to adjust the minimum length.

@falko17
Copy link
Contributor Author

falko17 commented Nov 26, 2024

We should maybe consider having a slider to adjust the minimum length.

That sounds like a good idea, but I think I'd do that in a separate issue and PR after this one has been merged.

@BrentTechDesigns
Copy link

@falko17 Do you have a Ko-Fi or somewhere that we could support your efforts? I really appreciate the work you’ve put in to resurrect this plugin.

Copy link
Owner

@OMGDuke OMGDuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, thanks for all your hard work @falko17 and thanks for the reviews @eXhumer

I'll put together a PR after work tomorrow to submit the update to the decky store

@OMGDuke OMGDuke merged commit 516300e into OMGDuke:main Nov 26, 2024
1 check passed
@falko17 falko17 deleted the yt-dlp branch November 28, 2024 16:20
@falko17
Copy link
Contributor Author

falko17 commented Nov 28, 2024

@falko17 Do you have a Ko-Fi or somewhere that we could support your efforts?

That's very kind, thanks @BrentTechDesigns! I do now, but it'll probably be at least a few weeks before I implement another feature for this plugin.

Approved

Great, thanks, and thank you to @eXhumer from me as well for the helpful comments.

@libewa
Copy link

libewa commented Dec 15, 2024

@OMGDuke hows the submission going? i dont see the update.

@OMGDuke
Copy link
Owner

OMGDuke commented Dec 15, 2024

@OMGDuke hows the submission going? i dont see the update.

SteamDeckHomebrew/decky-plugin-database#730

Awaiting review from the Decky team. No ETA unfortunately. They're all volunteers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants