Skip to content

SDK / exoplayer

Go to a project
Commit 1c0e6978 by ibaker Committed by Ian Baker
Options

Clear existing Spans when applying CSS styles in WebvttCueParser 

Relying on the precedence of spans seems risky - I can't find it
defined anywhere. It might have changed in Android 6.0?
https://stackoverflow.com/q/34631851

PiperOrigin-RevId: 287989365
parent f76b4fe6
Inline Side-by-side
Showing with 31 additions and 17 deletions
library/core/src/main/java/com/google/android/exoplayer2/text/webvtt/WebvttCueParser.java
...@@ -492,8 +492,7 @@ public final class WebvttCueParser { ...@@ -492,8 +492,7 @@ public final class WebvttCueParser {
return; return;
} }
if (style.getStyle() != WebvttCssStyle.UNSPECIFIED) { if (style.getStyle() != WebvttCssStyle.UNSPECIFIED) {
spannedText.setSpan(new StyleSpan(style.getStyle()), start, end, addOrReplaceSpan(spannedText, new StyleSpan(style.getStyle()), start, end);
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
} }
if (style.isLinethrough()) { if (style.isLinethrough()) {
spannedText.setSpan(new StrikethroughSpan(), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); spannedText.setSpan(new StrikethroughSpan(), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
...@@ -502,34 +501,29 @@ public final class WebvttCueParser { ...@@ -502,34 +501,29 @@ public final class WebvttCueParser {
spannedText.setSpan(new UnderlineSpan(), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); spannedText.setSpan(new UnderlineSpan(), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
} }
if (style.hasFontColor()) { if (style.hasFontColor()) {
spannedText.setSpan(new ForegroundColorSpan(style.getFontColor()), start, end, addOrReplaceSpan(spannedText, new ForegroundColorSpan(style.getFontColor()), start, end);
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
} }
if (style.hasBackgroundColor()) { if (style.hasBackgroundColor()) {
spannedText.setSpan(new BackgroundColorSpan(style.getBackgroundColor()), start, end, addOrReplaceSpan(
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE); spannedText, new BackgroundColorSpan(style.getBackgroundColor()), start, end);
} }
if (style.getFontFamily() != null) { if (style.getFontFamily() != null) {
spannedText.setSpan(new TypefaceSpan(style.getFontFamily()), start, end, addOrReplaceSpan(spannedText, new TypefaceSpan(style.getFontFamily()), start, end);
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
} }
Layout.Alignment textAlign = style.getTextAlign(); Layout.Alignment textAlign = style.getTextAlign();
if (textAlign != null) { if (textAlign != null) {
spannedText.setSpan( addOrReplaceSpan(spannedText, new AlignmentSpan.Standard(textAlign), start, end);
new AlignmentSpan.Standard(textAlign), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
} }
switch (style.getFontSizeUnit()) { switch (style.getFontSizeUnit()) {
case WebvttCssStyle.FONT_SIZE_UNIT_PIXEL: case WebvttCssStyle.FONT_SIZE_UNIT_PIXEL:
spannedText.setSpan(new AbsoluteSizeSpan((int) style.getFontSize(), true), start, end, addOrReplaceSpan(
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); spannedText, new AbsoluteSizeSpan((int) style.getFontSize(), true), start, end);
break; break;
case WebvttCssStyle.FONT_SIZE_UNIT_EM: case WebvttCssStyle.FONT_SIZE_UNIT_EM:
spannedText.setSpan(new RelativeSizeSpan(style.getFontSize()), start, end, addOrReplaceSpan(spannedText, new RelativeSizeSpan(style.getFontSize()), start, end);
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
break; break;
case WebvttCssStyle.FONT_SIZE_UNIT_PERCENT: case WebvttCssStyle.FONT_SIZE_UNIT_PERCENT:
spannedText.setSpan(new RelativeSizeSpan(style.getFontSize() / 100), start, end, addOrReplaceSpan(spannedText, new RelativeSizeSpan(style.getFontSize() / 100), start, end);
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
break; break;
case WebvttCssStyle.UNSPECIFIED: case WebvttCssStyle.UNSPECIFIED:
// Do nothing. // Do nothing.
...@@ -538,6 +532,26 @@ public final class WebvttCueParser { ...@@ -538,6 +532,26 @@ public final class WebvttCueParser {
} }
/** /**
* Adds {@code span} to {@code spannedText} between {@code start} and {@code end}, removing any
* existing spans of the same type and with the same indices.
*
* <p>This is useful for types of spans that don't make sense to duplicate and where the
* evaluation order might have an unexpected impact on the final text, e.g. {@link
* ForegroundColorSpan}.
*/
private static void addOrReplaceSpan(
SpannableStringBuilder spannedText, Object span, int start, int end) {
Object[] existingSpans = spannedText.getSpans(start, end, span.getClass());
for (Object existingSpan : existingSpans) {
if (spannedText.getSpanStart(existingSpan) == start
&& spannedText.getSpanEnd(existingSpan) == end) {
spannedText.removeSpan(existingSpan);
}
}
spannedText.setSpan(span, start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
}
/**
* Returns the tag name for the given tag contents. * Returns the tag name for the given tag contents.
* *
* @param tagExpression Characters between &amp;lt: and &amp;gt; of a start or end tag. * @param tagExpression Characters between &amp;lt: and &amp;gt; of a start or end tag.
......
library/core/src/test/java/com/google/android/exoplayer2/text/webvtt/WebvttDecoderTest.java
...@@ -405,7 +405,7 @@ public class WebvttDecoderTest { ...@@ -405,7 +405,7 @@ public class WebvttDecoderTest {
Spanned s4 = getUniqueSpanTextAt(subtitle, /* timeUs= */ 25000000); Spanned s4 = getUniqueSpanTextAt(subtitle, /* timeUs= */ 25000000);
assertThat(s1.getSpans(/* start= */ 0, s1.length(), ForegroundColorSpan.class)).hasLength(1); assertThat(s1.getSpans(/* start= */ 0, s1.length(), ForegroundColorSpan.class)).hasLength(1);
assertThat(s1.getSpans(/* start= */ 0, s1.length(), BackgroundColorSpan.class)).hasLength(1); assertThat(s1.getSpans(/* start= */ 0, s1.length(), BackgroundColorSpan.class)).hasLength(1);
assertThat(s2.getSpans(/* start= */ 0, s2.length(), ForegroundColorSpan.class)).hasLength(2); assertThat(s2.getSpans(/* start= */ 0, s2.length(), ForegroundColorSpan.class)).hasLength(1);
assertThat(s3.getSpans(/* start= */ 10, s3.length(), UnderlineSpan.class)).hasLength(1); assertThat(s3.getSpans(/* start= */ 10, s3.length(), UnderlineSpan.class)).hasLength(1);
assertThat(s4.getSpans(/* start= */ 0, /* end= */ 16, BackgroundColorSpan.class)).hasLength(2); assertThat(s4.getSpans(/* start= */ 0, /* end= */ 16, BackgroundColorSpan.class)).hasLength(2);
assertThat(s4.getSpans(/* start= */ 17, s4.length(), StyleSpan.class)).hasLength(1); assertThat(s4.getSpans(/* start= */ 17, s4.length(), StyleSpan.class)).hasLength(1);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment