datasone / MPVMediaControl

Windows 10 System Media Transport Controls (SMTC) for MPV (or any other programs)
MIT License
63 stars 3 forks source link

[Minor Bug] Chapters are displayed instead of title of video when streaming #7

Open NaiveInvestigator opened 2 years ago

NaiveInvestigator commented 2 years ago

Expected Result: image

Actual result: image

Link to the video (just in case if you want to reproduce the issue): https://www.youtube.com/watch?v=6C4u8l00cnE&t=9s

Nothing too big just letting you know. Have a great day pls! :)

Oh and please do check out this suggestion too at #6, in case if you missed it!

Edit: Oh I forgot to mention (again sorry ;-; ) but as you can see there is a play next and play previous button, even tho there is only one video on the playlist. I suspect that is cause of the chapter bug.

NaiveInvestigator commented 2 years ago

Also could we add a feature where the thumbnail of the YouTube vid will be used instead of a screenshot of the video in the SMTC box. We could use yt-dlp to download the thumbnail and maybe scrap some metadata (if possible).

Also as a side note could we delay the screenshot for the thumbnail for about a few milliseconds or so. The reason for this is as for in my case, the thumbnail often appears blank as takes a screenshot before the video is even played and some videos start also start with a black screen.

Maybe taking a screenshot after the video is played or create a mpv sub-process dedicated to seek to the middle of the video (or any random arbitrary value works ig) takes a screenshot and closes after it is done, similar to how the mpv thumbnail script works. (or at least that is how i think it works). It shouldn't be too resource intensive imo. we could disable hardware decoding and audio and also reduce the scaling to save resources.

Edit: We can use ffmpeg for the thumbnails but i think that would be more resource intensive (not sure) plus that just increases the number of dependencies which is not ideal imo.

Just throwing some ideas for a solution, really sorry if I have offended you in any way.

NaiveInvestigator commented 2 years ago

Also could we add a feature where the thumbnail of the YouTube vid will be used instead of a screenshot of the video in the SMTC box.

https://gist.github.com/xarblu/3cad5763ab699f331559d736c2462db0

I think this script could help to do just that.

Edit: I put this link here so that i don't forget. Sorry if you were disturbed.

datasone commented 2 years ago

Well, feel free to comment on issues or suggestions, it's always good to have different views :)

Sorry for a long absence (due to half busy and half lazy), but I think now the issues can be sort out to the following four:

  1. The chapters in video is used instead of the title.
  2. It is better to place the MPVMediaControl.exe in the scripts directory.
  3. YouTube cover image is a better choice for videos with it.
  4. Screenshoting video later can get better result for cover image.

For 1., thanks for pointing out this issue! I mainly used this on local files and especially musics, splitting media by chapters is used for getting tracks in cue files, so I didn't thought of videos with chapters. A quick fix that disables chapter splits for video should be OK. (That's how different views benifit, you see?😀)

For 2, I think it would be better to place it in the mpv's config directory, that is the parent directory of scripts. As mpv autoloads all files in scripts directory and treat them as lua scripts, I think it would be better to place it in the config dir. This is not a big issue so we could discuss about it more later.

For 3 and 4, it's good idea for getting this to work, I will look into the script you mentioned and try to get it to work. For postponing video screenshot, a configurable parameter in the script would be good. I'm also thinking of refactoring the way program handles screenshots (currently it takes screenshot to file and blocks the player while the playback starts), so this may take a little more time.

datasone commented 2 years ago

Why would I closed it...maybe misclick, anyway the video chapters issue has been fixed in 86a80262 , and I've found a way to quickly fix the block issue, so I guess fixing 3 and 4 would be quick

datasone commented 2 years ago

Support for youtube and delayed screenshot has been added, I've tested for a little but not much, so it would be good if you may help test if there are some bugs in the fixes.

An release is postponed as not fully tested and only the lua script is changed. But updating only notify_media.lua from master is sufficient.

NaiveInvestigator commented 2 years ago

It works like a charm! The titles are displayed correctly!

But sometimes some vids don't show the thumbnail, don't really know why. (Chances of this happening is really low tho) image

And also if the urls are shortened, for e.g: https://youtu.be/TcriY4d2EQs (interesting vid btw lol if you haven't watched it yet) It doesn't work and reverts back to the old behaviour instead, (noticed it by mistake lol).

I was thinking maybe instead of writing a whole function dedicated to this we can use yt-dlp to grab the thumbnail for us! I'm sorry for mentioning this late ;-; Pick the one which is easier to implement ig.

Even tho the chapter bug is fixed, the issue where the play next and play previous buttons is still present, really confused to why this happens. I only have that one video in the playlist lol I swear!

And also if we could add the channel's/ uploader's name in music artist field would be really cool too imo when playing YouTube or Twitch Streams.

Another request i have, (not really a important feature but would be very cool to have) is to control playback via SMTC, just like how Spotify and the UWP version of VLC does! Main reason of this is cause of this project lol.

Edit: Adding this line should download download and convert the thumbnails to jpg but doesn't handle the outputting. yt-dlp --skip-download --ignore-config --write-thumbnail --convert-thumbnails jpg "https://youtu.be/TcriY4d2EQs" Reading this should help : https://github.com/yt-dlp/yt-dlp#output-template

datasone commented 2 years ago

Can you give some examples for those without cover image? Or is it happening occasionally with the same video?

Cover image handling with yt-dlp or youtube-dl is a good idea, while it could also solve the shortened URL issue. But it seems there are several ways to specify paths, considering those programs usually may not be in %PATH% for Windows users. I will look further into the integrated ytdl_hook.lua (the one that call yt-dl programs for youtube links in mpv) and see the best way to deal with it.

As for ModernFlyouts, it's a really cool project, but it doesn't work well on my PCs (i.e. it starts on some but won't on mainly used ones...). I see there is an issue for that and they plan to solve this in 0.11 release, so once I got it working on my PCs, I think I could work on more support for ModernFlyouts.

NaiveInvestigator commented 2 years ago

Can you give some examples for those without cover image? Or is it happening occasionally with the same video?

It happens usually on vids which don't use a high resolution thumbnail mostly, as I have noticed. And it happens always with the same video (atleast for me) which confirms my suspicions. This video in particular : https://www.youtube.com/watch?v=l0Xw6NMlRQ4 The thumbnail for that vid is these four (I might have missed out others): https://i.ytimg.com/vi/l0Xw6NMlRQ4/maxresdefault.jpg https://i.ytimg.com/vi/l0Xw6NMlRQ4/sddefault.jpg https://i.ytimg.com/vi/l0Xw6NMlRQ4/hqdefault.jpg https://i.ytimg.com/vi/l0Xw6NMlRQ4/mqdefault.jpg

As you can see the maxres one doesn't work sometimes so if it could default to the the two as backup would be neat!

Cover image handling with yt-dlp or youtube-dl is a good idea, while it could also solve the shortened URL issue. But it seems there are several ways to specify paths, considering those programs usually may not be in %PATH% for Windows users. I will look further into the integrated ytdl_hook.lua (the one that call yt-dl programs for youtube links in mpv) and see the best way to deal with it.

I hope it works out! Dunno if this helps but yt-dlp is usually found on the root directory of mpv, if they used the update script (that comes along with it) to update mpv. That was how I originally found it lol. So we might not have to like worry about if yt-dlp is on the path or not. If it is not there, it is most likely that yt-dlp is not even downloaded on the PC so the user might not even use it. Not really sure tho.

Screenshot of the mpv directory: image

Screenshot of the updater script: image

As for ModernFlyouts, it's a really cool project, but it doesn't work well on my PCs (i.e. it starts on some but won't on mainly used ones...). I see there is an issue for that and they plan to solve this in 0.11 release, so once I got it working on my PCs, I think I could work on more support for ModernFlyouts.

I hope it works for you!! Really looking forward on that one!

Edit: Sometimes the chapters are still shown, don't really know the reason tho, it was working perfectly yesterday and it worked on the video before this, kinda a hit or a miss. image

Edit - 2: Just discovered this other rep: https://github.com/x0wllaar/MPV-SMTC Just posting so i don't forget, I think taking a look at it may help.

Edit - 3: Could we add an icon so it looks nice on the the tray? https://stackoverflow.com/questions/43219007/cs-script-for-notepad-prepare-exe-for-distribution-with-icon

Edit - 4:

Another request i have, (not really a important feature but would be very cool to have) is to control playback via SMTC, just like how Spotify and the UWP version of VLC does! Main reason of this is cause of this project lol.

This may help for implementing that feature: https://bugzilla.mozilla.org/show_bug.cgi?id=1689538