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

Switch from youtube-dl to yt-dlp (docker) #924

Open
1 of 10 tasks
Nightenom opened this issue Sep 22, 2023 · 9 comments
Open
1 of 10 tasks

Switch from youtube-dl to yt-dlp (docker) #924

Nightenom opened this issue Sep 22, 2023 · 9 comments

Comments

@Nightenom
Copy link

Iris version

3.68.0 (which itself still reports as 3.67.0)

Operating system(s) affected

  • Windows
  • MacOS
  • iOS
  • Android
  • Linux
  • Other

Browser(s) affected

  • Firefox
  • Chrome
  • Edge
  • Other

What happened?

Hi,

youtube-dl doesn't work for me, there was no release since youtube-dl 2021.12.17, but there is sth happening on the repo, not sure what/why/when it will release again

yt-dlp is also available on pip and mopidy-youtube supports it as counterpart to youtube-dl, yt-dlp library says it's better fork of above mentioned

switching to yt-dlp after adding it via environment: \n - PIP_PACKAGES=yt-dlp works good, although it's slower (iirc, not really sure how fast/slow is youtube-dl?), with cache it's fine, also it has significantly smaller usage of youtube api (which has small quota of 10k queries/day)

proposed solutions:
a) just add yt-dlp into default pip list of docker image
b) also adjust default configs to yt-dlp and enable youtube cache

Logs

No response

@jaedb
Copy link
Owner

jaedb commented Oct 10, 2023

Removing Bug flag as this isn't really a bug; the actual dependencies you install is entirely configurable. I am tempted to remove youtube entirely from the default dependencies as support of those plugins is somewhat patchy.

@PureTryOut
Copy link
Contributor

PureTryOut commented Oct 27, 2023

@jaedb maybe the Docker image shouldn't pre-install any extension besides Mopidy-Local? Not everybody might want Soundcloud or Youtube, YTMusic has been broken with the latest version of ytmusicapi for ages and is thus disabled on startup automatically by Mopidy, etc etc.

People can already install whatever extension they want by specifying the PIP_PACKAGES variable and modifying mopidy.conf, I see no need to pre-install extensions. Besides Spotify I suppose, since that is currently in a bit of a special state and I'm not sure you can just install it with pip yet.

@Nightenom
Copy link
Author

Well, while I do agree with removing it since there is PIP_PACKAGES you should also take for consideration that this variable requires install pip packages every docker start vs packages are baked into docker layers (much faster startup - pip is slow)

@kingosticks
Copy link
Contributor

Perhaps PIP_PACKAGES should be a docker build arg instead (or as well)?

@PureTryOut
Copy link
Contributor

It only has to install the packages once on first startup, afterwards pip realizes they are already installed and just does nothing.

Perhaps PIP_PACKAGES should be a docker build arg instead (or as well)?

How does that work? Don't you then need to build your own image if you want different extensions installed?

@kingosticks
Copy link
Contributor

kingosticks commented Oct 27, 2023

It only has to install the packages once on first startup, afterwards pip realizes they are already installed and just does nothing.

Not sure I follow how those changes are persisted across runs. Are you manually re-starting and re-attaching to the same container ID? Or are you using docker commit to build a new image and using that? Or is there some magic in docker compose that does this all for you (I've never used it but that seems really unlikely).

Don't you then need to build your own image if you want different extensions installed?

Yes. To be fair it's a bit OTT. A custom dockerfile based on the iris image, or using docker commit makes more sense. Even more so if your pip packages require extra setup/config.

@jaedb
Copy link
Owner

jaedb commented Oct 27, 2023

There may be some system-level caching of PIP happening, preventing the need to fully reinstall for every container start. But if you create a volume in docker-compose for where pip installs its packages, then you could more deliberately persist this.

My initial objective was to have an Iris image that is ready-to-go for 90% of users, without needing to manage extra dependencies. Given the nature of needing to tailor dependencies to suit each user's music sources, perhaps this is a moot exercise. The benefit of not including anything but the necessary dependencies would make the base Iris image more resilient to dependency issues.

This all being said, some Mopidy extensions require system extensions to be installed, in addition to those available via PIP. Mopidy-Spotify is an easy example.

So perhaps, as one of the most downloaded Mopidy backends (~290 downloads last month), Mopidy-Spotify remains but other backends will be up to the user to setup. Thoughts?

@PureTryOut
Copy link
Contributor

@kingosticks if you don't destroy the container but just restart it your changes persist. Only upon actual deletion of the container docker rm mopidy the changes are gone. Of course the container is also destroyed if you upgrade it to a different version, but across reboots and restarts it will remain.

Given the nature of needing to tailor dependencies to suit each user's music sources, perhaps this is a moot exercise.

I think so yes. Some system extensions like Spotify are the exception but otherwise I wouldn't really pre-install any extensions. Like you said it prevents problems and most people choose their own list of extensions anyway.

@kingosticks
Copy link
Contributor

Sure. but docker cli makes this pretty cumbersome and it's not the workflow you normally see. However, it does seem that docker compose up behaves quite differently and does just reattach to an existing container. Seems odd that they are so different but Docker is a mess so I shouldn't be surprised.

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

Successfully merging a pull request may close this issue.

4 participants