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

Directstream MusicLibrary Support #159

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@Memesa

Memesa commented Dec 11, 2016

Directstream Musiclibrary has been requested by several people and listed bugs (#14 and #21) that it kind of breaks kodi now. I had a try at implementing this feature.

When the same method is used as for movies; that is, directing the library path to the corresponding plugin. This doesn't work for the music library as it doesn't allow launching a plugin from the library.
This can be circumvented by using .strm files, these are picked up and can be used to redirect the music entry to the pkc plugin.

The current method is to generate .strm files in addon_data/pkc/musicstreamfiles/dbid.strm with the pkc entry (e.g. plugin://plugin.video.plexkodiconnect.music/?dbid=1&mode=play&id=20182) and start these instead. Some optimizations had to be added to the script to take music files into account.

key features:

  • generate *.strm file with plugin, dbid and itemid
  • using plugin.video.plexkodiconnect.music or plugin.video.plexkodiconnect.movies (they are both thesame and automatically chosen, but the former preferred)
  • *.strm files added to library
  • complete metadata support
  • the music scraper still doesn't like this approach, and deletes the entries from the library. A solution was found by introducing the directory as blank and the absolute filenames used to directly access the files. It does show the following in the log: "MUSIC_INFO::CMusicInfoScanner::Process directory '' does not exist - skipping scan." but this is harmless.
  • Plexcompanion does work for music, though metadata is not correct while playing (it is correct in the playlist)

For now tested on Windows, Kodi 16.1 using NAS PMS (synology 1.3.2.3112) or Windows PMS (1.3.2.3112).

Some bugs still need to be fixed:

  • metadata in plexcompanion
  • playlists / albums don't seem to work

Memesa added some commits Dec 11, 2016

Adding music library with .strm file redirection
Added music library files through a redirection in .strm files instead
of a direct link to the plex server (giving issues with music scraper)
or direct link to plugin (not allowed by kodi)

Basically; to the same as for movies but now through .strm files which
are saved in the addon_data folder.
Though, this wasn't a simple task. First of all the scraper was deleting
files if it couldn't verify their integrity (which it couldn't because a
.strm is not a music file for the scraper). Therefore the directory is
not told to the scraper, but instead the absolute path is parsed as
filename and the dir set to ''.
Because this path is usually followed for video files, some breakout
were installed for correct function.
small audioplugin fix and correctly set plugin calling itself
The check if the audioplugin existed was incorrect - fixed

The plugin directly called the playurl on the second attempt, this was
incorrect and caused metadata bugs. It is now correctly calling itself.
bugfix
Removed plex update (viewoffset) error message keep popping up because
it wasn't set correctly for the websocket_client. (though I actually
wonder if it is working or wether it should be working at all on music
files.)

@Memesa Memesa changed the title from MusicLibrary Support to Directstream MusicLibrary Support Dec 11, 2016

@Memesa Memesa closed this Dec 12, 2016

@Memesa

This comment has been minimized.

Memesa commented Dec 12, 2016

I'm closing it for now to work out the bugs.

@croneter

This comment has been minimized.

Owner

croneter commented Dec 12, 2016

I was just writing you back! It was looking awesome! :-)

Any show-stopper bugs?

@Memesa

This comment has been minimized.

Memesa commented Dec 13, 2016

It's just the playlist / albums that don't seem to work that is most annoying. In the current implementation this works fine, and is thereby more functional. In case a single item is played, it works fine though. But when you try to play more than a single song or queue a song, for some reason it tries to parse a single file as a directory and fails:

10:55:14 T:17188 DEBUG: CGUIMediaWindow::GetDirectory (musicdb://albums/1/14.strm?albumid=1)
10:55:14 T:17188 DEBUG: ParentPath = [musicdb://albums/]
10:55:14 T:17188 ERROR: XFILE::CDirectory::GetDirectory - Error getting musicdb://albums/1/14.strm?albumid=1

I'm wondering what causes it, because "musicdb://albums/1/14.strm?albumid=1" can be played back without issues. So the big question is now, why is it parsing it as a directory? Actually, a little testing showed that .strm, .m3u and .pls files are all parsed as directory when you trying to queue them, and that is independent on if they are in the music library or not...

@Memesa

This comment has been minimized.

Memesa commented Dec 13, 2016

From what I can tell now, is that this is a bug on how kodi queues playlist files...

I tried to go through the source code of kodi, and the queue seems to be initiated from this file:
https://github.com/xbmc/xbmc/blob/master/xbmc/music/windows/GUIWindowMusicBase.cpp

The class function CGUIWindowMusicBase::AddItemToPlayList contains the line:
if (pItem->m_bIsFolder || (g_windowManager.GetActiveWindow() == WINDOW_MUSIC_NAV && pItem->IsPlayList()))
This line requests if item is a folder OR if the user is in the music navigation screen AND item is a playlist. The first is false, the second and third are true and therefore this line returns true, it tries to list the directory (GetDirectory(pItem->GetPath(), items);) and fails. This idea is strengthened by the fact that if you start the album from the webserver, it queues up just fine. I'm not an expert at c++, so there is a small chance I'm incorrect here, but it looks like this is the issue.
I actually also wonder why this line is in the source code, because it does not seem possible anyway to browse into a playlist...

@Memesa

This comment has been minimized.

Memesa commented Dec 22, 2016

An update on this Pull Request; the suggested method of using .strm files might work to overcome the scraper giving errors of not being able to scrape the files on the plex server. There is one issue though, and that is that nothing can be queued and only single items can be played. This is caused by a bug in Kodi, which tries to parse playlist files as a directory and fails. I've opened a Pull Request at xbmc/xbmc#11175, and hope team Kodi will fix this bug before the release of version 17.0. I haven't found any useful detours yet to circumvent this bug. So until then, I don't consider this method useful as you can't play more than a single song at a time with it.

A different method to tackle this issue might be by making the library in the add-on instead of injecting it in the library of Kodi. I mean by that; parsing it in the same way as the current implementation of the photo add-on.

@croneter

This comment has been minimized.

Owner

croneter commented Dec 23, 2016

Hey @Memesa, that looks great, thanks for the update! I hope the Kodi team will merge your pull request

@croneter

This comment has been minimized.

Owner

croneter commented Jan 30, 2017

@Memesa, can you drive the discussion on the Kodi side? See xbmc/xbmc#11175

@DaveTBlake

This comment has been minimized.

DaveTBlake commented Feb 11, 2017

Well the Kodi PR has merged.

For something else I am looking at it, can you tell me if you are expecting Kodi to process the metadata tags from streams? Many addons do it themselves. And there is the suggestion that Kodi could stop passing internet streams to TagLib for efficiency (rather than trying to read tags we know aren't there, or read them but not use them). Would that impact your work?

@croneter

This comment has been minimized.

Owner

croneter commented Feb 12, 2017

Very nice! :-)

I don't think PKC needs Kodi to process stream metadata. It will pull all the relevant metadata information either from the Kodi music db directly or from the Plex Media Server.

@Memesa

This comment has been minimized.

Memesa commented Feb 12, 2017

@DaveTBlake Kodi does try to process the metadata tags from (music) streams, and obviously fails. This leads to the many errors people are seeing and Kodi unable to shut down. I guess for many addons that unofficially modify the database, removing parsing of internet streams would make Kodi more stable when having these addons installed. (I'm assuming you call internet streams direct links instead of .strm files)
But the video library parser and music library parser behave differently. So in the PlexKodiConnect addon, video files work fine (database injected with plugin:// links) and music files are less easy to manage (database injected with https:// links because plugin:// links are not accepted).

The PR I sent in was based on a bug I found when trying to circumvent the use of internet streams, by using plugin:// links in .strm files (quite a funny thing that this does work..). These .strm files inserted in the database will be parsed for metadata though and the parser then removes the entries from the library because of the lack of metadata. This can be fixed again by putting the .strm files in a path that is excluded from library scans (set through advancedsettings.xml).

The ideal way for the PlexKodiConnect addon to work would be that plugin:// files are accepted from the music library screen and that the music parser does not remove or try to read their metadata.

@davidtron

This comment has been minimized.

davidtron commented Mar 11, 2017

Since https://github.com/xbmc/xbmc/pull/11175 has been released in 17.1-RC1
Will I be able to use plexcompanion to play my music collection without triggering #209 if I apply these changes locally?

@Memesa

This comment has been minimized.

Memesa commented Mar 15, 2017

I haven't started on testing the implementation on 17.1-RC1 yet. There is also another change I would like to add to circumvent the library from removing the files again. But of course, you could try to see what happens if you apply them locally and if it doesn't work as expected revert to the original.

@davidtron

This comment has been minimized.

davidtron commented Mar 19, 2017

I rebased @Memesa change on top of 1.7.3-beta and am testing against Kodi 17.1-RC1
It's stopped triggering #209
If I attempt to add a whole album it only plays the first song and then logs errors. I'll keep digging to try and find out why.

kodi-queue.txt

@croneter

This comment has been minimized.

Owner

croneter commented Apr 6, 2017

@Memesa
Did you change or even create a plugin.video.plexkodiconnect.music? Could you share that?

@Memesa

This comment has been minimized.

Memesa commented Apr 6, 2017

@croneter
Yes I did, to allow me to tweak the commands that were sent to pkc, but also to make it easier to test. In the end this was a direct copy of the movie plugin, so for that reason I made it check if pkc.music existed and if not just use pkc.movies.

I tried to rebase this PR into the current pkc version some weeks ago, and noticed you changed quite a lot on the queuing commands. It looks much cleaner now, but I should dive in more to get this PR functional. So feel free to try.

Another thing is that in this PR, the way to hide the .strm files from the scraper is by setting the wrong directory. This is not a long lasting method, after playing a music file the directory will be added to the scanned directories and the files removed. A better way to do this is by setting the files correctly with directory and file in the database, and subsequently set the folder in the excluded directories in advancedsettings.xml (http://kodi.wiki/view/advancedsettings.xml#audio). It is also much less hacky. When I'm home I could post the code I've used (though I haven't made it automatically add the dir to as.xml, was part of my to do.

@croneter

This comment has been minimized.

Owner

croneter commented Apr 6, 2017

@Memesa
So the pkc.music did not contain any additional intelligence - good to know ;-)

Concerning the last paragraph:
Do you mean that we could link the Kodi music db directly with e.g. mp3s instead of stream files? I did try messing with the advancedsettings.xml to prevent a subsequent scanning of the files, but was unsuccessful.

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