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

[hhu] Add new extractor #22660

Closed
wants to merge 6 commits into from
Closed

[hhu] Add new extractor #22660

wants to merge 6 commits into from

Conversation

YtvwlD
Copy link

@YtvwlD YtvwlD commented Oct 9, 2019

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
  • 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?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

HHU Mediathek contains videos created by the Heinrich-Heine-Universität Düsseldorf (mostly recordings of lectures).
This extractor allows the download of single, publicly visible videos.

@dstftw
Copy link
Collaborator

dstftw commented Oct 9, 2019

Read coding conventions.

@YtvwlD
Copy link
Author

YtvwlD commented Oct 9, 2019

I read the coding conventions before but I didn't notice the rule regarding closing braces/brackets/parentheses. (I'm used to the guidelines in PEP8, sorry about that.)

@dstftw
Copy link
Collaborator

dstftw commented Oct 9, 2019

It does not say anything about brackets and braces. Also mostly it's nothing to do with closing parentheses which are perfectly valid PEP8.
As for extractor: jwplayer is already covered by generic extractor that should be extended to support such cases.

Some labels are of the form 'low quality (320p)'.
This commit changes the regex so,
that the whole label is searched for the number, not just the beginning.
@YtvwlD
Copy link
Author

YtvwlD commented Oct 9, 2019

The generic extractor doesn't work here because _find_jwplayer_data doesn't recognize the player config because the used regex fails at the encodeURI call.

I modified my PR to at least use _parse_jwplayer_data.
Also, I had to touch the regex in _parse_jwplayer_formats to also accept format descriptions that are not at the beginning of the label text. I really hope that this won't break existing stuff - I tested a few extractors using these functions, but most of them seem have broken already before my change.

@dstftw
Copy link
Collaborator

dstftw commented Oct 9, 2019

As already said jwplayer extraction code should be extended to support sanitizing function calls out of options JSON. HHU extractor must be removed. Extraction should be processed by generic extractor.

@YtvwlD
Copy link
Author

YtvwlD commented Oct 10, 2019

For finding the options there is already #21132 which works in this case (but isn't merged yet).

So, I'm guessing I should close this PR and open two new ones - one for dada9f6 and the other one for parsing this JS?

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.

3 participants