skorokithakis/catt

Fix yt-dlp support

Closed this issue · 22 comments

Actually, it seems fe90975 needs a fixup to port stream_info.py (and also test_catt.py) to use yt-dlp.

Here are the docs for that: https://github.com/yt-dlp/yt-dlp#embedding-yt-dlp, and this is the most relevant bit:

Tip: If you are porting your code from youtube-dl to yt-dlp, one important point to look out for is that we do not guarantee the return value of YoutubeDL.extract_info to be json serializable, or even be a dictionary. It will be dictionary-like, but if you want to ensure it is a serializable dictionary, pass it through YoutubeDL.sanitize_info as shown in the example above

Originally posted by @FranciscoPombal in #354 (comment)

The latest Docker build fails with the error below:

FROM python:3.7
RUN pip install catt

Running catt results in:

Traceback (most recent call last):
  File "/usr/local/bin/catt", line 5, in <module>
    from catt.cli import main
  File "/usr/local/lib/python3.7/site-packages/catt/cli.py", line 14, in <module>
    from .controllers import CastState
  File "/usr/local/lib/python3.7/site-packages/catt/controllers.py", line 22, in <module>
    from .stream_info import StreamInfo
  File "/usr/local/lib/python3.7/site-packages/catt/stream_info.py", line 4, in <module>
    import youtube_dl
ModuleNotFoundError: No module named 'youtube_dl'
Traceback (most recent call last):
  File "/usr/local/bin/catt", line 5, in <module>
    from catt.cli import main
  File "/usr/local/lib/python3.7/site-packages/catt/cli.py", line 14, in <module>
    from .controllers import CastState
  File "/usr/local/lib/python3.7/site-packages/catt/controllers.py", line 22, in <module>
    from .stream_info import StreamInfo
  File "/usr/local/lib/python3.7/site-packages/catt/stream_info.py", line 4, in <module>
    import youtube_dl
ModuleNotFoundError: No module named 'youtube_dl'

@skorokithakis I believe you did not change catt/stream_info.py, tests/test_catt.py, and README.rst where youtube_dl is referenced.

@skorokithakis a new "pipx install catt" also fails with the same error when trying to run catt cast.

Ah, damn it. I didn't realize we had changed the dependency. Can you perhaps make a PR, @anthonyrocom? I'll cut a new release afterwards.

Hey @skorokithakis I didn't mean to leave you hanging. I haven't had time to analyze the changes needed. For anyone who needs it, you can RUN pip install youtube-dl before RUN pip install catt if you're building your Dockerfile.

I'll see if I can get a PR done at the end of this week.

Oh, no problem. However, youtube-dl is discontinued and probably doesn't work with many sites. We're switching over to yt-dlp, so I don't know if the workaround you mention in your comment would actually work.

@skorokithakis

youtube-dl is discontinued

Is this official?

@theychx I don't know about official, it hasn't had any activity since July so I'd say that's as official as it's going to get...

My workaround makes catt run again but it doesn't mean YouTube or other links will cast 😅 I am streaming a local source so I wasn't thinking of it.

It's a good call seeing youtube-dl as inactive since they previously had a monthly or more release cadence.

@skorokithakis

The main youtube-dl maintainer just resurfaced from his hiatus, and made a fresh release. So I suggest we switch back to youtube-dl.

Hmm, that's very interesting, has he said if he's going to be maintaining it? I don't know if he's just going to go away again...

#369 relates to this.

I just finished the pull request too :D

Haha, yes, excellent timing :P By the way, as per the comment above, I think extract_info needs to be passed through sanitize_info first. I'm not entirely sure if we serialize it (I think not), but it can't hurt...

I also noticed you just updated catt/cli.py so I'm out of date too.

Maybe we just need to choose youtube-dl or yt-dlp, one or the other for now, because right now the build is broken if you don't have youtube-dl installed. Up to you, make the call! :)

I'm not sure what extract_info or sanitize_info does.

I also noticed you just updated catt/cli.py so I'm out of date too.

That shouldn't be an issue, your code has no conflicts.

Maybe we just need to choose youtube-dl or yt-dlp, one or the other for now, because right now the build is broken if you don't have youtube-dl installed. Up to you, make the call! :)

Yeah, we do need to choose one. On one hand, I'm inclined to go with the original, on the other, we know that yt-dlp is maintained, but YouTube-dl's status is uncertain.

@theychx I had a look and it seems that ytdl just released the 6-month-old code. There doesn't seem to be any new code merged or any work done, so I don't know how active the maintainer is. Given that catt is currently breaking, I think we should merge @anthonyrocom's PR and release, and we can think about switching back later if ytdl catches up to yt-dlp at some point. The latter seems to be much better maintained nowadays anyway.

I agree with going to yt-dlp right now because you have the agility to easily switch. Right now that's the well maintained project.

Looks like ytdl is officially dead, at least for now: ytdl-org/youtube-dl@21b7590

The tests are failing. I did some work on this in a branch, but yt-dlp seems to have changed a few things related to finding the best format and I need to figure out how to do that before we can release a fixed version...

Hey @skorokithakis sorry, I've been offline due to holidays. Anything I can help with? I admit I did a simple test but nothing more extensive. Is test_catt.py not passing?

Ah, no problem. Yes, the tests are failing, but I haven't managed to figure out why. It seems to be because they changed the format specifiers, but I didn't look into it more.