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

[Heise] Add support for new c't uplink episodes #14108

Closed
wants to merge 5 commits into from

Conversation

kayb94
Copy link
Contributor

@kayb94 kayb94 commented Sep 3, 2017

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:

  • I am the original author of this code and I am willing to release it under Unlicense

What is the purpose of your pull request?

  • Improvement

Description of your pull request and other information

Add support for new c't uplink episodes.

The previous way of extraction still works for all content before somewhat 2 weeks ago and might be the only way for some content, thereby still kept.

@kayb94
Copy link
Contributor Author

kayb94 commented Sep 16, 2017

Can someone merge this? I would really like to have other people be able to download the new c't uplink episodes with youtube-dl, too!

video_id, "Downloading alternative XML (SD)")
hd_rss_feed = self._download_xml(
'https://blog.ct.de/ctuplink/ctuplinkvideohd.rss',
video_id, "Downloading alternative XML (HD)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code duplication.

descriptions = hd_rss_feed.findall('./channel/item/description')

sd_video_urls = sd_rss_feed.findall('./channel/item/guid')
hd_video_urls = hd_rss_feed.findall('./channel/item/guid')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code duplication.


formats.append({
'url': hd_video_urls[episode_index].text,
'height': 720})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code duplication.

@kayb94
Copy link
Contributor Author

kayb94 commented Sep 17, 2017

Thanks for the review. Resolved those.
I also added an audio RSS that I additionally found and manually replaced some URLs with their redirects (speed things up).

video_id, "Downloading alternative XML (SD)"),
self._download_xml(
'https://www.heise.de/ct/uplink/ctuplinkvideohd.rss',
video_id, "Downloading alternative XML (HD)")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Code duplication.
  2. Each should be non fatal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

for path, format_id in (('', 'audio'), ('video', 'sd'), ('videohd', 'hd')):
    self._download_xml(
        'https://www.heise.de/ct/uplink/ctuplink%s.rss' % path,
        video_id, 'Downloading %s XML' % format_id, fatal=False)

@kayb94
Copy link
Contributor Author

kayb94 commented Sep 17, 2017

Hope this is okay. Just saw your comment afterwards at my mails!

@kayb94
Copy link
Contributor Author

kayb94 commented Sep 27, 2017

@dstftw Friendly ping: is this ready?

@kayb94
Copy link
Contributor Author

kayb94 commented Sep 30, 2017

Do not merge yet, please. I tried it again today (new episode) and it did not work. The latest episode is not in the RSS feeds. So either they just did not yet put it in there, or they stopped updating in. Have to investigate first.

Regards

@mpenkov
Copy link

mpenkov commented Aug 20, 2018

@kayb94 Just a friendly ping: how's it going? Do you think you can manage to finish this PR?

@kayb94
Copy link
Contributor Author

kayb94 commented Aug 20, 2018

@mpenkov This Pull Requests should have been closed, as the original issue has been solved (again) by 8e70c1b
But I aggree with you, that currently the c't uplink episodes are not extracted properly -- only the audio, right? And the type of extraction from this PR would indeed solve that. It looks, like they did not stop updating RSS (why I said "do not merge yet").

But I won't undertake any new efforts, before my long standing open pull requests #16880 and #15855 aren't looked after.

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defunct PR source branch is not accessible pending-fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants