Return true from CuesResolver.addCues if the output changed

This belongs in the resolver, because it depends on the resolution
algorithm (and therefore the logic can't live in `TextRenderer`).

This also fixes a bug in `TextRenderer` where we were doing arithmetic
with `cues.durationUs` without checking if it was `TIME_UNSET` first.

#minor-release

PiperOrigin-RevId: 571332750
(cherry picked from commit 272428734b)
This commit is contained in:
ibaker 2023-10-06 07:31:29 -07:00 committed by Rohit Singh
parent 6ad54d72ad
commit 1818d46077
6 changed files with 173 additions and 40 deletions

View file

@ -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}.

View file

@ -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

View file

@ -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

View file

@ -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:

View file

@ -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();

View file

@ -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();