fix direct playback of playable plugin favorites which have slash at end #1032

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@dersphere
Member

addresses my trac ticket http://trac.xbmc.org/ticket/13074

PlayMedia was assuming that every vfs-path with a slash at the end is a folder - which isn't correct. I fixed this with the additional condition that the item need to be a non-plugin item. But still wondering if this switch is ever needed because folder-favorites (even plugin folders) are using ActivateWindow(vfs) and not PlayMedia(vfs).

@dersphere dersphere fix direct playback of playable plugin favorites which have slash at end
addresses my trac ticket http://trac.xbmc.org/ticket/13074

PlayMedia was assuming that every vfs-path with a slash at the end is a folder - which isn't correct. I fixed this with the additional condition that the item need to be a non-plugin item. But still wondering if this switch is ever needed because folder-favorites (even plugin folders) are using ActivateWindow(vfs) and not PlayMedia(vfs).
864f96f
@jmarshallnz
Member

Typically a slash at end does denote a folder. For most plugins this would be correct, right?

We need something to denote whether things are a folder if we want the ability for PlayMedia to be able to play folder listings.

If the problem is only favourites, then when adding the favourite we probably know this information, so can remove the slash at end at that point.

Member

Hi Jonathan,
thanks for your thoughts and comments!

There is a possibility to add folder-playback to favorites?

Yes on most plugins this isn't an issue.

But there are a few possible examples for vfs-urls of plugins which are playable but ending with a slash (unproved, just some ideas):PlayMedia("rtmp://foo.bar:1935 playpath=/library/video1234/") (rtmp with playpath) PlayMedia("plugin://plugin.video.foo/?mode=play&url=http://website.com/video1234/") (callback url) PlayMedia("http://website.com/video1234/") (direct video) PlayMedia("plugin://plugin.audio.radio_de/station/2264/") (xbmcswift addon)
Also, at least on websites it has become a common pattern to have a trailing slash on some page-urls.

Folders which are meant to be opened as folder listing via favorites ([smart]playlists, phyisical/logical-folders, plugin-listings) have fav entries with ActivateWindow().

Btw, removing the slash by xbmc won't work on xbmcswift addons because the url pattern matching differs between having and not having a trailing slash.

I opened this PR assuming that on plugins there is only a folder (ActivateWindow) or a playable item (PlayMedia) possible - and while adding, this is detected and set. But if you feel that the current behavior is not wrong or it would break some (future) feature, you can just close this and we need to fix it in some plugins ;-)

Member

If the problem is only favourites, then when you "Add Favourite" to an item we can ensure no slash is on the end. As far as I can tell, all your examples should work with the slash removed from the end of the URLs or the slash being there, or do you have examples where the slash MUST be there for the URL to function?

@dersphere
Member

I see the problem only on favourites, yes. If I didn't overlook something the builtins.cpp:PlayMedia() Method is also only used there.

I don't think that all of the examples would work with a removed trailing slash - it depends on the remote source. I know some rtmp sources which definitely require a trailing slash at the end of the playpath. And like I said, xbmcswift based Addons (Radio, Khan Academy, Al jazeera, Khan Academy, MyVideo.de, overall >10 in official repo ...) definitly require the slash if its set in the plugin.route decorator (which is atm in all xbmcswift based plugins) - it does not work if the slash becomes removed by xbmc while adding a fav entry.

Personally I would prioritize the possible solutions in that order (best first):

  • Me and the other (xbmcswift-) addon authors should alter the plugin urls to not end with a slash (and close this) if its not a folder.
  • This patch got's accepted
  • XBMC get's altered to remove the slash, me and the other addon authors have to alter the plugins to match both urls (with and without trailing slash - new entries without).

But I'm still wondering why it is needed to detect if an entry points to a folder or a playable entry at this place. This can be 100% sure detected while adding (listitems.isFolder) something to the favs or while calling the plugin (returns one or more listitems) to open the fav entry.

@jmarshallnz
Member

This one fell off my radar (not sure why github only shows some of the issues in my list!)

Either way, I think you're right: XBMC should know this when calling Add Favourite, so we should be able to set the correct thing there and then. Unfortunately we can't just change PlayMedia() to do something else, but we can probably provide a separate routine PlayItem() or some such that will skip that check.

Does that sound like a better solution, and if so, will you do it? :)

@dersphere
Member

If I would add such a method we would have three relevant methods which would be used for fav entries:

  • ActivateWindow() (for showing a list of items)
  • PlayMedia() (for playing a playable item AND showing a list of items - depending on trailing slash or isDir param, ignoring isFolder-bool)
  • PlayItem() (for playing a playable item) (NEW)

In theory I would prefer exactly one method for listing and one method for playing ;)

IMO adding another method would be overkill ;)
Best and easiest would be closing this and change all addons to not have trailing slash in playable item url...

@TylonHH
TylonHH commented Apr 21, 2016

How can I find playable items?
In my case I use runaddon(plugin.audio.rautemusik). Thats works. But I want to start a specific station, headless.

@MartijnKaijser
Member

@TylonHH use the forum for question.

@TylonHH
TylonHH commented Apr 21, 2016

I added the radio station to the fav and looked into that file. That string there I used for my keymap. success.

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