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

updates jellyfin rockon configuration #345

Closed
wants to merge 1 commit into from

Conversation

kanecko
Copy link
Contributor

@kanecko kanecko commented Apr 2, 2023

the jellyfin docker image offers two additional ports which enable Service and Client Discovery (DLNA).
these are now configurable via the RockOn UI interface

To ease and accelerate the PR submission, please use the template below to provide all requested information.
You can find guidelines on which docker image to use in the Rockstor's documentation,
as well as examples of proper formatting in other rock-ons (here
and here)

Fixes #344 .

Checklist

  • Passes JSONlint validation
  • [N/A] Entry added to root.json in alphabetical order (for new rock-on only)
  • "description" object lists and links to the docker image used
  • "description" object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)
  • "website" object links to project's main website

the jellyfin docker image offers two additional ports which enable Service and Client Discovery (DLNA).
these are now configurable via the RockOn UI interface
@phillxnet phillxnet added the needs review Test install, function, on / off behaviour, all links / info. label Apr 3, 2023
@Hooverdan96
Copy link
Member

Hooverdan96 commented Apr 4, 2023

@kanecko thanks for submitting the updated Rockon. Enabling the DLNA discovery among other things is a good idea. A couple of things:

  • While it's described in the linuxserver documentation as "optional" for some of the ports, within the Rockon framework there is nothing optional about these, unfortunately. Once they have been defined in the json file, they are considered mandatory at this time (this might change at some point in the future, but will likely depend on priorities and other things).
    So I would suggest to remove the "optional" in the port descriptions that you have maintained. Because, if you leave it blank, Rockstor will complain as it considers these mandatory.
  • We now have 2 WebUI ports (of course only one is connected to the Jellyfin UI button in the Rockon screen), the original one that's under http:// and the new one that drives it to https://. I did a test install which went fine, and the requirement to have the http WebUi port set at 8096 still seems to hold (inadvertently had a different one in there, and then I couldn't access the WebUI. However, I was not able to access the https port address.
    Do you know whether there is a constraint that only one or the other has to be defined?

@FroggyFlox
Copy link
Member

Thanks a lot for keeping an eye on that and for sharing an update, @kanecko!

@Hooverdan96 made two excellent points so I have nothing to add there.

I have to admit that I am surprised to see that these two ports need to be defined for this Rock-On, though. If I remember correctly, we indeed use host networking on this one for the same reason(s) as for Emby (and Plex): DLNA and other automated discovery services that go with these media servers to facilitate users' experience. I was thus thinking that DLNA for Jellyfin would work without specifying these two ports. Note that because we are using host networking, adding these two ports to the Rock-On definition should not change the current situation as Docker simply ignores the -p XXXX:YYYY in such cases.
Can you confirm DLNA doesn't work for the moment?

That being said, maybe we should consider adding these two ports as you do here, and remove host networking if that will not hurt the user experience (read ease of client setup and use). After all, it would be far better from a security standpoint not to use host networking.

@Hooverdan96
Copy link
Member

@FroggyFlox another good point from you. It would be good if @kanecko can confirm. If I compare the write-up on linuxserver's jellyfin vs. plex, it seems like jellyfin does not actually need the net=host option, but then in turn would require all optional ports defined.
The benefit I see for the net=host setup from a user perspective would be that it makes the setup fairly painless, as no other ports (like for DLNA) should need to be defined, other than the WebUI port - but again would need confirmation from @kanecko on your question above whether with the current Rockon definition the port specifications are really needed.
Deducing from the conversation that happened during its original pull request (and I couldn't determine whether the net option was dropped during the evolution of the jellyfin docker image). It might be that @StephenBrown2 introduced the `net=host' option to make the setup as simple as possible (i.e. other than the UI port the user would not need to have any knowledge about how DLNA operates, etc.):

#227 (comment)

@kanecko
Copy link
Contributor Author

kanecko commented Apr 8, 2023

Thanks for the comments!

  • I will remove the "optional" in port descriptions.

  • "Do you know whether there is a constraint that only one or the other has to be defined?" I haven´t checked, I assume both should work. Docs mention that one needs to setup a certificate. HTTPS doesnt work for me either OOB. I will search the internet, if I can find anything. Jellyfin by default disables HTTPS port via configuration:
    image

  • "Can you confirm DLNA doesn't work for the moment?" DLNA on my TV could not discover Jellyfin before the change. That was the whole reason for this PR. I will do some more testing however.

  • "host networking" wow! I havent even noticed! Why couldnt my TV discover jellyfin through DLNA? That doesnt make sense. I will do more testing.

So.. I will do more testing with the original rockon, and then I will try to remove the host networking to see if everything still works.

@phillxnet
Copy link
Member

@kanecko We haven't forgotten about this PR by the way. Are you still interested in following-through. Just doing a tidy re backlog in Rock-ons repo? Would you rather resubmit a new pull request once you are ready?

Thanks.

@kanecko
Copy link
Contributor Author

kanecko commented Jan 18, 2024

I do not have time currently.

@phillxnet
Copy link
Member

@kanecko No worries. I'll close by way of house keeping.

If anyone is interested in pursuing this further please note your progress from where this left off and we can re-open accordingly.

@phillxnet phillxnet closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Test install, function, on / off behaviour, all links / info.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jellyfin DLNA support
4 participants