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

[ie/orf:on] Improve extraction #9677

Merged
merged 13 commits into from
May 23, 2024
Merged

Conversation

TuxCoder
Copy link
Contributor

@TuxCoder TuxCoder commented Apr 12, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

  • add better DRM detection: ref: on.orf.at not complete DRM detection #9652
  • add parsing of the v4.3 api for subtitles
  • better url parsing
    example for 7 digits: https://on.orf.at/video/3229173
  • better tests
    the new test data has killdate: None as attribute, so this should be more stable for the future
    sadly I did't found any video with age restriction and right: "worldwide" only with restriction to Austria and/or with a killdate < 2025, IMHO not worth to add as will create future problem
Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check all of the following options that apply:

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

Open Questions:

  • I did't get the other subtitles formats working 'xml', 'srt', 'sami', 'ttml', 'stl' that are available so I comment them out
  • should display_id be remove?! as it is not included with all urls from on.orf.at

As this is my first PR to yt-dlp I hope I didn't oversaw anything from the good written guidelines.
If so feel free to point that out.

tuxcoder added 2 commits April 12, 2024 23:14
use the linked subtitles in the v4.3 api
some urls are without slug
also some video-Id's are shorter that 8 digits
and could probably be bigger than 8
@seproDev seproDev added site-enhancement Feature request for some website site-bug Issue with a specific website labels Apr 20, 2024
yt_dlp/extractor/orf.py Show resolved Hide resolved
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
yt_dlp/extractor/orf.py Show resolved Hide resolved
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
@seproDev seproDev added the pending-fixes PR has had changes requested label May 19, 2024
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
tuxcoder added 4 commits May 20, 2024 13:15
some videos formats were not reported as DRM protected
fixes: yt-dlp#9652
new test file has no kill_date,
so I hope it will be online for longer
old one will be not available after "2024-08-12T21:05:00+02:00"
as display_id is optional it does not make sense to keep it
@seproDev
Copy link
Collaborator

Please do not force push. All commits will be squashed upon merge, and it makes reviewing harder.

@TuxCoder
Copy link
Contributor Author

thanks for the Feedback, try to add the changes and rerun the tests.

Force Push,
Sorry didn't know you squash the commit for the merge.
tried to have a clean commit history.
Will not do it in the future

yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
@TuxCoder
Copy link
Contributor Author

Thanks again for the good feedback.

I'm not sure about the subtitles as I not wanted to break --embed-subs by default.
Is there another solution for this?

Otherwise should be good now.

@seproDev seproDev changed the title [on.orf.at] better/more data parsing [ie/orf:on] Improve extraction May 20, 2024
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
tuxcoder added 2 commits May 20, 2024 14:51
interrate over all subtitles and sort that `vtt` is default,
so `--embed-subs` is working out of the box
Copy link
Collaborator

@seproDev seproDev left a comment

Choose a reason for hiding this comment

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

Last comments from me

yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
tuxcoder and others added 2 commits May 20, 2024 15:13
as `vtt_url` is the last in the list,
it will get priority
Co-authored-by: sepro <4618135+seproDev@users.noreply.github.com>
@seproDev seproDev added pending-review PR needs a review and removed pending-fixes PR has had changes requested labels May 20, 2024
@Grub4K Grub4K removed the pending-review PR needs a review label May 22, 2024
yt_dlp/extractor/orf.py Outdated Show resolved Hide resolved
@seproDev seproDev merged commit 0dd53fa into yt-dlp:master May 23, 2024
5 checks passed
@TuxCoder
Copy link
Contributor Author

Thanks again for the nice Review!

I got today motivated to publish my documentation for the API
you can find it here as openapi3 format:
https://git.o-g.at/tuxcoder/orf_fetcher/src/branch/main/src/orf-api_v4.3.yaml

Feel free to us it

@TuxCoder TuxCoder deleted the on_orf_at_more_parsing branch May 23, 2024 19:35
@TuxCoder TuxCoder restored the on_orf_at_more_parsing branch June 2, 2024 16:00
@TuxCoder TuxCoder deleted the on_orf_at_more_parsing branch June 2, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-bug Issue with a specific website site-enhancement Feature request for some website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants