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

Support BBC Reel URIs #28268

Closed
wants to merge 16 commits into from
Closed

Support BBC Reel URIs #28268

wants to merge 16 commits into from

Conversation

dirkf
Copy link
Contributor

@dirkf dirkf commented Feb 25, 2021

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

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

Extracted from PR #23415.

Fixes Issue #21870 (etc).

Test URL: https://www.bbc.com/reel/video/p07c6sb6/how-positive-thinking-is-harming-your-happiness

Extracted from PR #23415.

Fixes Issue #21870.
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
Using this logic:
* find the first object in the nested JSON that has `kind` and `versionID` attributes
* return the value of the `versionID`.
Copy link
Collaborator

@remitamine remitamine left a comment

Choose a reason for hiding this comment

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

Add a test.

youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
Assume it's the `clipPID` of the first object in `initData`'s `items`.

Not finding valid initial data is a warning.
Assumes only one item in `smpData['items']` that gives the clip details.

However this seems much simpler, although it finds no description:
```
        if not programme_id:
            programme_id = self._search_regex(
                r'/reel/video/(?P<id>%s)/' % self._ID_REGEX, url, 'Reel pid', de
            if programme_id:
                return self.url_result(
                            'https://www.bbc.co.uk/programmes/%s' % programme_id
                            ie=BBCCoUkIE.ie_key())
```
Also reinstate thumbnails
@dirkf
Copy link
Contributor Author

dirkf commented Feb 28, 2021

Here's a version with a test that succeeds and returns with metadata after parsing the JSON.

However this seems much simpler, although it finds no description:

        if not programme_id:
            programme_id = self._search_regex(
                r'/reel/video/(?P<id>%s)/' % self._ID_REGEX, url, 'Reel pid', default=None)
            if programme_id:
                return self.url_result(
                            'https://www.bbc.co.uk/programmes/%s' % programme_id,
                            ie=BBCCoUkIE.ie_key())

youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
youtube_dl/extractor/bbc.py Outdated Show resolved Hide resolved
Let `_sort_formats()` punt if no formats found.
Then get optional metadata.
Only return one of `thumbnails`, `thumbnail`
Also, don't include default `alt_title` if `title` is set, and only omit `None` values, not just any falsey.
@dirkf dirkf requested a review from remitamine March 1, 2021 18:10
@remitamine remitamine closed this in e465b25 Mar 2, 2021
@dirkf
Copy link
Contributor Author

dirkf commented Mar 2, 2021

Thanks for the guidance and topping and tailing the result. I did have the feeling that you knew how to implement it yourself and actually that might have been easier.

PRs ought to come with tests but I couldn't see any spec of how to populate the _TESTS list. Extractor mods are more likely to come with tests if it's obvious how to do it -- maybe put it in the template?

@remitamine
Copy link
Collaborator

remitamine commented Mar 2, 2021

PRs ought to come with tests but I couldn't see any spec of how to populate the _TESTS list. Extractor mods are more likely to come with tests if it's obvious how to do it -- maybe put it in the template?

_TESTS is simply an array of test cases that has the same spec as _TEST dict that has an exmple in step 4 of Adding support for a new site section.

github-actions bot added a commit to hellopony/youtube-dl that referenced this pull request Mar 2, 2021
* https://github.com/ytdl-org/youtube-dl:
  [9c9media] fix extraction for videos with multiple ContentPackages(closes ytdl-org#28309)
  [bbc] correct catched exception type
  [bbc] add support for BBC Reel videos(closes ytdl-org#21870, closes ytdl-org#23660, closes ytdl-org#28268)
  release 2021.03.02
  [ChangeLog] Actualize [ci skip]
  [zdf] Rework extractors (closes ytdl-org#11606, closes ytdl-org#13473, closes ytdl-org#17354, closes ytdl-org#21185, closes ytdl-org#26711, closes ytdl-org#27068, closes ytdl-org#27930, closes ytdl-org#28198, closes ytdl-org#28199, closes ytdl-org#28274)
leshasmlesha pushed a commit to leshasmlesha/youtube-dl that referenced this pull request Mar 7, 2021
leshasmlesha pushed a commit to leshasmlesha/youtube-dl that referenced this pull request Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants