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

Support Youtube URLs with both a video and a playlist ID #60

Closed
anohren opened this issue May 23, 2022 · 7 comments · Fixed by #63
Closed

Support Youtube URLs with both a video and a playlist ID #60

anohren opened this issue May 23, 2022 · 7 comments · Fixed by #63

Comments

@anohren
Copy link
Collaborator

anohren commented May 23, 2022

A Youtube URL can refer to a video inside a playlist, and has IDs for both (and optionally, apparent from the code below, an index referring to the video in the list). The desired result is to load the playlist into the play queue, and start the queue at the specified video. Currently this kind of URL seems to play from the beginning of the queue.

I don't remember from the top of my head how youtube-dl handles these URLs, but I know IINA has this feature and that IINA uses mpv. Mpv in turn handles it in lua like this it seems:

local function parse_yt_playlist(url, json)
    -- return 0-based index to use with --playlist-start

    if not json.extractor or json.extractor ~= "youtube:playlist" then
        return nil
    end

    local query = url:match("%?.+")
    if not query then return nil end

    local args = {}
    for arg, param in query:gmatch("(%a+)=([^&?]+)") do
        if arg and param then
            args[arg] = param
        end
    end

    local maybe_idx = tonumber(args["index"])

    -- if index matches v param it's probably the requested item
    if maybe_idx and #json.entries >= maybe_idx and
        json.entries[maybe_idx].id == args["v"] then
        msg.debug("index matches requested video")
        return maybe_idx - 1
    end

    -- if there's no index or it doesn't match, look for video
    for i = 1, #json.entries do
        if json.entries[i] == args["v"] then
            msg.debug("found requested video in index " .. (i - 1))
            return i - 1
        end
    end

    msg.debug("requested video not found in playlist")
    -- if item isn't on the playlist, give up
    return nil
end

Seems straightforward. We could simply extract the query parameters and apply similar logic.

@nullket
Copy link
Collaborator

nullket commented May 23, 2022

I played a bit with a playlist in yt-dlp. The cleanest would be to have a field in the result indicating the position within the playlist. But it seems like there is no such field.

Picking up the lua code something like this could work:

def get_original_playlist_position(result):
   # incorporate logic for every (youtube, vimeo etc.) page here, if not available default to playlist position 0 
   if result['extractor'] in ["youtube:tab", "youtube:playlist"]:
      # ideally one would use urllib but this starts another mess with python2/3
      url_query = result['original_url'].split("?")[1]
      url_params = {x[0] : x[1] for x in [x.split("=") for x in url_query[1:].split("&") ]} 
      return url_params[index] # should be sanity checked for a number
  return 0

# original part of code
if 'entries' in result:
    # Playlist
    pl = xbmc.PlayList(1)
    pl.PlayOffset =  get_original_playlist_position() # this is new

BUT this would require to implement site specific things in this plugin and last time @firsttris was not so happy about that in #36 (comment) @firsttris whats your opinion? Playlists playing at the correct position to the price of website specific parsing?

@anohren
Copy link
Collaborator Author

anohren commented May 23, 2022

Nice that you took a look! But keep in mind that solving this will be pointless until we've solved #15. That issue stalled because (some versions of) Kodi doesn't seem to handle the play queue properly (or alternatively isn't clear on how to construct the queued objects so that they don't completely break the queue).

It'd be really nice if we could get that merged, even if we have to probe for the Kodi version.

What's needed there is basically testing which versions break.

@firsttris
Copy link
Owner

i dont think there anything speaking against something like this.

i still see this in the area of youtube_dl - kodi integration.

and also generally, if you think thats a good idea i will not overrule or judge your decision.

@anohren
Copy link
Collaborator Author

anohren commented May 29, 2022

Well #15 doesn't seem to be a problem anymore.

Picking up the lua code something like this could work

Yeah, something like that. Not sure about the [1:] in url_query[1:].split("&") though.

Mpv is GPLv2, so while we can use the same logic to determine whether index actually refers to an entry in the playlist, I suppose we should avoid doing a direct translation.

I played around a bit and I don't think urllib should complicate it (I usually don't use Python, so might not be idiomatic).

url = 'https://youtube.com?v=xyz&index=2'

result = {
    'entries': [
        { 'id': "abc"},
        { 'id': "jkl"},
        { 'id': "xyz"}
    ]
}

def playlistIndex(url, result):
    import sys
    if sys.version_info >= (3, 0):
        from urllib.parse import urlparse, parse_qs
    else:
        from urlparse import urlparse, parse_qs 
    
    query = urlparse(url).query
    queryParams = parse_qs(query)
    
    if 'v' not in queryParams:
        return None
    
    v = queryParams['v'][0]
    
    try:
        index = int(queryParams.get('index')[0])
        if result['entries'][index]['id'] == v:
            return index
    except:
        pass
    
    for i, entry in enumerate(result['entries']):
        if entry['id'] == v:
            return i
    
    return None

# test
playlistIndex(url, result)

@anohren
Copy link
Collaborator Author

anohren commented May 30, 2022

Feel free to try this out in #63 (and point out my off by one errors)

@nullket
Copy link
Collaborator

nullket commented May 30, 2022

I just had a look at this during lunch and it seems perfect. But shouldn’t we add another „check if base url is YouTube“ part? Otherwise urls with a Playlist which might contain any „v“ param lead to wrong playlist starts?

Could you also do me the favor and remove all the print statements and replace them with a Kodi (debug) log call if you prefer them in the final version?

I will try out your PR as soon as I can.

@anohren
Copy link
Collaborator Author

anohren commented May 31, 2022

Thanks for taking a look!

shouldn’t we add another „check if base url is YouTube“ part? Otherwise urls with a Playlist which might contain any „v“ param lead to wrong playlist starts?

Checking which extractor was used is a good idea. I didn't notice that Mpv did that too, so I kind of assumed this way of determining the starting point was also applicable to some other video sites.

I doubt there's going to be any undesirable results whether we do or not though. What are the chances that an arbitrary v parameter will actually match a video ID but at the same time not be a hint at where to start? But the intent will be clearer I suppose.

Could you also do me the favor and remove all the print statements and replace them with a Kodi (debug) log call if you prefer them in the final version?

Sure. It was just some temporary assistance for lazy testing.

Let's just squash everything before merging though. I noticed the previous PR got spread out over multiple years with lots of auto commits in between, which was a bit distracting in git log. One commit per feature should make it easier to follow.

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

Successfully merging a pull request may close this issue.

3 participants