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

Process stdout should not use CharStream.copy #354

Open
Euklios opened this issue Aug 22, 2024 · 2 comments
Open

Process stdout should not use CharStream.copy #354

Euklios opened this issue Aug 22, 2024 · 2 comments
Labels

Comments

@Euklios
Copy link
Collaborator

Euklios commented Aug 22, 2024

Describe the bug
setProcessOutputStream takes in an Appendable, and later uses CharStreams.copy to transfer the output as text.
This does not work for most cases, as stdout is used to write the output streams.
This means, an encoded file would be interpreted as chars, causing some bytes being lost due to interpretation.

To Reproduce

// appendable should be a handle to a file

FFmpeg ffmpeg = new FFmpeg();
ffmpeg.setProcessOutputStream(appenable);

ffmpeg.run(new FFmpegBuilder().addInput("input.mp4")).done().addStdoutOutput().done().build());

Expected behavior
processOutputStream should be treated as binary.

Version (if applicable):

  • OS: any
  • Java Version: any
  • FFmpeg version: any
@Euklios Euklios added the bug label Aug 22, 2024
@Euklios
Copy link
Collaborator Author

Euklios commented Aug 22, 2024

It's a bit more nuanced then what I wrote above:
FFmpeg will output to stderr and send the resulting file to stdout, but only if the command succeeded.
If, for example, the output does not specify the format (no -f) or specifies a format that does not support seaking (-f mp4), it will still output to stdout.

However, replacing CharStreams with ByteStreams does not make a difference in that case, as the chars are just coppied as is

@Euklios
Copy link
Collaborator Author

Euklios commented Aug 22, 2024

Fix will be provided together with #334

Euklios added a commit to Euklios/ffmpeg-cli-wrapper that referenced this issue Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant