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

Allow user to replace all uses of STDOUT/STDERR for logging #355

Open
AlexanderSchuetz97 opened this issue Sep 23, 2024 · 2 comments
Open

Comments

@AlexanderSchuetz97
Copy link

Hello,

I deploy this library in an environment where the java processes stdout and stderr is discarded and there is no way for the developer
maintainer to see it.

I would like the ffmpeg errors to be captured without involving stderr as they are usefull to diagnose some errors.
To capture the errors you would need to either log them into java.util.logging or the slf4j logging facade.

I understand that you may not wish to include slf4j as a dependency.
I know that some people hate java.util.logging (and would prefer not to have to use it)

So my suggestion would be to have a global static variable that the user can set that determines where log lines get pumped to.
The default implementation would pump to stdout/stderr just like it currently does but I could then in my bootstrap of my application call
"FFmpegCommon.setLoggingSystem(FFmpegLoggingManager instance)" (just an example)

If I were to design this interface then I would give it only 1 method that looks like this:

void appendLog(Process ffmpegProc, boolean isStdErr, byte[] data, int off, int len);

If you do not want to hand in the process then please pass some unique identifier (aka process pid for example or a random UUID instead so I can tell appart error messages from multiple concurrent ffmpeg jobs)

I would outsource line breaks and all that thing to concrete implementations.
Currently I have to hack very deep into your library using inheritance to be able to accomplish this.

The default implementation that dumps everything to stdout/stderr could probably be as simple as this:

public void appendLog(Process ffmpegProc, boolean isStdErr, byte[] data, int off, int len) {
  PrintStream stream = isStdErr ? System.err : System.out;
  try {
    stream.write(data, off, len);
  } catch(Exception e) {
    //IGNORED
  }
}

Sincerely
Alexander Schütz

@AlexanderSchuetz97 AlexanderSchuetz97 changed the title Allow user to replace all uses of STDOUT/STDERR Allow user to replace all uses of STDOUT/STDERR for logging Sep 23, 2024
@Euklios
Copy link
Collaborator

Euklios commented Oct 1, 2024

I disslike the current stdout/stderr/stdin situation quite a bit and would like to see something like this.
An additional problem with the current solution would be, that stdout and stderr are merged into a single stream, this should be addressed as well.

slf4j-api is already included as a dependency, so adding support for a slf4j sink would be relatively easy.
But I'd say we should keep the default on System.out/System.err.

As for providing a reference to Process:
I see how hard it can be to identify what command is actually running.
I had a similar problem, where I had to interrupt a process that was running on a different thread.
I think providing a reference like you mentioned would be useful, but I'd prefer to make it something we (this library) owns.
So not (or at least not directly) java.lang.Process.

@AlexanderSchuetz97
Copy link
Author

AlexanderSchuetz97 commented Oct 2, 2024

Fyi: Getting the command line from a java Process Object is trivial. Files.readAllBytes("/proc/pid/cmdline") pid can be retrieved via Process.pid(). I do not require it in the log manager impl provided I can fetch the same id that you will hand me to the Ffmpeg obj that logs. I already have to "track" the Process objects anyways (I use a custom run process function that remembers the last process) so I can call destroyForcibly on it in case a client thats receiving transcoded video goes away. I do not forsee a situation where the logger would actually cancel the ffmpeg job anyways. (This would be a terrible idea)

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

No branches or pull requests

2 participants