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

YoutubeUserIE improvements #18

Closed
wants to merge 9 commits into from
Closed

YoutubeUserIE improvements #18

wants to merge 9 commits into from

Conversation

ppawel
Copy link

@ppawel ppawel commented Nov 6, 2010

This branch contains some improvements to the way specific user's videos are downloaded. Before it didn't work (at least when I tested it), probably due to the fact that YT user channel pages now use AJAX to download a list of user videos.

I changed the approach by using YT Data API to collect all video ids. Let me know what you think.

TODO:

  1. Properly handle playlist start/end parameters.
  2. Perhaps handle command line in the form of "ytuser:" - there is no real need for giving any URL's to this InfoExtractor, username should be enough (?).

@rg3
Copy link
Collaborator

rg3 commented Nov 6, 2010

Fantastic. Just keep working on the code. The two features you mention would be nice. As with other InfoExtractors that work on a list of videos, the playlist start and end parameters could be used and would be a nice feature.

About the URL, I'm not sure. I don't "use" YouTube that much except for watching specific videos. If there's a common "user" URL you can find in many places, it could be supported by the InfoExtractor. Else, or apart from that, you could handle a "ytuser:" pseudo-URL too if you think it could be useful for our case.

I gave a quick glance at the code and it looks fine. We'll address minor problems later. As with the Vimeo InfoExtractor, mail me or comment on this issue when the changes are ready.

Cheers!

@rg3
Copy link
Collaborator

rg3 commented Jan 12, 2011

Pawel,

I never got around reviewing more code from this and didn't receive more pull requests or message notifications. Are the two aspect implemented and the YoutubeUserIE stable? Did you finally add support for "ytuser:" URLs? If so, we could take a few moments and finally merge this.

@ppawel
Copy link
Author

ppawel commented Jan 27, 2011

Sorry, perhaps I got a little lazy with this branch in the last few months - after I finished fixing this IE I've been using it without problems on a daily basis, doing only small tweaks and merging master into it. I will address you comments and create another pull request.

… it seems to work fine after recent commits on master - so uncommenting it.
@rg3
Copy link
Collaborator

rg3 commented Jan 27, 2011

Thanks for your time and code.

There is another user who wrote me by email, Forrest Shoemake, who was working on this too. He submitted a bit of code but it didn't fit with the rest of youtube-dl much. He hasn't created any commit yet, but he's supposed to be working here:

https://github.com/1337-Ac3/youtube-dl

@ppawel
Copy link
Author

ppawel commented Jan 28, 2011

OK, so I've just added support for the two missing features. Let me know what else you want to have in this branch before merging it.

BTW, at first I based playlist start/end support code on the one from YoutubePlaylistIE but I think it is wrong:

            playliststart = self._downloader.params.get('playliststart', 1) - 1
            playlistend = self._downloader.params.get('playlistend', -1)
            video_ids = video_ids[playliststart:playlistend]

If playlistend is -1 (ie. user didn't specify --playlist-end option) then the last video id will always be missing. I fixed it in YoutubeUserIE.

@rg3
Copy link
Collaborator

rg3 commented Jan 28, 2011

Apparently, in playlists without using the gdata YouTube API, the last item in the playlist is always bogus. That's why the code is correct even if it doesn't look like it.

@rg3
Copy link
Collaborator

rg3 commented Jan 29, 2011

Pawel,

I cannot merge your branch as is. It contains too many unwanted changes and a convoluted history. I think you should have rebased your changes instead of merging from my master branch several times. For example, this commit is not needed as we have a -i option to ignore download errors and continue:

https://github.com/ppawel/youtube-dl/commit/afef7c15994e73da7272bcfa6ed761405824f106

Or, in another commit, you disable the gzip compression stuff that was not working at some point but now is.

As I cannot merge that branch, I'll do the following: From your branch, I'll take the InfoExtractor code and replace the current one with yours. That should work, right? If you see a problem with that, create a new branch from scratch and put your changes there as a single commit, and I'll cleanly merge that.

@rg3
Copy link
Collaborator

rg3 commented Jan 29, 2011

I just imported your InfoExtractor, tweaked the code a little bit and added your name to the list of authors. Please let me know if you find any problems with the current code. I'm closing the pull request.

@ppawel
Copy link
Author

ppawel commented Jan 29, 2011

It's fine by me. Thanks for merging the code.

phihag added a commit that referenced this pull request Jan 12, 2013
@FelixChery FelixChery mentioned this pull request Apr 24, 2019
joedborg referenced this pull request in joedborg/youtube-dl Nov 17, 2020
[pull] master from rg3:master
This pull request was closed.
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 this pull request may close these issues.

2 participants