diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5b14ac0405..836e557b32 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -46,6 +46,9 @@ `MediaItem.Builder.setSubtitleConfigurations`, and instead only load one if it is selected by track selection ([#1721](https://github.com/androidx/media/issues/1721)). + * Fix bug in `ReplacingCuesResolver.discardCuesBeforeTimeUs` where the cue + active at `timeUs` (started before but not yet ended) was incorrectly + discarded ([#1939](https://github.com/androidx/media/issues/1939)). * Metadata: * Image: * DRM: 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 d8e957621f..d09557f349 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 @@ -80,9 +80,15 @@ import java.util.ArrayList; @Override public void discardCuesBeforeTimeUs(long timeUs) { int indexToDiscardTo = getIndexOfCuesStartingAfter(timeUs); - if (indexToDiscardTo > 0) { - cuesWithTimingList.subList(0, indexToDiscardTo).clear(); + if (indexToDiscardTo == 0) { + // Either the first cue starts after timeUs, or the cues list is empty. + return; } + CuesWithTiming lastCueToDiscard = cuesWithTimingList.get(indexToDiscardTo - 1); + if (lastCueToDiscard.endTimeUs == C.TIME_UNSET || lastCueToDiscard.endTimeUs >= timeUs) { + indexToDiscardTo--; + } + cuesWithTimingList.subList(0, indexToDiscardTo).clear(); } @Override @@ -142,7 +148,7 @@ import java.util.ArrayList; /** * Returns the index of the first {@link CuesWithTiming} in {@link #cuesWithTimingList} where - * {@link CuesWithTiming#startTimeUs} is strictly less than {@code timeUs}. + * {@link CuesWithTiming#startTimeUs} is strictly greater than {@code timeUs}. * *

Returns the size of {@link #cuesWithTimingList} if all cues are before timeUs */ 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 4a2b41d20f..c5b815328c 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 @@ -217,6 +217,47 @@ public final class ReplacingCuesResolverTest { assertThat(replacingCuesResolver.getNextCueChangeTimeUs(4_999_990)).isEqualTo(6_000_000); } + @Test + public void discardCuesBeforeTimeUs_retainsActiveCueWithSetDuration() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming activeCue = + new CuesWithTiming(FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ 4_000_000); + CuesWithTiming laterCue = + new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 8_000_000, /* durationUs= */ 2_000_000); + replacingCuesResolver.addCues(activeCue, /* currentPositionUs= */ 5_000_000); + replacingCuesResolver.addCues(laterCue, /* currentPositionUs= */ 5_000_000); + + // Discard cues before 5_000_000. activeCue should remain active because it ends at 7_000_000. + replacingCuesResolver.discardCuesBeforeTimeUs(5_000_000); + + // Query at a time within activeCue's range to verify it's still there. + assertThat(replacingCuesResolver.getCuesAtTimeUs(6_000_000)).isEqualTo(FIRST_CUES); + // Ensure that laterCue is unaffected. + assertThat(replacingCuesResolver.getCuesAtTimeUs(9_000_000)).isEqualTo(SECOND_CUES); + } + + @Test + public void discardCuesBeforeTimeUs_retainsActiveCueWithUnsetDuration() { + ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver(); + CuesWithTiming activeCue = + new CuesWithTiming( + FIRST_CUES, /* startTimeUs= */ 3_000_000, /* durationUs= */ C.TIME_UNSET); + CuesWithTiming laterCue = + new CuesWithTiming(SECOND_CUES, /* startTimeUs= */ 8_000_000, /* durationUs= */ 2_000_000); + replacingCuesResolver.addCues(activeCue, /* currentPositionUs= */ 5_000_000); + replacingCuesResolver.addCues(laterCue, /* currentPositionUs= */ 5_000_000); + + // Discard cues before 5_000_000. activeCue should remain active because its + // duration is unset, meaning it should remain visible until replaced by a subsequent cue + // starting at 8_000_000. + replacingCuesResolver.discardCuesBeforeTimeUs(5_000_000); + + // Query at a time within activeCue's range to verify it's still there. + assertThat(replacingCuesResolver.getCuesAtTimeUs(6_000_000)).isEqualTo(FIRST_CUES); + // Ensure that laterCue is unaffected. + assertThat(replacingCuesResolver.getCuesAtTimeUs(9_000_000)).isEqualTo(SECOND_CUES); + } + @Test public void clear_clearsAllCues() { ReplacingCuesResolver replacingCuesResolver = new ReplacingCuesResolver();