Commit f11a204b by Oliver Woodman

Remove caption parser strict modes.

They don't seem particularly useful; they don't technically force
strict compliance, but rather just catch a few token things in
each case. Furthermore, for playback, probably the right thing to
do is to always turn strict mode off.
parent 4fbe4fc7
...@@ -15,8 +15,6 @@ ...@@ -15,8 +15,6 @@
*/ */
package com.google.android.exoplayer.text.subrip; package com.google.android.exoplayer.text.subrip;
import com.google.android.exoplayer.ParserException;
import android.test.InstrumentationTestCase; import android.test.InstrumentationTestCase;
import java.io.IOException; import java.io.IOException;
...@@ -35,7 +33,7 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -35,7 +33,7 @@ public final class SubripParserTest extends InstrumentationTestCase {
private static final String NO_END_TIMECODES_FILE = "subrip/no_end_timecodes"; private static final String NO_END_TIMECODES_FILE = "subrip/no_end_timecodes";
public void testParseEmpty() throws IOException { public void testParseEmpty() throws IOException {
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(EMPTY_FILE); InputStream inputStream = getInputStream(EMPTY_FILE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
// Assert that the subtitle is empty. // Assert that the subtitle is empty.
...@@ -44,7 +42,7 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -44,7 +42,7 @@ public final class SubripParserTest extends InstrumentationTestCase {
} }
public void testParseTypical() throws IOException { public void testParseTypical() throws IOException {
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(TYPICAL_FILE); InputStream inputStream = getInputStream(TYPICAL_FILE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
assertEquals(6, subtitle.getEventTimeCount()); assertEquals(6, subtitle.getEventTimeCount());
...@@ -54,7 +52,7 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -54,7 +52,7 @@ public final class SubripParserTest extends InstrumentationTestCase {
} }
public void testParseTypicalExtraBlankLine() throws IOException { public void testParseTypicalExtraBlankLine() throws IOException {
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(TYPICAL_EXTRA_BLANK_LINE); InputStream inputStream = getInputStream(TYPICAL_EXTRA_BLANK_LINE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
assertEquals(6, subtitle.getEventTimeCount()); assertEquals(6, subtitle.getEventTimeCount());
...@@ -64,19 +62,9 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -64,19 +62,9 @@ public final class SubripParserTest extends InstrumentationTestCase {
} }
public void testParseTypicalMissingTimecode() throws IOException { public void testParseTypicalMissingTimecode() throws IOException {
// Strict parsing should fail. // Parsing should succeed, parsing the first and third cues only.
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(TYPICAL_MISSING_TIMECODE); InputStream inputStream = getInputStream(TYPICAL_MISSING_TIMECODE);
try {
parser.parse(inputStream);
fail();
} catch (ParserException e) {
// Expected.
}
// Non-strict parsing should succeed, parsing the first and third cues only.
parser = new SubripParser(false);
inputStream = getInputStream(TYPICAL_MISSING_TIMECODE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
assertEquals(4, subtitle.getEventTimeCount()); assertEquals(4, subtitle.getEventTimeCount());
assertTypicalCue1(subtitle, 0); assertTypicalCue1(subtitle, 0);
...@@ -84,19 +72,9 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -84,19 +72,9 @@ public final class SubripParserTest extends InstrumentationTestCase {
} }
public void testParseTypicalMissingSequence() throws IOException { public void testParseTypicalMissingSequence() throws IOException {
// Strict parsing should fail. // Parsing should succeed, parsing the first and third cues only.
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(TYPICAL_MISSING_SEQUENCE); InputStream inputStream = getInputStream(TYPICAL_MISSING_SEQUENCE);
try {
parser.parse(inputStream);
fail();
} catch (ParserException e) {
// Expected.
}
// Non-strict parsing should succeed, parsing the first and third cues only.
parser = new SubripParser(false);
inputStream = getInputStream(TYPICAL_MISSING_SEQUENCE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
assertEquals(4, subtitle.getEventTimeCount()); assertEquals(4, subtitle.getEventTimeCount());
assertTypicalCue1(subtitle, 0); assertTypicalCue1(subtitle, 0);
...@@ -104,7 +82,7 @@ public final class SubripParserTest extends InstrumentationTestCase { ...@@ -104,7 +82,7 @@ public final class SubripParserTest extends InstrumentationTestCase {
} }
public void testParseNoEndTimecodes() throws IOException { public void testParseNoEndTimecodes() throws IOException {
SubripParser parser = new SubripParser(true); SubripParser parser = new SubripParser();
InputStream inputStream = getInputStream(NO_END_TIMECODES_FILE); InputStream inputStream = getInputStream(NO_END_TIMECODES_FILE);
SubripSubtitle subtitle = parser.parse(inputStream); SubripSubtitle subtitle = parser.parse(inputStream);
......
...@@ -470,10 +470,9 @@ public final class TtmlParserTest extends InstrumentationTestCase { ...@@ -470,10 +470,9 @@ public final class TtmlParserTest extends InstrumentationTestCase {
} }
private TtmlSubtitle getSubtitle(String file) throws IOException { private TtmlSubtitle getSubtitle(String file) throws IOException {
TtmlParser ttmlParser = new TtmlParser(false); TtmlParser ttmlParser = new TtmlParser();
InputStream inputStream = getInstrumentation().getContext() InputStream inputStream = getInstrumentation().getContext()
.getResources().getAssets().open(file); .getResources().getAssets().open(file);
return (TtmlSubtitle) ttmlParser.parse(inputStream); return (TtmlSubtitle) ttmlParser.parse(inputStream);
} }
} }
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
package com.google.android.exoplayer.text.subrip; package com.google.android.exoplayer.text.subrip;
import com.google.android.exoplayer.C; import com.google.android.exoplayer.C;
import com.google.android.exoplayer.ParserException;
import com.google.android.exoplayer.text.Cue; import com.google.android.exoplayer.text.Cue;
import com.google.android.exoplayer.text.SubtitleParser; import com.google.android.exoplayer.text.SubtitleParser;
import com.google.android.exoplayer.util.LongArray; import com.google.android.exoplayer.util.LongArray;
...@@ -47,24 +46,14 @@ public final class SubripParser implements SubtitleParser { ...@@ -47,24 +46,14 @@ public final class SubripParser implements SubtitleParser {
Pattern.compile("(?:(\\d+):)?(\\d+):(\\d+),(\\d+)"); Pattern.compile("(?:(\\d+):)?(\\d+):(\\d+),(\\d+)");
private final StringBuilder textBuilder; private final StringBuilder textBuilder;
private final boolean strictParsing;
/**
* Equivalent to {@code SubripParser(false)}.
*/
public SubripParser() { public SubripParser() {
this(false); textBuilder = new StringBuilder();
} }
/** @Override
* @param strictParsing If true, {@link #parse(InputStream)} will throw a {@link ParserException} public boolean canParse(String mimeType) {
* if the stream contains invalid data. If false, the parser will make a best effort to ignore return MimeTypes.APPLICATION_SUBRIP.equals(mimeType);
* minor errors in the stream. Note however that a {@link ParserException} will still be
* thrown when this is not possible.
*/
public SubripParser(boolean strictParsing) {
this.strictParsing = strictParsing;
textBuilder = new StringBuilder();
} }
@Override @Override
...@@ -85,12 +74,8 @@ public final class SubripParser implements SubtitleParser { ...@@ -85,12 +74,8 @@ public final class SubripParser implements SubtitleParser {
try { try {
Integer.parseInt(currentLine); Integer.parseInt(currentLine);
} catch (NumberFormatException e) { } catch (NumberFormatException e) {
if (!strictParsing) {
Log.w(TAG, "Skipping invalid index: " + currentLine); Log.w(TAG, "Skipping invalid index: " + currentLine);
continue; continue;
} else {
throw new ParserException("Expected numeric counter: " + currentLine);
}
} }
// Read and parse the timing line. // Read and parse the timing line.
...@@ -104,11 +89,9 @@ public final class SubripParser implements SubtitleParser { ...@@ -104,11 +89,9 @@ public final class SubripParser implements SubtitleParser {
haveEndTimecode = true; haveEndTimecode = true;
cueTimesUs.add(parseTimecode(matcher.group(2))); cueTimesUs.add(parseTimecode(matcher.group(2)));
} }
} else if (!strictParsing) { } else {
Log.w(TAG, "Skipping invalid timing: " + currentLine); Log.w(TAG, "Skipping invalid timing: " + currentLine);
continue; continue;
} else {
throw new ParserException("Expected timing line: " + currentLine);
} }
// Read and parse the text. // Read and parse the text.
...@@ -133,11 +116,6 @@ public final class SubripParser implements SubtitleParser { ...@@ -133,11 +116,6 @@ public final class SubripParser implements SubtitleParser {
return new SubripSubtitle(cuesArray, cueTimesUsArray); return new SubripSubtitle(cuesArray, cueTimesUsArray);
} }
@Override
public boolean canParse(String mimeType) {
return MimeTypes.APPLICATION_SUBRIP.equals(mimeType);
}
private static long parseTimecode(String s) throws NumberFormatException { private static long parseTimecode(String s) throws NumberFormatException {
Matcher matcher = SUBRIP_TIMESTAMP.matcher(s); Matcher matcher = SUBRIP_TIMESTAMP.matcher(s);
if (!matcher.matches()) { if (!matcher.matches()) {
......
...@@ -82,23 +82,8 @@ public final class TtmlParser implements SubtitleParser { ...@@ -82,23 +82,8 @@ public final class TtmlParser implements SubtitleParser {
private static final int DEFAULT_TICKRATE = 1; private static final int DEFAULT_TICKRATE = 1;
private final XmlPullParserFactory xmlParserFactory; private final XmlPullParserFactory xmlParserFactory;
private final boolean strictParsing;
/**
* Equivalent to {@code TtmlParser(false)}.
*/
public TtmlParser() { public TtmlParser() {
this(false);
}
/**
* @param strictParsing If true, {@link #parse(InputStream)} will throw a {@link ParserException}
* if the stream contains invalid data. If false, the parser will make a best effort to ignore
* minor errors in the stream. Note however that a {@link ParserException} will still be
* thrown when this is not possible.
*/
public TtmlParser(boolean strictParsing) {
this.strictParsing = strictParsing;
try { try {
xmlParserFactory = XmlPullParserFactory.newInstance(); xmlParserFactory = XmlPullParserFactory.newInstance();
} catch (XmlPullParserException e) { } catch (XmlPullParserException e) {
...@@ -107,6 +92,11 @@ public final class TtmlParser implements SubtitleParser { ...@@ -107,6 +92,11 @@ public final class TtmlParser implements SubtitleParser {
} }
@Override @Override
public boolean canParse(String mimeType) {
return MimeTypes.APPLICATION_TTML.equals(mimeType);
}
@Override
public Subtitle parse(InputStream inputStream) throws IOException { public Subtitle parse(InputStream inputStream) throws IOException {
try { try {
XmlPullParser xmlParser = xmlParserFactory.newPullParser(); XmlPullParser xmlParser = xmlParserFactory.newPullParser();
...@@ -134,15 +124,11 @@ public final class TtmlParser implements SubtitleParser { ...@@ -134,15 +124,11 @@ public final class TtmlParser implements SubtitleParser {
parent.addChild(node); parent.addChild(node);
} }
} catch (ParserException e) { } catch (ParserException e) {
if (strictParsing) {
throw e;
} else {
Log.w(TAG, "Suppressing parser error", e); Log.w(TAG, "Suppressing parser error", e);
// Treat the node (and by extension, all of its children) as unsupported. // Treat the node (and by extension, all of its children) as unsupported.
unsupportedNodeDepth++; unsupportedNodeDepth++;
} }
} }
}
} else if (eventType == XmlPullParser.TEXT) { } else if (eventType == XmlPullParser.TEXT) {
parent.addChild(TtmlNode.buildTextNode(xmlParser.getText())); parent.addChild(TtmlNode.buildTextNode(xmlParser.getText()));
} else if (eventType == XmlPullParser.END_TAG) { } else if (eventType == XmlPullParser.END_TAG) {
...@@ -287,11 +273,6 @@ public final class TtmlParser implements SubtitleParser { ...@@ -287,11 +273,6 @@ public final class TtmlParser implements SubtitleParser {
return style == null ? new TtmlStyle() : style; return style == null ? new TtmlStyle() : style;
} }
@Override
public boolean canParse(String mimeType) {
return MimeTypes.APPLICATION_TTML.equals(mimeType);
}
private TtmlNode parseNode(XmlPullParser parser, TtmlNode parent) throws ParserException { private TtmlNode parseNode(XmlPullParser parser, TtmlNode parent) throws ParserException {
long duration = 0; long duration = 0;
long startTime = TtmlNode.UNDEFINED_TIME; long startTime = TtmlNode.UNDEFINED_TIME;
......
...@@ -32,15 +32,15 @@ import java.io.InputStream; ...@@ -32,15 +32,15 @@ import java.io.InputStream;
public final class Tx3gParser implements SubtitleParser { public final class Tx3gParser implements SubtitleParser {
@Override @Override
public boolean canParse(String mimeType) {
return MimeTypes.APPLICATION_TX3G.equals(mimeType);
}
@Override
public Subtitle parse(InputStream inputStream) throws IOException { public Subtitle parse(InputStream inputStream) throws IOException {
DataInputStream dataInputStream = new DataInputStream(inputStream); DataInputStream dataInputStream = new DataInputStream(inputStream);
String cueText = dataInputStream.readUTF(); String cueText = dataInputStream.readUTF();
return new Tx3gSubtitle(new Cue(cueText)); return new Tx3gSubtitle(new Cue(cueText));
} }
@Override
public boolean canParse(String mimeType) {
return MimeTypes.APPLICATION_TX3G.equals(mimeType);
}
} }
...@@ -44,34 +44,23 @@ public final class WebvttParser implements SubtitleParser { ...@@ -44,34 +44,23 @@ public final class WebvttParser implements SubtitleParser {
private static final Pattern HEADER = Pattern.compile("^\uFEFF?WEBVTT((\u0020|\u0009).*)?$"); private static final Pattern HEADER = Pattern.compile("^\uFEFF?WEBVTT((\u0020|\u0009).*)?$");
private static final Pattern COMMENT_BLOCK = Pattern.compile("^NOTE((\u0020|\u0009).*)?$"); private static final Pattern COMMENT_BLOCK = Pattern.compile("^NOTE((\u0020|\u0009).*)?$");
private static final Pattern METADATA_HEADER = Pattern.compile("\\S*[:=]\\S*");
private static final Pattern CUE_HEADER = Pattern.compile("^(\\S+)\\s+-->\\s+(\\S+)(.*)?$"); private static final Pattern CUE_HEADER = Pattern.compile("^(\\S+)\\s+-->\\s+(\\S+)(.*)?$");
private static final Pattern CUE_SETTING = Pattern.compile("\\S+?:\\S+"); private static final Pattern CUE_SETTING = Pattern.compile("\\S+?:\\S+");
private final PositionHolder positionHolder; private final PositionHolder positionHolder;
private final StringBuilder textBuilder; private final StringBuilder textBuilder;
private final boolean strictParsing;
/**
* Equivalent to {@code WebvttParser(false)}.
*/
public WebvttParser() { public WebvttParser() {
this(false);
}
/**
* @param strictParsing If true, {@link #parse(InputStream)} will throw a {@link ParserException}
* if the stream contains invalid data. If false, the parser will make a best effort to ignore
* minor errors in the stream. Note however that a {@link ParserException} will still be
* thrown when this is not possible.
*/
public WebvttParser(boolean strictParsing) {
this.strictParsing = strictParsing;
positionHolder = new PositionHolder(); positionHolder = new PositionHolder();
textBuilder = new StringBuilder(); textBuilder = new StringBuilder();
} }
@Override @Override
public final boolean canParse(String mimeType) {
return MimeTypes.TEXT_VTT.equals(mimeType);
}
@Override
public final WebvttSubtitle parse(InputStream inputStream) throws IOException { public final WebvttSubtitle parse(InputStream inputStream) throws IOException {
ArrayList<WebvttCue> subtitles = new ArrayList<>(); ArrayList<WebvttCue> subtitles = new ArrayList<>();
...@@ -94,13 +83,6 @@ public final class WebvttParser implements SubtitleParser { ...@@ -94,13 +83,6 @@ public final class WebvttParser implements SubtitleParser {
// We read the newline that separates the header from the body. // We read the newline that separates the header from the body.
break; break;
} }
if (strictParsing) {
Matcher matcher = METADATA_HEADER.matcher(line);
if (!matcher.find()) {
throw new ParserException("Unexpected line: " + line);
}
}
} }
// Process the cues and text. // Process the cues and text.
...@@ -187,11 +169,6 @@ public final class WebvttParser implements SubtitleParser { ...@@ -187,11 +169,6 @@ public final class WebvttParser implements SubtitleParser {
return new WebvttSubtitle(subtitles); return new WebvttSubtitle(subtitles);
} }
@Override
public final boolean canParse(String mimeType) {
return MimeTypes.TEXT_VTT.equals(mimeType);
}
private static long parseTimestampUs(String s) throws NumberFormatException { private static long parseTimestampUs(String s) throws NumberFormatException {
long value = 0; long value = 0;
String[] parts = s.split("\\.", 2); String[] parts = s.split("\\.", 2);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment