Skip to content

Commit

Permalink
CEA-608: Only truncate to 32 visible characters
Browse files Browse the repository at this point in the history
We introduced truncation to 32 chars in <unknown commit>
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 e8fdd83)
  • Loading branch information
icbaker authored and tianyif committed Jun 30, 2023
1 parent aefba8a commit aa34db4
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 70 deletions.
5 changes: 5 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1675,25 +1675,6 @@ public static long toLong(int mostSignificantBits, int leastSignificantBits) {
return (toUnsignedLong(mostSignificantBits) << 32) | toUnsignedLong(leastSignificantBits);
}

/**
* Truncates a sequence of ASCII characters to a maximum length.
*
* <p>This preserves span styling in the {@link CharSequence}. If that's not important, use {@link
* Ascii#truncate(CharSequence, int, String)}.
*
* <p><b>Note:</b> 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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@
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;
Expand Down Expand Up @@ -896,45 +892,6 @@ public void toLong_withBigNegativeValue_returnsValue() {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
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;
Expand Down Expand Up @@ -949,8 +948,7 @@ public void backspace() {
}

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);
}
Expand All @@ -968,24 +966,23 @@ public void rollUp() {

@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.
return null;
}

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;
Expand Down

0 comments on commit aa34db4

Please sign in to comment.