search

ytdl-org / youtube-dl

Command-line program to download videos from YouTube.com and other video sites
http://ytdl-org.github.io/youtube-dl/
The Unlicense
132.72k stars 10.07k forks source link

MKV thumbnail not correctly embedded #10359

Open ghost opened 8 years ago

ghost commented 8 years ago

When using --embed-thumbnail, it seems that the thumbnail isn't added as an image resource (cover.jpg) to the mkv video file but instead added as a second video stream. Seemingly using the same code as is used for embedding cover art for a mp3. This results in the embedded thumbnail not being displayed and instead no thumbnail or a automatically generated one being shown.

Tested with this video: https://www.youtube.com/watch?v=k_okcNVZqqI youtube-dl https://www.youtube.com/watch?v=k_okcNVZqqI --embed-thumbnail Using youtube-dl 2016.08.13 and ffmpeg-20160815-3282e31-win64-static

MediaInfo, thumbnail embedded with --embed-thumbnail: INK DROPS 4K (ULTRA HD)-k_okcNVZqqI.mkv.MediaInfo.txt

MediaInfo, thumbnail embedded with mkvpropedit: INK DROPS 4K (ULTRA HD)-k_okcNVZqqI-proper.mkv.MediaInfo.txt

Related to this the request for thumbnail embedding support for webm files when this is fixed. (#10360)

yan12125 commented 8 years ago

This results in the embedded thumbnail not being displayed

Sorry for being late. Which player are you using?

ghost commented 8 years ago

I mainly use a software called Icaros which enables MKV files showing thumbnails in windows explorer using the thumbnail inside the file or, if there is no thumbnail inside it, using a generated one.

But I see the same thing with Windows Media Player and VLC media player. The correct thumbnail from youtube is only correctly displayed for the file where I used --write-thumbnail to download the thumbnail and mkvpropedit to embed the thumbnail. For the file where I used the --embed-thumbnail option to download and embed it directly, either no thumbnail is being displayed or one is generated from the video.

Screenshot

u1735067 commented 7 years ago

Hi, you might be interested in merging https://github.com/Alex131089/youtube-dl/compare/Alex131089-mkv-thumbnail or https://github.com/Alex131089/youtube-dl/compare/Alex131089-mkv-thumbnail-filename, I was able to embed thumbnail without issue in VLC or MPC-HC with this. The second branch tries to respect the filename convention (there's no orientation check nor dimensions enforcement/conversion), but using another/video's filename works too, at least in VLC. I guess this would fix #6046 ? Thanks for this utility :)

yan12125 commented 7 years ago

Awesome! Does it support non-ASCII thumbnail filenames? And I wonder how accurate guess_type is.

u1735067 commented 7 years ago

Awesome! Does it support non-ASCII thumbnail filenames?

No idea, but as much as for the MP3 I guess.

And I wonder how accurate guess_type is.

Mimetypes module seems to rely on the extension, so it's very "light".

And with only 2 extensions recognized by the convention, I came with this much simpler version : https://github.com/Alex131089/youtube-dl/tree/Alex131089-mkv-thumbnail-simpler. For other filetypes, you'd still need mimetype detection.

grenzor commented 6 years ago

This looks good, any reason it isn't merged yet?

srussel commented 6 years ago

Any progress on this? I am also experiencing this bug.

Dialga commented 6 years ago

Any chance of merging this soon?