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

Return list of videos or call FD.download in playlist IEs #608

Open
FiloSottile opened this issue Dec 29, 2012 · 1 comment
Open

Return list of videos or call FD.download in playlist IEs #608

FiloSottile opened this issue Dec 29, 2012 · 1 comment

Comments

@FiloSottile
Copy link
Collaborator

@FiloSottile FiloSottile commented Dec 29, 2012

@phihag I can't make my mind on this. (We can also have a IRC chat if you want).

Here's the problem: ATM at the end of YoutubePlaylistIE we call FD.download on the found ids. This is a pretty practical solution. However, I think we should consider returning a list instead.

Here's the comparison.

Pro FD.download:

  • The info extraction of each video happens just before the download. This is a fondamental point.
  • It works well for URL shortener recognizing.

Pro return list:

  • It gives control over how many videos are being downloaded, avoiding call cascades. Try ytdl -o xxx PL63F0C78739B09958, this is a bug, and it is not easily and elegantly solvable ATM.
  • It is step towards APIfication, we really need to stop using the downloader object in the IEs. I am cleaning all those self._downloader but this is a blocking issue.

A compromise might be to return two lists, one of info_dicts, one of links to add to the download queue.

@phihag
Copy link
Contributor

@phihag phihag commented Dec 29, 2012

We should do neither. Instead, every IE - playlist or not - should return a JSON-serializable object describing its results. The _type property of this result should be one of:

  • video - An extracted video. Apart from title, upload_date, etc., it has a formats parameter, which would be a list of dictionaries with the entries format_id, url, and ext at the least (others would be resolution).
  • url - A reference to a video (we could also construct these for the initial invocation). Has the url property (and can have an ie_key property if the extractor knows the next extractor in line).
  • playlist - Can have an attribute such as title. Most importantly, has an attribute entries, which is a list of dictionaries, each in the same structure as presented here.

There should be helper functions for the common case of a one-format video and a playlist of URLs.

If the IE encounters an error, it should throw one. We should provide a generic ExtractorError, but also simply allow ValueError et al.

If the IE wants to provide feedback to the user, it should call a function that it gets passed upon construction. We can add a helper function to_screen in the default IE implementation that adds in the IE name.

We also need to pass the parameters we currently have to the IE then, and not (only) to the downloader.

jaimeMF added a commit that referenced this issue Apr 20, 2013
Except the ones in youtube subtypes (user, channels ..) all calls to _downloader.to_screen has been changed.
The calls not prefixed with the IE name hasn't been touched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.