From 478df81434f139baf584ad0cc4538955197e732a Mon Sep 17 00:00:00 2001 From: Mickael GREGORI Date: Fri, 24 Apr 2020 09:17:52 +0200 Subject: [PATCH 1/3] FEAT Define Appendable to read input and error streams of ffmpeg/ffprobe process --- src/main/java/net/bramp/ffmpeg/FFcommon.java | 31 +++++++++++++++++-- .../java/net/bramp/ffmpeg/FFmpegTest.java | 19 ++++++++++++ .../bramp/ffmpeg/lang/NewProcessAnswer.java | 11 ++++++- .../bramp/ffmpeg/fixtures/ffmpeg-no-such-file | 1 + 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-no-such-file diff --git a/src/main/java/net/bramp/ffmpeg/FFcommon.java b/src/main/java/net/bramp/ffmpeg/FFcommon.java index 9f8abcc2..f61b7237 100644 --- a/src/main/java/net/bramp/ffmpeg/FFcommon.java +++ b/src/main/java/net/bramp/ffmpeg/FFcommon.java @@ -9,6 +9,7 @@ import javax.annotation.Nonnull; import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; import java.util.List; @@ -29,6 +30,12 @@ abstract class FFcommon { /** Version string */ String version = null; + /** Process input stream */ + Appendable processInputStream = null; + + /** Process error stream */ + Appendable processErrorStream = null; + public FFcommon(@Nonnull String path) { this(path, new RunProcessFunction()); } @@ -39,8 +46,24 @@ protected FFcommon(@Nonnull String path, @Nonnull ProcessFunction runFunction) { this.path = path; } + public void setProcessInputStream(@Nonnull Appendable processInputStream) { + this.processInputStream = processInputStream; + } + + public void setProcessErrorStream(@Nonnull Appendable processErrorStream) { + this.processErrorStream = processErrorStream; + } + + private BufferedReader _wrapInReader(final InputStream inputStream) { + return new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8)); + } + protected BufferedReader wrapInReader(Process p) { - return new BufferedReader(new InputStreamReader(p.getInputStream(), StandardCharsets.UTF_8)); + return _wrapInReader(p.getInputStream()); + } + + protected BufferedReader wrapErrorInReader(Process p) { + return _wrapInReader(p.getErrorStream()); } protected void throwOnError(Process p) throws IOException { @@ -108,8 +131,10 @@ public void run(List args) throws IOException { // TODO Move the copy onto a thread, so that FFmpegProgressListener can be on this thread. // Now block reading ffmpeg's stdout. We are effectively throwing away the output. - CharStreams.copy(wrapInReader(p), System.out); // TODO Should I be outputting to stdout? - + CharStreams.copy( + wrapInReader(p), (processInputStream != null) ? processInputStream : System.out); + CharStreams.copy( + wrapErrorInReader(p), (processErrorStream != null) ? processErrorStream : System.err); throwOnError(p); } finally { diff --git a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java index e5f68936..7a1d1bb3 100644 --- a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java +++ b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java @@ -1,5 +1,6 @@ package net.bramp.ffmpeg; +import com.google.common.collect.Lists; import net.bramp.ffmpeg.fixtures.Codecs; import net.bramp.ffmpeg.fixtures.Formats; import net.bramp.ffmpeg.lang.NewProcessAnswer; @@ -8,6 +9,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.slf4j.event.Level; import java.io.IOException; import java.util.List; @@ -31,6 +33,8 @@ public void before() throws IOException { when(runFunc.run(argThatHasItem("-formats"))) .thenAnswer(new NewProcessAnswer("ffmpeg-formats")); when(runFunc.run(argThatHasItem("-codecs"))).thenAnswer(new NewProcessAnswer("ffmpeg-codecs")); + when(runFunc.run(argThatHasItem("toto.mp4"))) + .thenAnswer(new NewProcessAnswer("ffmpeg-version", "ffmpeg-no-such-file")); ffmpeg = new FFmpeg(runFunc); } @@ -65,4 +69,19 @@ public void testFormats() throws IOException { verify(runFunc, times(1)).run(argThatHasItem("-formats")); } + + @Test + public void testReadProcessStreams() throws IOException { + // process input stream + Appendable processInputStream = mock(Appendable.class); + ffmpeg.setProcessInputStream(processInputStream); + // process error stream + Appendable processErrStream = mock(Appendable.class); + ffmpeg.setProcessErrorStream(processErrStream); + // run ffmpeg with non existing file + ffmpeg.run(Lists.newArrayList("-i", "toto.mp4")); + // check calls to Appendables + verify(processInputStream, times(1)).append(any(CharSequence.class)); + verify(processErrStream, times(1)).append(any(CharSequence.class)); + } } diff --git a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java index 5895537b..68eaf5a8 100644 --- a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java +++ b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java @@ -7,12 +7,21 @@ public class NewProcessAnswer implements Answer { final String resource; + String errResource = null; + public NewProcessAnswer(String resource) { this.resource = resource; } + public NewProcessAnswer(String resource, String errResource) { + this.resource = resource; + this.errResource = errResource; + } + @Override public Process answer(InvocationOnMock invocationOnMock) throws Throwable { - return new MockProcess(Helper.loadResource(resource)); + return errResource == null + ? new MockProcess(Helper.loadResource(resource)) + : new MockProcess(null, Helper.loadResource(resource), Helper.loadResource(errResource)); } } diff --git a/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-no-such-file b/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-no-such-file new file mode 100644 index 00000000..070b5f6d --- /dev/null +++ b/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-no-such-file @@ -0,0 +1 @@ +toto.mp4: No such file or directory \ No newline at end of file From c475345177edf4440515b8c03d1a61a4937c5f10 Mon Sep 17 00:00:00 2001 From: Joel Widmer Date: Tue, 5 Mar 2024 13:28:22 +0100 Subject: [PATCH 2/3] chore: specify default for processOutputStream and processErrorStream As the default is no longer null, we can skip checking for null in run. I've backed this assumption by addition preconditions to the setters. Last: The processInputStream is actually the output stream, the code now reflects that as well. --- src/main/java/net/bramp/ffmpeg/FFcommon.java | 16 ++++++++-------- src/test/java/net/bramp/ffmpeg/FFmpegTest.java | 3 +-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/bramp/ffmpeg/FFcommon.java b/src/main/java/net/bramp/ffmpeg/FFcommon.java index f61b7237..a35a130b 100644 --- a/src/main/java/net/bramp/ffmpeg/FFcommon.java +++ b/src/main/java/net/bramp/ffmpeg/FFcommon.java @@ -31,10 +31,10 @@ abstract class FFcommon { String version = null; /** Process input stream */ - Appendable processInputStream = null; + Appendable processOutputStream = System.out; /** Process error stream */ - Appendable processErrorStream = null; + Appendable processErrorStream = System.err; public FFcommon(@Nonnull String path) { this(path, new RunProcessFunction()); @@ -46,11 +46,13 @@ protected FFcommon(@Nonnull String path, @Nonnull ProcessFunction runFunction) { this.path = path; } - public void setProcessInputStream(@Nonnull Appendable processInputStream) { - this.processInputStream = processInputStream; + public void setProcessOutputStream(@Nonnull Appendable processOutputStream) { + Preconditions.checkNotNull(processOutputStream); + this.processOutputStream = processOutputStream; } public void setProcessErrorStream(@Nonnull Appendable processErrorStream) { + Preconditions.checkNotNull(processErrorStream); this.processErrorStream = processErrorStream; } @@ -131,10 +133,8 @@ public void run(List args) throws IOException { // TODO Move the copy onto a thread, so that FFmpegProgressListener can be on this thread. // Now block reading ffmpeg's stdout. We are effectively throwing away the output. - CharStreams.copy( - wrapInReader(p), (processInputStream != null) ? processInputStream : System.out); - CharStreams.copy( - wrapErrorInReader(p), (processErrorStream != null) ? processErrorStream : System.err); + CharStreams.copy(wrapInReader(p), processOutputStream); + CharStreams.copy(wrapErrorInReader(p), processErrorStream); throwOnError(p); } finally { diff --git a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java index 7a1d1bb3..c0c2ffc2 100644 --- a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java +++ b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java @@ -9,7 +9,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.slf4j.event.Level; import java.io.IOException; import java.util.List; @@ -74,7 +73,7 @@ public void testFormats() throws IOException { public void testReadProcessStreams() throws IOException { // process input stream Appendable processInputStream = mock(Appendable.class); - ffmpeg.setProcessInputStream(processInputStream); + ffmpeg.setProcessOutputStream(processInputStream); // process error stream Appendable processErrStream = mock(Appendable.class); ffmpeg.setProcessErrorStream(processErrStream); From 0cb85115f5b98fa0e06f741c6246cf05c68ad1e5 Mon Sep 17 00:00:00 2001 From: Joel Widmer Date: Tue, 5 Mar 2024 13:32:30 +0100 Subject: [PATCH 3/3] chore: make errResource final --- src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java index 68eaf5a8..25154120 100644 --- a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java +++ b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java @@ -7,10 +7,10 @@ public class NewProcessAnswer implements Answer { final String resource; - String errResource = null; + final String errResource; public NewProcessAnswer(String resource) { - this.resource = resource; + this(resource, null); } public NewProcessAnswer(String resource, String errResource) {