diff --git a/RELEASENOTES.md b/RELEASENOTES.md index c49ef4aa8b..c9b3bb1a32 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -24,6 +24,8 @@ `DefaultAudioSink` to stall. * Video: * Text: + * TTML: Add support for referencing `tts:origin` and `tts:extent` via + `style` ([#2953](https://github.com/google/ExoPlayer/issues/2953)). * Metadata: * Image: * DataSource: diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderTtmlParserTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderTtmlParserTest.java index adf392fc11..085a815277 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderTtmlParserTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/text/DelegatingSubtitleDecoderTtmlParserTest.java @@ -308,13 +308,9 @@ public final class DelegatingSubtitleDecoderTtmlParserTest { cue = getOnlyCueAtTimeUs(subtitle, 9_000_000); assertThat(cue.text.toString()).isEqualTo("dolor"); - assertThat(cue.position).isEqualTo(Cue.DIMEN_UNSET); - assertThat(cue.line).isEqualTo(Cue.DIMEN_UNSET); - assertThat(cue.size).isEqualTo(Cue.DIMEN_UNSET); - // TODO: Should be as below, once https://github.com/google/ExoPlayer/issues/2953 is fixed. - // assertEquals(10f / 100f, cue.position); - // assertEquals(80f / 100f, cue.line); - // assertEquals(1f, cue.size); + assertThat(cue.position).isEqualTo(10f / 100f); + assertThat(cue.line).isEqualTo(80f / 100f); + assertThat(cue.size).isEqualTo(1f); cue = getOnlyCueAtTimeUs(subtitle, 21_000_000); assertThat(cue.text.toString()).isEqualTo("They first said this"); diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlNode.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlNode.java index 47f7ba4da8..3405e02228 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlNode.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlNode.java @@ -53,6 +53,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; public static final String ANONYMOUS_REGION_ID = ""; public static final String ATTR_ID = "id"; + public static final String ATTR_STYLE = "style"; public static final String ATTR_TTS_ORIGIN = "origin"; public static final String ATTR_TTS_EXTENT = "extent"; public static final String ATTR_TTS_DISPLAY_ALIGN = "displayAlign"; diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlParser.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlParser.java index d1d615657a..4b11f487fd 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlParser.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlParser.java @@ -315,7 +315,8 @@ public final class TtmlParser implements SubtitleParser { globalStyles.put(styleId, style); } } else if (XmlPullParserUtil.isStartTag(xmlParser, TtmlNode.TAG_REGION)) { - @Nullable TtmlRegion ttmlRegion = parseRegionAttributes(xmlParser, cellRows, ttsExtent); + @Nullable + TtmlRegion ttmlRegion = parseRegionAttributes(xmlParser, cellRows, ttsExtent, globalStyles); if (ttmlRegion != null) { globalRegions.put(ttmlRegion.id, ttmlRegion); } @@ -350,7 +351,10 @@ public final class TtmlParser implements SubtitleParser { */ @Nullable private static TtmlRegion parseRegionAttributes( - XmlPullParser xmlParser, int cellRows, @Nullable TtsExtent ttsExtent) { + XmlPullParser xmlParser, + int cellRows, + @Nullable TtsExtent ttsExtent, + Map globalStyles) { @Nullable String regionId = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_ID); if (regionId == null) { return null; @@ -361,6 +365,15 @@ public final class TtmlParser implements SubtitleParser { @Nullable String regionOrigin = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_TTS_ORIGIN); + if (regionOrigin == null) { + String styleId = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_STYLE); + if (styleId != null) { + TtmlStyle style = globalStyles.get(styleId); + if (style != null) { + regionOrigin = style.getOrigin(); + } + } + } if (regionOrigin != null) { Matcher originPercentageMatcher = PERCENTAGE_COORDINATES.matcher(regionOrigin); Matcher originPixelMatcher = PIXEL_COORDINATES.matcher(regionOrigin); @@ -393,19 +406,24 @@ public final class TtmlParser implements SubtitleParser { return null; } } else { - Log.w(TAG, "Ignoring region without an origin"); - return null; - // TODO: Should default to top left as below in this case, but need to fix - // https://github.com/google/ExoPlayer/issues/2953 first. // Origin is omitted. Default to top left. - // position = 0; - // line = 0; + position = 0; + line = 0; } float width; float height; @Nullable String regionExtent = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_TTS_EXTENT); + if (regionExtent == null) { + String styleId = XmlPullParserUtil.getAttributeValue(xmlParser, TtmlNode.ATTR_STYLE); + if (styleId != null) { + TtmlStyle style = globalStyles.get(styleId); + if (style != null) { + regionExtent = style.getExtent(); + } + } + } if (regionExtent != null) { Matcher extentPercentageMatcher = PERCENTAGE_COORDINATES.matcher(regionExtent); Matcher extentPixelMatcher = PIXEL_COORDINATES.matcher(regionExtent); @@ -439,13 +457,9 @@ public final class TtmlParser implements SubtitleParser { return null; } } else { - Log.w(TAG, "Ignoring region without an extent"); - return null; - // TODO: Should default to extent of parent as below in this case, but need to fix - // https://github.com/google/ExoPlayer/issues/2953 first. // Extent is omitted. Default to extent of parent. - // width = 1; - // height = 1; + width = 1; + height = 1; } @Cue.AnchorType int lineAnchor = Cue.ANCHOR_TYPE_START; @@ -626,6 +640,12 @@ public final class TtmlParser implements SubtitleParser { case TtmlNode.ATTR_TTS_SHEAR: style = createIfNull(style).setShearPercentage(parseShear(attributeValue)); break; + case TtmlNode.ATTR_TTS_ORIGIN: + style = createIfNull(style).setOrigin(attributeValue); + break; + case TtmlNode.ATTR_TTS_EXTENT: + style = createIfNull(style).setExtent(attributeValue); + break; default: // ignore break; diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlStyle.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlStyle.java index 1cb59ae211..b3b29bdf24 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlStyle.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/ttml/TtmlStyle.java @@ -96,6 +96,8 @@ import java.lang.annotation.Target; private @OptionalBoolean int textCombine; @Nullable private TextEmphasis textEmphasis; private float shearPercentage; + @Nullable private String origin; + @Nullable private String extent; public TtmlStyle() { linethrough = UNSPECIFIED; @@ -277,6 +279,12 @@ import java.lang.annotation.Target; if (shearPercentage == UNSPECIFIED_SHEAR) { shearPercentage = ancestor.shearPercentage; } + if (origin == null) { + origin = ancestor.origin; + } + if (extent == null) { + extent = ancestor.extent; + } // attributes not inherited as of http://www.w3.org/TR/ttml1/ if (chaining && !hasBackgroundColor && ancestor.hasBackgroundColor) { setBackgroundColor(ancestor.backgroundColor); @@ -382,4 +390,26 @@ import java.lang.annotation.Target; public float getFontSize() { return fontSize; } + + @CanIgnoreReturnValue + public TtmlStyle setOrigin(@Nullable String origin) { + this.origin = origin; + return this; + } + + @Nullable + public String getOrigin() { + return origin; + } + + @CanIgnoreReturnValue + public TtmlStyle setExtent(@Nullable String extent) { + this.extent = extent; + return this; + } + + @Nullable + public String getExtent() { + return extent; + } } diff --git a/libraries/extractor/src/test/java/androidx/media3/extractor/text/ttml/TtmlParserTest.java b/libraries/extractor/src/test/java/androidx/media3/extractor/text/ttml/TtmlParserTest.java index 38cee01100..8d5eee951c 100644 --- a/libraries/extractor/src/test/java/androidx/media3/extractor/text/ttml/TtmlParserTest.java +++ b/libraries/extractor/src/test/java/androidx/media3/extractor/text/ttml/TtmlParserTest.java @@ -74,6 +74,8 @@ public final class TtmlParserTest { private static final String RUBIES_FILE = "media/ttml/rubies.xml"; private static final String TEXT_EMPHASIS_FILE = "media/ttml/text_emphasis.xml"; private static final String SHEAR_FILE = "media/ttml/shear.xml"; + private static final String REGION_ATTRS_FROM_STYLE_FILE = + "media/ttml/inherit_region_attributes_from_style.xml"; @Test public void simple_allCues() throws Exception { @@ -485,13 +487,9 @@ public final class TtmlParserTest { cue = Iterables.getOnlyElement(allCues.get(2).cues); assertThat(cue.text.toString()).isEqualTo("dolor"); - assertThat(cue.position).isEqualTo(Cue.DIMEN_UNSET); - assertThat(cue.line).isEqualTo(Cue.DIMEN_UNSET); - assertThat(cue.size).isEqualTo(Cue.DIMEN_UNSET); - // TODO: Should be as below, once https://github.com/google/ExoPlayer/issues/2953 is fixed. - // assertEquals(10f / 100f, cue.position); - // assertEquals(80f / 100f, cue.line); - // assertEquals(1f, cue.size); + assertThat(cue.position).isEqualTo(10f / 100f); + assertThat(cue.line).isEqualTo(80f / 100f); + assertThat(cue.size).isEqualTo(1f); cue = Iterables.getOnlyElement(allCues.get(3).cues); assertThat(cue.text.toString()).isEqualTo("They first said this"); @@ -1093,6 +1091,26 @@ public final class TtmlParserTest { assertThat(eighthCue.shearDegrees).isWithin(0.01f).of(90f); } + @Test + public void regionAttrsFromStyle() throws Exception { + ImmutableList allCues = getAllCues(REGION_ATTRS_FROM_STYLE_FILE); + + Cue firstCue = Iterables.getOnlyElement(allCues.get(0).cues); + assertThat(firstCue.position).isEqualTo(10f / 100f); + assertThat(firstCue.line).isEqualTo(10f / 100f); + assertThat(firstCue.size).isEqualTo(20f / 100f); + + Cue secondCue = Iterables.getOnlyElement(allCues.get(1).cues); + assertThat(secondCue.position).isEqualTo(30f / 100f); + assertThat(secondCue.line).isEqualTo(30f / 100f); + assertThat(secondCue.size).isEqualTo(40f / 100f); + + Cue thirdCue = Iterables.getOnlyElement(allCues.get(2).cues); + assertThat(thirdCue.position).isEqualTo(30f / 100f); + assertThat(thirdCue.line).isEqualTo(30f / 100f); + assertThat(thirdCue.size).isEqualTo(20f / 100f); + } + private static Spanned getOnlyCueTextAtIndex(List allCues, int index) { Cue cue = getOnlyCueAtIndex(allCues, index); assertThat(cue.text).isInstanceOf(Spanned.class); diff --git a/libraries/test_data/src/test/assets/media/ttml/inherit_region_attributes_from_style.xml b/libraries/test_data/src/test/assets/media/ttml/inherit_region_attributes_from_style.xml new file mode 100644 index 0000000000..6f06538ae7 --- /dev/null +++ b/libraries/test_data/src/test/assets/media/ttml/inherit_region_attributes_from_style.xml @@ -0,0 +1,22 @@ + + + +