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

[svt] Fix download of wrong episode and update test #7789

Merged
merged 10 commits into from Sep 2, 2023

Conversation

wader
Copy link
Contributor

@wader wader commented Aug 8, 2023

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

Based on patch by @dirkf in comment #5595 (comment) Removes some REs that now accidentially finds id in a newly introduced NEXT_DATA JSON object.

Update SVTPlay test to a hopefully more stable URL (Rederiet episode 1, has no expire date)

Fixes #5595

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?

Copilot Summary

🤖 Generated by Copilot at 3964b06

Summary

🛠️📝🌐

Improved SVTPlayIE extractor by fixing a bug, simplifying code, and adding features. yt_dlp/extractor/svt.py now handles more metadata and subtitles for SVTPlay videos.

Rise from the ashes of the broken test case
Simplify the extraction of the video ID
Unleash the power of the metadata and subtitles
SVTPlayIE is the extractor of the night

Walkthrough

  • Simplify and update regex patterns for video ID extraction in SVTPlayIE extractor (link)
  • Update test case for SVTPlayIE extractor with a new video URL and ID, and add more metadata fields and subtitles (link)

@bashonly
Copy link
Member

bashonly commented Aug 21, 2023

Add a new test instead of replacing an old one. If the old test no longer works, add a skip key to the test dict (the value should be the reason why it's being skipped)

@bashonly bashonly added the pending-fixes PR has had changes requested label Aug 21, 2023
@wader wader force-pushed the svt-fix-wrong-video-downloaded branch from 3964b06 to a5c8ab7 Compare August 21, 2023 20:25
Based on patch by @dirkf in comment yt-dlp#5595 (comment)
Removes some REs that now accidentially finds id in a newly introduced __NEXT_DATA__ JSON object.

Update SVTPlay test to a hopefully more stable URL (Rederiet episode 1, has no expire date)

Fixes yt-dlp#5595
@wader wader force-pushed the svt-fix-wrong-video-downloaded branch from a5c8ab7 to 6ee025b Compare August 21, 2023 20:25
@wader
Copy link
Contributor Author

wader commented Aug 21, 2023

Add a new test instead of replacing an old one. If the old test no longer works, add a skip key to the test dict (the value should be the reason why it's being skipped)

Ok, add skip reason. No sure what is going with md5 hmm

$ python3 test/test_download.py TestDownload.test_SVTPlay
Skipping SVTPlay: Episode is no longer available
s
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK (skipped=1)

$ python3 test/test_download.py TestDownload.test_SVTPlay_1
[debug] Loaded 1864 extractors
[debug] Using fake IP 78.69.195.245 (SE) as X-Forwarded-For
[SVTPlay] Extracting URL: https://www.svtplay.se/video/emBxBQj
[SVTPlay] emBxBQj: Downloading webpage
[SVTPlay] eyBd9aj: Downloading JSON metadata
[SVTPlay] eyBd9aj: Downloading MPD manifest
[SVTPlay] eyBd9aj: Downloading m3u8 information
[SVTPlay] eyBd9aj: Downloading MPD manifest
[SVTPlay] eyBd9aj: Downloading m3u8 information
[SVTPlay] eyBd9aj: Downloading MPD manifest
[SVTPlay] eyBd9aj: Downloading m3u8 information
[SVTPlay] eyBd9aj: Downloading MPD manifest
[SVTPlay] eyBd9aj: Downloading m3u8 information
[SVTPlay] eyBd9aj: Downloading m3u8 information
[SVTPlay] eyBd9aj: Downloading MPD manifest
[SVTPlay] eyBd9aj: Downloading m3u8 information
[debug] Formats sorted by: hasvid, ie_pref, lang, quality, res, fps, hdr:12(7), vcodec:vp9.2(10), channels, acodec, size, br, asr, proto, vext, aext, hasaud, source, id
[info] eyBd9aj: Downloading 1 format(s): dash-hbbtv-avc-0
[info] Writing video metadata as JSON to: test_SVTPlay_1_eyBd9aj.info.json
.
----------------------------------------------------------------------
Ran 1 test in 2.388s

OK

@bashonly
Copy link
Member

Don't worry about md5, I edited my comment after realizing there was a skip_download param in the test

In the future, please avoid force-pushing. All commits will squashed before merge

Patch looks good

@bashonly bashonly added pending-review PR needs a review and removed pending-fixes PR has had changes requested labels Aug 21, 2023
yt_dlp/extractor/svt.py Outdated Show resolved Hide resolved
yt_dlp/extractor/svt.py Outdated Show resolved Hide resolved
yt_dlp/extractor/svt.py Outdated Show resolved Hide resolved
wader and others added 3 commits August 27, 2023 09:57
Co-authored-by: pukkandan <pukkandan.ytdlp@gmail.com>
Co-authored-by: pukkandan <pukkandan.ytdlp@gmail.com>
Co-authored-by: pukkandan <pukkandan.ytdlp@gmail.com>
@wader
Copy link
Contributor Author

wader commented Aug 27, 2023

Thanks. Successfully ran tests and tried a bit with some random svtplay URLs, then i noticed that there are URLs that work with the old version but not with these changes like https://www.svtplay.se/klipp/e9dBZ9R/det-kan-handa-efter-prigozjins-formodade-dod

@wader
Copy link
Contributor Author

wader commented Aug 27, 2023

Re-adding r'["\']videoSvtId\\?["\']\s*:\s*\\?["\']([\da-zA-Z-]+)', seems to fix https://www.svtplay.se/klipp/e9dBZ9R/det-kan-handa-efter-prigozjins-formodade-dod

Wonder if we should readd more? add test?

@bashonly
Copy link
Member

bashonly commented Aug 30, 2023

Re-adding r'["\']videoSvtId\\?["\']\s*:\s*\\?["\']([\da-zA-Z-]+)', seems to fix...

Wasn't that one of the problematic patterns that was matching the wrong video ID? If we re-add more back, what is the point of this PR? Should more research be done into which patterns yield the wrong IDs?

Perhaps the JSON should be properly parsed instead

@wader
Copy link
Contributor Author

wader commented Aug 31, 2023

@bashonly tried and yes it break things again :( so yes some more proper less patchy solution is probably needed

@bashonly
Copy link
Member

@wader Test my changes and let me know

@wader
Copy link
Contributor Author

wader commented Aug 31, 2023

@wader
Copy link
Contributor Author

wader commented Sep 1, 2023

Retried, all good 👍

@wader
Copy link
Contributor Author

wader commented Sep 2, 2023

Let me know if i should help testing etc

@bashonly bashonly merged commit 2301b5c into yt-dlp:master Sep 2, 2023
13 checks passed
@bashonly bashonly removed the pending-review PR needs a review label Sep 2, 2023
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dowloading file from svtplay.se sometimes results in the wrong file being downloaded
3 participants