From b5a564c0bf23474846881a372d209db01915c94d Mon Sep 17 00:00:00 2001 From: Andrew Brampton Date: Mon, 11 Mar 2024 08:47:34 -0700 Subject: [PATCH 1/7] Updated Github Action to use latest maven-dependency-submission and allow it to fail without causing the job to fail. --- .github/workflows/maven.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index a06f41cb..b9681e9e 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -49,4 +49,5 @@ jobs: # Optional: Uploads the full dependency graph to GitHub to improve the quality of Dependabot alerts this repository can receive - name: Update dependency graph - uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6 + uses: advanced-security/maven-dependency-submission-action@v4 + continue-on-error: true From 0e8d1739af4b1598298b892c45bc6f5c0a55bdbf Mon Sep 17 00:00:00 2001 From: Joel Widmer Date: Mon, 11 Mar 2024 15:38:09 +0100 Subject: [PATCH 2/7] Add additional N/A checks to FFmpegUtils --- src/main/java/net/bramp/ffmpeg/FFmpegUtils.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java b/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java index 6c778313..fb42e733 100644 --- a/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java +++ b/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java @@ -74,10 +74,15 @@ public static String toTimecode(long duration, TimeUnit units) { * format "hour:minute:second", where second can be a decimal number. * * @param time the timecode to parse. - * @return the number of nanoseconds. + * @return the number of nanoseconds or -1 if time is 'N/A' */ public static long fromTimecode(String time) { checkNotEmpty(time, "time must not be empty string"); + + if (time.equals("N/A")) { + return -1; + } + Matcher m = TIME_REGEX.matcher(time); if (!m.find()) { throw new IllegalArgumentException("invalid time '" + time + "'"); @@ -97,6 +102,8 @@ public static long fromTimecode(String time) { * @return the bitrate in bits per second or -1 if bitrate is 'N/A' */ public static long parseBitrate(String bitrate) { + checkNotEmpty(bitrate, "bitrate must not be empty string"); + if ("N/A".equals(bitrate)) { return -1; } From 6966a234b50faa5cce3260d8e29abc46e3bfe5a6 Mon Sep 17 00:00:00 2001 From: Joel Widmer Date: Mon, 11 Mar 2024 15:38:31 +0100 Subject: [PATCH 3/7] Test N/A values in all Progress parser implementations --- .../net/bramp/ffmpeg/fixtures/Progresses.java | 14 +++++++-- .../progress/StreamProgressParserTest.java | 15 ++++++++-- .../progress/TcpProgressParserTest.java | 30 +++++++++++++++++-- .../progress/UdpProgressParserTest.java | 29 ++++++++++++++++-- .../bramp/ffmpeg/fixtures/ffmpeg-progress-na | 11 +++++++ 5 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-progress-na diff --git a/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java b/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java index 7aaf4866..b82932b3 100644 --- a/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java +++ b/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java @@ -10,12 +10,20 @@ private Progresses() { } public static final ImmutableList allFiles = - ImmutableList.of("ffmpeg-progress-0", "ffmpeg-progress-1", "ffmpeg-progress-2"); + ImmutableList.of( + "ffmpeg-progress-0", + "ffmpeg-progress-1", + "ffmpeg-progress-2"); + + public static final ImmutableList naProgressFile = ImmutableList.of("ffmpeg-progress-na"); public static final ImmutableList allProgresses = ImmutableList.of( new Progress(5, 0.0f, 800, 48, 512000000, 0, 0, 1.01f, Progress.Status.CONTINUE), new Progress(118, 23.4f, -1, -1, 5034667000L, 0, 0, -1, Progress.Status.CONTINUE), - new Progress( - 132, 23.1f, 1935500, 1285168, 5312000000L, 0, 0, 0.929f, Progress.Status.END)); + new Progress(132, 23.1f, 1935500, 1285168, 5312000000L, 0, 0, 0.929f, Progress.Status.END)); + + public static final ImmutableList naProgresses = ImmutableList.of( + new Progress(0, 0.0f, -1, -1, -1, 0, 0, -1, Progress.Status.END) + ); } diff --git a/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java b/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java index 18f41993..80d2c5bd 100644 --- a/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java +++ b/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java @@ -6,7 +6,6 @@ import java.io.IOException; import java.io.InputStream; -import java.util.List; import net.bramp.ffmpeg.fixtures.Progresses; import org.junit.Test; @@ -23,6 +22,18 @@ public void testNormal() throws IOException { InputStream inputStream = combineResource(Progresses.allFiles); parser.processStream(inputStream); - assertThat(listener.progesses, equalTo((List) Progresses.allProgresses)); + assertThat(listener.progesses, equalTo(Progresses.allProgresses)); + } + + @Test + public void testNaProgressPackets() throws IOException { + listener.reset(); + + StreamProgressParser parser = new StreamProgressParser(listener); + + InputStream inputStream = combineResource(Progresses.naProgressFile); + parser.processStream(inputStream); + + assertThat(listener.progesses, equalTo(Progresses.naProgresses)); } } diff --git a/src/test/java/net/bramp/ffmpeg/progress/TcpProgressParserTest.java b/src/test/java/net/bramp/ffmpeg/progress/TcpProgressParserTest.java index 4a7de8dc..82071824 100644 --- a/src/test/java/net/bramp/ffmpeg/progress/TcpProgressParserTest.java +++ b/src/test/java/net/bramp/ffmpeg/progress/TcpProgressParserTest.java @@ -12,7 +12,6 @@ import java.io.OutputStream; import java.net.Socket; import java.net.URISyntaxException; -import java.util.List; import net.bramp.ffmpeg.fixtures.Progresses; import org.junit.Test; @@ -44,12 +43,37 @@ public void testNormal() throws IOException, InterruptedException, URISyntaxExce parser.stop(); assertThat(bytes, greaterThan(0L)); - assertThat(progesses, equalTo((List) Progresses.allProgresses)); + assertThat(progesses, equalTo(Progresses.allProgresses)); + } + + + + @Test + public void testNaProgressPackets() throws IOException, InterruptedException, URISyntaxException { + parser.start(); + + Socket client = new Socket(uri.getHost(), uri.getPort()); + assertTrue("Socket is connected", client.isConnected()); + + InputStream inputStream = combineResource(Progresses.naProgressFile); + OutputStream outputStream = client.getOutputStream(); + + long bytes = ByteStreams.copy(inputStream, outputStream); + + // HACK, but give the TcpProgressParser thread time to actually handle the connection/data + // before the client is closed, and the parser is stopped. + Thread.sleep(100); + + client.close(); + parser.stop(); + + assertThat(bytes, greaterThan(0L)); + assertThat(progesses, equalTo(Progresses.naProgresses)); } @Test public void testPrematureDisconnect() - throws IOException, InterruptedException, URISyntaxException { + throws IOException { parser.start(); new Socket(uri.getHost(), uri.getPort()).close(); parser.stop(); diff --git a/src/test/java/net/bramp/ffmpeg/progress/UdpProgressParserTest.java b/src/test/java/net/bramp/ffmpeg/progress/UdpProgressParserTest.java index 7f9ccf43..bb06fc51 100644 --- a/src/test/java/net/bramp/ffmpeg/progress/UdpProgressParserTest.java +++ b/src/test/java/net/bramp/ffmpeg/progress/UdpProgressParserTest.java @@ -24,7 +24,7 @@ public ProgressParser newParser(ProgressListener listener) } @Test - public void testNormal() throws IOException, InterruptedException, URISyntaxException { + public void testNormal() throws IOException, InterruptedException { parser.start(); final InetAddress addr = InetAddress.getByName(uri.getHost()); @@ -41,10 +41,35 @@ public void testNormal() throws IOException, InterruptedException, URISyntaxExce } } + Thread.sleep(1000); // HACK: Wait a short while to avoid closing the receiving socket + + parser.stop(); + + assertThat(progesses, equalTo(Progresses.allProgresses)); + } + + @Test + public void testNaProgressPackets() throws IOException, InterruptedException, URISyntaxException { + parser.start(); + + final InetAddress addr = InetAddress.getByName(uri.getHost()); + final int port = uri.getPort(); + + try (DatagramSocket socket = new DatagramSocket()) { + // Load each Progress Fixture, and send in a single datagram packet + for (String progressFixture : Progresses.naProgressFile) { + InputStream inputStream = loadResource(progressFixture); + byte[] bytes = ByteStreams.toByteArray(inputStream); + + DatagramPacket packet = new DatagramPacket(bytes, bytes.length, addr, port); + socket.send(packet); + } + } + Thread.sleep(100); // HACK: Wait a short while to avoid closing the receiving socket parser.stop(); - assertThat(progesses, equalTo((List) Progresses.allProgresses)); + assertThat(progesses, equalTo(Progresses.naProgresses)); } } diff --git a/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-progress-na b/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-progress-na new file mode 100644 index 00000000..dde3b21c --- /dev/null +++ b/src/test/resources/net/bramp/ffmpeg/fixtures/ffmpeg-progress-na @@ -0,0 +1,11 @@ +frame=0 +fps=0 +stream_0_0_q=-1.0 +bitrate=N/A +total_size=N/A +out_time_ms=N/A +out_time=N/A +dup_frames=0 +drop_frames=0 +speed=N/A +progress=end From 4bc84da39d41969adcb7d233a9bd48bb658ac825 Mon Sep 17 00:00:00 2001 From: Euklios Date: Mon, 11 Mar 2024 17:27:37 +0100 Subject: [PATCH 4/7] Replace animal-sniffer with javac parameter (#312) * Replace animal-sniffer with javac parameter / maven.compiler.release property --- pom.xml | 45 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/pom.xml b/pom.xml index 9048c681..b4fb888f 100644 --- a/pom.xml +++ b/pom.xml @@ -40,6 +40,8 @@ 1.8 + ${base.java.version} + ${base.java.version} 1.5.2 @@ -243,11 +245,6 @@ maven-surefire-plugin 3.2.1 - - org.codehaus.mojo - animal-sniffer-maven-plugin - 1.23 - org.codehaus.mojo cobertura-maven-plugin @@ -485,11 +482,6 @@ extra-enforcer-rules 1.5.1 - - org.codehaus.mojo - animal-sniffer-enforcer-rule - 1.21 - @@ -520,27 +512,6 @@ - - org.codehaus.mojo - animal-sniffer-maven-plugin - - - check-java-api - test - - check - - - - org.codehaus.mojo.signature - java17 - 1.0 - - - - - - org.codehaus.mojo @@ -740,4 +711,16 @@ + + + + set-compiler-release + + [9,) + + + ${base.java.version} + + + From 8890104886e34b606c8e75d5de65f12db52ea23b Mon Sep 17 00:00:00 2001 From: Euklios Date: Mon, 11 Mar 2024 17:30:53 +0100 Subject: [PATCH 5/7] Feature/pr214 Appendable to output and error streams - changes (#304) * FEAT Define Appendable to read input and error streams of ffmpeg/ffprobe process Co-authored-by: Mickael GREGORI --- 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 b5746102..e766a1ca 100644 --- a/src/main/java/net/bramp/ffmpeg/FFcommon.java +++ b/src/main/java/net/bramp/ffmpeg/FFcommon.java @@ -8,6 +8,7 @@ import com.google.common.io.CharStreams; 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; @@ -28,6 +29,12 @@ abstract class FFcommon { /** Version string */ String version = null; + /** Process input stream */ + Appendable processOutputStream = System.out; + + /** Process error stream */ + Appendable processErrorStream = System.err; + public FFcommon(@Nonnull String path) { this(path, new RunProcessFunction()); } @@ -38,8 +45,26 @@ protected FFcommon(@Nonnull String path, @Nonnull ProcessFunction runFunction) { this.path = path; } + public void setProcessOutputStream(@Nonnull Appendable processOutputStream) { + Preconditions.checkNotNull(processOutputStream); + this.processOutputStream = processOutputStream; + } + + public void setProcessErrorStream(@Nonnull Appendable processErrorStream) { + Preconditions.checkNotNull(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 { @@ -107,8 +132,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), System.out); // TODO Should I be outputting to stdout? - + 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 7a4902fe..91e31f6f 100644 --- a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java +++ b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java @@ -7,6 +7,8 @@ import java.io.IOException; import java.util.List; + +import com.google.common.collect.Lists; import net.bramp.ffmpeg.fixtures.Codecs; import net.bramp.ffmpeg.fixtures.Filters; import net.bramp.ffmpeg.fixtures.Formats; @@ -35,6 +37,8 @@ public void before() throws IOException { when(runFunc.run(argThatHasItem("-codecs"))).thenAnswer(new NewProcessAnswer("ffmpeg-codecs")); when(runFunc.run(argThatHasItem("-pix_fmts"))) .thenAnswer(new NewProcessAnswer("ffmpeg-pix_fmts")); + when(runFunc.run(argThatHasItem("toto.mp4"))) + .thenAnswer(new NewProcessAnswer("ffmpeg-version", "ffmpeg-no-such-file")); when(runFunc.run(argThatHasItem("-filters"))) .thenAnswer(new NewProcessAnswer("ffmpeg-filters")); @@ -72,6 +76,21 @@ 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.setProcessOutputStream(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)); + } + @Test public void testPixelFormat() throws IOException { // Run twice, the second should be cached diff --git a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java index 5895537b..25154120 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; + final String errResource; + public NewProcessAnswer(String resource) { + this(resource, null); + } + + 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 053c32ce8086d431573113f135d18b65963d764e Mon Sep 17 00:00:00 2001 From: Andrew Brampton Date: Mon, 11 Mar 2024 11:16:23 -0700 Subject: [PATCH 6/7] Created JDK 8 and 9 maven profiles, and allow JDK 8 to use old versions of test dependencies. --- pom.xml | 68 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index b4fb888f..59d91669 100644 --- a/pom.xml +++ b/pom.xml @@ -39,7 +39,7 @@ - 1.8 + 8 ${base.java.version} ${base.java.version} @@ -127,7 +127,7 @@ org.glassfish.grizzly grizzly-http-server - 4.0.0 + 4.0.2 @@ -531,17 +531,6 @@ UTF-8 - - com.spotify.fmt - fmt-maven-plugin - - - - format - - - - install @@ -714,13 +703,64 @@ - set-compiler-release + + Java 8 + + 1.8 + + + + + + ch.qos.logback + logback-classic + + 1.3.14 + + + org.mockito + mockito-core + + 4.11.0 + + + org.glassfish.grizzly + grizzly-http-server + + 3.0.1 + + + + + + + Java 9+ [9,) + ${base.java.version} + + + + + com.spotify.fmt + fmt-maven-plugin + + + + + From be9d457e6d053ce337ee79f5c1c90727fc4de561 Mon Sep 17 00:00:00 2001 From: Andrew Brampton Date: Mon, 11 Mar 2024 11:17:04 -0700 Subject: [PATCH 7/7] Add JDK 8 to the Github actions for testing. --- .github/workflows/maven.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index b9681e9e..56478613 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -15,10 +15,7 @@ jobs: strategy: matrix: # Long term supported versions - java-version: [11, 17, 21] - - # TODO Add support for 8, but it fails with - # java.lang.UnsupportedClassVersionError: com/spotify/fmt/FMT has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0 + java-version: [8, 11, 17, 21] # TODO Should we test locales? The old travis setup did, see: # https://github.com/bramp/ffmpeg-cli-wrapper/pull/55