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/NHKRadiru] Switch to new metadata API #10106

Merged
merged 9 commits into from
Jun 13, 2024

Conversation

garret1317
Copy link
Collaborator

@garret1317 garret1317 commented Jun 4, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

NHK changed the metadata API Radiru uses. It didn't matter for a few weeks, but they've taken away the old API now, so the extractor is broken.
This PR switches to the new API. It also improves the extended metadata handling a bit, since some of the metadata is now only available there.

It also breaks radio news, since they're still on the old system. but that's a whole can of worms which I don't want to get into right now. As a temporary measure, people can use the podcast version https://www.nhk.or.jp/s-media/news/podcast/list/v1/all.xml.

I should probably note, the original idea when I wrote most of this was to have the extended metadata thing be the "one true source"- but then I encountered a programme that didn't exist on the extended metadata thing, so I had to backtrack
so it might be a little rough around the edges in that regard
most of the metadata is still available from the fallback (at least the bits i use in my filename template 🚎)

Fixes #10105

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?

@garret1317 garret1317 added site-bug Issue with a specific website labels Jun 5, 2024
Copy link
Member

@bashonly bashonly left a comment

Choose a reason for hiding this comment

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

I went ahead and pushed changes, but here's a retrospective review in case you're wondering why I did some things

yt_dlp/extractor/nhk.py Outdated Show resolved Hide resolved
yt_dlp/extractor/nhk.py Outdated Show resolved Hide resolved
yt_dlp/extractor/nhk.py Show resolved Hide resolved
yt_dlp/extractor/nhk.py Outdated Show resolved Hide resolved
@bashonly bashonly self-assigned this Jun 13, 2024
@garret1317
Copy link
Collaborator Author

All LGTM, + I checked the -j json output with https://jsondiff.com/ and it was all the same 👍

I was originally going to try and put the old/news code in NHKRadioNewsPageIE; now they're off in their own separate universe, it doesn't make sense to keep them in the main Radiru extractor. But then I realised they still use the same player for individual bulletins, so keeping them here seems better for now.

@bashonly bashonly merged commit b8e2a5e into yt-dlp:master Jun 13, 2024
6 checks passed
@garret1317
Copy link
Collaborator Author

thank you 👍

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.

[NhkRadiru] Unable to download JSON metadata: HTTP Error 404 / NHK
3 participants