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

[ie/Turbo] Fix turbo extractor #8927

Merged
merged 11 commits into from Jan 9, 2024
Merged

Conversation

nbr23
Copy link
Contributor

@nbr23 nbr23 commented Jan 4, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

The Turbo site has changes, and the extractor is currently broken. This fixes it, and adds support for "playlists" (through their RSS feeds).

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

@seproDev seproDev added site-enhancement Feature request for some website site-bug Issue with a specific website labels Jan 4, 2024
@seproDev
Copy link
Collaborator

seproDev commented Jan 7, 2024

I don't think the extractor should be changed like this to add playlist support. If you do want to add playlist support, that should be done in a dedicated extractor. However, I am not convinced that adding the RSS feeds like this is super useful, as most posts don't contain videos. In its current form, the code also seems broken? Do you have an example where parsing the RSS feeds would be useful?

Looking at the site, it may be worth to split viously.com in to its own embed extractor instead. That would also deal more easily with the case where a single page contains multiple videos. And as a added bonus, it would also work on other sites using that CDN.

@seproDev seproDev added the pending-fixes PR has had changes requested label Jan 7, 2024
@nbr23
Copy link
Contributor Author

nbr23 commented Jan 7, 2024

Thanks a bunch for the feedback @seproDev, it all makes perfect sense to me!
I've made this into a viously.com embed extractor (this actually solves the RSS feed thing by design, so I love it).

Removing the old Turbo extractor too since it's broken anyway.

supportedsites.md Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@seproDev seproDev left a comment

Choose a reason for hiding this comment

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

Please don't force push

yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
yt_dlp/extractor/viously.py Outdated Show resolved Hide resolved
@bashonly bashonly added pending-review PR needs a review and removed pending-fixes PR has had changes requested pending-review PR needs a review labels Jan 9, 2024
@seproDev seproDev merged commit 95e8234 into yt-dlp:master Jan 9, 2024
6 checks passed
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Replaces Turbo extractor

Authored by: nbr23, seproDev

Co-authored-by: sepro <4618135+seproDev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-bug Issue with a specific website site-enhancement Feature request for some website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants