Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Use raw ports instead of depending on Porcelain #82

Open
cronokirby opened this issue Jul 27, 2019 · 10 comments
Open

Use raw ports instead of depending on Porcelain #82

cronokirby opened this issue Jul 27, 2019 · 10 comments

Comments

@cronokirby
Copy link
Owner

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
Copy link

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
Copy link
Owner Author

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
Copy link

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
Copy link
Owner Author

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

@OvermindDL1
Copy link

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
Copy link
Contributor

curz46 commented Jul 29, 2019

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

@cronokirby
Copy link
Owner Author

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
Copy link
Contributor

curz46 commented Jul 29, 2019

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

@cronokirby
Copy link
Owner Author

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
Copy link

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.

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

No branches or pull requests

3 participants