From f548c7c8fa3c1a49c1e0dd897ba39cd55e89f0ff Mon Sep 17 00:00:00 2001 From: Dylan De Faoite Date: Wed, 16 Sep 2026 19:45:33 +0100 Subject: [PATCH] fix tests --- .../services/media/StreamActionsService.java | 36 +++++----- .../media/StreamActionsServiceTest.java | 67 ++++++++----------- 2 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java b/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java index ced6ebc..ec88229 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java @@ -43,13 +43,14 @@ public class StreamActionsService { * A resolved section of a stream, ready to be muxed by {@link #saveSection}. * * @param segments the segments to concatenate, in playback order; never empty - * @param trimOffset seconds to trim from the start of {@code segments.getFirst()} - * so playback begins at the requested moment, or {@code 0} - * when the first segment already starts there - * @param duration seconds of footage to keep, measured from the trim point. - * When the requested start fell in a recording gap and the - * leading segment was dropped, this is measured from the - * later real start so the section still ends where requested + * @param trimOffset seconds between the start of {@code segments.getFirst()} and + * the requested start, or {@code 0} when the first segment + * already starts there. Segments are copied whole, so this is + * lead-in the section carries before the requested moment, not + * footage that gets trimmed away + * @param duration seconds of footage to keep, measured from the start of + * {@code segments.getFirst()} so the section ends at the + * requested end time with the lead-in included */ public record SegmentSelection(List segments, float trimOffset, float duration) {} @@ -90,18 +91,22 @@ public class StreamActionsService { trimOffset = 0f; } - // Measure duration from where playback actually begins so a dropped - // leading segment shortens the section rather than overrunning endTime. - long effectiveStartMs = firstMs + (long) (trimOffset * 1000); - float duration = (endTime.toEpochMilli() - effectiveStartMs) / 1000f; + // Segments are copied whole, so playback begins at the first segment's + // boundary rather than at the requested start. Measure the duration from + // there so the section still ends at endTime, lead-in included. + float duration = (endTime.toEpochMilli() - firstMs) / 1000f; return new SegmentSelection(segments, trimOffset, duration); } /** - * Muxes a {@link SegmentSelection} to {@code outputFile}, trimming the leading - * segment to {@code selection.trimOffset()} and keeping {@code selection.duration()} - * seconds of footage. + * Muxes a {@link SegmentSelection} to {@code outputFile}, keeping + * {@code selection.duration()} seconds of footage. + *

+ * Segments are concatenated byte-wise through ffmpeg's {@code concat:} protocol, + * which preserves the continuous timestamps nginx writes across fragments. The + * leading segment is copied whole rather than trimmed, so playback begins up to + * {@code selection.trimOffset()} seconds before the requested moment. *

* Runs asynchronously; failures (process-launch errors, ffmpeg failures, * interruption) complete the returned future exceptionally rather than being @@ -140,8 +145,9 @@ public class StreamActionsService { List command = List.of( "ffmpeg", "-y", + "-progress", "pipe:1", "-i", concatInput, - "-t", String.valueOf(duration + trimOffset), + "-t", String.valueOf(duration), "-c", "copy", "-avoid_negative_ts", "make_zero", outputFile.toAbsolutePath().toString() diff --git a/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java b/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java index eef3327..89a5a18 100644 --- a/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java +++ b/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java @@ -57,23 +57,6 @@ class StreamActionsServiceTest { when(metadataService.getVideoMetadata(any())).thenReturn(CompletableFuture.completedFuture(meta)); } - /** - * Stubs the two-arg {@link CommandRunner#run} so the head re-encode pass - * actually writes a non-empty file at its output path — {@code encodeHead} - * verifies its output exists and is non-empty before continuing. The concat - * pass (no {@code mpegts} muxer) is left untouched. Returns {@code output} - * for both invocations. - */ - private void stubRunnerProducingHead(CommandOutput output) throws IOException, InterruptedException { - when(commandRunner.run(anyList(), any())).thenAnswer(inv -> { - List cmd = inv.getArgument(0); - if (cmd.contains("mpegts")) { - Files.writeString(Path.of(cmd.getLast()), "x"); - } - return output; - }); - } - // --------------------------------------------------------------- // selectSegments: missing / invalid directories // --------------------------------------------------------------- @@ -247,8 +230,10 @@ class StreamActionsServiceTest { assertThat(selection.segments()).extracting(p -> p.getFileName().toString()) .containsExactly("30.ts", "33.ts"); assertThat(selection.trimOffset()).isEqualTo(1.5f); - // effective start equals the requested 31.5s, so duration runs to endTime: 40 - 31.5 - assertThat(selection.duration()).isEqualTo(8.5f); + // 30.ts is copied whole, so the section starts at 30s rather than the + // requested 31.5s; duration runs from there to endTime: 40 - 30. The + // 1.5s trimOffset is carried as lead-in, not cut away. + assertThat(selection.duration()).isEqualTo(10f); } @Test @@ -355,7 +340,7 @@ class StreamActionsServiceTest { ProgressTracker progress = new ProgressTracker(); CommandOutput commandOutput = new CommandOutput(); - stubRunnerProducingHead(commandOutput); + when(commandRunner.run(anyList(), any())).thenReturn(commandOutput); var future = service.saveSection( new StreamActionsService.SegmentSelection(List.of(segmentA, segmentB), 5.0f, 12.0f), output, progress); @@ -363,24 +348,29 @@ class StreamActionsServiceTest { assertThat(future.get()).isSameAs(commandOutput); assertThat(progress.isComplete()).isTrue(); - // With a non-zero trim offset saveSection issues two ffmpeg passes: the - // head re-encode (trims the first segment via -ss) followed by the - // concat-demuxer stream copy (-f concat / -t / -c copy). + // The section is one stream-copy pass over the byte-concatenated segments: + // the concat: protocol joins them without rebasing the timestamps nginx + // wrote across fragments, which is what keeps the joins seamless. @SuppressWarnings("unchecked") ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); - Mockito.verify(commandRunner, Mockito.times(2)).run(captor.capture(), any()); - List headCommand = captor.getAllValues().get(0); - List concatCommand = captor.getAllValues().get(1); + Mockito.verify(commandRunner, Mockito.times(1)).run(captor.capture(), any()); + List command = captor.getValue(); - assertThat(headCommand).containsSequence("-ss", "5.0"); - assertThat(concatCommand).containsSequence("-f", "concat"); - assertThat(concatCommand).containsSequence("-t", "12.0"); - assertThat(concatCommand).containsSequence("-c", "copy"); - assertThat(concatCommand).endsWith(output.toAbsolutePath().toString()); + assertThat(command).containsSequence( + "-i", "concat:" + segmentA.toAbsolutePath() + "|" + segmentB.toAbsolutePath()); + assertThat(command).containsSequence("-t", "12.0"); + assertThat(command).containsSequence("-c", "copy"); + assertThat(command).containsSequence("-avoid_negative_ts", "make_zero"); + assertThat(command).endsWith(output.toAbsolutePath().toString()); + + // The leading segment is copied whole, so nothing seeks and the concat + // demuxer (-f concat) is deliberately not used. + assertThat(command).doesNotContain("-ss"); + assertThat(command).doesNotContainSequence("-f", "concat"); } @Test - void saveSection_zeroTrimOffset_skipsHeadReencodeAndRunsSingleConcatPass() throws Exception { + void saveSection_zeroTrimOffset_buildsTheSameSingleCopyPass() throws Exception { Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); CommandOutput commandOutput = new CommandOutput(); @@ -394,14 +384,14 @@ class StreamActionsServiceTest { assertThat(future.get()).isSameAs(commandOutput); assertThat(progress.isComplete()).isTrue(); - // trimOffset 0 means the first segment already starts at the requested point: - // no head re-encode pass, just a single concat-demuxer copy (no -ss). + // trimOffset no longer changes the command shape: with or without lead-in + // the section is the same single copy pass over the joined segments. @SuppressWarnings("unchecked") ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); Mockito.verify(commandRunner, Mockito.times(1)).run(captor.capture(), any()); List command = captor.getValue(); - assertThat(command).containsSequence("-f", "concat"); + assertThat(command).containsSequence("-i", "concat:/tmp/a.ts|/tmp/b.ts"); assertThat(command).containsSequence("-t", "10.0"); assertThat(command).doesNotContain("-ss"); } @@ -411,8 +401,7 @@ class StreamActionsServiceTest { Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); - // With a non-zero trim offset the head re-encode is the first ffmpeg pass; - // its failure must surface. + // The copy pass is the only ffmpeg invocation, so its failure must surface. when(commandRunner.run(anyList(), any())).thenThrow(new IOException("ffmpeg boom")); var future = service.saveSection( @@ -442,8 +431,8 @@ class StreamActionsServiceTest { void saveSection_nullOutputFile_completesExceptionally() { ProgressTracker progress = new ProgressTracker(); - // trimOffset 0 skips the head pass; building the concat command dereferences - // the null output path, so no ffmpeg pass ever runs. + // Building the command dereferences the null output path, so no ffmpeg + // pass ever runs. var future = service.saveSection( new StreamActionsService.SegmentSelection(List.of(Path.of("/tmp/a.ts")), 0f, 10f), null, progress);