-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Update api call to v5 spec in TwitchPlaylistBaseIE #25606
Conversation
Also changed instances of `var is None` to `var == None`, and replaced `var.replace('http%3A', 'http:')` with a regex
URLs like `https://www.screencast.com/users/cindyhailes/folders/Jing/media/c9be177c-5808-4c4f-af56-eadceb3a7c82` weren't being accepted before
sorry about the garbage commits |
Is there any reason why this is not being merged? It'd be nice to have this functionality back in youtube-dl – manually going through a list of videos and downloading new ones individually is a bit tedious. |
Is there anything anyone in the community can do to assist the youtube-dl team with getting this PR merged or at least feedback as to why it isn't merged so that it can either be re-worked/fixed so it can be merged? |
Please merge this for the next release if this works. My project is literally missing internet history because of the bug this (hopefully) fixes. |
@dstftw @phihag @remitamine Sorry about tagging you all – but please consider merging this, or at least giving any amount of feedback at all. |
@vxbinaca It looks like your use case is very similar to mine. With this patch, my archival scripts are working with Twitch again. Fortunately this project is quite easy to build locally, so hopefully you can deploy a temporary build while waiting for this to merge. TL;DR for this project: this patch works for me |
If anyone is desperate to get this working you can apply the patch yourself easily:
Note: this patch works correctly with the "old" URL for all twitch vods (i.e. https://www.twitch.tv/CHANNELNAME/videos/all) |
Or ya know, this could be merged up so I don't have to maintain a separate youtube-dl. I'm thinking of getting ahold of the maintainers and politely requesting this be merged this is a ridiculous bug to literally let sit. It's holding back testing we want to do to fix our project that relies on youtube-dl because we can't properly test a YTDL flag without Twitch channels working. Edit 2: I don't feel like re-pointing library calls to the administrative nightmare that would be a separate youtube-dl package just to get Twitch channel support. I'm forced to wait for this to be merged in along with the 600+ other PR's this project has waiting. But in the meantime internet history is being lost because my scripts rely on Twitch channels as targets w/ crontabs to automate ingestion to Archive.org. Twitch channel extraction is literally broken right now, therefor unless I manually enuumerte a channels videos they don't get ingested. So for the last 20 days internet history hasn't been getting saved, because the bug this PR fixes hasn't been fixed. The upgrade we want to test that this bug is preventing would also likely fix Periscope ingestion. |
You could always just temporarily apply this patch as outlined above, then revert to mainline It's a bit of a hassle, of course, but it'll get your internet history preservation project back up and running faster than demanding that the (unpaid) maintainers merge this patch and cut a release immediately. |
It takes two mouse clicks to merge this fix. The users of my script are much more diverse than just me. Probably hundreds by now. I'll just find all of them, somehow, and tell them if they need to save Twitch just do this complicated series of steps to get it to work - oh and be sure to keep up to date with the rest of the extractors and code changes too until two mouse clicks happen and it's merged then you can go back to the vanilla install from pypi. Totally that will work just fine. Or, ya know, two mouse clicks. |
There was no indication in your previous comments that you weren't the only user of your script – that sure complicates things! Mind you, I hope for a swift merge as well. |
I can confirm this PR does fix all of the issues I was running in to. I'm building a custom youtube-dl binary for my projects now. I'd still prefer this be merged, but we're at 3 weeks with no feedback from the project about if this PR has any issues or needs any change, while other issues have been "managed" (updated/closed/triaged) by project contributors. If you don't want to go the python module route that @Pete5x5 provided above, this is what I had to do to build a drop in replacement for /usr/local/bin/youtube-dl
Any errors I got were due to a dependency being missing which is why I installed the 4 packages above. It probably won't be much help for you @vxbinaca but it's at least a better solution for some use cases than using a patched version of the python module. |
@dstftw Sorry for the ping, but would it be possible to get any feedback on this PR/issue please? I think there are multiple people willing to update it and/or address feedback if needed and multiple people have been testing this patch without reporting any issues. |
Can I point ytdl at whatever that ID number is and would it pick up the channel or is this problem a of the URL structure changing? |
Well, there's also this small bug regarding URL schemes that wasn't fixed for more than a year: #24263 #21811 I wonder how you guys were handling these new twitch URLs with "videos?filter=archives" when downloading all videos from a channel even before this "channel ID" bug? Because that's what someone gets from URL bar and youtube-dl doesn't recognize them for more than a year. No way to get URLs like "videos/past-broadcasts" anymore. |
Seriously, its been like a month since the last release. Can they soon merge this? |
Show of reacts: Who is interested in a 1:1 fork of youtube-dl, with update mirrors going into the future, I change none of the code myself (because I can't code), but I merge as many of the PRs in this repo as are viable and call it "video-dl" (if it's not taken in pypi) to avoid trademark infringement? I could spend a few hours banging out improvements (see: clicking the mouse twice), and just push a version of video-dl (or whatever I'll call it), and this would be the first code I merge. Looking into merging xvideos fixes among others after that. I'm serious. I'm a get things done kind of guy. Dstfw wants us to pay attention to submitted issues and PRs, and it seems I'm among the small number of people who actually do this unlike the maintainers. So react away boys, I'll log into Github in a few days and see whos interested. Edit: Oh wow, dstfw is tagging a probably bad PR as "do-not-merge" but his desired solution has been in discussions since 2016.. We've burned half-way through 2020 as of this comment. Wow. |
I don't want to drag the maintainer here - this is a project that only lives because of people's unpaid contributions in their spare time - but I do get the sense that one of two things is happening:
|
@dstftw / @remitamine Any chance one of you could lend a hand with getting this PR merged? Do you need us to update/fix/rework anything? |
I have been using this to download a few videos, but just recently got bugs. |
@notcrusage looks like you are having a network/DNS issue unrelated to the issue here. Confirm that this pull request still solves the issue on my end. I've resorted to writing a script that replaces the twitch.py extractor with the one from this PR every time youtube-dl updates on my server. |
Install |
@dstftw @phihag @remitamine Any progress on this? |
Before submitting a pull request make sure you have:
In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:
What is the purpose of your pull request?
Description of your pull request and other information
This PR closes #25553. The v5 api uses a numerical string for the channel ID instead of the channel name, and to get it you have to request
/kraken/users?login=NAME
instead of/kraken/channels/NAME
: v5 docs.