mpv icon indicating copy to clipboard operation
mpv copied to clipboard

ytdl_hook.lua: support playlist_title

Open Earnestly opened this issue 1 year ago • 1 comments

This PR mostly serves as a discussion about how this issue might be resolved.

ytdl_hook is aware of many useful playlist metadata tags which are essentially lost during the creation of a playlist as yt-dlp is called twice: once for the playlist url and then again for each entry. Here the json representing the playlist and its metadata is no longer available in the json produced for each entry.

It is not clear whether yt-dlp should provide the metadata or if ytdl_hook should go out of its way to try and capture this information and provide it during another instance of run_ytdl_hook() and what consequences that might have.

See also https://github.com/yt-dlp/yt-dlp/issues/11234

Earnestly avatar Oct 15 '24 09:10 Earnestly

If you play a regular youtube video as the second argument after a playlist it keeps the previous playlist title, is there a way to reset it? The commit works otherwise.

guidocella avatar Oct 24 '24 18:10 guidocella

If you play a regular youtube video as the second argument after a playlist it keeps the previous playlist title, is there a way to reset it? The commit works otherwise.

I'm not sure. The general heuristic here is to assume if a playlist title was available at some point that any subsequent video(s) are part of that playlist, which is pretty silly.

Because the playlist title is not directly associated with videos there is no reliable relationship to use. This is primarily why I think yt-dlp should provide it even with --flat-playlist as per https://github.com/yt-dlp/yt-dlp/issues/11234

I can't personally think of any nice way to track this state and still believe yt-dlp needs to provide this. (E.g. By using non-standard #PLAYLIST entries in the generated m3u, trying to track some combination of playlist- properties to detect a non-playlist item, etc. None of these seem reasonable to me but I am open to better insights here.)

Edit: Maybe one possible way is to create a table that associates each playlist entry with a playlist title and only to update the title if the single video exists in that table.

Earnestly avatar Oct 24 '24 22:10 Earnestly

Something like this seems to work but can be improved with a better designed structure.


local playlist_titles = {}

-- ...

        -- preserve playlist metadata
        playlist_metadata["playlist_title"] = json["title"]
        playlist_metadata["playlist_id"] = json["id"]

        for _, entry in pairs(json.entries) do
            playlist_titles[entry.url] = playlist_metadata
        end

-- ...

    else -- probably a video
        -- restore playlist metadata if it exists
        local title_metadata = playlist_titles[json.original_url]

        if title_metadata ~= nil then
            for key, value in pairs(title_metadata) do
                if value then
                    json[key] = value
                end
            end
        end

        add_single_video(json)

Earnestly avatar Oct 24 '24 22:10 Earnestly

That seems fine.

guidocella avatar Oct 25 '24 06:10 guidocella

Had some time so here is a second attempt.

This does still have the downside that if a playlist and url is provided and that url happens to be part of the playlist, it will still retain the playlist's metadata.

I'm also not very familiar with lua so I'm not sure if it's memory sane to assign the metadata to every url (which can be thousands). Does lua make copies of the tables or store a reference to the same table?

Earnestly avatar Oct 25 '24 18:10 Earnestly

Tables are assigned by reference. But always using the same metadata for all URLs is wrong if you play multiple playlists and navigate back and forth between them, so metadata needs to be local to where it's assigned.

guidocella avatar Oct 25 '24 19:10 guidocella

always using the same metadata for all URLs is wrong if you play multiple playlists

While I am assigning the metadata to all urls in a playlist, each new playlist that is loaded will overwrite the entires, so the new metadata is implicitly available during the lookup later.

if you play multiple playlists and navigate back and forth between them, so metadata needs to be local to where it's assigned.

This does currently work for me* when testing using a youtube channel (videos, live, shorts, etc.), however the issue of multiple playlists (including single videos interspersed within) is nevertheless prescient. Under the current approach all urls are essentially "global" keys, so if any of the urls are shared between playlists then the metadata assigned to a specific url will be whatever playlist was seen last.

* With one minor change, if entry.url then playlist.urls[entry.url] = playlist.metadata end.

Do you have any suggestions? I've tried thinking of a few but I keep coming back to the same problem.

Edit: The test I'm using for multiple playlists is the g-p binding to switch between them.

Earnestly avatar Oct 25 '24 21:10 Earnestly

I mean if you play 2 unrelated playlists, go to the second one, then go back to the first one, it now has the playlist title and id of the second one.

guidocella avatar Oct 25 '24 21:10 guidocella

To associate the correct metadata to videos contained in multiple playlists, can't you associate metadata to playlist urls instead of video urls, and set it based on playlist-path?

guidocella avatar Oct 25 '24 21:10 guidocella

Here it is:

diff --git a/player/lua/ytdl_hook.lua b/player/lua/ytdl_hook.lua
index 7e31177ef7..d878238a2d 100644
--- a/player/lua/ytdl_hook.lua
+++ b/player/lua/ytdl_hook.lua
@@ -27,6 +27,7 @@ end)
 
 local chapter_list = {}
 local playlist_cookies = {}
+local playlist_metadata = {}
 
 local function Set (t)
     local set = {}
@@ -1079,6 +1080,11 @@ local function run_ytdl_hook(url)
             return
         end
 
+        playlist_metadata[url] = {
+            playlist_title = json["title"],
+            playlist_id = json["id"],
+        }
+
         local self_redirecting_url =
             json.entries[1]["_type"] ~= "url_transparent" and
             json.entries[1]["webpage_url"] and
@@ -1193,6 +1199,12 @@ local function run_ytdl_hook(url)
         end
 
     else -- probably a video
+        local metadata = playlist_metadata[mp.get_property("playlist-path")] or {}
+
+        for key, value in pairs(metadata) do
+            json[key] = value
+        end
+
         add_single_video(json)
     end
     msg.debug('script running time: '..os.clock()-start_time..' seconds')

guidocella avatar Oct 25 '24 21:10 guidocella