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

strip ytdl:// from direct links in ytdl_hook.lua #5003

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
1 participant
@parktheredcar

parktheredcar commented Oct 17, 2017

When youtube-dl returns a playlist of links that are not youtube videos, all links in the playlist file are prepended with ytdl:// by ytdl_hook.lua

When each of these links is passed off individually to ytdl_hook.lua during on_load and a direct link is detected, the ytdl:// is not appropriately stripped off, which results in a link beginning with 'ytdl://http://', ending in an unsupported protocol error back on the player side for each item in the playlist.

One example of this issue in action is mpv http://shoutengine.com/NyaniShortPhilosophicalStoriesinTamil.xml.

To resolve this, if a direct link is detected, save it back to the player after the ytdl:// has been stripped off.

I agree that my changes can be relicensed to LGPL 2.1 or later.

strip ytdl:// from direct links in ytdl_hook.lua
When youtube-dl returns a playlist of links that are not youtube videos, all links in the playlist file are prepended with ytdl:// by ytdl_hook.lua

When each of these links is passed off individually to ytdl_hook.lua during on_load and a direct link is detected, the ytdl:// is not appropriately stripped off, which results in a link beginning with 'ytdl://http://', resulting in an unsupported protocol error back on the player side.

One example of this issue in action is `mpv https://feeds.feedburner.com/WelcomeToNightVale`

To resolve this, if a direct link is detected, save it back to the player after the ytdl:// has been stripped off.
@parktheredcar

This comment has been minimized.

parktheredcar commented Oct 17, 2017

Closing this until I can figure out all the details, might have overlooked something.

@parktheredcar

This comment has been minimized.

parktheredcar commented Oct 17, 2017

After further investigation I have confirmed this but have found a better example, which has been edited in to the original comment.

There is some question in my mind as to whether it would be better to just save back the url, or to call the full blown add_single_video(), but I have gone with the former since it seems this is supposed to be a minimal case. The down side of this decision is that the requested http headers won't be set.

@parktheredcar parktheredcar reopened this Oct 17, 2017

@thebombzen thebombzen force-pushed the mpv-player:master branch from f958f52 to 0433162 Dec 5, 2017

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Dec 22, 2017

ytdl_hook: don't preappend ytdl:// to non-youtube links in playlists
Also, convert string concat to table concat, should be faster.

Close mpv-player#5003

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Dec 23, 2017

ytdl_hook: don't preappend ytdl:// to non-youtube links in playlists
Also, convert string concat to table concat, should be faster.

Close mpv-player#5003

wiiaboo added a commit to wiiaboo/mpv that referenced this pull request Dec 24, 2017

@kevmitch kevmitch closed this in 1623430 Dec 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment