From b3f94e3768577abfa365fcea88cbd04f5f2a1827 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 30 Oct 2019 16:39:14 +0000 Subject: [PATCH] Update WebvttCue to use defaults from the spec Cue's UNSET and null values should be used when the source data doesn't specify *and* the spec doesn't provide a clear default. In the WebVTT case, the defaults are clear, so we use them explicitly. PiperOrigin-RevId: 277522899 --- .../exoplayer2/text/webvtt/WebvttCue.java | 197 ++++++++++++++---- .../text/webvtt/WebvttCueParser.java | 28 ++- .../text/webvtt/WebvttSubtitle.java | 3 + .../src/test/assets/webvtt/with_positioning | 5 +- .../text/webvtt/Mp4WebvttDecoderTest.java | 11 +- .../text/webvtt/WebvttDecoderTest.java | 50 ++--- 6 files changed, 206 insertions(+), 88 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCue.java b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCue.java index 8b382ca3a5..2442302373 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCue.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCue.java @@ -15,15 +15,23 @@ */ package com.google.android.exoplayer2.text.webvtt; +import static java.lang.annotation.RetentionPolicy.SOURCE; + import android.text.Layout.Alignment; +import androidx.annotation.IntDef; +import androidx.annotation.Nullable; import com.google.android.exoplayer2.text.Cue; import com.google.android.exoplayer2.util.Log; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; /** * A representation of a WebVTT cue. */ public final class WebvttCue extends Cue { + private static final float DEFAULT_POSITION = 0.5f; + public final long startTime; public final long endTime; @@ -50,26 +58,73 @@ public final class WebvttCue extends Cue { * @return Whether this cue should be placed in the default position. */ public boolean isNormalCue() { - return (line == DIMEN_UNSET && position == DIMEN_UNSET); + return (line == DIMEN_UNSET && position == DEFAULT_POSITION); } - /** - * Builder for WebVTT cues. - */ + /** Builder for WebVTT cues. */ @SuppressWarnings("hiding") public static class Builder { + /** + * Valid values for {@link #setTextAlignment(int)}. + * + *

We use a custom list (and not {@link Alignment} directly) in order to include both {@code + * START}/{@code LEFT} and {@code END}/{@code RIGHT}. The distinction is important for {@link + * #derivePosition(int)}. + * + *

These correspond to the valid values for the 'align' cue setting in the WebVTT spec. + */ + @Documented + @Retention(SOURCE) + @IntDef({ + TextAlignment.START, + TextAlignment.CENTER, + TextAlignment.END, + TextAlignment.LEFT, + TextAlignment.RIGHT + }) + public @interface TextAlignment { + /** + * See WebVTT's align:start. + */ + int START = 1; + /** + * See WebVTT's align:center. + */ + int CENTER = 2; + /** + * See WebVTT's align:end. + */ + int END = 3; + /** + * See WebVTT's align:left. + */ + int LEFT = 4; + /** + * See WebVTT's align:right. + */ + int RIGHT = 5; + } + private static final String TAG = "WebvttCueBuilder"; private long startTime; private long endTime; - private CharSequence text; - private Alignment textAlignment; + @Nullable private CharSequence text; + @TextAlignment private int textAlignment; private float line; - private int lineType; - private int lineAnchor; + // Equivalent to WebVTT's snap-to-lines flag: + // https://www.w3.org/TR/webvtt1/#webvtt-cue-snap-to-lines-flag + @LineType private int lineType; + @AnchorType private int lineAnchor; private float position; - private int positionAnchor; + @AnchorType private int positionAnchor; private float width; // Initialization methods @@ -82,23 +137,43 @@ public final class WebvttCue extends Cue { startTime = 0; endTime = 0; text = null; - textAlignment = null; + // Default: https://www.w3.org/TR/webvtt1/#webvtt-cue-text-alignment + textAlignment = TextAlignment.CENTER; line = Cue.DIMEN_UNSET; - lineType = Cue.TYPE_UNSET; - lineAnchor = Cue.TYPE_UNSET; + // Defaults to NUMBER (true): https://www.w3.org/TR/webvtt1/#webvtt-cue-snap-to-lines-flag + lineType = Cue.LINE_TYPE_NUMBER; + // Default: https://www.w3.org/TR/webvtt1/#webvtt-cue-line-alignment + lineAnchor = Cue.ANCHOR_TYPE_START; position = Cue.DIMEN_UNSET; positionAnchor = Cue.TYPE_UNSET; - width = Cue.DIMEN_UNSET; + // Default: https://www.w3.org/TR/webvtt1/#webvtt-cue-size + width = 1.0f; } // Construction methods. public WebvttCue build() { - if (position != Cue.DIMEN_UNSET && positionAnchor == Cue.TYPE_UNSET) { - derivePositionAnchorFromAlignment(); + line = computeLine(line, lineType); + + if (position == Cue.DIMEN_UNSET) { + position = derivePosition(textAlignment); } - return new WebvttCue(startTime, endTime, text, textAlignment, line, lineType, lineAnchor, - position, positionAnchor, width); + + if (positionAnchor == Cue.TYPE_UNSET) { + positionAnchor = derivePositionAnchor(textAlignment); + } + + return new WebvttCue( + startTime, + endTime, + text, + convertTextAlignment(textAlignment), + line, + lineType, + lineAnchor, + position, + positionAnchor, + width); } public Builder setStartTime(long time) { @@ -116,7 +191,7 @@ public final class WebvttCue extends Cue { return this; } - public Builder setTextAlignment(Alignment textAlignment) { + public Builder setTextAlignment(@TextAlignment int textAlignment) { this.textAlignment = textAlignment; return this; } @@ -126,12 +201,12 @@ public final class WebvttCue extends Cue { return this; } - public Builder setLineType(int lineType) { + public Builder setLineType(@LineType int lineType) { this.lineType = lineType; return this; } - public Builder setLineAnchor(int lineAnchor) { + public Builder setLineAnchor(@AnchorType int lineAnchor) { this.lineAnchor = lineAnchor; return this; } @@ -141,7 +216,7 @@ public final class WebvttCue extends Cue { return this; } - public Builder setPositionAnchor(int positionAnchor) { + public Builder setPositionAnchor(@AnchorType int positionAnchor) { this.positionAnchor = positionAnchor; return this; } @@ -151,29 +226,69 @@ public final class WebvttCue extends Cue { return this; } - private Builder derivePositionAnchorFromAlignment() { - if (textAlignment == null) { - positionAnchor = Cue.TYPE_UNSET; + // https://www.w3.org/TR/webvtt1/#webvtt-cue-line + private static float computeLine(float line, @LineType int lineType) { + if (line != Cue.DIMEN_UNSET + && lineType == Cue.LINE_TYPE_FRACTION + && (line < 0.0f || line > 1.0f)) { + return 1.0f; // Step 1 + } else if (line != Cue.DIMEN_UNSET) { + // Step 2: Do nothing, line is already correct. + return line; + } else if (lineType == Cue.LINE_TYPE_FRACTION) { + return 1.0f; // Step 3 } else { - switch (textAlignment) { - case ALIGN_NORMAL: - positionAnchor = Cue.ANCHOR_TYPE_START; - break; - case ALIGN_CENTER: - positionAnchor = Cue.ANCHOR_TYPE_MIDDLE; - break; - case ALIGN_OPPOSITE: - positionAnchor = Cue.ANCHOR_TYPE_END; - break; - default: - Log.w(TAG, "Unrecognized alignment: " + textAlignment); - positionAnchor = Cue.ANCHOR_TYPE_START; - break; - } + // Steps 4 - 10 (stacking multiple simultaneous cues) are handled by WebvttSubtitle#getCues + // and WebvttCue#isNormalCue. + return DIMEN_UNSET; } - return this; } - } + // https://www.w3.org/TR/webvtt1/#webvtt-cue-position + private static float derivePosition(@TextAlignment int textAlignment) { + switch (textAlignment) { + case TextAlignment.LEFT: + return 0.0f; + case TextAlignment.RIGHT: + return 1.0f; + case TextAlignment.START: + case TextAlignment.CENTER: + case TextAlignment.END: + default: + return DEFAULT_POSITION; + } + } + // https://www.w3.org/TR/webvtt1/#webvtt-cue-position-alignment + @AnchorType + private static int derivePositionAnchor(@TextAlignment int textAlignment) { + switch (textAlignment) { + case TextAlignment.LEFT: + case TextAlignment.START: + return Cue.ANCHOR_TYPE_START; + case TextAlignment.RIGHT: + case TextAlignment.END: + return Cue.ANCHOR_TYPE_END; + case TextAlignment.CENTER: + default: + return Cue.ANCHOR_TYPE_MIDDLE; + } + } + + private static Alignment convertTextAlignment(@TextAlignment int textAlignment) { + switch (textAlignment) { + case TextAlignment.START: + case TextAlignment.LEFT: + return Alignment.ALIGN_NORMAL; + case TextAlignment.CENTER: + return Alignment.ALIGN_CENTER; + case TextAlignment.END: + case TextAlignment.RIGHT: + return Alignment.ALIGN_OPPOSITE; + default: + Log.w(TAG, "Unknown textAlignment: " + textAlignment); + return null; + } + } + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java index c233fda13f..d8215e9847 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java @@ -16,7 +16,6 @@ package com.google.android.exoplayer2.text.webvtt; import android.graphics.Typeface; -import android.text.Layout.Alignment; import android.text.Spannable; import android.text.SpannableStringBuilder; import android.text.Spanned; @@ -87,7 +86,7 @@ public final class WebvttCueParser { * Parses the next valid WebVTT cue in a parsable array, including timestamps, settings and text. * * @param webvttData Parsable WebVTT file data. - * @param builder Builder for WebVTT Cues. + * @param builder Builder for WebVTT Cues (output parameter). * @param styles List of styles defined by the CSS style blocks preceding the cues. * @return Whether a valid Cue was found. */ @@ -252,14 +251,11 @@ public final class WebvttCueParser { // Internal methods - private static void parseLineAttribute(String s, WebvttCue.Builder builder) - throws NumberFormatException { + private static void parseLineAttribute(String s, WebvttCue.Builder builder) { int commaIndex = s.indexOf(','); if (commaIndex != -1) { builder.setLineAnchor(parsePositionAnchor(s.substring(commaIndex + 1))); s = s.substring(0, commaIndex); - } else { - builder.setLineAnchor(Cue.TYPE_UNSET); } if (s.endsWith("%")) { builder.setLine(WebvttParserUtil.parsePercentage(s)).setLineType(Cue.LINE_TYPE_FRACTION); @@ -274,18 +270,16 @@ public final class WebvttCueParser { } } - private static void parsePositionAttribute(String s, WebvttCue.Builder builder) - throws NumberFormatException { + private static void parsePositionAttribute(String s, WebvttCue.Builder builder) { int commaIndex = s.indexOf(','); if (commaIndex != -1) { builder.setPositionAnchor(parsePositionAnchor(s.substring(commaIndex + 1))); s = s.substring(0, commaIndex); - } else { - builder.setPositionAnchor(Cue.TYPE_UNSET); } builder.setPosition(WebvttParserUtil.parsePercentage(s)); } + @Cue.AnchorType private static int parsePositionAnchor(String s) { switch (s) { case "start": @@ -301,20 +295,24 @@ public final class WebvttCueParser { } } - private static Alignment parseTextAlignment(String s) { + @WebvttCue.Builder.TextAlignment + private static int parseTextAlignment(String s) { switch (s) { case "start": + return WebvttCue.Builder.TextAlignment.START; case "left": - return Alignment.ALIGN_NORMAL; + return WebvttCue.Builder.TextAlignment.LEFT; case "center": case "middle": - return Alignment.ALIGN_CENTER; + return WebvttCue.Builder.TextAlignment.CENTER; case "end": + return WebvttCue.Builder.TextAlignment.END; case "right": - return Alignment.ALIGN_OPPOSITE; + return WebvttCue.Builder.TextAlignment.RIGHT; default: Log.w(TAG, "Invalid alignment value: " + s); - return null; + // Default value: https://www.w3.org/TR/webvtt1/#webvtt-cue-text-alignment + return WebvttCue.Builder.TextAlignment.CENTER; } } 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 b7ce477894..5d3339c85a 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 @@ -83,6 +83,9 @@ import java.util.List; list = new ArrayList<>(); } WebvttCue 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 (cue.isNormalCue()) { // 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 diff --git a/library/core/src/test/assets/webvtt/with_positioning b/library/core/src/test/assets/webvtt/with_positioning index 06a81df901..6bb86b7c93 100644 --- a/library/core/src/test/assets/webvtt/with_positioning +++ b/library/core/src/test/assets/webvtt/with_positioning @@ -21,13 +21,12 @@ NOTE Line as absolute negative number and without line alignment. 00:06.000 --> 00:07.000 line:-10 align:middle This is the fourth subtitle. -NOTE The positioning alignment should be inherited from align. +NOTE The position and positioning alignment should be inherited from align. -00:07.000 --> 00:08.000 position:10% align:end size:10% +00:07.000 --> 00:08.000 align:right This is the fifth subtitle. NOTE In newer drafts, align:middle has been replaced by align:center 00:10.000 --> 00:11.000 line:45%,end align:center size:35% This is the sixth subtitle. - diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/Mp4WebvttDecoderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/Mp4WebvttDecoderTest.java index f930c4590f..341c3eb41b 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/Mp4WebvttDecoderTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/Mp4WebvttDecoderTest.java @@ -87,7 +87,8 @@ public final class Mp4WebvttDecoderTest { public void testSingleCueSample() throws SubtitleDecoderException { Mp4WebvttDecoder decoder = new Mp4WebvttDecoder(); Subtitle result = decoder.decode(SINGLE_CUE_SAMPLE, SINGLE_CUE_SAMPLE.length, false); - Cue expectedCue = new Cue("Hello World"); // Line feed must be trimmed by the decoder + // Line feed must be trimmed by the decoder + Cue expectedCue = new WebvttCue.Builder().setText("Hello World").build(); assertMp4WebvttSubtitleEquals(result, expectedCue); } @@ -95,8 +96,8 @@ public final class Mp4WebvttDecoderTest { public void testTwoCuesSample() throws SubtitleDecoderException { Mp4WebvttDecoder decoder = new Mp4WebvttDecoder(); Subtitle result = decoder.decode(DOUBLE_CUE_SAMPLE, DOUBLE_CUE_SAMPLE.length, false); - Cue firstExpectedCue = new Cue("Hello World"); - Cue secondExpectedCue = new Cue("Bye Bye"); + Cue firstExpectedCue = new WebvttCue.Builder().setText("Hello World").build(); + Cue secondExpectedCue = new WebvttCue.Builder().setText("Bye Bye").build(); assertMp4WebvttSubtitleEquals(result, firstExpectedCue, secondExpectedCue); } @@ -104,7 +105,9 @@ public final class Mp4WebvttDecoderTest { public void testNoCueSample() throws SubtitleDecoderException { Mp4WebvttDecoder decoder = new Mp4WebvttDecoder(); Subtitle result = decoder.decode(NO_CUE_SAMPLE, NO_CUE_SAMPLE.length, false); - assertMp4WebvttSubtitleEquals(result); + assertThat(result.getEventTimeCount()).isEqualTo(1); + assertThat(result.getEventTime(0)).isEqualTo(0); + assertThat(result.getCues(0)).isEmpty(); } // Negative tests. 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 49e30d8be0..a879a38b8d 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 @@ -224,8 +224,8 @@ public class WebvttDecoderTest { "This is the first subtitle.", Alignment.ALIGN_NORMAL, /* line= */ Cue.DIMEN_UNSET, - /* lineType= */ Cue.TYPE_UNSET, - /* lineAnchor= */ Cue.TYPE_UNSET, + /* lineType= */ Cue.LINE_TYPE_NUMBER, + /* lineAnchor= */ Cue.ANCHOR_TYPE_START, /* position= */ 0.1f, /* positionAnchor= */ Cue.ANCHOR_TYPE_START, /* size= */ 0.35f); @@ -237,10 +237,10 @@ public class WebvttDecoderTest { "This is the second subtitle.", Alignment.ALIGN_OPPOSITE, /* line= */ Cue.DIMEN_UNSET, - /* lineType= */ Cue.TYPE_UNSET, - /* lineAnchor= */ Cue.TYPE_UNSET, - /* position= */ Cue.DIMEN_UNSET, - /* positionAnchor= */ Cue.TYPE_UNSET, + /* lineType= */ Cue.LINE_TYPE_NUMBER, + /* lineAnchor= */ Cue.ANCHOR_TYPE_START, + /* position= */ 0.5f, + /* positionAnchor= */ Cue.ANCHOR_TYPE_END, /* size= */ 0.35f); assertCue( subtitle, @@ -252,8 +252,8 @@ public class WebvttDecoderTest { /* line= */ 0.45f, /* lineType= */ Cue.LINE_TYPE_FRACTION, /* lineAnchor= */ Cue.ANCHOR_TYPE_END, - /* position= */ Cue.DIMEN_UNSET, - /* positionAnchor= */ Cue.TYPE_UNSET, + /* position= */ 0.5f, + /* positionAnchor= */ Cue.ANCHOR_TYPE_MIDDLE, /* size= */ 0.35f); assertCue( subtitle, @@ -262,12 +262,12 @@ public class WebvttDecoderTest { /* endTimeUs= */ 7000000, "This is the fourth subtitle.", Alignment.ALIGN_CENTER, - /* line= */ -11f, + /* line= */ -11.0f, /* lineType= */ Cue.LINE_TYPE_NUMBER, - /* lineAnchor= */ Cue.TYPE_UNSET, - /* position= */ Cue.DIMEN_UNSET, - /* positionAnchor= */ Cue.TYPE_UNSET, - /* size= */ Cue.DIMEN_UNSET); + /* lineAnchor= */ Cue.ANCHOR_TYPE_START, + /* position= */ 0.5f, + /* positionAnchor= */ Cue.ANCHOR_TYPE_MIDDLE, + /* size= */ 1.0f); assertCue( subtitle, /* eventTimeIndex= */ 8, @@ -276,11 +276,11 @@ public class WebvttDecoderTest { "This is the fifth subtitle.", Alignment.ALIGN_OPPOSITE, /* line= */ Cue.DIMEN_UNSET, - /* lineType= */ Cue.TYPE_UNSET, - /* lineAnchor= */ Cue.TYPE_UNSET, - /* position= */ 0.1f, + /* lineType= */ Cue.LINE_TYPE_NUMBER, + /* lineAnchor= */ Cue.ANCHOR_TYPE_START, + /* position= */ 1.0f, /* positionAnchor= */ Cue.ANCHOR_TYPE_END, - /* size= */ 0.1f); + /* size= */ 1.0f); assertCue( subtitle, /* eventTimeIndex= */ 10, @@ -291,8 +291,8 @@ public class WebvttDecoderTest { /* line= */ 0.45f, /* lineType= */ Cue.LINE_TYPE_FRACTION, /* lineAnchor= */ Cue.ANCHOR_TYPE_END, - /* position= */ Cue.DIMEN_UNSET, - /* positionAnchor= */ Cue.TYPE_UNSET, + /* position= */ 0.5f, + /* positionAnchor= */ Cue.ANCHOR_TYPE_MIDDLE, /* size= */ 0.35f); } @@ -412,13 +412,13 @@ public class WebvttDecoderTest { startTimeUs, endTimeUs, text, - /* textAlignment= */ null, + /* textAlignment= */ Alignment.ALIGN_CENTER, /* line= */ Cue.DIMEN_UNSET, - /* lineType= */ Cue.TYPE_UNSET, - /* lineAnchor= */ Cue.TYPE_UNSET, - /* position= */ Cue.DIMEN_UNSET, - /* positionAnchor= */ Cue.TYPE_UNSET, - /* size= */ Cue.DIMEN_UNSET); + /* lineType= */ Cue.LINE_TYPE_NUMBER, + /* lineAnchor= */ Cue.ANCHOR_TYPE_START, + /* position= */ 0.5f, + /* positionAnchor= */ Cue.ANCHOR_TYPE_MIDDLE, + /* size= */ 1.0f); } private static void assertCue(