From 06c17f51522c21ee753f2e1f26163a6595c4bb13 Mon Sep 17 00:00:00 2001 From: ibaker Date: Wed, 24 Jun 2020 11:19:30 +0100 Subject: [PATCH] Redefine numeric Cue.line in terms of viewport lines, ignore lineAnchor Numerical lines conceptually map to a grid of lines in the viewport, with the Cue text lines being aligned to one of the viewport lines. It doesn't make sense to position a single-line cue differently based on lineAnchor when it's expected to 'snap' to a particular line on the viewport grid. So we redefine the position to be in terms of the cue lines rather than the bounds of the cue box. It's also not possible to always handle ANCHOR_TYPE_MIDDLE when lineType=NUMBER (as it relies on the number of lines in the cue being odd), so it's easier to ignore lineAnchor completely. PiperOrigin-RevId: 318034664 --- RELEASENOTES.md | 2 + demos/main/src/main/assets/media.exolist.json | 7 +++ .../google/android/exoplayer2/text/Cue.java | 60 +++++++++---------- .../exoplayer2/text/cea/Cea608Decoder.java | 5 +- .../text/webvtt/WebvttCueParser.java | 8 +-- .../text/webvtt/WebvttSubtitle.java | 11 +--- .../text/webvtt/WebvttDecoderTest.java | 13 +--- .../exoplayer2/ui/SubtitlePainter.java | 17 +++--- .../exoplayer2/ui/WebViewSubtitleOutput.java | 16 ++--- 9 files changed, 58 insertions(+), 81 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 76aee2f7f3..1b3ac65ec8 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -133,6 +133,8 @@ * Add support for WebVTT's `ruby-position` CSS property. * Fix positioning for CEA-608 roll-up captions in the top half of screen ([#7475](https://github.com/google/ExoPlayer/issues/7475)). + * Redefine `Cue.lineType=LINE_TYPE_NUMBER` in terms of aligning the cue + text lines to grid of viewport lines, and ignore `Cue.lineAnchor`. * DRM: * Add support for attaching DRM sessions to clear content in the demo app. * Remove `DrmSessionManager` references from all renderers. diff --git a/demos/main/src/main/assets/media.exolist.json b/demos/main/src/main/assets/media.exolist.json index a1ea669f0e..9bdd697394 100644 --- a/demos/main/src/main/assets/media.exolist.json +++ b/demos/main/src/main/assets/media.exolist.json @@ -525,6 +525,13 @@ "subtitle_mime_type": "application/ttml+xml", "subtitle_language": "en" }, + { + "name": "WebVTT line positioning", + "uri": "https://html5demos.com/assets/dizzy.mp4", + "subtitle_uri": "https://storage.googleapis.com/exoplayer-test-media-1/webvtt/numeric-lines.vtt", + "subtitle_mime_type": "text/vtt", + "subtitle_language": "en" + }, { "name": "SSA/ASS position & alignment", "uri": "https://storage.googleapis.com/exoplayer-test-media-1/gen-3/screens/dash-vod-single-segment/video-avc-baseline-480.mp4", diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/Cue.java b/library/core/src/main/java/com/google/android/exoplayer2/text/Cue.java index cb3dcbfad9..98ce0dfc93 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/Cue.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/Cue.java @@ -144,10 +144,9 @@ public final class Cue { @Nullable public final Bitmap bitmap; /** - * The position of the {@link #lineAnchor} of the cue box within the viewport in the direction - * orthogonal to the writing direction (determined by {@link #verticalType}), or {@link - * #DIMEN_UNSET}. When set, the interpretation of the value depends on the value of {@link - * #lineType}. + * The position of the cue box within the viewport in the direction orthogonal to the writing + * direction (determined by {@link #verticalType}), or {@link #DIMEN_UNSET}. When set, the + * interpretation of the value depends on the value of {@link #lineType}. * *

The measurement direction depends on {@link #verticalType}: * @@ -167,40 +166,35 @@ public final class Cue { * *

- * - *

Note that it's particularly important to consider the effect of {@link #lineAnchor} when - * using {@link #LINE_TYPE_NUMBER}. - * - *

*/ public final @LineType int lineType; /** - * The cue box anchor positioned by {@link #line}. One of {@link #ANCHOR_TYPE_START}, {@link - * #ANCHOR_TYPE_MIDDLE}, {@link #ANCHOR_TYPE_END} and {@link #TYPE_UNSET}. + * The cue box anchor positioned by {@link #line} when {@link #lineType} is {@link + * #LINE_TYPE_FRACTION}. + * + *

One of: + * + *

* *

For the normal case of horizontal text, {@link #ANCHOR_TYPE_START}, {@link * #ANCHOR_TYPE_MIDDLE} and {@link #ANCHOR_TYPE_END} correspond to the top, middle and bottom of @@ -584,8 +578,8 @@ public final class Cue { } /** - * Sets the position of the {@code lineAnchor} of the cue box within the viewport in the - * direction orthogonal to the writing direction. + * Sets the position of the cue box within the viewport in the direction orthogonal to the + * writing direction. * * @see Cue#line * @see Cue#lineType diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java b/library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java index 1990cde9c2..14b5be0504 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/cea/Cea608Decoder.java @@ -944,17 +944,14 @@ public final class Cea608Decoder extends CeaDecoder { break; } - int lineAnchor; int line; // Note: Row indices are in the range [1-15], Cue.line counts from 0 (top) and -1 (bottom). if (row > (BASE_ROW / 2)) { - lineAnchor = Cue.ANCHOR_TYPE_END; line = row - BASE_ROW; // Two line adjustments. The first is because line indices from the bottom of the window // start from -1 rather than 0. The second is a blank row to act as the safe area. line -= 2; } else { - lineAnchor = Cue.ANCHOR_TYPE_START; // The `row` of roll-up cues positions the bottom line (even for cues shown in the top // half of the screen), so we need to consider the number of rows in this cue. In // non-roll-up, we don't need any further adjustments because we leave the first line @@ -968,7 +965,7 @@ public final class Cea608Decoder extends CeaDecoder { Alignment.ALIGN_NORMAL, line, Cue.LINE_TYPE_NUMBER, - lineAnchor, + /* lineAnchor= */ Cue.TYPE_UNSET, position, positionAnchor, Cue.DIMEN_UNSET); 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 c56fa080a0..8e804968b3 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 @@ -391,13 +391,7 @@ public final class WebvttCueParser { builder.line = WebvttParserUtil.parsePercentage(s); builder.lineType = Cue.LINE_TYPE_FRACTION; } else { - int lineNumber = Integer.parseInt(s); - if (lineNumber < 0) { - // WebVTT defines line -1 as last visible row when lineAnchor is ANCHOR_TYPE_START, where-as - // Cue defines it to be the first row that's not visible. - lineNumber--; - } - builder.line = lineNumber; + builder.line = Integer.parseInt(s); builder.lineType = Cue.LINE_TYPE_NUMBER; } } 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 6832033165..4a8f5a5471 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 @@ -85,16 +85,7 @@ import java.util.List; 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()); + currentCues.add(cue.buildUpon().setLine((float) (-1 - i), Cue.LINE_TYPE_NUMBER).build()); } 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 88de97ae62..7b8c695182 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 @@ -203,10 +203,6 @@ public class WebvttDecoderTest { // Unspecified values should use WebVTT defaults 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); @@ -232,7 +228,7 @@ public class WebvttDecoderTest { assertThat(subtitle.getEventTime(7)).isEqualTo(7_000_000L); Cue fourthCue = Iterables.getOnlyElement(subtitle.getCues(subtitle.getEventTime(6))); assertThat(fourthCue.text.toString()).isEqualTo("This is the fourth subtitle."); - assertThat(fourthCue.line).isEqualTo(-11f); + assertThat(fourthCue.line).isEqualTo(-10f); assertThat(fourthCue.lineAnchor).isEqualTo(Cue.ANCHOR_TYPE_START); assertThat(fourthCue.textAlignment).isEqualTo(Alignment.ALIGN_CENTER); // Derived from `align:middle`: @@ -280,7 +276,6 @@ public class WebvttDecoderTest { 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); @@ -288,18 +283,15 @@ public class WebvttDecoderTest { .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); @@ -307,19 +299,16 @@ public class WebvttDecoderTest { .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 diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java index d3ef1a1a87..fd7c3bffee 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java @@ -337,21 +337,24 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull; int textTop; if (cueLine != Cue.DIMEN_UNSET) { - int anchorPosition; if (cueLineType == Cue.LINE_TYPE_FRACTION) { - anchorPosition = Math.round(parentHeight * cueLine) + parentTop; + int anchorPosition = Math.round(parentHeight * cueLine) + parentTop; + textTop = + cueLineAnchor == Cue.ANCHOR_TYPE_END + ? anchorPosition - textHeight + : cueLineAnchor == Cue.ANCHOR_TYPE_MIDDLE + ? (anchorPosition * 2 - textHeight) / 2 + : anchorPosition; } else { // cueLineType == Cue.LINE_TYPE_NUMBER int firstLineHeight = textLayout.getLineBottom(0) - textLayout.getLineTop(0); if (cueLine >= 0) { - anchorPosition = Math.round(cueLine * firstLineHeight) + parentTop; + textTop = Math.round(cueLine * firstLineHeight) + parentTop; } else { - anchorPosition = Math.round((cueLine + 1) * firstLineHeight) + parentBottom; + textTop = Math.round((cueLine + 1) * firstLineHeight) + parentBottom - textHeight; } } - textTop = cueLineAnchor == Cue.ANCHOR_TYPE_END ? anchorPosition - textHeight - : cueLineAnchor == Cue.ANCHOR_TYPE_MIDDLE ? (anchorPosition * 2 - textHeight) / 2 - : anchorPosition; + if (textTop + textHeight > parentBottom) { textTop = parentBottom - textHeight; } else if (textTop < parentTop) { diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java index ecab29ddff..70ab6bc9c9 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java @@ -180,16 +180,18 @@ import java.util.List; float linePercent; int lineTranslatePercent; - @Cue.AnchorType int lineAnchor; + int lineAnchorTranslatePercent; if (cue.line != Cue.DIMEN_UNSET) { switch (cue.lineType) { case Cue.LINE_TYPE_NUMBER: if (cue.line >= 0) { linePercent = 0; lineTranslatePercent = Math.round(cue.line) * 100; + lineAnchorTranslatePercent = 0; } else { linePercent = 100; lineTranslatePercent = Math.round(cue.line + 1) * 100; + lineAnchorTranslatePercent = -100; } break; case Cue.LINE_TYPE_FRACTION: @@ -197,18 +199,16 @@ import java.util.List; default: linePercent = cue.line * 100; lineTranslatePercent = 0; + lineAnchorTranslatePercent = + cue.verticalType == Cue.VERTICAL_TYPE_RL + ? -anchorTypeToTranslatePercent(cue.lineAnchor) + : anchorTypeToTranslatePercent(cue.lineAnchor); } - lineAnchor = cue.lineAnchor; } else { linePercent = (1.0f - bottomPaddingFraction) * 100; lineTranslatePercent = 0; - // If Cue.line == DIMEN_UNSET then ignore Cue.lineAnchor and assume ANCHOR_TYPE_END. - lineAnchor = Cue.ANCHOR_TYPE_END; + lineAnchorTranslatePercent = -100; } - int lineAnchorTranslatePercent = - cue.verticalType == Cue.VERTICAL_TYPE_RL - ? -anchorTypeToTranslatePercent(lineAnchor) - : anchorTypeToTranslatePercent(lineAnchor); String size = cue.size != Cue.DIMEN_UNSET