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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure dir existence for thumbnails and subtitles #7985
Conversation
Fixes output template `pl_thumbnail`. This problem went unnoticed while downloading videos because they're written into an already ensured dir, while playlists called the thumbnail writing method directly.
a675ef1
to
adddcf2
Compare
There, I've applied all suggestions. Sorry for the delay! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In the future, please do not force push; all commits will be squashed upon merge anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fine to me
Closes yt-dlp#8203 Authored by: Riteo
IMPORTANT: PRs without the template will be CLOSED
Description of your pull request and other information
Fixes output template
pl_thumbnail
.Before this PR, trying to download a playlist with an output template of type
pl_thumbnail
pointing to a missing subdirectory doesn't create it, instead erroring out.This problem went unnoticed while downloading videos because they're written into an already ensured dir, while playlists called the thumbnail writing method directly.
The subtitle change, while technically not required (playlists don't usually have subs) is done for consistency with all the other metadata writing methods. I can remove it if there's the need but it shouldn't hurt.Also I'm not really sure of the expected order of conditions, like, whether I should've put the checks right at the beginning or somewhere else. Please tell me if you'd like them somewhere else, I can change them no problem.
Example of a fixed invocation (tested with commit 4b3a6ef):
yt-dlp -vU --no-config --flat-playlist --write-thumbnail -o 'pl_thumbnail:test/%(id)s.%(ext)s' https://youtube.com/playlist?list=PLFG57rtsH8Jso3qfd5eO6egyBtW3nyvir
verbose log before fix
verbose log after fix
Fixes... Nothing I could find with a couple of searches with various keywords.
Sorry, it's my first time here. Do you allow direct PRs?
CONTRIBUTING.MD
doesn't say much either. If that's not the case, I can open a simple issue right away.I hope that making the fix directly is better (merged or not I still need some workaround for this issue in the meantime so I'm not wasting time for myself anyways).
Closes #8203
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:
What is the purpose of your pull request?
Copilot Summary
馃 Generated by Copilot at a675ef1
Summary
馃悰馃摑馃毃
Add checks for output directory existence in
_write_subtitles
and_write_thumbnails
. Update docstring of_write_thumbnails
.Walkthrough
_write_thumbnails
to reflect this possibility (link)