Use CuesWithTiming.durationUs in SubripParser instead of empty cues

This fixes two things in one go:
1. In order to indicate 'end of a cue' **without** relying on
   `CuesWithTiming.durationUs`, `SubripParser` should have been emitting
   an empty `List<Cue>` instead of the current behaviour of a
   single-item list with `cue.text=""`.
2. There's no need for the empty cue (or cue list), we can use
   `durationUs` to indicate the end of each list of cues.

There's no real need to ever have a `Cue` with `text=""`, so also
deprecate `Cue.EMPTY`.

PiperOrigin-RevId: 548938874
This commit is contained in:
ibaker 2023-07-18 10:27:40 +01:00 committed by Ian Baker
parent 8655429af7
commit 0fa66534fe
4 changed files with 57 additions and 63 deletions

View File

@ -1188,7 +1188,7 @@ package androidx.media3.common.text {
field public static final int ANCHOR_TYPE_MIDDLE = 1; // 0x1
field public static final int ANCHOR_TYPE_START = 0; // 0x0
field public static final float DIMEN_UNSET = -3.4028235E38f;
field public static final androidx.media3.common.text.Cue EMPTY;
field @Deprecated public static final androidx.media3.common.text.Cue EMPTY;
field public static final int LINE_TYPE_FRACTION = 0; // 0x0
field public static final int LINE_TYPE_NUMBER = 1; // 0x1
field public static final int TEXT_SIZE_TYPE_ABSOLUTE = 2; // 0x2

View File

@ -50,8 +50,11 @@ import org.checkerframework.dataflow.qual.Pure;
// information around in a sidecar object.
public final class Cue implements Bundleable {
/** The empty cue. */
public static final Cue EMPTY = new Cue.Builder().setText("").build();
/**
* @deprecated There's no general need for a cue with an empty text string. If you need one,
* create it yourself.
*/
@Deprecated public static final Cue EMPTY = new Cue.Builder().setText("").build();
/** An unset position, width or size. */
// Note: We deliberately don't use Float.MIN_VALUE because it's positive & very close to zero.

View File

@ -22,11 +22,9 @@ import android.text.Spanned;
import android.text.TextUtils;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import androidx.media3.common.C;
import androidx.media3.common.text.Cue;
import androidx.media3.common.util.Assertions;
import androidx.media3.common.util.Log;
import androidx.media3.common.util.LongArray;
import androidx.media3.common.util.ParsableByteArray;
import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util;
@ -82,8 +80,7 @@ public final class SubripParser implements SubtitleParser {
@Nullable
@Override
public ImmutableList<CuesWithTiming> parse(byte[] data, int offset, int length) {
ArrayList<Cue> cues = new ArrayList<>();
LongArray startTimesUs = new LongArray();
ImmutableList.Builder<CuesWithTiming> cues = new ImmutableList.Builder<>();
if (dataScratch.length < length) {
dataScratch = new byte[length];
@ -115,10 +112,12 @@ public final class SubripParser implements SubtitleParser {
break;
}
long startTimeUs;
long endTimeUs;
Matcher matcher = SUBRIP_TIMING_LINE.matcher(currentLine);
if (matcher.matches()) {
startTimesUs.add(parseTimecode(matcher, /* groupOffset= */ 1));
startTimesUs.add(parseTimecode(matcher, /* groupOffset= */ 6));
startTimeUs = parseTimecode(matcher, /* groupOffset= */ 1);
endTimeUs = parseTimecode(matcher, /* groupOffset= */ 6);
} else {
Log.w(TAG, "Skipping invalid timing: " + currentLine);
continue;
@ -147,21 +146,13 @@ public final class SubripParser implements SubtitleParser {
break;
}
}
cues.add(buildCue(text, alignmentTag));
cues.add(Cue.EMPTY);
}
ImmutableList.Builder<CuesWithTiming> cuesWithStartTimeAndDuration = ImmutableList.builder();
for (int i = 0; i < cues.size(); i++) {
cuesWithStartTimeAndDuration.add(
cues.add(
new CuesWithTiming(
/* cues= */ ImmutableList.of(cues.get(i)),
/* startTimeUs= */ startTimesUs.get(i),
/* durationUs= */ i == cues.size() - 1
? C.TIME_UNSET
: startTimesUs.get(i + 1) - startTimesUs.get(i)));
ImmutableList.of(buildCue(text, alignmentTag)),
startTimeUs,
/* durationUs= */ endTimeUs - startTimeUs));
}
return cuesWithStartTimeAndDuration.build();
return cues.build();
}
@Override

View File

@ -65,10 +65,10 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue3(allCues.get(4));
assertTypicalCue2(allCues.get(1));
assertTypicalCue3(allCues.get(2));
}
@Test
@ -78,11 +78,11 @@ public final class SubripParserTest {
ImmutableList<CuesWithTiming> allCues = parser.parse(bytes, 10, bytes.length - 15);
assertThat(allCues).hasSize(4);
assertThat(allCues).hasSize(2);
// Because of the offset, we skip the first line of dialogue
assertTypicalCue2(allCues.get(0));
// Because of the length restriction, we only partially parse the third line of dialogue
Cue thirdCue = allCues.get(2).cues.get(0);
Cue thirdCue = allCues.get(1).cues.get(0);
assertThat(thirdCue.text.toString()).isEqualTo("This is the third subti");
}
@ -95,10 +95,10 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue3(allCues.get(4));
assertTypicalCue2(allCues.get(1));
assertTypicalCue3(allCues.get(2));
}
@Test
@ -110,10 +110,10 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue3(allCues.get(4));
assertTypicalCue2(allCues.get(1));
assertTypicalCue3(allCues.get(2));
}
@Test
@ -126,9 +126,9 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(4);
assertThat(allCues).hasSize(2);
assertTypicalCue1(allCues.get(0));
assertTypicalCue3(allCues.get(2));
assertTypicalCue3(allCues.get(1));
}
@Test
@ -141,9 +141,9 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(4);
assertThat(allCues).hasSize(2);
assertTypicalCue1(allCues.get(0));
assertTypicalCue3(allCues.get(2));
assertTypicalCue3(allCues.get(1));
}
@Test
@ -156,7 +156,7 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(2);
assertThat(allCues).hasSize(1);
assertTypicalCue3(allCues.get(0));
}
@ -169,9 +169,9 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(4);
assertThat(allCues).hasSize(2);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue2(allCues.get(1));
}
@Test
@ -182,10 +182,10 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue3(allCues.get(4));
assertTypicalCue2(allCues.get(1));
assertTypicalCue3(allCues.get(2));
}
@Test
@ -196,10 +196,10 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertTypicalCue2(allCues.get(2));
assertTypicalCue3(allCues.get(4));
assertTypicalCue2(allCues.get(1));
assertTypicalCue3(allCues.get(2));
}
@Test
@ -212,22 +212,22 @@ public final class SubripParserTest {
assertThat(allCues).isNotNull();
assertThat(allCues.get(0).cues.get(0).text.toString()).isEqualTo("This is the first subtitle.");
assertThat(allCues.get(2).cues.get(0).text.toString())
assertThat(allCues.get(1).cues.get(0).text.toString())
.isEqualTo("This is the second subtitle.\nSecond subtitle with second line.");
assertThat(allCues.get(4).cues.get(0).text.toString()).isEqualTo("This is the third subtitle.");
assertThat(allCues.get(6).cues.get(0).text.toString())
assertThat(allCues.get(2).cues.get(0).text.toString()).isEqualTo("This is the third subtitle.");
assertThat(allCues.get(3).cues.get(0).text.toString())
.isEqualTo("This { \\an2} is not a valid tag due to the space after the opening bracket.");
assertThat(allCues.get(8).cues.get(0).text.toString())
assertThat(allCues.get(4).cues.get(0).text.toString())
.isEqualTo("This is the fifth subtitle with multiple valid tags.");
assertAlignmentCue(allCues.get(10), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_START); // {/an1}
assertAlignmentCue(allCues.get(12), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_MIDDLE); // {/an2}
assertAlignmentCue(allCues.get(14), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_END); // {/an3}
assertAlignmentCue(allCues.get(16), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_START); // {/an4}
assertAlignmentCue(allCues.get(18), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_MIDDLE); // {/an5}
assertAlignmentCue(allCues.get(20), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_END); // {/an6}
assertAlignmentCue(allCues.get(22), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_START); // {/an7}
assertAlignmentCue(allCues.get(24), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_MIDDLE); // {/an8}
assertAlignmentCue(allCues.get(26), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_END); // {/an9}
assertAlignmentCue(allCues.get(5), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_START); // {/an1}
assertAlignmentCue(allCues.get(6), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_MIDDLE); // {/an2}
assertAlignmentCue(allCues.get(7), Cue.ANCHOR_TYPE_END, Cue.ANCHOR_TYPE_END); // {/an3}
assertAlignmentCue(allCues.get(8), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_START); // {/an4}
assertAlignmentCue(allCues.get(9), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_MIDDLE); // {/an5}
assertAlignmentCue(allCues.get(10), Cue.ANCHOR_TYPE_MIDDLE, Cue.ANCHOR_TYPE_END); // {/an6}
assertAlignmentCue(allCues.get(11), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_START); // {/an7}
assertAlignmentCue(allCues.get(12), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_MIDDLE); // {/an8}
assertAlignmentCue(allCues.get(13), Cue.ANCHOR_TYPE_START, Cue.ANCHOR_TYPE_END); // {/an9}
}
@Test
@ -239,11 +239,11 @@ public final class SubripParserTest {
List<CuesWithTiming> allCues = parser.parse(bytes);
assertThat(allCues).hasSize(6);
assertThat(allCues).hasSize(3);
assertTypicalCue1(allCues.get(0));
assertThat(allCues.get(2).startTimeUs).isEqualTo(2_000_000);
assertThat(allCues.get(3).startTimeUs).isEqualTo(3_000_000);
assertTypicalCue3(allCues.get(4));
assertThat(allCues.get(1).startTimeUs).isEqualTo(2_000_000);
assertThat(allCues.get(1).durationUs).isEqualTo(1_000_000);
assertTypicalCue3(allCues.get(2));
}
private static void assertTypicalCue1(CuesWithTiming cuesWithTiming) {