[PBS] Fix AttributeError: 'NoneType' #16623
Conversation
This is a fix for #15373
youtube_dl/extractor/pbs.py
Outdated
| try: | ||
| video_id = self._search_regex( | ||
| r'<div\s+id="video_([0-9]+)"', player_page, 'video ID') | ||
| except: |
There was a problem hiding this comment.
Never use bare except. Use list of regexes in _search_regex instead.
youtube_dl/extractor/pbs.py
Outdated
| @@ -455,7 +455,9 @@ def _extract_webpage(self, url): | |||
|
|
|||
| if not url: | |||
| url = self._og_search_url(webpage) | |||
|
|
|||
|
|
|||
| if url.strip().startswith("//"): | |||
youtube_dl/extractor/pbs.py
Outdated
|
|
||
|
|
||
| if url.strip().startswith("//"): | ||
| url = "https:" + url.strip() |
youtube_dl/extractor/pbs.py
Outdated
| @@ -466,7 +467,7 @@ def _extract_webpage(self, url): | |||
| url, display_id, note='Downloading player page', | |||
| errnote='Could not download player page') | |||
| video_id = self._search_regex( | |||
| r'<div\s+id="video_([0-9]+)"', player_page, 'video ID') | |||
| [r'<div\s+id="video_([0-9]+)"', r'"id":[\s]*"([0-9]+)"'], player_page, 'video ID') | |||
There was a problem hiding this comment.
[] is pointless. Must be more relaxed.
There was a problem hiding this comment.
What ? I need to use [] for making a list like you want.
There was a problem hiding this comment.
So execuse me but I don't understand your comment.
What do I have to change?
There was a problem hiding this comment.
[] is superfluous in your regex. What's not clear here?
There was a problem hiding this comment.
Sorry I thought you were talking about the [] around the 2 regex.
|
Is there anything else to do before the merge ? |
|
Add a test. |
youtube_dl/extractor/pbs.py
Outdated
| @@ -466,7 +467,7 @@ def _extract_webpage(self, url): | |||
| url, display_id, note='Downloading player page', | |||
| errnote='Could not download player page') | |||
| video_id = self._search_regex( | |||
| r'<div\s+id="video_([0-9]+)"', player_page, 'video ID') | |||
| [r'<div\s+id="video_([0-9]+)"', r'"id":\s*"([0-9]+)"'], player_page, 'video ID') | |||
There was a problem hiding this comment.
I've already pointed out: relax regex. It must match arbitrary quotes and whitespace.
There was a problem hiding this comment.
I'm sorry but I can't change the new regex otherwise it will point to other things that are very similar but incorrect.
There was a problem hiding this comment.
Actually this approach is redundant since id can be extracted right from webpage.
There was a problem hiding this comment.
In rare cases it cannot be extracted directly from the webpage (like embed video)
There was a problem hiding this comment.
That's not an excuse for not adding another id patern for extraction from webpage that works fine with your test. Also add a test with "rare" case.
There was a problem hiding this comment.
Yes, it seems that it works due to this commit (d7be705) which was published after my pull request but merge before me. However my test is valid without the previous commit.
I also maintain that my id extraction can serve but if you insist I can remove it.
There was a problem hiding this comment.
Again: https://www.pbs.org/wgbh/masterpiece/episodes/victoria-s2-e1/ can be processed without downloading player_page by looking for media id in webpage.
|
This pull request is also a fix for #16684 |
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
This is a fix for #15373