From e8fdd83558dcb0a4f886c52c8b32e2c1f3637a48 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). #minor-release PiperOrigin-RevId: 544677965 --- RELEASENOTES.md | 4 ++ .../androidx/media3/common/util/Util.java | 19 -------- .../androidx/media3/common/util/UtilTest.java | 43 ------------------- .../extractor/text/cea/Cea608Decoder.java | 13 +++--- 4 files changed, 9 insertions(+), 70 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 0712da9f0b..b220d49c41 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -68,6 +68,10 @@ * Allow `MediaCodecVideoRenderer` to use a custom `VideoFrameProcessor.Factory`. * 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)). * Metadata: * DRM: * Effect: 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 f05c52bdb3..e66d409478 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 @@ -1674,25 +1674,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 f5efdfd53b..c8b10201c4 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 @@ -36,7 +36,6 @@ import androidx.media3.common.util.Log; import androidx.media3.common.util.NullableType; 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; @@ -952,8 +951,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); } @@ -971,17 +969,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. @@ -989,6 +984,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;