diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/CuesResolver.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/CuesResolver.java index 86652b9e13..4f8c161bb9 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/CuesResolver.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/CuesResolver.java @@ -30,8 +30,11 @@ import com.google.common.collect.ImmutableList; */ /* package */ interface CuesResolver { - /** Adds cues to this instance. */ - void addCues(CuesWithTiming cues); + /** + * Adds {@code cues} to this instance, returning whether this changes the cues displayed at {@code + * currentPositionUs}. + */ + boolean addCues(CuesWithTiming cues, long currentPositionUs); /** * Returns the {@linkplain Cue cues} that should be shown at time {@code timeUs}. diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/MergingCuesResolver.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/MergingCuesResolver.java index aeb01bb338..f015f059fc 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/MergingCuesResolver.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/MergingCuesResolver.java @@ -60,16 +60,19 @@ import java.util.List; } @Override - public void addCues(CuesWithTiming cues) { + public boolean addCues(CuesWithTiming cues, long currentPositionUs) { checkArgument(cues.startTimeUs != C.TIME_UNSET); checkArgument(cues.durationUs != C.TIME_UNSET); + boolean cuesAreShownAtCurrentTime = + cues.startTimeUs <= currentPositionUs && currentPositionUs < cues.endTimeUs; for (int i = cuesWithTimingList.size() - 1; i >= 0; i--) { if (cues.startTimeUs >= cuesWithTimingList.get(i).startTimeUs) { cuesWithTimingList.add(i + 1, cues); - return; + return cuesAreShownAtCurrentTime; } } cuesWithTimingList.add(0, cues); + return cuesAreShownAtCurrentTime; } @Override diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/ReplacingCuesResolver.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/ReplacingCuesResolver.java index fec4add61a..d8e957621f 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/ReplacingCuesResolver.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/ReplacingCuesResolver.java @@ -15,6 +15,8 @@ */ package androidx.media3.exoplayer.text; +import static androidx.media3.common.util.Assertions.checkArgument; + import androidx.media3.common.C; import androidx.media3.common.text.Cue; import androidx.media3.extractor.text.CuesWithTiming; @@ -43,14 +45,23 @@ import java.util.ArrayList; } @Override - public void addCues(CuesWithTiming cues) { + public boolean addCues(CuesWithTiming cues, long currentPositionUs) { + checkArgument(cues.startTimeUs != C.TIME_UNSET); + boolean cuesAreShownAtCurrentTime = + cues.startTimeUs <= currentPositionUs + && (cues.endTimeUs == C.TIME_UNSET || currentPositionUs < cues.endTimeUs); for (int i = cuesWithTimingList.size() - 1; i >= 0; i--) { if (cues.startTimeUs >= cuesWithTimingList.get(i).startTimeUs) { cuesWithTimingList.add(i + 1, cues); - return; + return cuesAreShownAtCurrentTime; + } else if (cuesWithTimingList.get(i).startTimeUs <= currentPositionUs) { + // There's a cue that starts after the new cues, but before the current time, meaning + // the new cues will not be displayed at the current time. + cuesAreShownAtCurrentTime = false; } } cuesWithTimingList.add(0, cues); + return cuesAreShownAtCurrentTime; } @Override diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/TextRenderer.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/TextRenderer.java index 62ab211c06..1fb35b81c2 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/TextRenderer.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/TextRenderer.java @@ -312,12 +312,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { cueData.limit()); cueDecoderInputBuffer.clear(); - cuesResolver.addCues(cuesWithTiming); - - // Return whether the CuesWithTiming we added to CuesMerger changes the subtitles that - // should be on-screen *now*. - return cuesWithTiming.startTimeUs <= positionUs - && positionUs < cuesWithTiming.startTimeUs + cuesWithTiming.durationUs; + return cuesResolver.addCues(cuesWithTiming, positionUs); case C.RESULT_FORMAT_READ: case C.RESULT_NOTHING_READ: default: diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/MergingCuesResolverTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/MergingCuesResolverTest.java index 5807071317..c8ab32cf34 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/MergingCuesResolverTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/MergingCuesResolverTest.java @@ -62,8 +62,8 @@ public final class MergingCuesResolverTest { new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 6_000_000, /* durationUs= */ 1_000_000); // Reverse the addCues call to check everything still works (it should). - mergingCuesResolver.addCues(secondCuesWithTiming); - mergingCuesResolver.addCues(firstCuesWithTiming); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(mergingCuesResolver, 3_000_000); @@ -82,8 +82,8 @@ public final class MergingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 2_000_000, /* durationUs= */ 4_000_000); - mergingCuesResolver.addCues(firstCuesWithTiming); - mergingCuesResolver.addCues(secondCuesWithTiming); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(mergingCuesResolver, 1_000_000); assertCueTextBetween(mergingCuesResolver, 1_000_000, 2_000_000, "first cue"); @@ -109,8 +109,8 @@ public final class MergingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 1_000_000, /* durationUs= */ 3_000_000); - mergingCuesResolver.addCues(firstCuesWithTiming); - mergingCuesResolver.addCues(secondCuesWithTiming); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(mergingCuesResolver, 1_000_000); // secondCuesWithTiming has a shorter duration than firstCuesWithTiming, so should appear later @@ -134,8 +134,8 @@ public final class MergingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 2_000_000, /* durationUs= */ 3_000_000); - mergingCuesResolver.addCues(firstCuesWithTiming); - mergingCuesResolver.addCues(secondCuesWithTiming); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(mergingCuesResolver, 1_000_000); // secondCuesWithTiming has a shorter duration than firstCuesWithTiming, so should appear later @@ -158,7 +158,59 @@ public final class MergingCuesResolverTest { new CuesWithTiming( FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ C.TIME_UNSET); - assertThrows(IllegalArgumentException.class, () -> mergingCuesResolver.addCues(cuesWithTiming)); + assertThrows( + IllegalArgumentException.class, + () -> mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 0)); + } + + @Test + public void addCues_cuesStartAfterCurrentPosition() { + MergingCuesResolver mergingCuesResolver = new MergingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 1_000_000)) + .isFalse(); + } + + @Test + public void addCues_cuesStartAtCurrentPosition() { + MergingCuesResolver mergingCuesResolver = new MergingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 3_000_000)) + .isTrue(); + } + + @Test + public void addCues_cuesDisplayedAtCurrentPosition() { + MergingCuesResolver mergingCuesResolver = new MergingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 4_000_000)) + .isTrue(); + } + + @Test + public void addCues_cuesEndBeforeCurrentPosition() { + MergingCuesResolver mergingCuesResolver = new MergingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 6_000_000)) + .isFalse(); + } + + @Test + public void addCues_cuesEndAtCurrentPosition() { + MergingCuesResolver mergingCuesResolver = new MergingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(mergingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 5_000_000)) + .isFalse(); } @Test @@ -171,9 +223,9 @@ public final class MergingCuesResolverTest { CuesWithTiming thirdCuesWithTiming = new CuesWithTiming(THIRD_CUES, /* startTimeUs= */ 8_000_000, /* durationUs= */ 4_000_000); - mergingCuesResolver.addCues(firstCuesWithTiming); - mergingCuesResolver.addCues(secondCuesWithTiming); - mergingCuesResolver.addCues(thirdCuesWithTiming); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(thirdCuesWithTiming, /* currentPositionUs= */ 0); // Remove only firstCuesWithTiming (secondCuesWithTiming should be kept because it ends after // this time). @@ -195,9 +247,9 @@ public final class MergingCuesResolverTest { CuesWithTiming thirdCuesWithTiming = new CuesWithTiming(THIRD_CUES, /* startTimeUs= */ 8_000_000, /* durationUs= */ 4_000_000); - mergingCuesResolver.addCues(firstCuesWithTiming); - mergingCuesResolver.addCues(secondCuesWithTiming); - mergingCuesResolver.addCues(thirdCuesWithTiming); + mergingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); + mergingCuesResolver.addCues(thirdCuesWithTiming, /* currentPositionUs= */ 0); mergingCuesResolver.clear(); diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/ReplacingCuesResolverTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/ReplacingCuesResolverTest.java index 0f4cbf9948..4a2b41d20f 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/ReplacingCuesResolverTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/ReplacingCuesResolverTest.java @@ -64,8 +64,8 @@ public final class ReplacingCuesResolverTest { SECOND_CUES, /* startTimeUs= */ 6_000_000, /* durationUs= */ C.TIME_UNSET); // Reverse the addCues call to check everything still works (it should). - replacingCuesResolver.addCues(secondCuesWithTiming); - replacingCuesResolver.addCues(cuesWithTiming); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(replacingCuesResolver, 3_000_000); assertCueTextBetween(replacingCuesResolver, 3_000_000, 6_000_000, "first cue"); @@ -81,8 +81,8 @@ public final class ReplacingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 6_000_000, /* durationUs= */ 1_000_000); - replacingCuesResolver.addCues(firstCuesWithTiming); - replacingCuesResolver.addCues(secondCuesWithTiming); + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(replacingCuesResolver, 3_000_000); @@ -101,8 +101,8 @@ public final class ReplacingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 2_000_000, /* durationUs= */ 4_000_000); - replacingCuesResolver.addCues(firstCuesWithTiming); - replacingCuesResolver.addCues(secondCuesWithTiming); + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(replacingCuesResolver, 1_000_000); assertCueTextBetween(replacingCuesResolver, 1_000_000, 2_000_000, "first cue"); @@ -119,8 +119,8 @@ public final class ReplacingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 1_000_000, /* durationUs= */ 3_000_000); - replacingCuesResolver.addCues(firstCuesWithTiming); - replacingCuesResolver.addCues(secondCuesWithTiming); + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); assertCuesStartAt(replacingCuesResolver, 1_000_000); assertCueTextBetween( @@ -128,6 +128,75 @@ public final class ReplacingCuesResolverTest { assertCuesEndAt(replacingCuesResolver, 4_000_000); } + @Test + public void addCues_cuesStartAfterCurrentPosition() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 1_000_000)) + .isFalse(); + } + + @Test + public void addCues_cuesStartAtCurrentPosition() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 3_000_000)) + .isTrue(); + } + + @Test + public void addCues_cuesDisplayedAtCurrentPosition() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 4_000_000)) + .isTrue(); + } + + @Test + public void addCues_cuesDisplayedAtCurrentPosition_butAlreadyReplacedByLaterCues() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming firstCuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 3_000_000); + CuesWithTiming secondCuesWithTiming = + new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 4_000_000, /* durationUs= */ 3_000_000); + CuesWithTiming thirdCuesWithTiming = + new CuesWithTiming(THIRD_CUES, /* startTimeUs= */ 5_000_000, /* durationUs= */ 3_000_000); + replacingCuesResolver.addCues(thirdCuesWithTiming, /* currentPositionUs= */ 0); + + assertThat( + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 5_000_000)) + .isFalse(); + assertThat( + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 5_500_000)) + .isFalse(); + } + + @Test + public void addCues_cuesEndBeforeCurrentPosition() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 6_000_000)) + .isFalse(); + } + + @Test + public void addCues_cuesEndAtCurrentPosition() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming cuesWithTiming = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 2_000_000); + + assertThat(replacingCuesResolver.addCues(cuesWithTiming, /* currentPositionUs= */ 5_000_000)) + .isFalse(); + } + @Test public void discardCuesBeforeTimeUs() { ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); @@ -136,8 +205,8 @@ public final class ReplacingCuesResolverTest { CuesWithTiming secondCuesWithTiming = new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 6_000_000, /* durationUs= */ 1_000_000); - replacingCuesResolver.addCues(firstCuesWithTiming); - replacingCuesResolver.addCues(secondCuesWithTiming); + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); // Remove firstCuesWithTiming replacingCuesResolver.discardCuesBeforeTimeUs(5_500_000); @@ -158,9 +227,9 @@ public final class ReplacingCuesResolverTest { CuesWithTiming thirdCuesWithTiming = new CuesWithTiming(THIRD_CUES, /* startTimeUs= */ 8_000_000, /* durationUs= */ 4_000_000); - replacingCuesResolver.addCues(firstCuesWithTiming); - replacingCuesResolver.addCues(secondCuesWithTiming); - replacingCuesResolver.addCues(thirdCuesWithTiming); + replacingCuesResolver.addCues(firstCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(secondCuesWithTiming, /* currentPositionUs= */ 0); + replacingCuesResolver.addCues(thirdCuesWithTiming, /* currentPositionUs= */ 0); replacingCuesResolver.clear();