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

Add client native mode path replacement #926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sundermann
Copy link

@sundermann sundermann commented Oct 3, 2024

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

Copy link
Member

@oddstr13 oddstr13 left a 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.

jellyfin_kodi/helper/api.py Show resolved Hide resolved
@oddstr13 oddstr13 added Native Mode Issue is related to Native playback mode needs-testing Some testing is requested/required before merging labels Oct 3, 2024
Copy link

codecov bot commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 21.90%. Comparing base (6cc0bb6) to head (8b77c14).
Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
jellyfin_kodi/helper/api.py 16.66% 5 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@oddstr13
Copy link
Member

oddstr13 commented Oct 3, 2024

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

@sundermann
Copy link
Author

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.

@oddstr13
Copy link
Member

oddstr13 commented Oct 3, 2024

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.
I have not investigated the Jellyfin APIs around this, and I don't plan on investing much time with it at this moment.

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.

Copy link

sonarcloud bot commented Oct 5, 2024

@sundermann
Copy link
Author

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.

@oddstr13
Copy link
Member

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.

@EraYaN
Copy link
Member

EraYaN commented Oct 31, 2024

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.

@marcovolpato00
Copy link

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:

  • Native path replace N is the native path of the library on the server, for example /data/movies
  • Native path replace with N is the path on the client that should be used to retrieve media files from, for example nfs://myhomeserver/whatever/data/movies

@fcr
Copy link

fcr commented Nov 2, 2024

@sundermann I also installed your changes and they restored dual subtitle functionality that had been lost due to Jellyfin 10.10.

Thank you

@youhitgit
Copy link

youhitgit commented Nov 5, 2024

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 attempt

msgctxt "#33203"
msgid "/data/movies"
msgstr "/data/movies"

msgctxt "#33204"
msgid "nfs://myhomeserver/whatever/data/movies"
msgstr "nfs://myhomeserver/whatever/data/movies"


2nd attempt

msgctxt "#33203"
msgid "Native path replace 1"
msgstr "/data/movies"

msgctxt "#33204"
msgid "Native path replace with 1"
msgstr "nfs://myhomeserver/whatever/data/movies"


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

@fcr
Copy link

fcr commented Nov 5, 2024

@youhitgit after you replace the file in the addon you should not edit any of the files.
The file to edit is in the userdata/addon_data/plugin.video.jellyfin/settings.xml as in @marcovolpato00's instructions above.

@youhitgit
Copy link

youhitgit commented Nov 5, 2024

@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.

  1. Download https://lon1.mirror.jellyfin.org/files/client/kodi/py3/plugin.video.jellyfin/plugin.video.jellyfin-1.0.5+py3.zip
  2. Open that zip file on your PC and Replace "api.py", "strings.po" and "settings.xml" with @sundermann versions of those files.
  3. In kodi "install addon from zip" and select your modified plugin.video.jellyfin-1.0.5+py3.zip file.
  4. When it asks to select your server or to enter the server manually just press cancel
  5. Open file manager in kodi and "Add source", "Browse", "Add network location"
  6. example... Protocol = NFS, Server address = 1.1.1.1, Remote Path = whatever/data/movies
  7. Do this for each library you need.
  8. Go to Addons in Kodi, Select Jellyfin and either Settings or Configure
  9. Under the "sync" tab fill in "Native path replace 1" ( native path of the library on the server, for example /data/movies )
  10. Click "Native path replace with 1" ( Select the network location you added earlier )
  11. Do that for each library you need.
  12. Check all the other settings are to your liking ( I deselected "Force HTTP playback" )
  13. Reboot
  14. Wait for jellyfin to pop up and ask you to select your server.
  15. Carry on as usual ( when asked whether you want plugin mode or Native paths obviously go for Native paths )

@sundermann THANK YOU for your excellent work
@fcr @marcovolpato00 Thank you for your help getting it working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Native Mode Issue is related to Native playback mode needs-testing Some testing is requested/required before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants