Skip to content

Commit

Permalink
Fix ReplacingCuesResolver.discardCuesBeforeTimeUs to retain active cue
Browse files Browse the repository at this point in the history
The method previously discarded the cue that was active at `timeUs`,
meaning it had started before but had not ended by `timeUs`.

Issue: #1939

PiperOrigin-RevId: 702707611
(cherry picked from commit e927d7b)
  • Loading branch information
rohitjoins authored and shahdDaghash committed Dec 4, 2024
1 parent 121b79a commit 5d9badc
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 3 deletions.
7 changes: 7 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,15 @@ public ImmutableList<Cue> 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
Expand Down Expand Up @@ -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}.
*
* <p>Returns the size of {@link #cuesWithTimingList} if all cues are before timeUs
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 5d9badc

Please sign in to comment.