danb35 / freenas-iocage-plex

Scripted installation of Plex Media Server in a FreeNAS jail
GNU General Public License v3.0
56 stars 9 forks source link

Optionally configure hardware transcode ... #7

Closed thorsteneb closed 4 years ago

thorsteneb commented 4 years ago

... and optionally mount media directory

danb35 commented 4 years ago

I like the idea of being able to configure hardware transcode, but the instructions seem a little cryptic. Could you flesh out the README a bit, or link to further discussion/documentation?

thorsteneb commented 4 years ago

Further discussion for hardware transcode is currently at the bottom of the readme. I can add a reference further up, to check the bottom for instructions on how to create the script that creates the ruleset. Or would you prefer that to be structured another way?

Currently, creation of the startup script and adding it to startup tasks is left to the user. This could be automated - check whether it exists, create if not - and, I didn’t think going that far fit with what you want this script to be. Definitely a topic for discussion.

Re stopping and not restarting: Yes exactly, the theory is that the user will mount a directory from GUI. This is a tiny QoL change, and can absolutely be removed again if it harshes your flow :).

danb35 commented 4 years ago

Further discussion for hardware transcode is currently at the bottom of the readme.

Ah, I should pay more attention to the PRs to my README. But yes, I think a pointer there, and a note that there would be other work to be done, would be helpful.

Currently, creation of the startup script and adding it to startup tasks is left to the user.

...which I'd agree with. This script is intended to cover the general use case; system-specific things would be out of its scope--but allowing the user to specify the ruleset on jail creation does seem like an improvement.

This is a tiny QoL change, and can absolutely be removed again if it harshes your flow :).

No, it's just mildly annoying that the GUI requires a jail to be stopped before messing with its mountpoints--I don't see any good reason for that, and it wasn't the behavior of the old GUI (admittedly under warden rather than iocage).

thorsteneb commented 4 years ago

The script to create the ruleset is not very system-specific, by the way. The only system-specific item there is whether to create it, or whether it already exists. One further improvement could be to take all necessary steps for this if the HW_TRANSCODE_RULESET is set to -1. 0 would mean "don't", positive numbers mean "I already have a script, use this ruleset" and -1 would mean "I don't have a script, create it for me and set it to run on boot". Or, you know, be even more specific, why even fiddle with -1: "CREATE" means to do just that, create it.

Edit: Or even easier, change it to USE_HW_TRANSCODE. 0 means no, 1 means "yes and create the ruleset script if it doesn't exist". The ruleset script can be queried with 'midclt call initshutdownscript.query | grep plex-ruleset.sh', easy enough, and then created with initshutdownscript.create if it doesn't already exist. Assumption would be that ruleset 10 is being used, which is what the Plex docs suggest. People who want this more granular can always set USE_HW_TRANSCODE to 0, and do the requisite work themselves. Thoughts?

thorsteneb commented 4 years ago

Ping. Do you want to:

danb35 commented 4 years ago

I think I prefer the behavior you describe with USE_HW_TRANSCODE. I'm at a bit of a disadvantage in that I don't have suitable hardware to test this on.

thorsteneb commented 4 years ago

You got it. Let me make the changes.

thorsteneb commented 4 years ago

Changes are in. Now it's USE_HW_TRANSCODE, and there is a lot of sanity-checking code to make sure we are on supported hardware, in a version of FreeNAS that's been tested with this, the drivers will load, and we're not "stepping on" anything the user may have already done.