From aa34db41f7d64009615a9df1fe97fac36b1f22ce Mon Sep 17 00:00:00 2001 From: ibaker Date: Fri, 30 Jun 2023 16:35:46 +0000 Subject: [PATCH] CEA-608: Only truncate to 32 visible characters We introduced truncation to 32 chars in and included indent and offset in the calculation. I think this is technically correct, but it causes problems with the content in Issue: google/ExoPlayer#11019 and it doesn't seem a problem to only truncate actual cue text (i.e. ignore offset and indent). PiperOrigin-RevId: 544677965 (cherry picked from commit e8fdd83558dcb0a4f886c52c8b32e2c1f3637a48) --- RELEASENOTES.md | 5 +++ .../androidx/media3/common/util/Util.java | 19 -------- .../androidx/media3/common/util/UtilTest.java | 43 ------------------- .../extractor/text/cea/Cea608Decoder.java | 13 +++--- 4 files changed, 10 insertions(+), 70 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 1a3083f52a..4173c8bba1 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -5,6 +5,11 @@ * ExoPlayer: * Fix issue in `PlaybackStatsListener` where spurious `PlaybackStats` are created after the playlist is cleared. +* Text: + * CEA-608: Change cue truncation logic to only consider visible text. + Previously indent and tab offset were included when limiting the cue + length to 32 characters (which was technically correct by the spec) + ([#11019](https://github.com/google/ExoPlayer/issues/11019)). ## 1.1 ### 1.1.0 (2023-07-05) diff --git a/libraries/common/src/main/java/androidx/media3/common/util/Util.java b/libraries/common/src/main/java/androidx/media3/common/util/Util.java index 52e6e80142..6665a545dc 100644 --- a/libraries/common/src/main/java/androidx/media3/common/util/Util.java +++ b/libraries/common/src/main/java/androidx/media3/common/util/Util.java @@ -1675,25 +1675,6 @@ public final class Util { return (toUnsignedLong(mostSignificantBits) << 32) | toUnsignedLong(leastSignificantBits); } - /** - * Truncates a sequence of ASCII characters to a maximum length. - * - *

This preserves span styling in the {@link CharSequence}. If that's not important, use {@link - * Ascii#truncate(CharSequence, int, String)}. - * - *

Note: This is not safe to use in general on Unicode text because it may separate - * characters from combining characters or split up surrogate pairs. - * - * @param sequence The character sequence to truncate. - * @param maxLength The max length to truncate to. - * @return {@code sequence} directly if {@code sequence.length() <= maxLength}, otherwise {@code - * sequence.subsequence(0, maxLength}. - */ - @UnstableApi - public static CharSequence truncateAscii(CharSequence sequence, int maxLength) { - return sequence.length() <= maxLength ? sequence : sequence.subSequence(0, maxLength); - } - /** * Returns a byte array containing values parsed from the hex string provided. * diff --git a/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java b/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java index b91c4f5152..54b0f09a76 100644 --- a/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java +++ b/libraries/common/src/test/java/androidx/media3/common/util/UtilTest.java @@ -38,10 +38,6 @@ import android.net.Uri; import android.os.Handler; import android.os.HandlerThread; import android.os.Looper; -import android.text.SpannableString; -import android.text.Spanned; -import android.text.style.StrikethroughSpan; -import android.text.style.UnderlineSpan; import android.util.SparseLongArray; import androidx.media3.common.C; import androidx.media3.test.utils.TestUtil; @@ -896,45 +892,6 @@ public class UtilTest { assertThat(Util.toLong(0xFEDCBA, 0x87654321)).isEqualTo(0xFEDCBA_87654321L); } - @Test - public void truncateAscii_shortInput_returnsInput() { - String input = "a short string"; - - assertThat(Util.truncateAscii(input, 100)).isSameInstanceAs(input); - } - - @Test - public void truncateAscii_longInput_truncated() { - String input = "a much longer string"; - - assertThat(Util.truncateAscii(input, 5).toString()).isEqualTo("a muc"); - } - - @Test - public void truncateAscii_preservesStylingSpans() { - SpannableString input = new SpannableString("a short string"); - input.setSpan(new UnderlineSpan(), 0, 10, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - input.setSpan(new StrikethroughSpan(), 4, 10, Spanned.SPAN_INCLUSIVE_EXCLUSIVE); - - CharSequence result = Util.truncateAscii(input, 7); - - assertThat(result).isInstanceOf(SpannableString.class); - assertThat(result.toString()).isEqualTo("a short"); - // TODO(internal b/161804035): Use SpannedSubject when it's available in a dependency we can use - // from here. - Spanned spannedResult = (Spanned) result; - Object[] spans = spannedResult.getSpans(0, result.length(), Object.class); - assertThat(spans).hasLength(2); - assertThat(spans[0]).isInstanceOf(UnderlineSpan.class); - assertThat(spannedResult.getSpanStart(spans[0])).isEqualTo(0); - assertThat(spannedResult.getSpanEnd(spans[0])).isEqualTo(7); - assertThat(spannedResult.getSpanFlags(spans[0])).isEqualTo(Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - assertThat(spans[1]).isInstanceOf(StrikethroughSpan.class); - assertThat(spannedResult.getSpanStart(spans[1])).isEqualTo(4); - assertThat(spannedResult.getSpanEnd(spans[1])).isEqualTo(7); - assertThat(spannedResult.getSpanFlags(spans[1])).isEqualTo(Spanned.SPAN_INCLUSIVE_EXCLUSIVE); - } - @Test public void toHexString_returnsHexString() { byte[] bytes = createByteArray(0x12, 0xFC, 0x06); diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Decoder.java b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Decoder.java index 9a975939af..530be9025e 100644 --- a/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Decoder.java +++ b/libraries/extractor/src/main/java/androidx/media3/extractor/text/cea/Cea608Decoder.java @@ -35,7 +35,6 @@ import androidx.media3.common.util.Assertions; import androidx.media3.common.util.Log; import androidx.media3.common.util.ParsableByteArray; import androidx.media3.common.util.UnstableApi; -import androidx.media3.common.util.Util; import androidx.media3.extractor.text.Subtitle; import androidx.media3.extractor.text.SubtitleDecoder; import androidx.media3.extractor.text.SubtitleDecoderException; @@ -949,8 +948,7 @@ public final class Cea608Decoder extends CeaDecoder { } public void append(char text) { - // Don't accept more than 32 chars. We'll trim further, considering indent & tabOffset, in - // build(). + // Don't accept more than 32 chars. if (captionStringBuilder.length() < SCREEN_CHARWIDTH) { captionStringBuilder.append(text); } @@ -968,17 +966,14 @@ public final class Cea608Decoder extends CeaDecoder { @Nullable public Cue build(@Cue.AnchorType int forcedPositionAnchor) { - // The number of empty columns before the start of the text, in the range [0-31]. - int startPadding = indent + tabOffset; - int maxTextLength = SCREEN_CHARWIDTH - startPadding; SpannableStringBuilder cueString = new SpannableStringBuilder(); // Add any rolled up captions, separated by new lines. for (int i = 0; i < rolledUpCaptions.size(); i++) { - cueString.append(Util.truncateAscii(rolledUpCaptions.get(i), maxTextLength)); + cueString.append(rolledUpCaptions.get(i)); cueString.append('\n'); } // Add the current line. - cueString.append(Util.truncateAscii(buildCurrentLine(), maxTextLength)); + cueString.append(buildCurrentLine()); if (cueString.length() == 0) { // The cue is empty. @@ -986,6 +981,8 @@ public final class Cea608Decoder extends CeaDecoder { } int positionAnchor; + // The number of empty columns before the start of the text, in the range [0-31]. + int startPadding = indent + tabOffset; // The number of empty columns after the end of the text, in the same range. int endPadding = SCREEN_CHARWIDTH - startPadding - cueString.length(); int startEndPaddingDelta = startPadding - endPadding;