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/ard] Revert to using old id #8916

Merged
merged 4 commits into from Jan 5, 2024
Merged

[ie/ard] Revert to using old id #8916

merged 4 commits into from Jan 5, 2024

Conversation

Grub4K
Copy link
Member

@Grub4K Grub4K commented Jan 2, 2024

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

Using the new id is inconvenient for users bc it is just too long. Since the old id might be removed eventually though, this PR provides a fallback to the new id if it ever breaks, with a warning to open an issue about the breakage

People should use --print-to-file "%(_old_archive_ids)#l" archive.txt to put the new id into their archives, so when the old id disappears the connection is not completely lost

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?

@dirkf
Copy link
Contributor

dirkf commented Jan 3, 2024

If the old ID really is unreliable, how about using a digest of the new ID?

@Grub4K
Copy link
Member Author

Grub4K commented Jan 3, 2024

Thats a fair comment, and will limit the id to a fixed length, however that means we cant get the id back from the hashed id easily. One benefit of the new ID is that we can get back to the media using that ID. Keeping the full id under display_id is def something we want to do either way though so that could cover that. Wed have to think about it @seproDev

@seproDev seproDev added the site-bug Issue with a specific website label Jan 4, 2024
yt_dlp/extractor/ard.py Outdated Show resolved Hide resolved
@bashonly bashonly self-requested a review January 5, 2024 17:09
yt_dlp/extractor/ard.py Outdated Show resolved Hide resolved
@Grub4K Grub4K merged commit b695127 into yt-dlp:master Jan 5, 2024
6 checks passed
@Grub4K Grub4K deleted the fix/ard-id branch January 5, 2024 21:55
@seproDev
Copy link
Collaborator

seproDev commented Jan 9, 2024

We reached out to ARD and asked if the contentId field will be removed. Here is their response:

Das genannte Feld wird mittelfristig noch weiterhin ausgegeben, wird aber längerfristig ausgebaut werden. Wann genau das sein wird, können wir leider noch nicht sagen.

This field will continue to be returned in the medium term, but will be removed in the longer term. Unfortunately, we cannot yet say exactly when this will be.

To reiterate what Grub4K said: If you are using the download archive, you should add --print-to-file "%(_old_archive_ids)#l" archive.txt to put the new ID into your archive. Otherwise, the connection between the old and new IDs will be completely lost once the old IDs are removed.

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.

None yet

4 participants