-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Add client native mode path replacement #926
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation seems to depend on all jf libraries being organized within a single source path, and that the whole of it is mountable in a specified location for Kodi to access.
In reality, a Jellyfin library can have any number of media paths specified, and they can be located anywhere on the filesystem.
Using what is implemented in this patch is possible (with the assumption that it works, I have not tested it), but it would potentially require replicating the entire tree below/mnt/
or even /
of the Jellyfin server on the local host.
Possible, but cumbersome.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #926 +/- ##
========================================
Coverage 21.90% 21.90%
========================================
Files 63 63
Lines 8634 8640 +6
Branches 1587 1404 -183
========================================
+ Hits 1891 1893 +2
- Misses 6722 6728 +6
+ Partials 21 19 -2 ☔ View full report in Codecov by Sentry. |
Black formatter also isn't happy with the formatting of the new code, please run black against your changes, or otherwise re-format the if statement such that black is happy. --- /home/runner/work/jellyfin-kodi/jellyfin-kodi/jellyfin_kodi/helper/api.py 2024-10-03 15:09:36.571631+00:00
+++ /home/runner/work/jellyfin-kodi/jellyfin-kodi/jellyfin_kodi/helper/api.py 2024-10-03 15:09:41.562261+00:00
@@ -206,11 +206,16 @@
if not path:
return ""
replace = settings("directPathReplace")
replace_with = settings("directPathReplaceWith")
- if settings("useDirectPaths") == "1" and replace and replace_with and path.startswith(replace):
+ if (
+ settings("useDirectPaths") == "1"
+ and replace
+ and replace_with
+ and path.startswith(replace)
+ ):
path = path.replace(replace, replace_with, 1)
if path.startswith("\\\\"):
path = (
path.replace("\\\\", "smb://", 1)
would reformat /home/runner/work/jellyfin-kodi/jellyfin-kodi/jellyfin_kodi/helper/api.py |
b650fb9
to
890db89
Compare
Applied reformatting. I figured this was an easy way to at least get back functionality even though it imposes some rules on how the library must be organized on the file system. Looking at the plex kodi plugin, they appear to have a hard coded five total path settings mapping to each library in plex. Since jellyfin can have an arbitrary amount of libraries this approach is not feasible. I figured ideally this would be an option per library but I couldn't come up with a clean solution. |
Not only can Jellyfin have an arbitrary amount of libraries, but each library can have an arbitrary number of paths associated with them, and each one could be located anywhere on the file system. I agree that this is a quick and dirty way of doing it, but I imagine it wouldn't fit with many user's setups. Native mode is something I've only played around with mostly for testing, several years back, and I don't intend on spending much time testing at the moment – I have other projects higher on the priority list right now. I might not dedicate time to testing this PR any time soon, so I would appreciate anyone who need and depend on Native mode testing this PR out. One thing I can think of which may be an issue, is that the path might need to be set during initial configuration for things to work, so more UI for the setup wizard is probably required. |
890db89
to
e4e5fab
Compare
e4e5fab
to
8b77c14
Compare
Quality Gate passedIssues Measures |
Unfortunately, kodi does not have an API to dynamically create setting entries. I have added 5 mapping settings which hopefully is enough to cover most setups and at least bring back functionality on the upcoming 10.10 release. |
Apologies for the late response I think we should bypass the Kodi config stuff entirely and storing the info in our own file. We are already storing json files in the addon_data dir for other things, so I think it would make sense to do the same for directory mappings. I think something like this would be a sane data structure; {
"server-id": {
"server-path": "local-path"
}
} Possibly with a sub-structure under server for libraries, if this makes more sense for the ergonomics of using the data. It would require creating an addition to the server connection flow, mapping paths to local directories after selecting libraries to sync. |
You will still need to create a Kodi file location for every SMB/NFS share that you need to access. Mostly to handle the credentials. This was done manually anyway in the old setup, so that ought to be fine to keep that manual. |
Installed the plugin with this functionality and can confirm it's working like a charm, thank you @sundermann! I agree that the implementation can be improved, but at least now I can play media files without annoying random hiccups. For anyone wondering how to configure path rewrites:
|
@sundermann I also installed your changes and they restored dual subtitle functionality that had been lost due to Jellyfin 10.10. Thank you |
I'm having some trouble with this. I downloaded https://lon1.mirror.jellyfin.org/files/client/kodi/py3/plugin.video.jellyfin/plugin.video.jellyfin-1.0.5+py3.zip and replaced api.py, strings.po and settings.xml with @sundermann versions of those files. I then edited "strings.po" like below ; 1st attemptmsgctxt "#33203" msgctxt "#33204" 2nd attemptmsgctxt "#33203" msgctxt "#33204" Both attempts did "not" work. I think maybe I also need to edit "settings.xml" but after starring at the code for an hour I have no idea what needs editing. @sundermann @fcr @marcovolpato00 can any of you give me a little help please. Thank you |
@youhitgit after you replace the file in the addon you should not edit any of the files. |
@fcr Thank you, your reply helped but wasn't how i done it in the end. For anyone else who's trying to do this.
@sundermann THANK YOU for your excellent work |
As an alternative to #925 this PR aims at bringing the removed functionality from jellyfin core into the addon itself.
Native mode is removed in 10.10 and was already phased out in jellyfin web in 10.9.
Closes #925