diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0c307749d0..f990b11ef8 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -109,6 +109,8 @@ seconds ([#7181](https://github.com/google/ExoPlayer/issues/7181)). * Add special-case positioning behaviour for vertical cues being rendered horizontally. + * Implement steps 4-10 of the + [WebVTT line computation algorithm](https://www.w3.org/TR/webvtt1/#cue-computed-line). * DRM: * Add support for attaching DRM sessions to clear content in the demo app. * Remove `DrmSessionManager` references from all renderers. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitle.java b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitle.java index 620e1ef491..6832033165 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitle.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitle.java @@ -15,7 +15,6 @@ */ package com.google.android.exoplayer2.text.webvtt; -import android.text.SpannableStringBuilder; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.text.Cue; import com.google.android.exoplayer2.text.Subtitle; @@ -23,6 +22,7 @@ import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; /** @@ -30,17 +30,16 @@ import java.util.List; */ /* package */ final class WebvttSubtitle implements Subtitle { - private final List cues; + private final List cueInfos; private final long[] cueTimesUs; private final long[] sortedCueTimesUs; /** Constructs a new WebvttSubtitle from a list of {@link WebvttCueInfo}s. */ public WebvttSubtitle(List cueInfos) { - this.cues = new ArrayList<>(cueInfos.size()); + this.cueInfos = Collections.unmodifiableList(new ArrayList<>(cueInfos)); cueTimesUs = new long[2 * cueInfos.size()]; for (int cueIndex = 0; cueIndex < cueInfos.size(); cueIndex++) { WebvttCueInfo cueInfo = cueInfos.get(cueIndex); - this.cues.add(cueInfo.cue); int arrayIndex = cueIndex * 2; cueTimesUs[arrayIndex] = cueInfo.startTimeUs; cueTimesUs[arrayIndex + 1] = cueInfo.endTimeUs; @@ -69,53 +68,34 @@ import java.util.List; @Override public List getCues(long timeUs) { - List list = new ArrayList<>(); - Cue firstNormalCue = null; - SpannableStringBuilder normalCueTextBuilder = null; - - for (int i = 0; i < cues.size(); i++) { + List currentCues = new ArrayList<>(); + List cuesWithUnsetLine = new ArrayList<>(); + for (int i = 0; i < cueInfos.size(); i++) { if ((cueTimesUs[i * 2] <= timeUs) && (timeUs < cueTimesUs[i * 2 + 1])) { - Cue cue = cues.get(i); - // TODO(ibaker): Replace this with a closer implementation of the WebVTT spec (keeping - // individual cues, but tweaking their `line` value): - // https://www.w3.org/TR/webvtt1/#cue-computed-line - if (isNormal(cue)) { - // We want to merge all of the normal cues into a single cue to ensure they are drawn - // correctly (i.e. don't overlap) and to emulate roll-up, but only if there are multiple - // normal cues, otherwise we can just append the single normal cue. - if (firstNormalCue == null) { - firstNormalCue = cue; - } else if (normalCueTextBuilder == null) { - normalCueTextBuilder = new SpannableStringBuilder(); - normalCueTextBuilder - .append(Assertions.checkNotNull(firstNormalCue.text)) - .append("\n") - .append(Assertions.checkNotNull(cue.text)); - } else { - normalCueTextBuilder.append("\n").append(Assertions.checkNotNull(cue.text)); - } + WebvttCueInfo cueInfo = cueInfos.get(i); + if (cueInfo.cue.line == Cue.DIMEN_UNSET) { + cuesWithUnsetLine.add(cueInfo); } else { - list.add(cue); + currentCues.add(cueInfo.cue); } } } - if (normalCueTextBuilder != null) { - // There were multiple normal cues, so create a new cue with all of the text. - list.add(WebvttCueParser.newCueForText(normalCueTextBuilder)); - } else if (firstNormalCue != null) { - // There was only a single normal cue, so just add it to the list. - list.add(firstNormalCue); + // Steps 4 - 10 of https://www.w3.org/TR/webvtt1/#cue-computed-line + // (steps 1 - 3 are handled by WebvttCueParser#computeLine(float, int)) + Collections.sort(cuesWithUnsetLine, (c1, c2) -> Long.compare(c1.startTimeUs, c2.startTimeUs)); + for (int i = 0; i < cuesWithUnsetLine.size(); i++) { + Cue cue = cuesWithUnsetLine.get(i).cue; + currentCues.add( + cue.buildUpon() + .setLine((float) (-1 - i), Cue.LINE_TYPE_NUMBER) + // WebVTT doesn't use 'line alignment' (i.e. Cue#lineAnchor) when computing position + // with snap-to-lines=true (i.e. Cue#LINE_TYPE_NUMBER) but Cue does use lineAnchor + // when describing how numeric cues should be displayed. So we have to manually set + // lineAnchor=ANCHOR_TYPE_END to avoid the bottom line of cues being off the screen. + // https://www.w3.org/TR/webvtt1/#processing-cue-settings + .setLineAnchor(Cue.ANCHOR_TYPE_END) + .build()); } - return list; - } - - /** - * Returns whether or not this cue should be placed in the default position and rolled-up with the - * other "normal" cues. - * - * @return Whether this cue should be placed in the default position. - */ - private static boolean isNormal(Cue cue) { - return (cue.line == Cue.DIMEN_UNSET && cue.position == WebvttCueParser.DEFAULT_POSITION); + return currentCues; } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java index 4bd26b8da2..3de75a249f 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java @@ -31,6 +31,7 @@ import com.google.android.exoplayer2.util.ColorParser; import com.google.common.collect.Iterables; import com.google.common.truth.Expect; import java.io.IOException; +import java.util.List; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -44,6 +45,8 @@ public class WebvttDecoderTest { private static final String TYPICAL_WITH_IDS_FILE = "webvtt/typical_with_identifiers"; private static final String TYPICAL_WITH_COMMENTS_FILE = "webvtt/typical_with_comments"; private static final String WITH_POSITIONING_FILE = "webvtt/with_positioning"; + private static final String WITH_OVERLAPPING_TIMESTAMPS_FILE = + "webvtt/with_overlapping_timestamps"; private static final String WITH_VERTICAL_FILE = "webvtt/with_vertical"; private static final String WITH_BAD_CUE_HEADER_FILE = "webvtt/with_bad_cue_header"; private static final String WITH_TAGS_FILE = "webvtt/with_tags"; @@ -194,9 +197,14 @@ public class WebvttDecoderTest { assertThat(firstCue.positionAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); assertThat(firstCue.textAlignment).isEqualTo(Alignment.ALIGN_NORMAL); assertThat(firstCue.size).isEqualTo(0.35f); + // Unspecified values should use WebVTT defaults - assertThat(firstCue.line).isEqualTo(Cue.DIMEN_UNSET); + assertThat(firstCue.line).isEqualTo(-1f); assertThat(firstCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + // WebVTT specifies START as the default, but it doesn't expect this to be used if + // lineType=NUMBER so we have to override it to END in this case, otherwise the Cue will be + // displayed off the bottom of the screen. + assertThat(firstCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); assertThat(firstCue.verticalType).isEqualTo(Cue.TYPE_UNSET); assertThat(subtitle.getEventTime(2)).isEqualTo(2_345_000L); @@ -260,6 +268,58 @@ public class WebvttDecoderTest { assertThat(eighthCue.positionAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); } + @Test + public void decodeWithOverlappingTimestamps() throws Exception { + WebvttSubtitle subtitle = getSubtitleForTestAsset(WITH_OVERLAPPING_TIMESTAMPS_FILE); + + assertThat(subtitle.getEventTimeCount()).isEqualTo(8); + + Cue firstCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(0))); + assertThat(firstCue.text.toString()).isEqualTo("Displayed at the bottom for 3 seconds."); + assertThat(firstCue.line).isEqualTo(-1f); + assertThat(firstCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(firstCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + + List firstAndSecondCue = subtitle.getCues(subtitle.getEventTime(1)); + assertThat(firstAndSecondCue).hasSize(2); + assertThat(firstAndSecondCue.get(0).text.toString()) + .isEqualTo("Displayed at the bottom for 3 seconds."); + assertThat(firstAndSecondCue.get(0).line).isEqualTo(-1f); + assertThat(firstAndSecondCue.get(0).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(firstAndSecondCue.get(0).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + assertThat(firstAndSecondCue.get(1).text.toString()) + .isEqualTo("Appears directly above for 1 second."); + assertThat(firstAndSecondCue.get(1).line).isEqualTo(-2f); + assertThat(firstAndSecondCue.get(1).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(firstAndSecondCue.get(1).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + + Cue thirdCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(4))); + assertThat(thirdCue.text.toString()).isEqualTo("Displayed at the bottom for 2 seconds."); + assertThat(thirdCue.line).isEqualTo(-1f); + assertThat(thirdCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(thirdCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + + List thirdAndFourthCue = subtitle.getCues(subtitle.getEventTime(5)); + assertThat(thirdAndFourthCue).hasSize(2); + assertThat(thirdAndFourthCue.get(0).text.toString()) + .isEqualTo("Displayed at the bottom for 2 seconds."); + assertThat(thirdAndFourthCue.get(0).line).isEqualTo(-1f); + assertThat(thirdAndFourthCue.get(0).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(thirdAndFourthCue.get(0).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + assertThat(thirdAndFourthCue.get(1).text.toString()) + .isEqualTo("Appears directly above the previous cue, then replaces it after 1 second."); + assertThat(thirdAndFourthCue.get(1).line).isEqualTo(-2f); + assertThat(thirdAndFourthCue.get(1).lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(thirdAndFourthCue.get(1).lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + + Cue fourthCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(6))); + assertThat(fourthCue.text.toString()) + .isEqualTo("Appears directly above the previous cue, then replaces it after 1 second."); + assertThat(fourthCue.line).isEqualTo(-1f); + assertThat(fourthCue.lineType).isEqualTo(Cue.LINE_TYPE_NUMBER); + assertThat(fourthCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_END); + } + @Test public void decodeWithVertical() throws Exception { WebvttSubtitle subtitle = getSubtitleForTestAsset(WITH_VERTICAL_FILE); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitleTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitleTest.java index af44120bc1..bf5c01ce3e 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitleTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttSubtitleTest.java @@ -21,6 +21,7 @@ import static java.lang.Long.MAX_VALUE; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.text.Cue; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -33,8 +34,6 @@ public class WebvttSubtitleTest { private static final String FIRST_SUBTITLE_STRING = "This is the first subtitle."; private static final String SECOND_SUBTITLE_STRING = "This is the second subtitle."; - private static final String FIRST_AND_SECOND_SUBTITLE_STRING = - FIRST_SUBTITLE_STRING + "\n" + SECOND_SUBTITLE_STRING; private static final WebvttSubtitle emptySubtitle = new WebvttSubtitle(Collections.emptyList()); @@ -143,12 +142,12 @@ public class WebvttSubtitleTest { assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, overlappingSubtitle.getCues(1_999_999)); // Test after first and second subtitle - assertSingleCueTextEquals( - FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_000_000)); - assertSingleCueTextEquals( - FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_500_000)); - assertSingleCueTextEquals( - FIRST_AND_SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(2_999_999)); + assertThat(getCueTexts(overlappingSubtitle.getCues(2_000_000))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); + assertThat(getCueTexts(overlappingSubtitle.getCues(2_500_000))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); + assertThat(getCueTexts(overlappingSubtitle.getCues(2_999_999))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); // Test second subtitle assertSingleCueTextEquals(SECOND_SUBTITLE_STRING, overlappingSubtitle.getCues(3_000_000)); @@ -184,9 +183,12 @@ public class WebvttSubtitleTest { assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, nestedSubtitle.getCues(1_999_999)); // Test after first and second subtitle - assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_000_000)); - assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_500_000)); - assertSingleCueTextEquals(FIRST_AND_SECOND_SUBTITLE_STRING, nestedSubtitle.getCues(2_999_999)); + assertThat(getCueTexts(nestedSubtitle.getCues(2_000_000))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); + assertThat(getCueTexts(nestedSubtitle.getCues(2_500_000))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); + assertThat(getCueTexts(nestedSubtitle.getCues(2_999_999))) + .containsExactly(FIRST_SUBTITLE_STRING, SECOND_SUBTITLE_STRING); // Test first subtitle assertSingleCueTextEquals(FIRST_SUBTITLE_STRING, nestedSubtitle.getCues(3_000_000)); @@ -241,4 +243,12 @@ public class WebvttSubtitleTest { assertThat(cues).hasSize(1); assertThat(cues.get(0).text.toString()).isEqualTo(expected); } + + private static List getCueTexts(List cues) { + List cueTexts = new ArrayList<>(); + for (int i = 0; i < cues.size(); i++) { + cueTexts.add(cues.get(i).text.toString()); + } + return cueTexts; + } } diff --git a/testdata/src/test/assets/webvtt/with_overlapping_timestamps b/testdata/src/test/assets/webvtt/with_overlapping_timestamps new file mode 100644 index 0000000000..088dc8b941 --- /dev/null +++ b/testdata/src/test/assets/webvtt/with_overlapping_timestamps @@ -0,0 +1,13 @@ +WEBVTT + +00:00:00.000 --> 00:00:03.000 +Displayed at the bottom for 3 seconds. + +00:01.000 --> 00:02.000 +Appears directly above for 1 second. + +00:04.000 --> 00:06.000 +Displayed at the bottom for 2 seconds. + +00:05.000 --> 00:07.000 +Appears directly above the previous cue, then replaces it after 1 second.