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

(Question) Recent changes - where is this heading ? #55

Open
ddscentral opened this issue Jan 16, 2023 · 4 comments
Open

(Question) Recent changes - where is this heading ? #55

ddscentral opened this issue Jan 16, 2023 · 4 comments

Comments

@ddscentral
Copy link

ddscentral commented Jan 16, 2023

Hello,

Just a question, not an issue. I'm not sure whether this is the right place to post this, if not, it would be good to know where should I post this question, perhaps the Jellyfin forum ?
I've forked rffmpeg a long time ago to add some extra functionality and make some tweaks for my setup (namely, support for remoting "vainfo" and also workarounds to get Intel VAAPI working properly).
I see there were a ton of changes merged since my fork. Now there's even a database of some kind, though it's not exactly clear to me what is it used for.
What is the purpose of these changes ? Because what was previously a simple remote ffmpeg invoker script seems to be turning into quite a complex program with a lot of extra functionality and some extra configuration steps. Are these changes intended purely for scaling rffmpeg to a lot of hosts or there are other purposes as well ?
I'm not sure whether I should merge these changes back to my fork or not.

@aleksasiriski
Copy link
Contributor

aleksasiriski commented Jan 16, 2023

Here's the explanation of my changes:

  1. servername field is used alongside host so that you can easily "label" your hosts, instead of having just the domain or ip adress. This is required so that my hcloud-rffmpeg script can easily delete created servers from Hetzner cloud.
  2. By adding Postgresql as an alternative to SQLite you have an option to use an External Database, which allows this script to be completely stateless, this means that you don't need persistent storage for this script to remember all the hosts and processes. Also Postgres is a lot more stable and has a lower chance of corrupting itself (but that's probably very unlikely to happen with this small script
  3. I also added some QoL code fixes which make the code shorter and more human readable (f"" change)

I don't remember what else I changed...

Now, other changes that I know of were a few bugfixes (SEGFAULT handling) and adding the option to use dated logs, which allows users to debug more easily if they need to see the log for different days, instead of scrolling they can just open the log with date in the name. Also there's a new clear command that clears all states and processes from the DB.

@aleksasiriski
Copy link
Contributor

But mostly commits are just changing the README or SETUP and after someone PRs a new feature they introduce little bugs that get fixed over ~10 additional commits..

@joshuaboniface
Copy link
Owner

While an excellent explanation @aleksasiriski I believe @ddscentral is referring to many even older changes, made after the 1.0 tag. ;-)

Most of these changes are, as you mention, to better support scaling to more than 1 target, as well as to make management "easier" (using CLI commands rather than editing configs), ability to autoconfigure in scripts, and just in general to make the whole thing more like a real program. Plus a ton of cleanup.

Don't fret - my usecase is also super basic, so all the changes work for that usecase too. But I've definitely seen a lot of desire to make this more than just a basic wrapper, hence a lot of the changes I did and @aleksasiriski is now doing.

Now, you don't have to update/backport for your own personal use. 1.0 before all the big changes does still work fine, and if it's working in your usecase, you definitely don't need to rebase it all to the current master. That said I'd love to see those changes (anything that makes it work better for more people), if you're willing to backport them or even just share the git patches!

@ddscentral
Copy link
Author

Not sure whether I'd consider using databases versus config files easier, but that's a matter of opinion. I personally prefer keeping it simple and just using config files but I know many users do prefer to use the command line instead as (like you mentioned) it's usually easier to automate than config files.

As for my code, it would need some clean-up first to be contributable and not sure how useful it would be for others as it includes changes specific to my setup.
For example, I have added some hacks, a couple of which do change the command line:

  • Jellyfin sends empty VAAPI device node name to ffmpeg (even though it is set correctly in admin dashboard) which causes transcoding to fail. I have to inject node name from wrapper.
  • VAAPI tries to use the iHD driver for Intel VAAPI which is unstable on my host, I have to change it to i965 driver.
  • Besides Jellyfin, I have other programs using rffmpeg, one of them needed stdin/stdout pipes to work in ffmpeg and those were disabled in rffmpeg for some reason.

Other changes include the addition of wrappers for vainfo and ffdetect (Emby utility, the original plan was to use rffmpeg with Emby, but I ended up switching to Jellyfin). There may be more (likely minor) changes, I haven't touched the code for a while.
If any of that could be useful, I could upload those changes to Github after some cleanup.

I believe my SSH configuration could also be of interest. I use GNU Rush to create a restricted user for rffmpeg which can only run commands necessary for transcoding but not anything else and also provides protection against shell injection. I do not think GNU Rush is completely foolproof, but it's much better security-wise than plain bash, especially for uses like ffmpeg remoting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants