fix tests
All checks were successful
CI / Maven unit tests (push) Successful in 14s
CI / Build and push images (push) Has been skipped
CI / Deploy to server (push) Has been skipped

This commit is contained in:
2026-09-16 19:45:33 +01:00
parent 4b40661d09
commit f548c7c8fa
2 changed files with 49 additions and 54 deletions

View File

@@ -43,13 +43,14 @@ public class StreamActionsService {
* A resolved section of a stream, ready to be muxed by {@link #saveSection}. * 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 segments the segments to concatenate, in playback order; never empty
* @param trimOffset seconds to trim from the start of {@code segments.getFirst()} * @param trimOffset seconds between the start of {@code segments.getFirst()} and
* so playback begins at the requested moment, or {@code 0} * the requested start, or {@code 0} when the first segment
* when the first segment already starts there * already starts there. Segments are copied whole, so this is
* @param duration seconds of footage to keep, measured from the trim point. * lead-in the section carries before the requested moment, not
* When the requested start fell in a recording gap and the * footage that gets trimmed away
* leading segment was dropped, this is measured from the * @param duration seconds of footage to keep, measured from the start of
* later real start so the section still ends where requested * {@code segments.getFirst()} so the section ends at the
* requested end time with the lead-in included
*/ */
public record SegmentSelection(List<Path> segments, float trimOffset, float duration) {} public record SegmentSelection(List<Path> segments, float trimOffset, float duration) {}
@@ -90,18 +91,22 @@ public class StreamActionsService {
trimOffset = 0f; trimOffset = 0f;
} }
// Measure duration from where playback actually begins so a dropped // Segments are copied whole, so playback begins at the first segment's
// leading segment shortens the section rather than overrunning endTime. // boundary rather than at the requested start. Measure the duration from
long effectiveStartMs = firstMs + (long) (trimOffset * 1000); // there so the section still ends at endTime, lead-in included.
float duration = (endTime.toEpochMilli() - effectiveStartMs) / 1000f; float duration = (endTime.toEpochMilli() - firstMs) / 1000f;
return new SegmentSelection(segments, trimOffset, duration); return new SegmentSelection(segments, trimOffset, duration);
} }
/** /**
* Muxes a {@link SegmentSelection} to {@code outputFile}, trimming the leading * Muxes a {@link SegmentSelection} to {@code outputFile}, keeping
* segment to {@code selection.trimOffset()} and keeping {@code selection.duration()} * {@code selection.duration()} seconds of footage.
* seconds of footage. * <p>
* 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.
* <p> * <p>
* Runs asynchronously; failures (process-launch errors, ffmpeg failures, * Runs asynchronously; failures (process-launch errors, ffmpeg failures,
* interruption) complete the returned future exceptionally rather than being * interruption) complete the returned future exceptionally rather than being
@@ -140,8 +145,9 @@ public class StreamActionsService {
List<String> command = List.of( List<String> command = List.of(
"ffmpeg", "ffmpeg",
"-y", "-y",
"-progress", "pipe:1",
"-i", concatInput, "-i", concatInput,
"-t", String.valueOf(duration + trimOffset), "-t", String.valueOf(duration),
"-c", "copy", "-c", "copy",
"-avoid_negative_ts", "make_zero", "-avoid_negative_ts", "make_zero",
outputFile.toAbsolutePath().toString() outputFile.toAbsolutePath().toString()

View File

@@ -57,23 +57,6 @@ class StreamActionsServiceTest {
when(metadataService.getVideoMetadata(any())).thenReturn(CompletableFuture.completedFuture(meta)); 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<String> cmd = inv.getArgument(0);
if (cmd.contains("mpegts")) {
Files.writeString(Path.of(cmd.getLast()), "x");
}
return output;
});
}
// --------------------------------------------------------------- // ---------------------------------------------------------------
// selectSegments: missing / invalid directories // selectSegments: missing / invalid directories
// --------------------------------------------------------------- // ---------------------------------------------------------------
@@ -247,8 +230,10 @@ class StreamActionsServiceTest {
assertThat(selection.segments()).extracting(p -> p.getFileName().toString()) assertThat(selection.segments()).extracting(p -> p.getFileName().toString())
.containsExactly("30.ts", "33.ts"); .containsExactly("30.ts", "33.ts");
assertThat(selection.trimOffset()).isEqualTo(1.5f); assertThat(selection.trimOffset()).isEqualTo(1.5f);
// effective start equals the requested 31.5s, so duration runs to endTime: 40 - 31.5 // 30.ts is copied whole, so the section starts at 30s rather than the
assertThat(selection.duration()).isEqualTo(8.5f); // 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 @Test
@@ -355,7 +340,7 @@ class StreamActionsServiceTest {
ProgressTracker progress = new ProgressTracker(); ProgressTracker progress = new ProgressTracker();
CommandOutput commandOutput = new CommandOutput(); CommandOutput commandOutput = new CommandOutput();
stubRunnerProducingHead(commandOutput); when(commandRunner.run(anyList(), any())).thenReturn(commandOutput);
var future = service.saveSection( var future = service.saveSection(
new StreamActionsService.SegmentSelection(List.of(segmentA, segmentB), 5.0f, 12.0f), output, progress); 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(future.get()).isSameAs(commandOutput);
assertThat(progress.isComplete()).isTrue(); assertThat(progress.isComplete()).isTrue();
// With a non-zero trim offset saveSection issues two ffmpeg passes: the // The section is one stream-copy pass over the byte-concatenated segments:
// head re-encode (trims the first segment via -ss) followed by the // the concat: protocol joins them without rebasing the timestamps nginx
// concat-demuxer stream copy (-f concat / -t / -c copy). // wrote across fragments, which is what keeps the joins seamless.
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> captor = ArgumentCaptor.forClass(List.class); ArgumentCaptor<List<String>> captor = ArgumentCaptor.forClass(List.class);
Mockito.verify(commandRunner, Mockito.times(2)).run(captor.capture(), any()); Mockito.verify(commandRunner, Mockito.times(1)).run(captor.capture(), any());
List<String> headCommand = captor.getAllValues().get(0); List<String> command = captor.getValue();
List<String> concatCommand = captor.getAllValues().get(1);
assertThat(headCommand).containsSequence("-ss", "5.0"); assertThat(command).containsSequence(
assertThat(concatCommand).containsSequence("-f", "concat"); "-i", "concat:" + segmentA.toAbsolutePath() + "|" + segmentB.toAbsolutePath());
assertThat(concatCommand).containsSequence("-t", "12.0"); assertThat(command).containsSequence("-t", "12.0");
assertThat(concatCommand).containsSequence("-c", "copy"); assertThat(command).containsSequence("-c", "copy");
assertThat(concatCommand).endsWith(output.toAbsolutePath().toString()); 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 @Test
void saveSection_zeroTrimOffset_skipsHeadReencodeAndRunsSingleConcatPass() throws Exception { void saveSection_zeroTrimOffset_buildsTheSameSingleCopyPass() throws Exception {
Path output = Path.of("/tmp/out/clip.mp4"); Path output = Path.of("/tmp/out/clip.mp4");
ProgressTracker progress = new ProgressTracker(); ProgressTracker progress = new ProgressTracker();
CommandOutput commandOutput = new CommandOutput(); CommandOutput commandOutput = new CommandOutput();
@@ -394,14 +384,14 @@ class StreamActionsServiceTest {
assertThat(future.get()).isSameAs(commandOutput); assertThat(future.get()).isSameAs(commandOutput);
assertThat(progress.isComplete()).isTrue(); assertThat(progress.isComplete()).isTrue();
// trimOffset 0 means the first segment already starts at the requested point: // trimOffset no longer changes the command shape: with or without lead-in
// no head re-encode pass, just a single concat-demuxer copy (no -ss). // the section is the same single copy pass over the joined segments.
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
ArgumentCaptor<List<String>> captor = ArgumentCaptor.forClass(List.class); ArgumentCaptor<List<String>> captor = ArgumentCaptor.forClass(List.class);
Mockito.verify(commandRunner, Mockito.times(1)).run(captor.capture(), any()); Mockito.verify(commandRunner, Mockito.times(1)).run(captor.capture(), any());
List<String> command = captor.getValue(); List<String> 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).containsSequence("-t", "10.0");
assertThat(command).doesNotContain("-ss"); assertThat(command).doesNotContain("-ss");
} }
@@ -411,8 +401,7 @@ class StreamActionsServiceTest {
Path output = Path.of("/tmp/out/clip.mp4"); Path output = Path.of("/tmp/out/clip.mp4");
ProgressTracker progress = new ProgressTracker(); ProgressTracker progress = new ProgressTracker();
// With a non-zero trim offset the head re-encode is the first ffmpeg pass; // The copy pass is the only ffmpeg invocation, so its failure must surface.
// its failure must surface.
when(commandRunner.run(anyList(), any())).thenThrow(new IOException("ffmpeg boom")); when(commandRunner.run(anyList(), any())).thenThrow(new IOException("ffmpeg boom"));
var future = service.saveSection( var future = service.saveSection(
@@ -442,8 +431,8 @@ class StreamActionsServiceTest {
void saveSection_nullOutputFile_completesExceptionally() { void saveSection_nullOutputFile_completesExceptionally() {
ProgressTracker progress = new ProgressTracker(); ProgressTracker progress = new ProgressTracker();
// trimOffset 0 skips the head pass; building the concat command dereferences // Building the command dereferences the null output path, so no ffmpeg
// the null output path, so no ffmpeg pass ever runs. // pass ever runs.
var future = service.saveSection( var future = service.saveSection(
new StreamActionsService.SegmentSelection(List.of(Path.of("/tmp/a.ts")), 0f, 10f), null, progress); new StreamActionsService.SegmentSelection(List.of(Path.of("/tmp/a.ts")), 0f, 10f), null, progress);