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

MKV thumbnail not correctly embedded #10359

Open
ghost opened this issue Aug 16, 2016 · 8 comments
Open

MKV thumbnail not correctly embedded #10359

ghost opened this issue Aug 16, 2016 · 8 comments

Comments

@ghost
Copy link

@ghost ghost commented Aug 16, 2016

  • I've verified and I assure that I'm running youtube-dl 2016.08.13
  • At least skimmed through README and most notably FAQ and BUGS sections
  • Searched the bugtracker for similar issues including closed ones

What is the purpose of your issue?

  • Bug report (encountered problems with youtube-dl)
  • Site support request (request for adding support for a new site)
  • Feature request (request for a new functionality)
  • Question
  • Other

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)

@ghost ghost mentioned this issue Aug 16, 2016
1 of 5 tasks complete
@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Aug 23, 2016

This results in the embedded thumbnail not being displayed

Sorry for being late. Which player are you using?

@ghost
Copy link
Author

@ghost ghost commented Aug 25, 2016

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

yan12125 added a commit that referenced this issue Oct 20, 2016
This reverts commit 7360db0.

This commit was added as an attempt to fix #6046. Unfortunately, the fix
is completely wrong. As reported on #10359, embedded thumbnails are not
displayed in VLC, and Se7en on IRC reports that the embedded thumbnail
misleads mpv as well.

The correct way is using -attachment of ffmpeg, while the current
run_ffmpeg_multiple_files API can't handle it cleanly.
@yan12125 yan12125 mentioned this issue Feb 22, 2017
4 of 8 tasks complete
@Alex131089
Copy link

@Alex131089 Alex131089 commented Nov 4, 2017

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
Copy link
Collaborator

@yan12125 yan12125 commented Nov 4, 2017

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

@Alex131089
Copy link

@Alex131089 Alex131089 commented Nov 5, 2017

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
Copy link

@grenzor grenzor commented Dec 17, 2017

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

@srussel
Copy link

@srussel srussel commented Jul 21, 2018

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

@Dialga
Copy link

@Dialga Dialga commented Nov 3, 2018

Any chance of merging this soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.