cronokirby / alchemy

A discord library for Elixir
MIT License
152 stars 34 forks source link

Use raw ports instead of depending on Porcelain #82

Open cronokirby opened 4 years ago

cronokirby commented 4 years ago

Removing a dependency on Porcelain would be nice just to avoid the startup message about downloading goon. The communication we're doing across pipes could be done with the existing port API anyways, so it's better to avoid incurring an additional and somewhat annoying dependency.

Resolving this will take a bit of effort, and isn't particularly urgent.

OvermindDL1 commented 4 years ago

Instead of porcelain, why not use erlexec, it comes with a built-in C handler that compiles with the library itself, has all the functionality of Porcelain (and a bit more), although it is an erlang interface.

You really don't want to manage ffmpeg by raw ports. Ports are only intended for things that speak Port, managing non-Port-aware processes can cause signal handler issues (on both sides), zombie processes, etc... etc... erlexec fixes all that by (like goon) using a port-aware program to handle everything needed for an external process management, including killing it, signal handling, and more if necessary.

cronokirby commented 4 years ago

Does it support windows though?

I think maintaining windows support is important, especially since Discord Bots are often people's first foray into programming, and I'd like to keep this library buildable without much fuss.

From what I've seen on the page, it doesn't mention Windows support?

OvermindDL1 commented 4 years ago

Does it support windows though?

It does not because windows lacks a lot of primitives it needs. They are open to accepting a PR to allow shim'd support at least. But still, if someone is running ffmpeg on windows there will be other issues they will have to deal with too.

cronokirby commented 4 years ago

Well, I wrote the voice feature back on windows. IIRC all that was need was providing visible ffmpeg and youtube-dl executables.

OvermindDL1 commented 4 years ago

Should be simple enough to fallback to a simple port (like porcelain does, could just keep using it), you might occasionally get zombie processes and hung connections, but otherwise...? ^.^;

curz46 commented 4 years ago

Is there a bigger reason than Porcelain's goon warnings to stop using it?

cronokirby commented 4 years ago

If something can be easily done without a dependency, I'd prefer not to have another dependency. It might just be aesthetic, but I think it improves build times, if even just slightly, and makes the software a bit simpler.

curz46 commented 4 years ago

Porcelain's existence makes it sound not easy. If the reasons it exists are not applicable to Alchemy then I agree with you.

cronokirby commented 4 years ago

Well I raised this issue because I know of a library author who wrote an implementation of voice (in elixir) with a comparable amount of work/lines of code. So I don't think that porcelain is a very necessary dependency.

OvermindDL1 commented 4 years ago

It is a very useful dependency for reliability however. There are a variety of cases that can cause ffmpeg to 'hang' on certain input, and as such it really needs a management process to handle it as running it on a long-running system leaves an exploit for a denial of service attack.