-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[postprocessor/embedthumbnail] Fix Youtube's WebP thumbnails by converting them to JPG #25717
Conversation
This fix works for me! One thing that might need to be tweaked is to have it strip the original thumbnail extension when converting - right now it leaves a file |
Thanks @empyrical, I changed the file name. |
I cloned the repo, added your "fix" and then ran youtube-dl with
|
Are you sure that you applied the patch correctly? I added a log message when a thumbnail conversion takes place and it is missing from your output.
As you can see, there is an additional message |
@alexmerkel I applied the fix correctly, the issue was that it wasn't running the version with the fix added, it was running the version I had previously installed using pip3. All is good now. |
An error occurs when there is "%" in the file title |
Oh, good catch @njoynj. |
I'd like to compile a binary of of Alex's patch but there is no instruction on how to compile it to a Windows exe binary it would be handy if someone wrote a wiki on how to do that but that is a side tangent |
For those who want to easily install youtube-dl with the fix implemented, you can do: |
I used the youtube-dl version that @BassThatHertz suggested and it works perfectly, after I cut out all the affected videos from my music playlist from |
Hi, StefSuse:/home/stefan # python3 -m youtube_dl --verbose --extract-audio --no-mtime --audio-quality 4 --embed-thumbnail --audio-format mp3 -o test.mp3 https://www.youtube.com/watch?v=Phsr8tC0R6w |
Can confirm @Labud98 's problem happens when you specify the output filename. Using output templates and not specifying a filename works fine. |
More about the bug @Labud98 found From original comment
youtube-dl seems to save the downloaded opus file as test.mp3 and skips converting the audio to actual mp3 format. This breaks ffmpeg when it tries to look for mp3 streams in an opus file.
I'm not that good at python to fix this but hope this helps. |
Using the unpatched (unrelated to @alexmerkel PR) youtube-dl version, I can confirm this ^ is a different bug. (my output: [debug] System config: [] ) The PR looks clean enough to me. |
Youtube-dl is broken for a very large number of use cases because of this bug. No new issues with the PR have been found in 2 weeks, and it's pretty small and straightforward. I hate to be pushy, but why hasn't this been merged? |
Somewhat related: #24882
|
Could we get a merge please @dstftw 😀 |
NOTE: The thumbnail embedding part is the cause of many conversion failures, and [this](ytdl-org/youtube-dl#25717) PR can probably fix it.
I just downloaded youtube-dl after youtube-dlg stopped working so I might be doing this wrong, but here's the output that I got with the same command:
It looks like it should work but the mp3 file still doesn't have the thumbnail embedded? |
@silariel Unfortunately I can't reproduce your issue. When I run your command, the resulting file has the embedded thumbnail. Have you tried opening the MP3 with a different program to see if the thumbnail appears there? |
What is different about the way thumbnails are added with youtube-dl to, say, official cover art of a song purchased from iTunes? Because I have noticed that the cover art added with youtube-dl does not show up in Groove Music and Windows Media Player but cover art of music that I have purchased does. @silariel If you're on Windows, try foobar2000. It shows the cover art. I also like the fact that it shows the bitrate in realtime (at the bottom-right) for VBR files. See the image below. As you can see, it shows the cover art. |
According to this stackoverflow answer, it might be because the ffmpeg command uses |
@alexmerkel I'm trying to test this myself by adding I'm getting the following error when running youtube-dl with
I get this error even without adding the |
@BassThatHertz no, the flag has to be added to the ffmpeg command that adds the thumbnail to the MP3 file
to
The other error should be the unrelated bug that occurs when specifying the output filename that was discussed above. |
@alexmerkel Just did some testing. With regards to the output filename issue, I can see that someone else has brought up the same issue but nobody has explained what's causing that issue. |
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.
Add '-id3v2_version', '3'
to the options
list to fix the issue of no cover art being seen when using Groove Music on Windows or Windows Media Player, and perhaps other applications.
I'm not sure this should be added to this pull request. |
@alexmerkel Makes sense. I would issue a new pull request but the issue is that your fix hasn't been merged with the master branch yet, and adding a fix for cover art visibility on some Windows applications doesn't make sense because right now the thumbnails don't even get added to the file (due to being WebP). In other words, with the current youtube-dl code, adding I guess I need to wait for your fix to be merged with the master branch, which is taking a while. |
@Labud98, @adithya-s-sekhar, @michaelb, @BassThatHertz I found the reason for the
So the file has the extension .mp3 but it isn't really an MP3 file. I think a new issue/pull request should be created to address this problem. |
I've noticed it too. This issue has nothing to do with this thumbnail fix. @alexmerkel @Labud98 @BassThatHertz Basically you shouldn't give .mp3 as an extension and use |
@Labud98, @adithya-s-sekhar, @michaelb, @alexmerkel I've submitted a fix for |
If possible, you should remove/decline my suggested changes. I don't want anything delaying this fix being merged to the master branch. If/when this fix gets merged, I may submit a pull request which passes |
Oops sorry, wasn't keeping track of this thread but this worked like a charm! :) ^ Thanks for the suggestions and help, y'all! |
Some older(?) Android devices/apps also needs the |
@AvinashReddy3108 FFmpeg only supports ID3v2 versions 3 and 4. |
somehow, 'ffmpeg' in my Kubuntu 18.04 cannot convert webp. thanks. |
I'm still having this issue as well when downloading mp4 videos on YouTube. Thumbnail downloads but doesn't embed. |
5e26784
to
da2069f
Compare
with open(encodeFilename(thumbnail_filename), "rb") as f: | ||
b = f.read(16) | ||
if b'\x57\x45\x42\x50' in b: # Binary for WEBP |
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.
This makes no sense if extension is not webp
, extension must be checked first.
@@ -41,6 +41,28 @@ def run(self, info): | |||
'Skipping embedding the thumbnail because the file is missing.') | |||
return [], info | |||
|
|||
# Check for mislabeled webp file | |||
with open(encodeFilename(thumbnail_filename), "rb") as f: | |||
b = f.read(16) |
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.
WebP header occupies 12 bytes, there is no point reading more.
# Check for mislabeled webp file | ||
with open(encodeFilename(thumbnail_filename), "rb") as f: | ||
b = f.read(16) | ||
if b'\x57\x45\x42\x50' in b: # Binary for WEBP |
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.
WebP header is clearly defined as 0-3 bytes == ASCII RIFF
, 8-11 bytes == ASCII WEBP
. This is what should be checked exactly and not for WEBP
somewhere is the first 16 bytes.
# Check for mislabeled webp file | ||
with open(encodeFilename(thumbnail_filename), "rb") as f: | ||
b = f.read(16) | ||
if b'\x57\x45\x42\x50' in b: # Binary for WEBP |
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.
Use b'WEBP`.
# If not a jpg or png thumbnail, convert it to jpg using ffmpeg | ||
if not os.path.splitext(thumbnail_filename)[1].lower() in ['.jpg', '.png']: | ||
jpg_thumbnail_filename = os.path.splitext(thumbnail_filename)[0] + ".jpg" | ||
jpg_thumbnail_filename = os.path.join(os.path.dirname(jpg_thumbnail_filename), os.path.basename(jpg_thumbnail_filename).replace('%', '_')) # ffmpeg interprets % as image sequence |
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.
%d
in path still does not work:
[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['--embed-thu', 'https://www.youtube.com/watch?v=bSBTcQNPNqc', '-f', '18', '-v']
[debug] Encodings: locale cp1251, fs utf-8, out utf-8, pref cp1251
[debug] youtube-dl version 2020.09.06
[debug] Git HEAD: bf6c312
[debug] Python version 3.7.0 (CPython) - Windows-10-10.0.10240-SP0
[debug] exe versions: ffmpeg N-85653-gb4330a0, ffprobe N-85653-gb4330a0, phantomjs 2.1.1, rtmpdump 2.4
[debug] Proxy map: {}
[youtube] bSBTcQNPNqc: Downloading webpage
[youtube] bSBTcQNPNqc: Downloading thumbnail ...
[youtube] bSBTcQNPNqc: Writing thumbnail to: How to print %d using printf in C language-bSBTcQNPNqc.jpg
[debug] Invoking downloader on '...'
[download] How to print %d using printf in C language-bSBTcQNPNqc.mp4 has already been downloaded
[download] 100% of 8.39MiB
[ffmpeg] Converting thumbnail "How to print %d using printf in C language-bSBTcQNPNqc.webp" to JPEG
[debug] ffmpeg command line: ffmpeg -y -loglevel "repeat+info" -i "file:How to print %d using printf in C language-bSBTcQNPNqc.webp" "-bsf:v" mjpeg2jpeg "file:How to print _d using printf in C language-bSBTcQNPNqc.jpg"
ERROR: file:How to print %d using printf in C language-bSBTcQNPNqc.webp: No such file or directory
Traceback (most recent call last):
File "C:\Dev\youtube-dl\master\youtube_dl\YoutubeDL.py", line 2065, in post_process
files_to_delete, info = pp.run(info)
File "C:\Dev\youtube-dl\master\youtube_dl\postprocessor\embedthumbnail.py", line 61, in run
self.run_ffmpeg(thumbnail_filename, jpg_thumbnail_filename, ['-bsf:v', 'mjpeg2jpeg'])
File "C:\Dev\youtube-dl\master\youtube_dl\postprocessor\ffmpeg.py", line 239, in run_ffmpeg
self.run_ffmpeg_multiple_files([path], out_path, opts)
File "C:\Dev\youtube-dl\master\youtube_dl\postprocessor\ffmpeg.py", line 235, in run_ffmpeg_multiple_files
raise FFmpegPostProcessorError(msg)
youtube_dl.postprocessor.ffmpeg.FFmpegPostProcessorError: file:How to print %d using printf in C language-bSBTcQNPNqc.webp: No such file or directory
|
||
# If not a jpg or png thumbnail, convert it to jpg using ffmpeg | ||
if not os.path.splitext(thumbnail_filename)[1].lower() in ['.jpg', '.png']: | ||
jpg_thumbnail_filename = os.path.splitext(thumbnail_filename)[0] + ".jpg" |
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.
utils.replace_extension
.
Before submitting a pull request make sure you have:
In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:
What is the purpose of your pull request?
Description of your pull request and other information
This change should fix #25687:
Youtube started using the WebP image format for thumbnail images. However, both
ffmpeg
andAtomicParsley
only support the embedding of JPEG and PNG files. Therefore, as @noname8753 suggested, a conversion step was added during which the thumbnail is converted to JPEG usingffmpeg
.