Update Plex Rock-On to allow optional support of Intel QuickSync transcoding

[Hooverdan96]

Merge updates

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 # .
Add QuickSync option (device) to enable Intel HW transcoding

General information on project

This pull request proposes to enhance an existing rock-on for the following project:

• name: Plex Rockon
• website: N/A
• description: using existing Plex RockOn definition under underlying Docker Image from linuxserver.io add option to enable QuickSync certain Intel CPUs.

Information on docker image

• docker image: https://hub.docker.com/r/linuxserver/plex
• is an official docker image available for this project?: yes, but since this is an enhancement to the existing RockOn, continuing to use the Linuxserver.io maintained docker (which is actively updated in line with official Plex releases).

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

[phillxnet]

@Hooverdan96 Thanks for this submission. Nice.

@FroggyFlox I know you are super busy but given this Rock-on uses one of the Rock-on features you recently introduced, would you mind take a quick look. Also if you could note here if you think this requires a pending pr of yours, given it is an instance of 2 different Rock-ons using the same image? Cheers.

Would be nice to have this intel QuickSync variant in as well as our generic offerings. But I'm wandering if the transcode dir is optional. In which case it could be added afterwards as per our regular plex Rock-on.

[FroggyFlox]

Thanks a lot for submitting this PR, @Hooverdan96!

I'll try to give it a try as soon as possible and review it.

Also if you could note here if you think this requires a pending pr of yours, given it is an instance of 2 different Rock-ons using the same image?

I believe it simply is an update to our existing Plex rock-on (only one so far, until #169 is merged, though) so there shouldn't be any requirement on a pending PR.

Would be nice to have this intel QuickSync variant in as well as our generic offerings. But I'm wandering if the transcode dir is optional. In which case it could be added afterwards as per our regular plex Rock-on.

I was thinking of having this one as an update to our current Plex rock-on and give the possibility to the users with compatible hardware to use hardware transcoding, the same way we currently do with the Emby server rock-on. As the "devices" info is optional during install, the user can simply skip that step if no hardware transcoding is needed. This would thus save us from having yet one more rock-on just for that.

As to the transcoding dir, @Hooverdan96 actually had the idea of making it optional but as we can't do that yet (good feature to implement in the future, though) I recommended him to go ahead and put it in the JSON anyway. I somehow didn't think of your solution, however, which is a lot better in my opinion. @Hooverdan96 , sorry for the bad advice on that one.

Of note, we might want to update our already very detailed Plex rock-on write-up in the Rockstor documentation with a section on the steps required for using QuickSync (how to get device path, create transcoding dir, proper permissions, etc...).
http://rockstor.com/docs/docker-based-rock-ons/plex-media-server.html

For context, here's the forum thread where this feature was discussed:
https://forum.rockstor.com/t/quicksync-with-plex-rock-on/6374?u=flox

[Hooverdan96]

Gentlemen,
thanks for the good feedback. And, yes, after discussing with @FroggyFlox the intent was to update the generic Plex variant that's already there. On the transcoding directory, I can take that out, and in one of the descriptions (for adding additional shares later maybe, highlight, that it's necessary to create if QuickSync is activated. I will update the JSON accordingly and commit it again.

[Hooverdan96]

I made the update and tested it locally. One thing I noticed, in my changes of the plex.json I named the container plex-linuxserver.io-qs (as opposed to the original container name plex-linuxserver.io). I did this, so I could more easily test without having to rename files and all that... is it better to change that back?

[FroggyFlox]

One thing I noticed, in my changes of the plex.json I named the container plex-linuxserver.io-qs (as opposed to the original container name plex-linuxserver.io). I did this, so I could more easily test without having to rename files and all that... is it better to change that back?

Yes, as this is an update to an existing rock-on, it is important to leave the container name as well as the rock-on name the same otherwise we risk creating unecessary conflicts.

I tested your json and have some suggestions that I will share in a proper review coming shortly.

[FroggyFlox]

Wow, you're too fast for me, @Hooverdan96, thanks for making these changes!

As you can see, most of my comments simply relate to "style" and information displayed in the description field, for instance, as we've recently been trying to harmonize this across all rock-ons.

@phillxnet, note that this rock-on is another example of a Linuxserver.io image recommending the --net=host option. Their image documentation reads:

Special note - If you'd like to run Plex without requiring --net=host (NOT recommended) then you will need the following ports in your docker create command:

-p 32400:32400
-p 32400:32400/udp
-p 32469:32469
-p 32469:32469/udp
-p 5353:5353/udp
-p 1900:1900/udp

As you can see, it seems to be possible to not use host networking and bind all these ports manually (which thanks to Rockstor is as easy as clicking next), but I'm not sure whether it would also require further customization in Plex's settings as a result. It would be nice to have a user's perspective on that.

[Hooverdan96]

@FroggyFlox, thanks for the review and changes/suggestions. Appreciate it.

My 2 cents on your comment about the net=host option ... I've looked around a little, and seem to find a few forum entries (reddit, plex, etc.) talking about not using the net=host but only binding the ports, but to me that also looks like an "advanced" concern about container security and network control. So, at the very least, it would be good to have both options (i.e. the one were the host network is being used and no special port mapping has to occur) and the bridge option where one then maps the explicit ports). For that to be possible we would need an extension to query the user for which network they want to use, correct? Or can those "switches" already be surfaced (I've not noticed it in the JSON files, that's why I assumed they were always "hard-coded")?

I found one more port (related to Roku apparently) that would need to be mapped:

TCP port 32400 for the Plex Media Server.
UDP port 1900 for the Plex DLNA Server.
TCP port 3005 for Plex Companion.
UDP port 5353 for network discovery.
TCP port 8324 for Roku via Plex Companion.
UDP port 32410, 32412, 32413, 32414 for network discovery.
TCP port 32469 for the Plex DLNA Server.

Within Plex I can manually specify an "external" port (aka 32400), but if the container port remains at 32400, then the Server does not need any port updates in its admin menu by the user after installation, since it would be managed by defining what the host port is.

Cheers,
Dan

[FroggyFlox]

Thanks for the additional information on these ports, @Hooverdan96, that is helpful, especially given those apparently needed for the Plex Companion on Roku devices, as well as those you labeled as "network discovery" seem to not be listed on the image's documentation. These also seem to indicate the --net=host would greatly facilitate things for the user as one can then simply set the port of choice in the Plex settings and be good to go (maybe that's why LSIO recommends it, then).

As the "philosophy" behind rock-ons is to greatly simplify the task for the user, I see it as an opportunity (and even a responsibility) to enforce best practices when possible, which includes using the best setup in terms of usability and security; this is why we try not the use host networking when possible and why I mentioned it in my previous post. In this case things may not be that simple, however, given the information you detailed...

For that to be possible we would need an extension to query the user for which network they want to use, correct? Or can those "switches" already be surfaced (I've not noticed it in the JSON files, that's why I assumed they were always "hard-coded")?

By default (i.e. unless specified otherwise), Docker connects all containers to a bridge network called docker0 and you can see this network in Rockstor's Network page, for instance. Docker does allow the creation of multiple such networks, however, and there is an extensive work well underway to implement this in Rockstor (see rockstor/rockstor-core#1982, rockstor/rockstor-core#2003, rockstor/rockstor-core#2009, rockstor/rockstor-core#2013). This will allow a lot more customization re-docker networks in Rockstor and helps this kind of scenario for users who need more customization.

[Hooverdan96]

As usual, a thoroughly detailed response, thanks, @FroggyFlox. Makes sense to me. Question is now where to go from here. Should this version be merged and become the default with a new issue/feature request opened to keep track? That way we could possibly pick it up again, once you've worked through the dev items and the OpenSUSE transition. In the meantime, I would tend to think the advanced users with need for full control and maximum security design options would "mod" this RockOn set up anyway.

[FroggyFlox]

I tend to agree with you, @Hooverdan96, especially considering that this rock-on already currently uses host networking and this PR is focused on adding QuickSync support.

I may miss an easy solution, though, so @phillxnet's input will be again particularly valuable.

[phillxnet]

@FroggyFlox @Hooverdan96 You've pretty much covered it here I think. But out of interest we did in the past remove the host networking from this Plex rock-on and ended up having to put it back in again for discovery (local particularly I think) to work. Don't think we then had a full complement of ports though but I suspect some discovery related ports are 'hard wired' within clients which presents us with a problem as the Rock-ons system will offer up what ever is next available on the 'outside' and so if we do offer mapping of all ports and drop host networking we may still end up with a broken Plex install which is of course not good. We may have need for a fixed port mapping option for such instances. And that would require a pre-check to inform users that required ports (those defined as immutable) are not available, and which Rock-on is currently holding them, and the ability to then edit these (though a remove and re-install is still there of course) !!

So yes I say the dropping of host networking for this Rock-on is beyond the scope of this pr.

Is their a consensus that this pr is now ready to publish? If so I'll do the honours when next able.

[Hooverdan96]

@phillxnet, yes, I concur. If we pick this up again, it would definitely require more in-depth testing on the port definition/network combination. So, from my perspective this PR is ready, but I'll leave the last word for @FroggyFlox ... Thanks for all the handholding through this process.

[FroggyFlox]

Thanks a lot for your input, @phillxnet and @Hooverdan96!

Sorry for the delay in answering this time, I wanted to test it again, just to make sure. It does work as expected (as tested on 3.9.2-50):

• can be installed without error
• can be turned OFF and back ON without error
• can be uninstalled without error

As mentioned in one of my previous messages, I can't test the QuickSync feature itself as I do not have compatible hardware, but it should in theory be ok as we are following the image's documentation on how to implement it.

Thanks again for your time and effort in preparing this PR, @Hooverdan96!

[phillxnet]

@Hooverdan96 Thanks form me also for your effort here. Much appreciated.

And Thanks to @FroggyFlox for reviewing testing and helping to shape this important Rock-on improvement.

About to merge and publish.

[phillxnet]

@Hooverdan96 and @FroggyFlox the changes in this pr are now published.

Thanks again for your efforts. It looks to be much improved and it's nice to now have this Rock-on brought up to scratch with our newer format.