-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[npr] Add new extractor #13446
base: master
Are you sure you want to change the base?
[npr] Add new extractor #13446
Conversation
youtube_dl/extractor/npr.py
Outdated
return formats | ||
|
||
|
||
class NprPlaylistIE(NprBaseIE): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't change extractor name.
youtube_dl/extractor/npr.py
Outdated
class NprIE(InfoExtractor): | ||
_VALID_URL = r'https?://(?:www\.)?npr\.org/player/v2/mediaPlayer\.html\?.*\bid=(?P<id>\d+)' | ||
class NprBaseIE(InfoExtractor): | ||
def extract_info(self, id): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't shadow built-in names.
youtube_dl/extractor/npr.py
Outdated
} | ||
|
||
def _real_extract(self, url): | ||
display_id = self._match_id(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a display id.
youtube_dl/extractor/npr.py
Outdated
|
||
return json_data['list']['story'][0] | ||
|
||
def extract_formats(self, media_info): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be private.
youtube_dl/extractor/npr.py
Outdated
class NprIE(InfoExtractor): | ||
_VALID_URL = r'https?://(?:www\.)?npr\.org/player/v2/mediaPlayer\.html\?.*\bid=(?P<id>\d+)' | ||
class NprBaseIE(InfoExtractor): | ||
def extract_info(self, id): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be private.
youtube_dl/extractor/npr.py
Outdated
'title': story.get('title', {}).get('$text'), | ||
'id': video.get('id'), | ||
'duration': int_or_none(video.get('duration', {}).get('$text')), | ||
'formats': self.extract_formats(video), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No url
and formats
at the same time.
youtube_dl/extractor/npr.py
Outdated
return { | ||
'url': url, | ||
'display_id': display_id, | ||
'title': story.get('title', {}).get('$text'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Title is mandatory.
Ok, pending fixes resolved and also problem with m3u8 formats. @dstftw
|
The issue with #13440 is a site error. the hls url in the page source is 404: This is because the 200000 quality is not actually present on the server. An edited hls url does work: There is also html5 mp4 progressive url (600000 quality) in the page source which does work. It can be extended to the higher qualities using dirty heuristics or the quality values from the hls url. Best quality: https://ondemand.npr.org/npr-mp4/npr/ascvid/2017/06/20170619_ascvid_tigersjaw-n-2000000.mp4 |
Interesting, I will add this to the extractor. Any suggestion with rtmp? @stinkteeth |
rtmp seems to work okay if --resume is removed. Not sure why. |
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
I rewritten Npr extractor and added support for #13440. However there is a problem with m3u8 playlists from NprVideo that returns error 404.
regards,
Giuseppe Fabiano