From 5d9badcb502aebc57027ba407959344b55731e9d Mon Sep 17 00:00:00 2001 From: rohks Date: Wed, 4 Dec 2024 06:48:43 -0800 Subject: [PATCH] Fix ReplacingCuesResolver.discardCuesBeforeTimeUs to retain active cue The method previously discarded the cue that was active at `timeUs`, meaning it had started before but had not ended by `timeUs`. Issue: androidx/media#1939 PiperOrigin-RevId: 702707611 (cherry picked from commit e927d7b986229a89a43d749a0b51cb9d53722d26) --- RELEASENOTES.md | 7 ++++ .../exoplayer/text/ReplacingCuesResolver.java | 12 ++++-- .../text/ReplacingCuesResolverTest.java | 41 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 1ce272571ae..1a254a80726 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -38,6 +38,13 @@ when provided while processing `onOutputFormatChanged` ([#1371](https://github.com/androidx/media/pull/1371)). * Text: + * Stop eagerly loading all subtitle files configured with + `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 d8e957621f2..d09557f349b 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 @@ public ImmutableList getCuesAtTimeUs(long timeUs) { @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 @@ public void clear() { /** * 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 4a2b41d20f0..c5b815328cd 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 void discardCuesBeforeTimeUs() { 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();