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

[playlist] Add support for reading XSPF Playlists #14585

Merged
merged 1 commit into from Oct 23, 2018

Conversation

@tylerszabo
Copy link
Contributor

commented Oct 12, 2018

Description

Allows for .xspf files to be read as playlists

Motivation and Context

I wanted to get Kodi to read playlists I had made using VLC. I searched to see if the functionality existed and I came upon this thread https://forum.kodi.tv/showthread.php?tid=209910. While I can convert, I thought it'd be handy to have Kodi also support this (powerful and extensible) format.

How Has This Been Tested?

Unit tests were created and run on Windows 10 & Linux.
Manual testing was done one Windows 10 64-bit and Ubuntu 18.04.1 64-bit VM.

  1. Create a music directory source that contains audio files in subdirectories and playlists that specify paths to those files based on a Windows system's file structure and some from a Linux system's file structure.
  2. Add a music directory as a music source
  3. Open the file "Files" browser for the music source and locate the playlist files
  4. Open the various playlist files and observe how they are populated when the file exists in the given filesystem and are playable. (Note that a playlist that includes location paths but no file exists at that location (on that system) are still loaded, but the respective entries are skipped, this is the behavior specified XSPF.)

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch from 93d3659 to 8dceafe Oct 13, 2018
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
xbmc/playlists/PlayListXSPF.cpp Outdated Show resolved Hide resolved
@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch 2 times, most recently from edf1d13 to 5e6c3f9 Oct 13, 2018
@garbear

This comment has been minimized.

Copy link
Member

commented Oct 13, 2018

Looking better

@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch 2 times, most recently from 28d1096 to 8d6902e Oct 13, 2018
@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch from 8d6902e to cb78451 Oct 14, 2018
xbmc/utils/Mime.cpp Outdated Show resolved Hide resolved
@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch 3 times, most recently from b3b698e to bab1201 Oct 15, 2018
@tylerszabo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2018

I'd like to thank everyone for reviewing this change - I realized it took time to ensure that the change would perform correctly and the style conformed to the way project's goals. I think this project can credit its readability to the care you take in keeping code quality high.

@tylerszabo tylerszabo force-pushed the tylerszabo:xspf_playlist branch 3 times, most recently from 4a251d4 to 1961882 Oct 22, 2018
@@ -320,6 +320,7 @@
<string>aif</string>
<string>aiff</string>
<string>wpl</string>
<string>xspf</string>

This comment has been minimized.

Copy link
@MartijnKaijser

MartijnKaijser Oct 22, 2018

Member

Might also need to be added to UWP (and other platforms) acceptable extensions we can open

This comment has been minimized.

Copy link
@pkerling

pkerling Oct 22, 2018

Member

@Rechi @Paxxi maybe?

Linux doesn't need it afaik

Copy link
Contributor

left a comment

Looks great.

@MartijnKaijser MartijnKaijser merged commit 9d3a68a into xbmc:master Oct 23, 2018
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

Thx for the patience

@DaveTBlake DaveTBlake added the v18 Leia label Oct 23, 2018
@DaveTBlake DaveTBlake added this to the Leia 18.0-beta4 milestone Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.